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

feat: Webhooks #165

Open
wants to merge 34 commits into
base: main
Choose a base branch
from
Open

feat: Webhooks #165

wants to merge 34 commits into from

Conversation

danjohnson95
Copy link
Contributor

@danjohnson95 danjohnson95 commented Jan 4, 2025

👋 Sorry for the giant PR here, got a bit carried away.

This PR introduces the webhooks functionality ✨ (Resolves #106)

It's using Spatie's library under the hood to handle sending and retries, but here's a brief overview of what it's doing:

  • Any events that should trigger a webhook should use the SendsWebhook trait.
  • There's a listener registered on the provider listening to all events fired by Cachet. (src/Listeners/SendWebhookListener.php). If any of them use the SendsWebhook trait, the DispatchWebhooks action will take it from here.
  • DispatchWebhooks will call the getWebhookEventName and getWebhookPayload methods on the event to build up the webhook call.
  • It'll find all webhook subscriptions that are interested in this event, and the Spatie package will fire off the event.
  • We're listening to the Spatie events to hear back when a webhook call was successful/unsuccessful, and creating a WebhookAttempt.

This means from the user point of view, they just need to:

  • Go to Settings -> Manage Webhooks
  • Add a new webhook
  • Specify their URL, a secret, an optional description, and pick which events they're interested in.

And they'll see a log of all the recent webhook attempts made to that endpoint.


From a Cachet contributor point of view, in order to make something webhookable, simply:

  • Add the event name to src/Enums/WebhookEventEnum
  • Make the component use the SendsWebhook trait
  • Define the getWebhookPayload() and getWebhookEventName() methods.

Let me know if you have any questions or suggestions!

Copy link
Member

@jbrooksuk jbrooksuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is amazing, thanks @danjohnson95! I've not yet reviewed in my laptop, but aside from this one question, it looks pretty great!

src/Actions/Webhook/DispatchWebhooks.php Show resolved Hide resolved
@jbrooksuk
Copy link
Member

@danjohnson95 okay, I had a moment to check on this PR. Here's my feedback:

  • The "Manage Webhooks" section should probably be full-width, with attempts underneath.
  • When editing a webhook, we shouldn't need to specify the secret unless changing it.
  • Can "webhook documentation" be linked to https://docs.cachethq.io/v3.x/guide/webhooks, which we'll have in place when we merge this.

@danjohnson95
Copy link
Contributor Author

Thanks for the review @jbrooksuk!

The "Manage Webhooks" section should probably be full-width, with attempts underneath.

Nice, I've made this change now. I initially put it in a section on the right because the Save button goes underneath the Attempts section. It could potentially be a bit confusing if a webhook has a long list of attempts and the Save button requires a scroll to see it.

  • We could paginate the list of attempts to keep scrolling to a minimum?
  • Or maybe there's a way we can add a section to go after the Save button? (Filament first timer here)

When editing a webhook, we shouldn't need to specify the secret unless changing it.

Good spot. How about something like how GitHub manages it, with an additional click to change the secret?

image

or did you have something else in mind?

Can "webhook documentation" be linked to https://docs.cachethq.io/v3.x/guide/webhooks, which we'll have in place when we merge this.

Done! I had a dig around in the Filament source to see how we can output HTML in the helperText and came up with a decent solution - I've created the Cachet\View\Htmlable\TextWithLink class

@jbrooksuk
Copy link
Member

jbrooksuk commented Jan 7, 2025

@danjohnson95 thanks for implementing these changes. I couldn't immediately see anything in the docs about showing the panel under the save button. Maybe we could make the width condition whether you're editing a webhook or not?

Either leave the secret field empty as to not update it, or have a button to change it (which could open an action modal) would be fine.

@jbrooksuk
Copy link
Member

@danjohnson95 I've just fixed the tests, but one thing I've noticed is that the "Edit Secret" button is visible when creating a webhook, we should make it conditional to the Edit page.

@danjohnson95
Copy link
Contributor Author

Looks like that's a Filament bug! I've raised a PR to fix it.

@jbrooksuk
Copy link
Member

Thanks @danjohnson95!

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.

Webhooks
2 participants