diff options
| -rw-r--r-- | app/services/notification_recipient_service.rb | 293 | ||||
| -rw-r--r-- | app/services/notification_service.rb | 323 | ||||
| -rw-r--r-- | spec/services/notification_service_spec.rb | 71 |
3 files changed, 382 insertions, 305 deletions
diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb new file mode 100644 index 00000000000..44ae23fad18 --- /dev/null +++ b/app/services/notification_recipient_service.rb @@ -0,0 +1,293 @@ +# +# Used by NotificationService to determine who should receive notification +# +class NotificationRecipientService + attr_reader :project + + def initialize(project) + @project = project + end + + def build_recipients(target, current_user, action: nil, previous_assignee: nil, skip_current_user: true) + custom_action = build_custom_key(action, target) + + recipients = target.participants(current_user) + + unless NotificationSetting::EXCLUDED_WATCHER_EVENTS.include?(custom_action) + recipients = add_project_watchers(recipients) + end + + recipients = add_custom_notifications(recipients, custom_action) + recipients = reject_mention_users(recipients) + + # Re-assign is considered as a mention of the new assignee so we add the + # new assignee to the list of recipients after we rejected users with + # the "on mention" notification level + if [:reassign_merge_request, :reassign_issue].include?(custom_action) + recipients << previous_assignee if previous_assignee + recipients << target.assignee + end + + recipients = reject_muted_users(recipients) + recipients = add_subscribed_users(recipients, target) + + if [:new_issue, :new_merge_request].include?(custom_action) + recipients = add_labels_subscribers(recipients, target) + end + + recipients = reject_unsubscribed_users(recipients, target) + recipients = reject_users_without_access(recipients, target) + + recipients.delete(current_user) if skip_current_user + + recipients.uniq + end + + def build_relabeled_recipients(target, current_user, labels:) + recipients = add_labels_subscribers([], target, labels: labels) + recipients = reject_unsubscribed_users(recipients, target) + recipients = reject_users_without_access(recipients, target) + recipients.delete(current_user) + recipients.uniq + end + + def build_new_note_recipients(note) + target = note.noteable + + ability, subject = if note.for_personal_snippet? + [:read_personal_snippet, note.noteable] + else + [:read_project, note.project] + end + + mentioned_users = note.mentioned_users.select { |user| user.can?(ability, subject) } + + # Add all users participating in the thread (author, assignee, comment authors) + recipients = + if target.respond_to?(:participants) + target.participants(note.author) + else + mentioned_users + end + + unless note.for_personal_snippet? + # Merge project watchers + recipients = add_project_watchers(recipients) + + # Merge project with custom notification + recipients = add_custom_notifications(recipients, :new_note) + end + + # Reject users with Mention notification level, except those mentioned in _this_ note. + recipients = reject_mention_users(recipients - mentioned_users) + recipients = recipients + mentioned_users + + recipients = reject_muted_users(recipients) + + recipients = add_subscribed_users(recipients, note.noteable) + recipients = reject_unsubscribed_users(recipients, note.noteable) + recipients = reject_users_without_access(recipients, note.noteable) + + recipients.delete(note.author) + recipients.uniq + end + + # Remove users with disabled notifications from array + # Also remove duplications and nil recipients + def reject_muted_users(users) + reject_users(users, :disabled) + end + + protected + + # Get project/group users with CUSTOM notification level + def add_custom_notifications(recipients, action) + user_ids = [] + + # Users with a notification setting on group or project + user_ids += user_ids_notifiable_on(project, :custom, action) + user_ids += user_ids_notifiable_on(project.group, :custom, action) + + # Users with global level custom + user_ids_with_project_level_global = user_ids_notifiable_on(project, :global) + user_ids_with_group_level_global = user_ids_notifiable_on(project.group, :global) + + global_users_ids = user_ids_with_project_level_global.concat(user_ids_with_group_level_global) + user_ids += user_ids_with_global_level_custom(global_users_ids, action) + + recipients.concat(User.find(user_ids)) + end + + def add_project_watchers(recipients) + recipients.concat(project_watchers).compact + end + + # Get project users with WATCH notification level + def project_watchers + project_members_ids = user_ids_notifiable_on(project) + + user_ids_with_project_global = user_ids_notifiable_on(project, :global) + user_ids_with_group_global = user_ids_notifiable_on(project.group, :global) + + 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_group_setting = select_group_members_ids(project.group, project_members_ids, user_ids_with_group_global, user_ids) + + User.where(id: user_ids_with_project_setting.concat(user_ids_with_group_setting).uniq).to_a + end + + # Remove users with notification level 'Mentioned' + def reject_mention_users(users) + reject_users(users, :mention) + end + + def add_subscribed_users(recipients, target) + return recipients unless target.respond_to? :subscribers + + recipients + target.subscribers(project) + end + + def user_ids_notifiable_on(resource, notification_level = nil, action = nil) + return [] unless resource + + if notification_level + settings = resource.notification_settings.where(level: NotificationSetting.levels[notification_level]) + settings = settings.select { |setting| setting.events[action] } if action.present? + settings.map(&:user_id) + else + resource.notification_settings.pluck(:user_id) + end + 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) + user_ids = user_ids_notifiable_on(project, :watch) + + # If project setting is global, add to watch list if global setting is watch + global_setting.each do |user_id| + if user_ids_global_level_watch.include?(user_id) + user_ids << user_id + end + end + + user_ids + end + + # Build a list of user_ids based on group notification settings + def select_group_members_ids(group, project_members, global_setting, user_ids_global_level_watch) + uids = user_ids_notifiable_on(group, :watch) + + # Group setting is watch, add to user_ids list if user is not project member + user_ids = [] + uids.each do |user_id| + if project_members.exclude?(user_id) + user_ids << user_id + end + end + + # Group setting is global, add to user_ids list if global setting is watch + global_setting.each do |user_id| + if project_members.exclude?(user_id) && user_ids_global_level_watch.include?(user_id) + user_ids << user_id + end + end + + user_ids + end + + def user_ids_with_global_level_watch(ids) + settings_with_global_level_of(:watch, ids).pluck(:user_id) + end + + def user_ids_with_global_level_custom(ids, action) + settings = settings_with_global_level_of(:custom, ids) + settings = settings.select { |setting| setting.events[action] } + settings.map(&:user_id) + end + + def settings_with_global_level_of(level, ids) + NotificationSetting.where( + user_id: ids, + source_type: nil, + level: NotificationSetting.levels[level] + ) + end + + # Reject users which has certain notification level + # + # Example: + # reject_users(users, :watch, project) + # + def reject_users(users, level) + level = level.to_s + + unless NotificationSetting.levels.keys.include?(level) + raise 'Invalid notification level' + end + + users = users.to_a.compact.uniq + users = users.select { |u| u.can?(:receive_notifications) } + + users.reject do |user| + global_notification_setting = user.global_notification_setting + + next global_notification_setting.level == level unless project + + setting = user.notification_settings_for(project) + + if project.group && (setting.nil? || setting.global?) + setting = user.notification_settings_for(project.group) + end + + # reject users who globally set mention notification and has no setting per project/group + next global_notification_setting.level == level unless setting + + # reject users who set mention notification in project + next true if setting.level == level + + # reject users who have mention level in project and disabled in global settings + setting.global? && global_notification_setting.level == level + end + end + + def reject_unsubscribed_users(recipients, target) + return recipients unless target.respond_to? :subscriptions + + recipients.reject do |user| + subscription = target.subscriptions.find_by_user_id(user.id) + subscription && !subscription.subscribed + end + end + + def reject_users_without_access(recipients, target) + ability = case target + when Issuable + :"read_#{target.to_ability_name}" + when Ci::Pipeline + :read_build # We have build trace in pipeline emails + end + + return recipients unless ability + + recipients.select do |user| + user.can?(ability, target) + end + end + + def add_labels_subscribers(recipients, target, labels: nil) + return recipients unless target.respond_to? :labels + + (labels || target.labels).each do |label| + recipients += label.subscribers(project) + end + + recipients + end + + # Build event key to search on custom notification level + # Check NotificationSetting::EMAIL_EVENTS + def build_custom_key(action, object) + "#{action}_#{object.class.model_name.name.underscore}".to_sym + end +end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index fdaba9b95fb..f9aa2346759 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -150,7 +150,10 @@ class NotificationService end def resolve_all_discussions(merge_request, current_user) - recipients = build_recipients(merge_request, merge_request.target_project, current_user, action: "resolve_all_discussions") + recipients = NotificationRecipientService.new(merge_request.target_project).build_recipients( + merge_request, + current_user, + action: "resolve_all_discussions") recipients.each do |recipient| mailer.resolved_all_discussions_email(recipient.id, merge_request.id, current_user.id).deliver_later @@ -164,64 +167,15 @@ class NotificationService end # Notify users on new note in system - # - # TODO: split on methods and refactor - # def new_note(note) return true unless note.noteable_type.present? # ignore gitlab service messages return true if note.cross_reference? && note.system? - target = note.noteable - - recipients = [] - - mentioned_users = note.mentioned_users - - ability, subject = if note.for_personal_snippet? - [:read_personal_snippet, note.noteable] - else - [:read_project, note.project] - end - - mentioned_users.select! do |user| - user.can?(ability, subject) - end - - # Add all users participating in the thread (author, assignee, comment authors) - participants = - if target.respond_to?(:participants) - target.participants(note.author) - else - mentioned_users - end - - recipients = recipients.concat(participants) - - unless note.for_personal_snippet? - # Merge project watchers - recipients = add_project_watchers(recipients, note.project) - - # Merge project with custom notification - recipients = add_custom_notifications(recipients, note.project, :new_note) - end - - # Reject users with Mention notification level, except those mentioned in _this_ note. - recipients = reject_mention_users(recipients - mentioned_users, note.project) - recipients = recipients + mentioned_users - - recipients = reject_muted_users(recipients, note.project) - - recipients = add_subscribed_users(recipients, note.project, note.noteable) - recipients = reject_unsubscribed_users(recipients, note.noteable) - recipients = reject_users_without_access(recipients, note.noteable) - - recipients.delete(note.author) - recipients = recipients.uniq - notify_method = "note_#{note.to_ability_name}_email".to_sym + recipients = NotificationRecipientService.new(note.project).build_new_note_recipients(note) recipients.each do |recipient| mailer.send(notify_method, recipient.id, note.id).deliver_later end @@ -290,7 +244,7 @@ class NotificationService def project_was_moved(project, old_path_with_namespace) recipients = project.team.members - recipients = reject_muted_users(recipients, project) + recipients = NotificationRecipientService.new(project).reject_muted_users(recipients) recipients.each do |recipient| mailer.project_was_moved_email( @@ -302,7 +256,7 @@ class NotificationService end def issue_moved(issue, new_issue, current_user) - recipients = build_recipients(issue, issue.project, current_user) + recipients = NotificationRecipientService.new(issue.project).build_recipients(issue, current_user) recipients.map do |recipient| email = mailer.issue_moved_email(recipient, issue, new_issue, current_user) @@ -324,9 +278,8 @@ class NotificationService return unless mailer.respond_to?(email_template) - recipients ||= build_recipients( + recipients ||= NotificationRecipientService.new(pipeline.project).build_recipients( pipeline, - pipeline.project, nil, # The acting user, who won't be added to recipients action: pipeline.status).map(&:notification_email) @@ -337,199 +290,8 @@ class NotificationService protected - # Get project/group users with CUSTOM notification level - def add_custom_notifications(recipients, project, action) - user_ids = [] - - # Users with a notification setting on group or project - user_ids += notification_settings_for(project, :custom, action) - user_ids += notification_settings_for(project.group, :custom, action) - - # Users with global level custom - users_with_project_level_global = notification_settings_for(project, :global) - users_with_group_level_global = notification_settings_for(project.group, :global) - - global_users_ids = users_with_project_level_global.concat(users_with_group_level_global) - user_ids += users_with_global_level_custom(global_users_ids, action) - - recipients.concat(User.find(user_ids)) - end - - # Get project users with WATCH notification level - def project_watchers(project) - project_members = notification_settings_for(project) - - users_with_project_level_global = notification_settings_for(project, :global) - users_with_group_level_global = notification_settings_for(project.group, :global) - - users = users_with_global_level_watch([users_with_project_level_global, users_with_group_level_global].flatten.uniq) - - users_with_project_setting = select_project_member_setting(project, users_with_project_level_global, users) - users_with_group_setting = select_group_member_setting(project.group, project_members, users_with_group_level_global, users) - - User.where(id: users_with_project_setting.concat(users_with_group_setting).uniq).to_a - end - - def notification_settings_for(resource, notification_level = nil, action = nil) - return [] unless resource - - if notification_level - settings = resource.notification_settings.where(level: NotificationSetting.levels[notification_level]) - settings = settings.select { |setting| setting.events[action] } if action.present? - settings.map(&:user_id) - else - resource.notification_settings.pluck(:user_id) - end - end - - def users_with_global_level_watch(ids) - settings_with_global_level_of(:watch, ids).pluck(:user_id) - end - - def users_with_global_level_custom(ids, action) - settings = settings_with_global_level_of(:custom, ids) - settings = settings.select { |setting| setting.events[action] } - settings.map(&:user_id) - end - - def settings_with_global_level_of(level, ids) - NotificationSetting.where( - user_id: ids, - source_type: nil, - level: NotificationSetting.levels[level] - ) - end - - # Build a list of users based on project notification settings - def select_project_member_setting(project, global_setting, users_global_level_watch) - users = notification_settings_for(project, :watch) - - # If project setting is global, add to watch list if global setting is watch - global_setting.each do |user_id| - if users_global_level_watch.include?(user_id) - users << user_id - end - end - - users - end - - # Build a list of users based on group notification settings - def select_group_member_setting(group, project_members, global_setting, users_global_level_watch) - uids = notification_settings_for(group, :watch) - - # Group setting is watch, add to users list if user is not project member - users = [] - uids.each do |user_id| - if project_members.exclude?(user_id) - users << user_id - end - end - - # Group setting is global, add to users list if global setting is watch - global_setting.each do |user_id| - if project_members.exclude?(user_id) && users_global_level_watch.include?(user_id) - users << user_id - end - end - - users - end - - def add_project_watchers(recipients, project) - recipients.concat(project_watchers(project)).compact - end - - # Remove users with disabled notifications from array - # Also remove duplications and nil recipients - def reject_muted_users(users, project = nil) - reject_users(users, :disabled, project) - end - - # Remove users with notification level 'Mentioned' - def reject_mention_users(users, project = nil) - reject_users(users, :mention, project) - end - - # Reject users which has certain notification level - # - # Example: - # reject_users(users, :watch, project) - # - def reject_users(users, level, project = nil) - level = level.to_s - - unless NotificationSetting.levels.keys.include?(level) - raise 'Invalid notification level' - end - - users = users.to_a.compact.uniq - users = users.select { |u| u.can?(:receive_notifications) } - - users.reject do |user| - global_notification_setting = user.global_notification_setting - - next global_notification_setting.level == level unless project - - setting = user.notification_settings_for(project) - - if project.group && (setting.nil? || setting.global?) - setting = user.notification_settings_for(project.group) - end - - # reject users who globally set mention notification and has no setting per project/group - next global_notification_setting.level == level unless setting - - # reject users who set mention notification in project - next true if setting.level == level - - # reject users who have mention level in project and disabled in global settings - setting.global? && global_notification_setting.level == level - end - end - - def reject_unsubscribed_users(recipients, target) - return recipients unless target.respond_to? :subscriptions - - recipients.reject do |user| - subscription = target.subscriptions.find_by_user_id(user.id) - subscription && !subscription.subscribed - end - end - - def reject_users_without_access(recipients, target) - ability = case target - when Issuable - :"read_#{target.to_ability_name}" - when Ci::Pipeline - :read_build # We have build trace in pipeline emails - end - - return recipients unless ability - - recipients.select do |user| - user.can?(ability, target) - end - end - - def add_subscribed_users(recipients, project, target) - return recipients unless target.respond_to? :subscribers - - recipients + target.subscribers(project) - end - - def add_labels_subscribers(recipients, project, target, labels: nil) - return recipients unless target.respond_to? :labels - - (labels || target.labels).each do |label| - recipients += label.subscribers(project) - end - - recipients - end - def new_resource_email(target, project, method) - recipients = build_recipients(target, project, target.author, action: "new") + recipients = NotificationRecipientService.new(project).build_recipients(target, target.author, action: "new") recipients.each do |recipient| mailer.send(method, recipient.id, target.id).deliver_later @@ -537,7 +299,7 @@ class NotificationService end def new_mentions_in_resource_email(target, project, new_mentioned_users, current_user, method) - recipients = build_recipients(target, project, current_user, action: "new") + recipients = NotificationRecipientService.new(project).build_recipients(target, current_user, action: "new") recipients = recipients & new_mentioned_users recipients.each do |recipient| @@ -548,9 +310,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 = build_recipients( + recipients = NotificationRecipientService.new(project).build_recipients( target, - project, current_user, action: action, skip_current_user: skip_current_user @@ -565,7 +326,12 @@ class NotificationService previous_assignee_id = previous_record(target, 'assignee_id') previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id - recipients = build_recipients(target, project, current_user, action: "reassign", previous_assignee: previous_assignee) + recipients = NotificationRecipientService.new(project).build_recipients( + target, + current_user, + action: "reassign", + previous_assignee: previous_assignee + ) recipients.each do |recipient| mailer.send( @@ -579,7 +345,7 @@ class NotificationService end def relabeled_resource_email(target, project, labels, current_user, method) - recipients = build_relabeled_recipients(target, project, current_user, labels: labels) + recipients = NotificationRecipientService.new(project).build_relabeled_recipients(target, current_user, labels: labels) label_names = labels.map(&:name) recipients.each do |recipient| @@ -588,58 +354,13 @@ class NotificationService end def reopen_resource_email(target, project, current_user, method, status) - recipients = build_recipients(target, project, current_user, action: "reopen") + recipients = NotificationRecipientService.new(project).build_recipients(target, current_user, action: "reopen") recipients.each do |recipient| mailer.send(method, recipient.id, target.id, status, current_user.id).deliver_later end end - def build_recipients(target, project, current_user, action: nil, previous_assignee: nil, skip_current_user: true) - custom_action = build_custom_key(action, target) - - recipients = target.participants(current_user) - - unless NotificationSetting::EXCLUDED_WATCHER_EVENTS.include?(custom_action) - recipients = add_project_watchers(recipients, project) - end - - recipients = add_custom_notifications(recipients, project, custom_action) - recipients = reject_mention_users(recipients, project) - - recipients = recipients.uniq - - # Re-assign is considered as a mention of the new assignee so we add the - # new assignee to the list of recipients after we rejected users with - # the "on mention" notification level - if [:reassign_merge_request, :reassign_issue].include?(custom_action) - recipients << previous_assignee if previous_assignee - recipients << target.assignee - end - - recipients = reject_muted_users(recipients, project) - recipients = add_subscribed_users(recipients, project, target) - - if [:new_issue, :new_merge_request].include?(custom_action) - recipients = add_labels_subscribers(recipients, project, target) - end - - recipients = reject_unsubscribed_users(recipients, target) - recipients = reject_users_without_access(recipients, target) - - recipients.delete(current_user) if skip_current_user - - recipients.uniq - end - - def build_relabeled_recipients(target, project, current_user, labels:) - recipients = add_labels_subscribers([], project, target, labels: labels) - recipients = reject_unsubscribed_users(recipients, target) - recipients = reject_users_without_access(recipients, target) - recipients.delete(current_user) - recipients.uniq - end - def mailer Notify end @@ -651,10 +372,4 @@ class NotificationService end end end - - # Build event key to search on custom notification level - # Check NotificationSetting::EMAIL_EVENTS - def build_custom_key(action, object) - "#{action}_#{object.class.model_name.name.underscore}".to_sym - end end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 82a4ec3f581..f7240969588 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -758,7 +758,7 @@ describe NotificationService, services: true do update_custom_notification(:reopen_issue, @u_custom_global) end - it 'sends email to issue assignee and issue author' do + it 'sends email to issue notification recipients' do notification.reopen_issue(issue, @u_disabled) should_email(issue.assignee) @@ -772,6 +772,7 @@ describe NotificationService, services: true do should_email(@watcher_and_subscriber) should_not_email(@unsubscriber) should_not_email(@u_participating) + should_not_email(@u_disabled) should_not_email(@u_lazy_participant) end @@ -781,6 +782,32 @@ describe NotificationService, services: true do let(:notification_trigger) { notification.reopen_issue(issue, @u_disabled) } end end + + describe '#issue_moved' do + let(:new_issue) { create(:issue) } + + it 'sends email to issue notification recipients' do + notification.issue_moved(issue, new_issue, @u_disabled) + + should_email(issue.assignee) + should_email(issue.author) + should_email(@u_watcher) + should_email(@u_guest_watcher) + should_email(@u_participant_mentioned) + should_email(@subscriber) + should_email(@watcher_and_subscriber) + should_not_email(@unsubscriber) + should_not_email(@u_participating) + should_not_email(@u_disabled) + should_not_email(@u_lazy_participant) + end + + it_behaves_like 'participating notifications' do + let(:participant) { create(:user, username: 'user-participant') } + let(:issuable) { issue } + let(:notification_trigger) { notification.issue_moved(issue, new_issue, @u_disabled) } + end + end end describe 'Merge Requests' do @@ -1192,6 +1219,48 @@ describe NotificationService, services: true do end end + describe 'Pipelines' do + describe '#pipeline_finished' do + let(:project) { create(:project, :public) } + let(:current_user) { create(:user) } + let(:u_member) { create(:user) } + let(:u_other) { create(:user) } + + let(:commit) { project.commit } + let(:pipeline) do + create(:ci_pipeline, :success, + project: project, + user: current_user, + ref: 'refs/heads/master', + sha: commit.id, + before_sha: '00000000') + end + + before do + project.add_master(current_user) + project.add_master(u_member) + reset_delivered_emails! + end + + context 'without custom recipients' do + it 'notifies the pipeline user' do + notification.pipeline_finished(pipeline) + + should_only_email(current_user, kind: :bcc) + end + end + + context 'with custom recipients' do + it 'notifies the custom recipients' do + users = [u_member, u_other] + notification.pipeline_finished(pipeline, users.map(&:notification_email)) + + should_only_email(*users, kind: :bcc) + end + end + end + end + def build_team(project) @u_watcher = create_global_setting_for(create(:user), :watch) @u_participating = create_global_setting_for(create(:user), :participating) |