diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb index 97ff1f99f0252d6c09b6cef953da96658afd6ca0..b36bc90c8849f94418b3b64fc37fb1c227a3368f 100644 --- a/app/services/notification_recipient_service.rb +++ b/app/services/notification_recipient_service.rb @@ -44,6 +44,10 @@ module NotificationRecipientService raise 'abstract' end + def project + target.project + end + def recipients @recipients ||= [] end @@ -138,7 +142,7 @@ module NotificationRecipientService user_ids = user_ids_with_global_level_watch((user_ids_with_project_global + user_ids_with_group_global).uniq) - user_ids_with_project_setting = select_project_members_ids(project, user_ids_with_project_global, user_ids) + user_ids_with_project_setting = select_project_members_ids(user_ids_with_project_global, user_ids) user_ids_with_group_setting = select_group_members_ids(project.group, project_members_ids, user_ids_with_group_global, user_ids) user_scope.where(id: user_ids_with_project_setting.concat(user_ids_with_group_setting).uniq) @@ -163,7 +167,7 @@ module NotificationRecipientService end # Build a list of user_ids based on project notification settings - def select_project_members_ids(project, global_setting, user_ids_global_level_watch) + def select_project_members_ids(global_setting, user_ids_global_level_watch) user_ids = user_ids_notifiable_on(project, :watch) # If project setting is global, add to watch list if global setting is watch @@ -230,14 +234,12 @@ module NotificationRecipientService end class Default < Base - attr_reader :project attr_reader :target attr_reader :current_user attr_reader :action attr_reader :previous_assignee attr_reader :skip_current_user - def initialize(project, target, current_user, action:, previous_assignee: nil, skip_current_user: true) - @project = project + def initialize(target, current_user, action:, previous_assignee: nil, skip_current_user: true) @target = target @current_user = current_user @action = action @@ -280,12 +282,10 @@ module NotificationRecipientService end class Relabeled < Base - attr_reader :project attr_reader :target attr_reader :current_user attr_reader :labels - def initialize(project, target, current_user, labels:) - @project = project + def initialize(target, current_user, labels:) @target = target @current_user = current_user @labels = labels @@ -297,13 +297,17 @@ module NotificationRecipientService end class NewNote < Base - attr_reader :project attr_reader :note - attr_reader :target - def initialize(project, note) - @project = project + def initialize(note) @note = note - @target = note.noteable + end + + def target + note.noteable + end + + def project + note.project end def read_ability diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index f5366b9ceabde4e3e0857983ba2a8077afe81e1e..c93f82999dc5d016148fca96ab9ae1ff796f0deb 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -42,7 +42,7 @@ class NotificationService # * users with custom level checked with "new issue" # def new_issue(issue, current_user) - new_resource_email(issue, issue.project, :new_issue_email) + new_resource_email(issue, :new_issue_email) end # When issue text is updated, we should send an email to: @@ -52,7 +52,6 @@ class NotificationService def new_mentions_in_issue(issue, new_mentioned_users, current_user) new_mentions_in_resource_email( issue, - issue.project, new_mentioned_users, current_user, :new_mention_in_issue_email @@ -67,7 +66,7 @@ class NotificationService # * users with custom level checked with "close issue" # def close_issue(issue, current_user) - close_resource_email(issue, issue.project, current_user, :closed_issue_email) + close_resource_email(issue, current_user, :closed_issue_email) end # When we reassign an issue we should send an email to: @@ -78,7 +77,6 @@ class NotificationService # def reassigned_issue(issue, current_user, previous_assignees = []) recipients = NotificationRecipientService.build_recipients( - issue.project, issue, current_user, action: "reassign", @@ -103,7 +101,7 @@ class NotificationService # * watchers of the issue's labels # def relabeled_issue(issue, added_labels, current_user) - relabeled_resource_email(issue, issue.project, added_labels, current_user, :relabeled_issue_email) + relabeled_resource_email(issue, added_labels, current_user, :relabeled_issue_email) end # When create a merge request we should send an email to: @@ -114,7 +112,7 @@ class NotificationService # * users with custom level checked with "new merge request" # def new_merge_request(merge_request, current_user) - new_resource_email(merge_request, merge_request.target_project, :new_merge_request_email) + new_resource_email(merge_request, :new_merge_request_email) end # When merge request text is updated, we should send an email to: @@ -124,7 +122,6 @@ class NotificationService def new_mentions_in_merge_request(merge_request, new_mentioned_users, current_user) new_mentions_in_resource_email( merge_request, - merge_request.target_project, new_mentioned_users, current_user, :new_mention_in_merge_request_email @@ -138,7 +135,7 @@ class NotificationService # * users with custom level checked with "reassign merge request" # def reassigned_merge_request(merge_request, current_user) - reassign_resource_email(merge_request, merge_request.target_project, current_user, :reassigned_merge_request_email) + reassign_resource_email(merge_request, current_user, :reassigned_merge_request_email) end # When we add labels to a merge request we should send an email to: @@ -146,21 +143,20 @@ class NotificationService # * watchers of the mr's labels # def relabeled_merge_request(merge_request, added_labels, current_user) - relabeled_resource_email(merge_request, merge_request.target_project, added_labels, current_user, :relabeled_merge_request_email) + relabeled_resource_email(merge_request, added_labels, current_user, :relabeled_merge_request_email) end def close_mr(merge_request, current_user) - close_resource_email(merge_request, merge_request.target_project, current_user, :closed_merge_request_email) + close_resource_email(merge_request, current_user, :closed_merge_request_email) end def reopen_issue(issue, current_user) - reopen_resource_email(issue, issue.project, current_user, :issue_status_changed_email, 'reopened') + reopen_resource_email(issue, current_user, :issue_status_changed_email, 'reopened') end def merge_mr(merge_request, current_user) close_resource_email( merge_request, - merge_request.target_project, current_user, :merged_merge_request_email, skip_current_user: !merge_request.merge_when_pipeline_succeeds? @@ -170,7 +166,6 @@ class NotificationService def reopen_mr(merge_request, current_user) reopen_resource_email( merge_request, - merge_request.target_project, current_user, :merge_request_status_email, 'reopened' @@ -179,7 +174,6 @@ class NotificationService def resolve_all_discussions(merge_request, current_user) recipients = NotificationRecipientService.build_recipients( - merge_request.target_project, merge_request, current_user, action: "resolve_all_discussions") @@ -204,7 +198,7 @@ class NotificationService notify_method = "note_#{note.to_ability_name}_email".to_sym - recipients = NotificationRecipientService.build_new_note_recipients(note.project, note) + recipients = NotificationRecipientService.build_new_note_recipients(note) recipients.each do |recipient| mailer.send(notify_method, recipient.id, note.id).deliver_later end @@ -284,7 +278,7 @@ class NotificationService end def issue_moved(issue, new_issue, current_user) - recipients = NotificationRecipientService.build_recipients(issue.project, issue, current_user, action: 'moved') + recipients = NotificationRecipientService.build_recipients(issue, current_user, action: 'moved') recipients.map do |recipient| email = mailer.issue_moved_email(recipient, issue, new_issue, current_user) @@ -320,16 +314,16 @@ class NotificationService protected - def new_resource_email(target, project, method) - recipients = NotificationRecipientService.build_recipients(project, target, target.author, action: "new") + def new_resource_email(target, method) + recipients = NotificationRecipientService.build_recipients(target, target.author, action: "new") recipients.each do |recipient| mailer.send(method, recipient.id, target.id).deliver_later end end - def new_mentions_in_resource_email(target, project, new_mentioned_users, current_user, method) - recipients = NotificationRecipientService.build_recipients(project, target, current_user, action: "new") + def new_mentions_in_resource_email(target, new_mentioned_users, current_user, method) + recipients = NotificationRecipientService.build_recipients(target, current_user, action: "new") recipients = recipients & new_mentioned_users recipients.each do |recipient| @@ -337,11 +331,10 @@ class NotificationService end end - def close_resource_email(target, project, current_user, method, skip_current_user: true) + def close_resource_email(target, current_user, method, skip_current_user: true) action = method == :merged_merge_request_email ? "merge" : "close" recipients = NotificationRecipientService.build_recipients( - project, target, current_user, action: action, @@ -353,12 +346,11 @@ class NotificationService end end - def reassign_resource_email(target, project, current_user, method) + def reassign_resource_email(target, current_user, method) previous_assignee_id = previous_record(target, 'assignee_id') previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id recipients = NotificationRecipientService.build_recipients( - project, target, current_user, action: "reassign", @@ -376,8 +368,8 @@ class NotificationService end end - def relabeled_resource_email(target, project, labels, current_user, method) - recipients = NotificationRecipientService.build_relabeled_recipients(project, target, current_user, labels: labels) + def relabeled_resource_email(target, labels, current_user, method) + recipients = NotificationRecipientService.build_relabeled_recipients(target, current_user, labels: labels) label_names = labels.map(&:name) recipients.each do |recipient| @@ -385,8 +377,8 @@ class NotificationService end end - def reopen_resource_email(target, project, current_user, method, status) - recipients = NotificationRecipientService.build_recipients(project, target, current_user, action: "reopen") + def reopen_resource_email(target, current_user, method, status) + recipients = NotificationRecipientService.build_recipients(target, current_user, action: "reopen") recipients.each do |recipient| mailer.send(method, recipient.id, target.id, status, current_user.id).deliver_later