Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add webhook stores #13

Closed
wants to merge 3 commits into from
Closed

Add webhook stores #13

wants to merge 3 commits into from

Conversation

rubenvanassche
Copy link
Member

Add the possibility to implement your own WebhookStore: your own implementation of storing a WebhookCall model in the database.

@rubenvanassche
Copy link
Member Author

Ideas for better name are welcome :)

@freekmurze
Copy link
Member

The intent isn't clear to me. If you want to use another model or table, you can specify that in the config. My feeling is that have a store layer on top is too complex.

Maybe you could guide me through the actual use case where this is needed.

@rubenvanassche
Copy link
Member Author

So in a project we're building right now we need some additional data on the model, though this data can also be found in the raw JSON payload, you don't want to use this field to query for certain webhooks(what if the format of the JSON changes through the application lifetime).

For example in our project we have a status attached to each webhook, we need to check if a status was already received in the past so we can accept/reject the webhook coming in. So a simple ->where('status', $status) is the best solution for this.

The problem with the current implementation of the package is that we have no control over the creation of the webhook model. This means our status column should be null so an initial webhook model can be constructed without errors. But at time of creating the model we have the status field, so why not fill it right in? This is where this PR become important, you can create a model with a status column filled in by creating a custom webhook store class which handles the creating of the model.

@freekmurze
Copy link
Member

How about creating your own model, and add a created callback on it. In that callback you could do whatever you want.

In the package we could to add a dedicated storeWebhook method on the the current WebhookCall model and use that instead of the create method. That way people can override that method as well. The upside of this solution is that we don't need to introduce an extra concept like store.

@rubenvanassche
Copy link
Member Author

In the package we could to add a dedicated storeWebhook method on the the current WebhookCall model and use that instead of the create method. That way people can override that method as well. The upside of this solution is that we don't need to introduce an extra concept like store.

Really like that idea! I will try to implement this as soon as possible

@freekmurze
Copy link
Member

Closing this in favour of your next PR :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants