Skip to content

Show why a notification email was sent

Mario de la Ossa requested to merge mdelaossa/gitlab-ce:41532-email-reason into master

What does this MR do?

Adds #build_notification_recipients to NotificationRecipientService that returns the NotificationRecipient objects in order to be able to access the new attribute reason.

This new attribute is populated inside the #build! method and used in the different notifier methods in order to add the reason as a header: X-GitLab-NotificationReason.

Only the reason with the most priority gets sent, decided by the NotificationRecipientService::Builder::Base::REASON_PRIORITY array.

So far only own_activity and assigned have been implemented. Also implemented is a priority list for each 'reason' in order to send the most relevant/important reason. own_activity gets set from within NotificationRecipient#own_activity? when it is detected that the user

This MR also adds the reason in the footer of email notifications.

Are there points in the code the reviewer needs to double check?

  • NotificationRecipient#reason is a string due to ActiveJob not being able to serialize symbols
  • new_merge_request_worker_spec and new_issue_worker_spec say they're testing "creates a notification for the assignee" but they're really testing a notification for a mentioned user? The assignee(s) relationship is empty/nil.
  • In NotificationRecipientService#build!, when adding assigned notifications for new issues/MRs, I'm adding them at the :participating level since existing specs (notification_service_spec.rb:509) expect it to be at a level below :mention. Is this correct? Shouldn't an assignment count as a mention?

Why was this MR needed?

See #41532 (closed). This allows users to filter their email notifications depending on the reason they were notified.

Screenshots (if relevant)

Does this MR meet the acceptance criteria?

What are the relevant issue numbers?

Closes #41532 (closed) and #1366 (closed)

Edited by Mario de la Ossa

Merge request reports