Merge branch 'fix-extra-emails-for-custom-notifications' into 'master'
Fix extra emails for custom notifications Closes #56861 See merge request gitlab-org/gitlab-ce!25607
This commit is contained in:
commit
0de554edc9
|
@ -47,14 +47,14 @@ class NotificationRecipient
|
|||
|
||||
def suitable_notification_level?
|
||||
case notification_level
|
||||
when :disabled, nil
|
||||
false
|
||||
when :custom
|
||||
custom_enabled? || %i[participating mention].include?(@type)
|
||||
when :watch, :participating
|
||||
!action_excluded?
|
||||
when :mention
|
||||
@type == :mention
|
||||
when :participating
|
||||
!excluded_participating_action? && %i[participating mention watch].include?(@type)
|
||||
when :custom
|
||||
custom_enabled? || %i[participating mention].include?(@type)
|
||||
when :watch
|
||||
!excluded_watcher_action?
|
||||
else
|
||||
false
|
||||
end
|
||||
|
@ -100,18 +100,14 @@ class NotificationRecipient
|
|||
end
|
||||
end
|
||||
|
||||
def action_excluded?
|
||||
excluded_watcher_action? || excluded_participating_action?
|
||||
end
|
||||
|
||||
def excluded_watcher_action?
|
||||
return false unless @custom_action && notification_level == :watch
|
||||
return false unless @custom_action
|
||||
|
||||
NotificationSetting::EXCLUDED_WATCHER_EVENTS.include?(@custom_action)
|
||||
end
|
||||
|
||||
def excluded_participating_action?
|
||||
return false unless @custom_action && notification_level == :participating
|
||||
return false unless @custom_action
|
||||
|
||||
NotificationSetting::EXCLUDED_PARTICIPATING_EVENTS.include?(@custom_action)
|
||||
end
|
||||
|
|
|
@ -135,7 +135,7 @@ module NotificationRecipientService
|
|||
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, custom_action)
|
||||
|
||||
add_recipients(user_scope.where(id: user_ids), :watch, nil)
|
||||
add_recipients(user_scope.where(id: user_ids), :custom, nil)
|
||||
end
|
||||
# rubocop: enable CodeReuse/ActiveRecord
|
||||
|
||||
|
@ -391,7 +391,7 @@ module NotificationRecipientService
|
|||
def build!
|
||||
return [] unless project
|
||||
|
||||
add_recipients(project.team.maintainers, :watch, nil)
|
||||
add_recipients(project.team.maintainers, :mention, nil)
|
||||
end
|
||||
|
||||
def acting_user
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Fix extra emails for custom notifications
|
||||
merge_request: 25607
|
||||
author:
|
||||
type: fixed
|
|
@ -643,6 +643,64 @@ describe NotificationService, :mailer do
|
|||
end
|
||||
end
|
||||
|
||||
describe 'Participating project notification settings have priority over group and global settings if available' do
|
||||
let!(:group) { create(:group) }
|
||||
let!(:maintainer) { group.add_owner(create(:user, username: 'maintainer')).user }
|
||||
let!(:user1) { group.add_developer(create(:user, username: 'user_with_project_and_custom_setting')).user }
|
||||
|
||||
let(:project) { create(:project, :public, namespace: group) }
|
||||
let(:issue) { create :issue, project: project, assignees: [assignee], description: '' }
|
||||
|
||||
before do
|
||||
reset_delivered_emails!
|
||||
|
||||
create_notification_setting(user1, project, :participating)
|
||||
end
|
||||
|
||||
context 'custom on group' do
|
||||
[nil, true].each do |new_issue_value|
|
||||
value_caption = new_issue_value || 'nil'
|
||||
it "does not send an email to user1 when a new issue is created and new_issue is set to #{value_caption}" do
|
||||
update_custom_notification(:new_issue, user1, resource: group, value: new_issue_value)
|
||||
|
||||
notification.new_issue(issue, maintainer)
|
||||
should_not_email(user1)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'watch on group' do
|
||||
it 'does not send an email' do
|
||||
user1.notification_settings_for(group).update!(level: :watch)
|
||||
|
||||
notification.new_issue(issue, maintainer)
|
||||
should_not_email(user1)
|
||||
end
|
||||
end
|
||||
|
||||
context 'custom on global, global on group' do
|
||||
it 'does not send an email' do
|
||||
user1.notification_settings_for(nil).update!(level: :custom)
|
||||
|
||||
user1.notification_settings_for(group).update!(level: :global)
|
||||
|
||||
notification.new_issue(issue, maintainer)
|
||||
should_not_email(user1)
|
||||
end
|
||||
end
|
||||
|
||||
context 'watch on global, global on group' do
|
||||
it 'does not send an email' do
|
||||
user1.notification_settings_for(nil).update!(level: :watch)
|
||||
|
||||
user1.notification_settings_for(group).update!(level: :global)
|
||||
|
||||
notification.new_issue(issue, maintainer)
|
||||
should_not_email(user1)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe 'Issues' do
|
||||
let(:group) { create(:group) }
|
||||
let(:project) { create(:project, :public, namespace: group) }
|
||||
|
@ -660,7 +718,7 @@ describe NotificationService, :mailer do
|
|||
end
|
||||
|
||||
describe '#new_issue' do
|
||||
it do
|
||||
it 'notifies the expected users' do
|
||||
notification.new_issue(issue, @u_disabled)
|
||||
|
||||
should_email(assignee)
|
||||
|
@ -1639,7 +1697,7 @@ describe NotificationService, :mailer do
|
|||
end
|
||||
|
||||
describe '#project_was_moved' do
|
||||
it do
|
||||
it 'notifies the expected users' do
|
||||
notification.project_was_moved(project, "gitlab/gitlab")
|
||||
|
||||
should_email(@u_watcher)
|
||||
|
|
|
@ -16,7 +16,9 @@ module EmailHelpers
|
|||
end
|
||||
|
||||
def should_email(user, times: 1, recipients: email_recipients)
|
||||
expect(sent_to_user(user, recipients: recipients)).to eq(times)
|
||||
amount = sent_to_user(user, recipients: recipients)
|
||||
failed_message = lambda { "User #{user.username} (#{user.id}): email test failed (expected #{times}, got #{amount})" }
|
||||
expect(amount).to eq(times), failed_message
|
||||
end
|
||||
|
||||
def should_not_email(user, recipients: email_recipients)
|
||||
|
|
|
@ -17,11 +17,15 @@ module NotificationHelpers
|
|||
|
||||
def create_user_with_notification(level, username, resource = project)
|
||||
user = create(:user, username: username)
|
||||
create_notification_setting(user, resource, level)
|
||||
|
||||
user
|
||||
end
|
||||
|
||||
def create_notification_setting(user, resource, level)
|
||||
setting = user.notification_settings_for(resource)
|
||||
setting.level = level
|
||||
setting.save
|
||||
|
||||
user
|
||||
end
|
||||
|
||||
# Create custom notifications
|
||||
|
|
Loading…
Reference in New Issue