Skip to content

Update GroupHook.select_active to include non-executable hooks

Alex Kalderimis requested to merge 387938-group-hooks-select-active into master

What does this MR do and why?

We updated Group Hooks to not be disabled when they receive failing responses. We also need to update the hook selection mechanism.

See: #387938 (closed)

How did this get fixed?

Rather than having auto-disable functionality in the super-class (WebHook) that is then selectively disabled, the behavior is moved to two concerns that are included selectively in hook classes.

Currently only project hooks are auto-disabling.

This also involved moving the test code out the super-class.

Database

Two queries changed:

Executable webhooks:

Disabled webhooks:

Screenshots or screen recordings

1673636664

How to set up and validate locally

Steps to reproduce

  1. Setup a group webhook pointing to: https://httpstat.us/500 for comment events
  2. Make 5 comments
  3. Check the hook details
  4. You'll see 5 recent events with 500 responses and no indication that anything is wrong (e.g. no indicator that the hook is disabled temporarily or permanently)
  5. Wait a while
  6. Make another comment
  7. Check the hook details
  8. You see a new event :)

whereas, for project hooks you should see:

  1. Setup a project webhook pointing to: https://httpstat.us/500 for comment events
  2. Make 5 comments
  3. Check the hook details
  4. You'll see 3 recent events with 500 responses and an indication that something is wrong (specifically an indicator that the hook is disabled temporarily)
  5. Make another comment
  6. Check the hook details, see that there is no new event
  7. Wait one minute
  8. Make another comment
  9. Check the hook details
  10. You see a new failed event, and the back-off time will have doubled

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Alex Kalderimis

Merge request reports