move the #build_* methods to static, parameterize the project

2 files changed, 40 insertions, 41 deletions

diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb
index 3d36660495f..788e06502ee 100644
--- a/app/services/notification_recipient_service.rb
+++ b/app/services/notification_recipient_service.rb
@@ -1,23 +1,21 @@
 #
 # Used by NotificationService to determine who should receive notification
 #
-class NotificationRecipientService
-  attr_reader :project
-
-  def self.notification_setting_for_user_project(user, project)
-    project_setting = project && user.notification_settings_for(project)
-
-    return project_setting unless project_setting.nil? || project_setting.global?
-
-    group_setting = project&.group && user.notification_settings_for(project.group)
+module NotificationRecipientService
+  def self.notifiable_users(*a)
+    Recipient.notifiable_users(*a)
+  end
 
-    return group_setting unless group_setting.nil? || group_setting.global?
+  def self.build_recipients(*a)
+    Builder::Default.new(*a).recipient_users
+  end
 
-    user.global_notification_setting
+  def self.build_relabeled_recipients(*a)
+    Builder::Relabeled.new(*a).recipient_users
   end
 
-  def initialize(project)
-    @project = project
+  def self.build_new_note_recipients(*a)
+    Builder::NewNote.new(*a).recipient_users
   end
 
   class Recipient
@@ -38,8 +36,7 @@ class NotificationRecipientService
     end
 
     def notification_setting
-      @notification_setting ||=
-        NotificationRecipientService.notification_setting_for_user_project(user, @project)
+      @notification_setting ||= find_notification_setting
     end
 
     def raw_notification_level
@@ -111,6 +108,20 @@ class NotificationRecipientService
 
       NotificationSetting::EXCLUDED_WATCHER_EVENTS.include?(@custom_action)
     end
+
+    private
+
+    def find_notification_setting
+      project_setting = project && user.notification_settings_for(project)
+
+      return project_setting unless project_setting.nil? || project_setting.global?
+
+      group_setting = project&.group && user.notification_settings_for(project.group)
+
+      return group_setting unless group_setting.nil? || group_setting.global?
+
+      user.global_notification_setting
+    end
   end
 
   module Builder
@@ -442,20 +453,4 @@ class NotificationRecipientService
       end
     end
   end
-
-  def self.notifiable_users(*a)
-    Recipient.notifiable_users(*a)
-  end
-
-  def build_recipients(*a)
-    Builder::Default.new(@project, *a).recipient_users
-  end
-
-  def build_relabeled_recipients(*a)
-    Builder::Relabeled.new(@project, *a).recipient_users
-  end
-
-  def build_new_note_recipients(*a)
-    Builder::NewNote.new(@project, *a).recipient_users
-  end
 end
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 8a6ed88923d..675b4536a26 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -77,7 +77,8 @@ class NotificationService
   #  * users with custom level checked with "reassign issue"
   #
   def reassigned_issue(issue, current_user, previous_assignees = [])
-    recipients = NotificationRecipientService.new(issue.project).build_recipients(
+    recipients = NotificationRecipientService.build_recipients(
+      issue.project,
       issue,
       current_user,
       action: "reassign",
@@ -177,7 +178,8 @@ class NotificationService
   end
 
   def resolve_all_discussions(merge_request, current_user)
-    recipients = NotificationRecipientService.new(merge_request.target_project).build_recipients(
+    recipients = NotificationRecipientService.build_recipients(
+      merge_request.target_project,
       merge_request,
       current_user,
       action: "resolve_all_discussions")
@@ -202,7 +204,7 @@ class NotificationService
 
     notify_method = "note_#{note.to_ability_name}_email".to_sym
 
-    recipients = NotificationRecipientService.new(note.project).build_new_note_recipients(note)
+    recipients = NotificationRecipientService.build_new_note_recipients(note.project, note)
     recipients.each do |recipient|
       mailer.send(notify_method, recipient.id, note.id).deliver_later
     end
@@ -282,7 +284,7 @@ class NotificationService
   end
 
   def issue_moved(issue, new_issue, current_user)
-    recipients = NotificationRecipientService.new(issue.project).build_recipients(issue, current_user, action: 'moved')
+    recipients = NotificationRecipientService.build_recipients(issue.project, issue, current_user, action: 'moved')
 
     recipients.map do |recipient|
       email = mailer.issue_moved_email(recipient, issue, new_issue, current_user)
@@ -317,7 +319,7 @@ class NotificationService
   protected
 
   def new_resource_email(target, project, method)
-    recipients = NotificationRecipientService.new(project).build_recipients(target, target.author, action: "new")
+    recipients = NotificationRecipientService.build_recipients(project, target, target.author, action: "new")
 
     recipients.each do |recipient|
       mailer.send(method, recipient.id, target.id).deliver_later
@@ -325,7 +327,7 @@ class NotificationService
   end
 
   def new_mentions_in_resource_email(target, project, new_mentioned_users, current_user, method)
-    recipients = NotificationRecipientService.new(project).build_recipients(target, current_user, action: "new")
+    recipients = NotificationRecipientService.build_recipients(project, target, current_user, action: "new")
     recipients = recipients & new_mentioned_users
 
     recipients.each do |recipient|
@@ -336,7 +338,8 @@ class NotificationService
   def close_resource_email(target, project, current_user, method, skip_current_user: true)
     action = method == :merged_merge_request_email ? "merge" : "close"
 
-    recipients = NotificationRecipientService.new(project).build_recipients(
+    recipients = NotificationRecipientService.build_recipients(
+      project,
       target,
       current_user,
       action: action,
@@ -352,7 +355,8 @@ class NotificationService
     previous_assignee_id = previous_record(target, 'assignee_id')
     previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id
 
-    recipients = NotificationRecipientService.new(project).build_recipients(
+    recipients = NotificationRecipientService.build_recipients(
+      project,
       target,
       current_user,
       action: "reassign",
@@ -371,7 +375,7 @@ class NotificationService
   end
 
   def relabeled_resource_email(target, project, labels, current_user, method)
-    recipients = NotificationRecipientService.new(project).build_relabeled_recipients(target, current_user, labels: labels)
+    recipients = NotificationRecipientService.build_relabeled_recipients(project, target, current_user, labels: labels)
     label_names = labels.map(&:name)
 
     recipients.each do |recipient|
@@ -380,7 +384,7 @@ class NotificationService
   end
 
   def reopen_resource_email(target, project, current_user, method, status)
-    recipients = NotificationRecipientService.new(project).build_recipients(target, current_user, action: "reopen")
+    recipients = NotificationRecipientService.build_recipients(project, target, current_user, action: "reopen")
 
     recipients.each do |recipient|
       mailer.send(method, recipient.id, target.id, status, current_user.id).deliver_later