Document generic webhooks

[astrojuanlu]

Continuation of #8522.

I considered renaming the file from build-notifications to build-notifications-webhooks, but was unsure if it was worth it. I ended up renaming the title though.

[astrojuanlu]

🤖 Rendered version: https://docs--8609.org.readthedocs.build/en/8609/build-notifications.html

[stsewd]

I was thinking we could provide some examples of payloads ready to use for slack & discord, so users just need to copy/paste things.

https://support.discord.com/hc/en-us/articles/228383668-Intro-to-Webhooks
https://app.slack.com/block-kit-builder/T02J81RRP

[stsewd]

oh, we should also document about the signature check

readthedocs.org/readthedocs/builds/tasks.py

Line 613 in c99df5d

headers['X-Hub-Signature'] = webhook.sign_payload(payload)

, maybe have a small python code as example on how to check the signature, which is the same process from

readthedocs.org/readthedocs/projects/models.py

Lines 1593 to 1600 in c99df5d

	def sign_payload(self, payload):
	"""Get the signature of `payload` using HMAC-SHA1 with the webhook secret."""
	digest = hmac.new(
	key=self.secret.encode(),
	msg=payload.encode(),
	digestmod=hashlib.sha1,
	)
	return digest.hexdigest()

for example

• https://stripe.com/docs/webhooks/signatures
• https://docs.github.com/en/developers/webhooks-and-events/webhooks/securing-your-webhooks#validating-payloads-from-github

[humitos]

I was thinking we could provide some examples of payloads ready to use for slack & discord, so users just need to copy/paste things.

I also think this is important 💯

[astrojuanlu]

I added Slack and Discord examples as suggested 👍🏽

[astrojuanlu]

Added another section on payload validation, please verify. I don't know if there are any coding standards for this kind of verifications.

[astrojuanlu]

There's a bit of git mess here (merge of merge of merge...) so @stsewd I will let you decide whether to merge this pull request to webhooks directly or if you want to rebase and merge.

[stsewd]

There's a bit of git mess here (merge of merge of merge...)

Since this isn't touching anything from the code on the other PR we could also just set the base branch as master, and then merge the two PRs the same day.

[stsewd]

I'll see if I can get some screenshots of the final result of the slack/discord messages. We could merge this when the other PR is merged and update any variables if needed.

[stsewd]

screenshots using the payload from the examples

[ericholscher]

This is a good refactor, but could still use a bit more context. I really feel that having to copy/paste a jumble of JSON to support very common webhooks is a poor product experience, so I'd really like us to see if we can support a nicer workflow there by default with just a button or dropdown Webhook format: Slack, Discord, Custom or something.

[ericholscher]

This feels like bad UX. I would strongly suggest that we implement a standard Slack webhook option, instead of forcing users to copy & paste large bits of JSON into our dashboard. /cc @stsewd

[stsewd]

would this be an option to pre-fill the content, or just an option where users don't have control over?

[stsewd]

We have this for automation rules for example

But I'd prefer the option to pre-fill the content

[stsewd]

and user would still need to setup webhook from their side on slack/discord.

[ericholscher]

@stsewd I think something that we control should be the default. Many users don't actually want to think about this, and having something that we control and can improve over time without users having to touch it feels like better UX?

[ericholscher]

In the same fashion, imagine that Slack changed their webhook format or something. It would be a huge pain if we had to email every user to ask them to update the blob of JSON in the config. We should definitely support "Slack" as a first-class option that we control the format of.

[stsewd]

I guess if that happens slack will provide users a different url or option, then we are forced to support both formats using an option or something.

My other worry here is if we change something, it will affect everyone, users may not like something from the new change and will request for options, and we end up adding a lot of options in the UI or the users will end up creating their own custom payload at the end.

[ericholscher]

@stsewd Valid concerns, but I think there are a large number of users who just want "Slack notifications" that work well. Asking them to customize them is an actively negative experience. I think we want both the simple & custom, so users can choose what works better for them.

[astrojuanlu]

Hi folks, I lost track of the discussion at #8522 and feature proposals that were suggested, too many messages. Is there consensus on the usage already? May I continue working on this? @stsewd @ericholscher

[stsewd]

@astrojuanlu this is it #8522 (comment)

[astrojuanlu]

Ready for another pass!

[ericholscher]

Looks good to me, with a couple nits 👍

[ericholscher]

@stsewd Valid concerns, but I think there are a large number of users who just want "Slack notifications" that work well. Asking them to customize them is an actively negative experience. I think we want both the simple & custom, so users can choose what works better for them.

[stsewd]

I think we should maybe start each section with the links to the docs to create the webhhok on slack/discord and then give them the payload to use (Follow this steps on discord/slack, use the given URL when creating a webhook on rtd and add this payload).

[astrojuanlu]

This has been approved already three times, and I keep having to catch up with the webhooks branch. To simplify things, I'm going to merge this already, and I think we can make changes in future PRs.