Add event that is being fired upon the webhook's dispatch #150
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
freekmurze
requested changes
Nov 27, 2023
src/WebhookCall.php
Outdated
| @@ -229,6 +230,16 @@ public function dispatch(): PendingDispatch | |||
| { | |||
| $this->prepareForDispatch(); | |||
|
|
|||
| event(new WebhookCallDispatchedEvent( | |||
There was a problem hiding this comment.
This event should be named DispatchingWebhookCallEvent as it is fired before the actual dispatch.
There was a problem hiding this comment.
Very good point, thank you. I amended the event's name and updated the README.md accordingly.
|
Thanks! |
|
@thannaske which database structure of table did you use for storing handling outgoing webhooks? |
|
@siarheipashkevich Please use GitHub's discussions for questions regarding the usage of the package, a (closed and over half a year old) Pull Request is not the right place for it. You can however just put all available properties of the DispatchingWebhookCallEvent into a table. The exact table structure depends on your individual needs. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The idea for this PR came in regards of discussion #91, which introduced the idea of a database persistence layer. While this would be currently out of scope for this package, I figured that you could easily achieve everything the creator of the idea intended to do by utilising the package's events.
The only thing that was missing to allow persisting the webhook in whichever way the developer wants, was an event that is being fired exactly once when a new webhook is being dispatched (not to confuse with the dispatching of the job which could occur multiple times when retrying). For this reason, this PR adds the
WebhookCallDispatchedEventwhich fires when thedispatch()method for theWebhookCallobject is being called.One could then simply listen to the
WebhookCallDispatchedEvent, persist it to the database and e.g. increase attempt counts or update the web hook's status based on firedWebhookCallFailedEvents andWebhookCallSucceededEvents.I was unable to reuse the abstract
WebhookCallEventas it contains a lot of attributes that are specific to an actual attempt to send the webhook to its target URL.Thank you for your work and of course no hard feelings if this PR is going to be declined.