Skip to content

Support notifications to be fired for protected branches also

Balasankar 'Balu' C requested to merge slack-protected-branch-notification into master

What does this MR do?

Related: https://gitlab.com/gitlab-org/gitlab-ce/issues/42776

Currently, notifications can be configured only for either all the branches or just the default branch, both of which are a bit extreme. This MR changes it to more options

  1. All branches
  2. Default branch only
  3. Protected branches only
  4. Default branch and Protected branches only

Technical decision timeline

  1. Thought of splitting off the attribute from properties column, but that included introducing a background migration. Since devopsplan has no plan to do that for all the properties right now, it didn't make sense to introduce a background migration just for one key. So, we are now considering both old and new key and set precedence to the new one.
  2. Extracted redundant code that was present in both chat notification service and pipeline email service to a module.

Screenshots

(Slack as an example)

Before

Before After
image image

Does this MR meet the acceptance criteria?

Conformity

Performance and Testing

Edited by Balasankar 'Balu' C

Merge request reports