Skip to content

G
gitlab-ce
Commit 90ade76b authored by Dongqing Hu's avatar Dongqing Hu Committed by Rémy Coutable
Browse files
Options

Resolve "Extract logic of who should receive notification into separate classes"

parent bbddd770
Hide whitespace changes
Inline Side-by-side
Showing with 382 additions and 305 deletions
app/services/notification_recipient_service.rb 0 → 100644
View file @ 90ade76b
#
# 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
app/services/notification_service.rb
View file @ 90ade76b
......@@ -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
spec/services/notification_service_spec.rb
View file @ 90ade76b
......@@ -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)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7