Repository navigation
feat: webhook on call notification - #3036
Conversation
|
Do you think we should mark this as a draft in the meantime? I think you mentioned this is blocked until the dropdown changes are made to the rule form. No one is actively working on that for the moment, to my knowledge, though. I created a separate issue to track that specific change: |
|
I mistakenly thought that the user groups PR was going to include the dropdown. This PR adds the dropdown, I was just worried it would conflict if there was another PR introducing one. Because the logic and components are already present here, I would say that this should come first and then a PR for #3045 would just separate user groups from channels. But it's up to you |
|
@andrewbenington That does make sense, though that makes the scope of this PR bigger due to coupling the UI redesign with the new webhook notification option. That said, I think it makes sense to do #3045 first because switching to the dropdown would best be done along with updating the user group behavior to match the new UI (since it is currently coupled with channels). Doing so with this PR would only serve to make it even larger and harder to get through review. |
There was a problem hiding this comment.
It looks like we're displaying the full URL, including sensitive parts:
In contact methods, we mask the password, strip query params, and truncate the path:
And escalation policies only show the hostname:
We should be consistent everywhere as a follow-up, but let's update this to follow one of the two existing methods.
@mastercactapus I think it's good to have the full URL to differentiate between webhooks with the same hostname. The name is now set to the "FriendlyValue", but the full URL will still be visible on edit. Should this be hidden too? |
|
@andrewbenington It's mostly consistent with contact method behavior, and I'd consider changing that to be out-of-scope for this PR. Mainly because the notification channel can be edited, and contact methods can not (since they also convey ownership). With it in experimental state, I'm okay with leaving it open for iteration in the future (e.g., maybe only show the FriendlyValue or allow replacing it entirely) but then consider how we can get EP and contact methods to all match |
make checkto catch common errors. Fixed any that came up.Description:
Adds webooks as a possible destination for Schedule On-Call Notifications.
valueuses achannelFieldssub-object instead of includingslackChannelandslackUserGroupin the base objectWhich issue(s) this PR fixes:
#2914
Describe any introduced user-facing changes:
Users can specify a webhook destination for Schedule On-Call Notifications if webhooks are enabled by an admin. Under a Schedule, the Quick Link for On-Call Notifications will now show up if webhooks are enabled and the chan-webhook experimental flag is set. When creating/editing an On-Call Notification rule, there is now a dropdown to select between Slack and a webhook, which will also display the appropriate form fields.
Describe any introduced API changes:
setScheduleOnCallNotificationRulescan now accept a rule with a target of type "chanWebhook", if it includes an ID that conforms with an approved webhook URL.UI Screenshots:
