craftcms / webhooks

Plugin for integrating Craft with Zapier and IFTTT.
https://plugins.craftcms.com/webhooks
MIT License
84 stars 12 forks source link

Idea/FR: If the requestBody is empty allow the webhook to be ignored. If a config setting is enabled #9

Closed gtettelaar closed 5 years ago

gtettelaar commented 5 years ago

Definitely linked to #6

When using the payload template using basic twig conditionals is really useful for altering the requestBody.

This PR allows you to enable a config setting that ignores webhooks (doesn't send them) when the requestBody is empty. I.E. if a conditional wasn't met in the payload template.

This is really useful when not using Zapier based webhooks.

The config setting defaults to false so existing installations shouldn't be affected.

brandonkelly commented 5 years ago

The plugin already allows empty bodies (necessarily for all GET requests, but also can be the case for POST requests), so this would prevent those from being sent.

gtettelaar commented 5 years ago

Whoops!

I added another commit that swaps out the global config variable with a per-webhook setting that can only be enabled if the method is post and if the customPayloadTemplate is selected. See here: https://github.com/lemiwinkz/webhooks/commit/372aada38acaea44067eaa458d26cd73b7a42fde

Would you be open to a PR with those changes? It shouldn't affect existing installations as the default setting is false.

It now only ignores webhook requests if all the below are met:

GET requests are now always sent by default and POST requests can only be ignored with explicit 'consent' from the administrator (on a per-webhook basis) and under specific conditions.

@brandonkelly

gtettelaar commented 5 years ago

@brandonkelly Any feedback on this?

brandonkelly commented 5 years ago

We just released Webhooks 2.1.0 with a new Event Filters feature. I think this is a better approach to determining whether the webhook should be executed. Whatever conditions you were going to put in your payload template, you could just put into your filter’s check() method instead.