Merge branch '30217-refactor-email-notification-code' into 'master'

Replace the Notify#recipient method

Closes #30217

See merge request gitlab-org/gitlab!17741

app/mailers/emails/issues.rb

@@ -85,7 +85,7 @@ module Emails
       @project = Project.find(project_id)
       @results = results
 
-      mail(to: recipient(@user.id, @project.group), subject: subject('Imported issues')) do |format|
+      mail(to: @user.notification_email_for(@project.group), subject: subject('Imported issues')) do |format|
         format.html { render layout: 'mailer' }
         format.text { render layout: 'mailer' }
       end
@@ -105,7 +105,7 @@ module Emails
     def issue_thread_options(sender_id, recipient_id, reason)
       {
         from: sender(sender_id),
-        to: recipient(recipient_id, @project.group),
+        to: User.find(recipient_id).notification_email_for(@project.group),
         subject: subject("#{@issue.title} (##{@issue.iid})"),
         'X-GitLab-NotificationReason' => reason
       }

app/mailers/emails/members.rb

@@ -13,7 +13,9 @@ module Emails
       @member_source_type = member_source_type
       @member_id = member_id
 
-      mail(to: recipient(recipient_id, notification_group),
+      user = User.find(recipient_id)
+
+      mail(to: user.notification_email_for(notification_group),
            subject: subject("Request to join the #{member_source.human_name} #{member_source.model_name.singular}"))
     end
 
@@ -21,7 +23,7 @@ module Emails
       @member_source_type = member_source_type
       @member_id = member_id
 
-      mail(to: recipient(member.user, notification_group),
+      mail(to: member.user.notification_email_for(notification_group),
            subject: subject("Access to the #{member_source.human_name} #{member_source.model_name.singular} was granted"))
     end
 
@@ -29,7 +31,9 @@ module Emails
       @member_source_type = member_source_type
       @member_source = member_source_class.find(source_id)
 
-      mail(to: recipient(user_id, notification_group),
+      user = User.find(user_id)
+
+      mail(to: user.notification_email_for(notification_group),
            subject: subject("Access to the #{member_source.human_name} #{member_source.model_name.singular} was denied"))
     end
 
@@ -47,7 +51,7 @@ module Emails
       @member_id = member_id
       return unless member.created_by
 
-      mail(to: recipient(member.created_by, notification_group),
+      mail(to: member.created_by.notification_email_for(notification_group),
            subject: subject('Invitation accepted'))
     end
 
@@ -58,7 +62,9 @@ module Emails
       @member_source = member_source_class.find(source_id)
       @invite_email = invite_email
 
-      mail(to: recipient(created_by_id, notification_group),
+      user = User.find(created_by_id)
+
+      mail(to: user.notification_email_for(notification_group),
            subject: subject('Invitation declined'))
     end
 

app/mailers/emails/merge_requests.rb

@@ -110,7 +110,7 @@ module Emails
     def merge_request_thread_options(sender_id, recipient_id, reason = nil)
       {
         from: sender(sender_id),
-        to: recipient(recipient_id, @project.group),
+        to: User.find(recipient_id).notification_email_for(@project.group),
         subject: subject("#{@merge_request.title} (#{@merge_request.to_reference})"),
         'X-GitLab-NotificationReason' => reason
       }

app/mailers/emails/notes.rb

@@ -55,7 +55,7 @@ module Emails
     def note_thread_options(recipient_id, reason)
       {
         from: sender(@note.author_id),
-        to: recipient(recipient_id, @project&.group || @group),
+        to: User.find(recipient_id).notification_email_for(@project&.group || @group),
         subject: subject("#{@note.noteable.title} (#{@note.noteable.reference_link_text})"),
         'X-GitLab-NotificationReason' => reason
       }

app/mailers/emails/pages_domains.rb

@@ -7,7 +7,7 @@ module Emails
       @project = domain.project
 
       mail(
-        to: recipient(recipient.id, @project.group),
+        to: recipient.notification_email_for(@project.group),
         subject: subject("GitLab Pages domain '#{domain.domain}' has been enabled")
       )
     end
@@ -17,7 +17,7 @@ module Emails
       @project = domain.project
 
       mail(
-        to: recipient(recipient.id, @project.group),
+        to: recipient.notification_email_for(@project.group),
         subject: subject("GitLab Pages domain '#{domain.domain}' has been disabled")
       )
     end
@@ -27,7 +27,7 @@ module Emails
       @project = domain.project
 
       mail(
-        to: recipient(recipient.id, @project.group),
+        to: recipient.notification_email_for(@project.group),
         subject: subject("Verification succeeded for GitLab Pages domain '#{domain.domain}'")
       )
     end
@@ -37,7 +37,7 @@ module Emails
       @project = domain.project
 
       mail(
-        to: recipient(recipient.id, @project.group),
+        to: recipient.notification_email_for(@project.group),
         subject: subject("ACTION REQUIRED: Verification failed for GitLab Pages domain '#{domain.domain}'")
       )
     end

app/mailers/emails/projects.rb

@@ -7,20 +7,20 @@ module Emails
       @project = Project.find project_id
       @target_url = project_url(@project)
       @old_path_with_namespace = old_path_with_namespace
-      mail(to: recipient(user_id, @project.group),
+      mail(to: @user.notification_email_for(@project.group),
            subject: subject("Project was moved"))
     end
 
     def project_was_exported_email(current_user, project)
       @project = project
-      mail(to: recipient(current_user.id, project.group),
+      mail(to: current_user.notification_email_for(project.group),
            subject: subject("Project was exported"))
     end
 
     def project_was_not_exported_email(current_user, project, errors)
       @project = project
       @errors = errors
-      mail(to: recipient(current_user.id, @project.group),
+      mail(to: current_user.notification_email_for(@project.group),
            subject: subject("Project export error"))
     end
 
@@ -28,7 +28,7 @@ module Emails
       @project = project
       @user = user
 
-      mail(to: recipient(user.id, project.group), subject: subject("Project cleanup has completed"))
+      mail(to: user.notification_email_for(project.group), subject: subject("Project cleanup has completed"))
     end
 
     def repository_cleanup_failure_email(project, user, error)
@@ -36,7 +36,7 @@ module Emails
       @user = user
       @error = error
 
-      mail(to: recipient(user.id, project.group), subject: subject("Project cleanup failure"))
+      mail(to: user.notification_email_for(project.group), subject: subject("Project cleanup failure"))
     end
 
     def repository_push_email(project_id, opts = {})

app/mailers/emails/remote_mirrors.rb

@@ -5,8 +5,9 @@ module Emails
     def remote_mirror_update_failed_email(remote_mirror_id, recipient_id)
       @remote_mirror = RemoteMirror.find_by_id(remote_mirror_id)
       @project = @remote_mirror.project
+      user = User.find(recipient_id)
 
-      mail(to: recipient(recipient_id, @project.group), subject: subject('Remote mirror update failed'))
+      mail(to: user.notification_email_for(@project.group), subject: subject('Remote mirror update failed'))
     end
   end
 end

app/mailers/notify.rb

@@ -71,20 +71,6 @@ class Notify < BaseMailer
     address.format
   end
 
-  # Look up a User's notification email for a particular context.
-  # Can look up by their ID or can accept a User object.
-  #
-  # recipient          - User object OR a User ID
-  # notification_group - The parent group of the notification
-  #
-  # Returns a String containing the User's email address.
-  def recipient(recipient, notification_group = nil)
-    user = recipient if recipient.is_a?(User)
-    user ||= User.find(recipient)
-
-    user.notification_email_for(notification_group)
-  end
-
   # Formats arguments into a String suitable for use as an email subject
   #
   # extra - Extra Strings to be inserted into the subject

changelogs/unreleased/30217-refactor-email-notification-code.yml

+---
+title: Refactor email notification code
+merge_request: 17741
+author: briankabiro
+type: other

ee/app/mailers/ee/emails/projects.rb

@@ -5,21 +5,24 @@ module EE
     module Projects
       def mirror_was_hard_failed_email(project_id, user_id)
         @project = ::Project.find(project_id)
+        user = ::User.find(user_id)
 
-        mail(to: recipient(user_id, @project.group),
+        mail(to: user.notification_email_for(@project.group),
              subject: subject('Repository mirroring paused'))
       end
 
       def project_mirror_user_changed_email(new_mirror_user_id, deleted_user_name, project_id)
         @project = ::Project.find(project_id)
         @deleted_user_name = deleted_user_name
+        user = ::User.find(new_mirror_user_id)
 
-        mail(to: recipient(new_mirror_user_id, @project.group),
+        mail(to: user.notification_email_for(@project.group),
              subject: subject('Mirror user changed'))
       end
 
       def prometheus_alert_fired_email(project_id, user_id, alert_payload)
         @project = ::Project.find(project_id)
+        user = ::User.find(user_id)
 
         @alert = ::Gitlab::Alerting::Alert
           .new(project: @project, payload: alert_payload)
@@ -27,7 +30,7 @@ module EE
         return unless @alert.valid?
 
         subject_text = "Alert: #{@alert.full_title}"
-        mail(to: recipient(user_id, @project.group), subject: subject(subject_text))
+        mail(to: user.notification_email_for(@project.group), subject: subject(subject_text))
       end
     end
   end

ee/app/mailers/emails/admin_notification.rb

@@ -3,14 +3,16 @@
 module Emails
   module AdminNotification
     def send_admin_notification(user_id, subject, body)
-      email = recipient(user_id)
+      user = User.find(user_id)
+      email = user.notification_email
       @unsubscribe_url = unsubscribe_url(email: Base64.urlsafe_encode64(email))
       @body = body
       mail to: email, subject: subject
     end
 
     def send_unsubscribed_notification(user_id)
-      email = recipient(user_id)
+      user = User.find(user_id)
+      email = user.notification_email
       mail to: email, subject: "Unsubscribed from GitLab administrator notifications"
     end
   end

ee/app/mailers/emails/csv_export.rb

@@ -10,7 +10,7 @@ module Emails
 
       filename = "#{project.full_path.parameterize}_issues_#{Date.today.iso8601}.csv"
       attachments[filename] = { content: csv_data, mime_type: 'text/csv' }
-      mail(to: recipient(user.id, @project.group), subject: subject("Exported issues")) do |format|
+      mail(to: user.notification_email_for(@project.group), subject: subject("Exported issues")) do |format|
         format.html { render layout: 'mailer' }
         format.text { render layout: 'mailer' }
       end

ee/app/mailers/emails/epics.rb

@@ -35,7 +35,7 @@ module Emails
     def epic_thread_options(sender_id, recipient_id, reason)
       {
         from: sender(sender_id),
-        to: recipient(recipient_id, @epic.group),
+        to: User.find(recipient_id).notification_email_for(@epic.group),
         subject: subject("#{@epic.title} (#{@epic.to_reference})"),
         'X-GitLab-NotificationReason' => reason
       }

ee/app/mailers/emails/reviews.rb

@@ -13,7 +13,7 @@ module Emails
     def review_thread_options(recipient_id)
       {
         from: sender(@author.id),
-        to: recipient(recipient_id, @merge_request.target_project.group),
+        to: User.find(recipient_id).notification_email_for(@merge_request.target_project.group),
         subject: subject("#{@merge_request.title} (#{@merge_request.to_reference})")
       }
     end