diff options
author | Douwe Maan <douwe@gitlab.com> | 2016-06-13 11:52:39 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2016-06-13 11:52:39 +0000 |
commit | c03f12590440c4071151d0097299c3f3dd50c5c7 (patch) | |
tree | c2a562e1dd53d19f3a9b685496213b646e1461b5 | |
parent | 65df6bcb898e067e380658431136b5ef9aaba3b0 (diff) | |
parent | 39ead205de72461e86db07525922f2fab5fff2a9 (diff) | |
download | gitlab-ce-c03f12590440c4071151d0097299c3f3dd50c5c7.tar.gz |
Merge branch 'issue_3359_2' into 'master'
Remove notification level from user model
part of #3359
See merge request !4494
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/controllers/profiles/notifications_controller.rb | 23 | ||||
-rw-r--r-- | app/helpers/notifications_helper.rb | 19 | ||||
-rw-r--r-- | app/models/notification_setting.rb | 1 | ||||
-rw-r--r-- | app/models/user.rb | 21 | ||||
-rw-r--r-- | app/services/notification_service.rb | 18 | ||||
-rw-r--r-- | app/views/profiles/notifications/_group_settings.html.haml | 2 | ||||
-rw-r--r-- | app/views/profiles/notifications/_project_settings.html.haml | 2 | ||||
-rw-r--r-- | app/views/profiles/notifications/show.html.haml | 28 | ||||
-rw-r--r-- | db/migrate/20160610140403_remove_notification_setting_not_null_constraints.rb | 11 | ||||
-rw-r--r-- | db/migrate/20160610201627_migrate_users_notification_level.rb | 21 | ||||
-rw-r--r-- | db/migrate/20160610301627_remove_notification_level_from_users.rb | 7 | ||||
-rw-r--r-- | spec/models/notification_setting_spec.rb | 1 | ||||
-rw-r--r-- | spec/services/notification_service_spec.rb | 301 |
14 files changed, 392 insertions, 64 deletions
diff --git a/CHANGELOG b/CHANGELOG index 364690286e1..549b8ea707d 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -57,6 +57,7 @@ v 8.9.0 (unreleased) - Improve error handling importing projects - Remove duplicated notification settings - Put project Files and Commits tabs under Code tab + - Decouple global notification level from user model - Replace Colorize with Rainbow for coloring console output in Rake tasks. - Add workhorse controller and API helpers - An indicator is now displayed at the top of the comment field for confidential issues. diff --git a/app/controllers/profiles/notifications_controller.rb b/app/controllers/profiles/notifications_controller.rb index 18ee55c839a..40d1906a53f 100644 --- a/app/controllers/profiles/notifications_controller.rb +++ b/app/controllers/profiles/notifications_controller.rb @@ -1,12 +1,13 @@ class Profiles::NotificationsController < Profiles::ApplicationController def show - @user = current_user - @group_notifications = current_user.notification_settings.for_groups - @project_notifications = current_user.notification_settings.for_projects + @user = current_user + @group_notifications = current_user.notification_settings.for_groups + @project_notifications = current_user.notification_settings.for_projects + @global_notification_setting = current_user.global_notification_setting end def update - if current_user.update_attributes(user_params) + if current_user.update_attributes(user_params) && update_notification_settings flash[:notice] = "Notification settings saved" else flash[:alert] = "Failed to save new settings" @@ -16,6 +17,18 @@ class Profiles::NotificationsController < Profiles::ApplicationController end def user_params - params.require(:user).permit(:notification_email, :notification_level) + params.require(:user).permit(:notification_email) + end + + def global_notification_setting_params + params.require(:global_notification_setting).permit(:level) + end + + private + + def update_notification_settings + return true unless global_notification_setting_params + + current_user.global_notification_setting.update_attributes(global_notification_setting_params) end end diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index b8e64b3890a..50c21fc0d49 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -61,4 +61,23 @@ module NotificationsHelper end end end + + def notification_level_radio_buttons + html = "" + + NotificationSetting.levels.each_key do |level| + level = level.to_sym + next if level == :global + + html << content_tag(:div, class: "radio") do + content_tag(:label, { value: level }) do + radio_button_tag(:"global_notification_setting[level]", level, @global_notification_setting.level.to_sym == level) + + content_tag(:div, level.to_s.capitalize, class: "level-title") + + content_tag(:p, notification_description(level)) + end + end + end + + html.html_safe + end end diff --git a/app/models/notification_setting.rb b/app/models/notification_setting.rb index 17fb15b08df..0ce87968e46 100644 --- a/app/models/notification_setting.rb +++ b/app/models/notification_setting.rb @@ -7,7 +7,6 @@ class NotificationSetting < ActiveRecord::Base belongs_to :source, polymorphic: true validates :user, presence: true - validates :source, presence: true validates :level, presence: true validates :user_id, uniqueness: { scope: [:source_type, :source_id], message: "already exists in source", diff --git a/app/models/user.rb b/app/models/user.rb index e0987e07e1f..7afbfbf112a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -10,6 +10,8 @@ class User < ActiveRecord::Base include CaseSensitivity include TokenAuthenticatable + DEFAULT_NOTIFICATION_LEVEL = :participating + add_authentication_token_field :authentication_token default_value_for :admin, false @@ -99,7 +101,6 @@ class User < ActiveRecord::Base presence: true, uniqueness: { case_sensitive: false } - validates :notification_level, presence: true validate :namespace_uniq, if: ->(user) { user.username_changed? } validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? } validate :unique_email, if: ->(user) { user.email_changed? } @@ -133,13 +134,6 @@ class User < ActiveRecord::Base # Note: When adding an option, it MUST go on the end of the array. enum project_view: [:readme, :activity, :files] - # Notification level - # Note: When adding an option, it MUST go on the end of the array. - # - # TODO: Add '_prefix: :notification' to enum when update to Rails 5. https://github.com/rails/rails/pull/19813 - # Because user.notification_disabled? is much better than user.disabled? - enum notification_level: [:disabled, :participating, :watch, :global, :mention] - alias_attribute :private_token, :authentication_token delegate :path, to: :namespace, allow_nil: true, prefix: true @@ -800,6 +794,17 @@ class User < ActiveRecord::Base notification_settings.find_or_initialize_by(source: source) end + # Lazy load global notification setting + # Initializes User setting with Participating level if setting not persisted + def global_notification_setting + return @global_notification_setting if defined?(@global_notification_setting) + + @global_notification_setting = notification_settings.find_or_initialize_by(source: nil) + @global_notification_setting.update_attributes(level: NotificationSetting.levels[DEFAULT_NOTIFICATION_LEVEL]) unless @global_notification_setting.persisted? + + @global_notification_setting + end + def assigned_open_merge_request_count(force: false) Rails.cache.fetch(['users', id, 'assigned_open_merge_request_count'], force: force) do assigned_merge_requests.opened.count diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 91ca82ed3b7..875a3f4fab6 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -279,10 +279,11 @@ class NotificationService end def users_with_global_level_watch(ids) - User.where( - id: ids, - notification_level: NotificationSetting.levels[:watch] - ).pluck(:id) + NotificationSetting.where( + user_id: ids, + source_type: nil, + level: NotificationSetting.levels[:watch] + ).pluck(:user_id) end # Build a list of users based on project notifcation settings @@ -352,7 +353,9 @@ class NotificationService users = users.reject(&:blocked?) users.reject do |user| - next user.notification_level == level unless project + global_notification_setting = user.global_notification_setting + + next global_notification_setting.level == level unless project setting = user.notification_settings_for(project) @@ -361,13 +364,13 @@ class NotificationService end # reject users who globally set mention notification and has no setting per project/group - next user.notification_level == level unless setting + 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? && user.notification_level == level + setting.global? && global_notification_setting.level == level end end @@ -456,7 +459,6 @@ class NotificationService def build_recipients(target, project, current_user, action: nil, previous_assignee: nil) recipients = target.participants(current_user) - recipients = add_project_watchers(recipients, project) recipients = reject_mention_users(recipients, project) diff --git a/app/views/profiles/notifications/_group_settings.html.haml b/app/views/profiles/notifications/_group_settings.html.haml index 89ae7ffda2b..f0cf82afe83 100644 --- a/app/views/profiles/notifications/_group_settings.html.haml +++ b/app/views/profiles/notifications/_group_settings.html.haml @@ -1,7 +1,7 @@ %li.notification-list-item %span.notification.fa.fa-holder.append-right-5 - if setting.global? - = notification_icon(current_user.notification_level) + = notification_icon(current_user.global_notification_setting.level) - else = notification_icon(setting.level) diff --git a/app/views/profiles/notifications/_project_settings.html.haml b/app/views/profiles/notifications/_project_settings.html.haml index 17c097154da..e0fad555c09 100644 --- a/app/views/profiles/notifications/_project_settings.html.haml +++ b/app/views/profiles/notifications/_project_settings.html.haml @@ -1,7 +1,7 @@ %li.notification-list-item %span.notification.fa.fa-holder.append-right-5 - if setting.global? - = notification_icon(current_user.notification_level) + = notification_icon(current_user.global_notification_setting.level) - else = notification_icon(setting.level) diff --git a/app/views/profiles/notifications/show.html.haml b/app/views/profiles/notifications/show.html.haml index 7696f112bb3..f2659ac14b5 100644 --- a/app/views/profiles/notifications/show.html.haml +++ b/app/views/profiles/notifications/show.html.haml @@ -26,33 +26,7 @@ = f.select :notification_email, @user.all_emails, { include_blank: false }, class: "select2" .form-group = f.label :notification_level, class: 'label-light' - .radio - = f.label :notification_level, value: :disabled do - = f.radio_button :notification_level, :disabled - .level-title - Disabled - %p You will not get any notifications via email - - .radio - = f.label :notification_level, value: :mention do - = f.radio_button :notification_level, :mention - .level-title - On Mention - %p You will receive notifications only for comments in which you were @mentioned - - .radio - = f.label :notification_level, value: :participating do - = f.radio_button :notification_level, :participating - .level-title - Participating - %p You will only receive notifications from related resources (e.g. from your commits or assigned issues) - - .radio - = f.label :notification_level, value: :watch do - = f.radio_button :notification_level, :watch - .level-title - Watch - %p You will receive notifications for any activity + = notification_level_radio_buttons .prepend-top-default = f.submit 'Update settings', class: "btn btn-create" diff --git a/db/migrate/20160610140403_remove_notification_setting_not_null_constraints.rb b/db/migrate/20160610140403_remove_notification_setting_not_null_constraints.rb new file mode 100644 index 00000000000..259abb08e47 --- /dev/null +++ b/db/migrate/20160610140403_remove_notification_setting_not_null_constraints.rb @@ -0,0 +1,11 @@ +class RemoveNotificationSettingNotNullConstraints < ActiveRecord::Migration + def up + change_column :notification_settings, :source_type, :string, null: true + change_column :notification_settings, :source_id, :integer, null: true + end + + def down + change_column :notification_settings, :source_type, :string, null: false + change_column :notification_settings, :source_id, :integer, null: false + end +end diff --git a/db/migrate/20160610201627_migrate_users_notification_level.rb b/db/migrate/20160610201627_migrate_users_notification_level.rb new file mode 100644 index 00000000000..760b766828e --- /dev/null +++ b/db/migrate/20160610201627_migrate_users_notification_level.rb @@ -0,0 +1,21 @@ +class MigrateUsersNotificationLevel < ActiveRecord::Migration + # Migrates only users who changed their default notification level :participating + # creating a new record on notification settings table + + def up + execute(%Q{ + INSERT INTO notification_settings + (user_id, level, created_at, updated_at) + (SELECT id, notification_level, created_at, updated_at FROM users WHERE notification_level != 1) + }) + end + + # Migrates from notification settings back to user notification_level + # If no value is found the default level of 1 will be used + def down + execute(%Q{ + UPDATE users u SET + notification_level = COALESCE((SELECT level FROM notification_settings WHERE user_id = u.id AND source_type IS NULL), 1) + }) + end +end diff --git a/db/migrate/20160610301627_remove_notification_level_from_users.rb b/db/migrate/20160610301627_remove_notification_level_from_users.rb new file mode 100644 index 00000000000..8afb14df2cf --- /dev/null +++ b/db/migrate/20160610301627_remove_notification_level_from_users.rb @@ -0,0 +1,7 @@ +class RemoveNotificationLevelFromUsers < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + def change + remove_column :users, :notification_level, :integer + end +end diff --git a/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb index 295081e9da1..4e24e89b008 100644 --- a/spec/models/notification_setting_spec.rb +++ b/spec/models/notification_setting_spec.rb @@ -10,7 +10,6 @@ RSpec.describe NotificationSetting, type: :model do subject { NotificationSetting.new(source_id: 1, source_type: 'Project') } it { is_expected.to validate_presence_of(:user) } - it { is_expected.to validate_presence_of(:source) } it { is_expected.to validate_presence_of(:level) } it { is_expected.to validate_uniqueness_of(:user_id).scoped_to([:source_id, :source_type]).with_message(/already exists in source/) } end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index cef5e0d8659..b99e02ba678 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -72,6 +72,7 @@ describe NotificationService, services: true do should_not_email(@u_disabled) should_not_email(@unsubscriber) should_not_email(@u_outsider_mentioned) + should_not_email(@u_lazy_participant) end it 'filters out "mentioned in" notes' do @@ -80,6 +81,20 @@ describe NotificationService, services: true do expect(Notify).not_to receive(:note_issue_email) notification.new_note(mentioned_note) end + + context 'participating' do + context 'by note' do + before do + ActionMailer::Base.deliveries.clear + note.author = @u_lazy_participant + note.save + notification.new_note(note) + end + + + it { should_not_email(@u_lazy_participant) } + end + end end describe 'new note on issue in project that belongs to a group' do @@ -106,6 +121,7 @@ describe NotificationService, services: true do should_not_email(note.author) should_not_email(@u_participating) should_not_email(@u_disabled) + should_not_email(@u_lazy_participant) end end end @@ -235,6 +251,7 @@ describe NotificationService, services: true do should_not_email(note.author) should_not_email(@u_participating) should_not_email(@u_disabled) + should_not_email(@u_lazy_participant) end it do @@ -248,10 +265,11 @@ describe NotificationService, services: true do should_not_email(note.author) should_not_email(@u_participating) should_not_email(@u_disabled) + should_not_email(@u_lazy_participant) end it do - @u_committer.update_attributes(notification_level: :mention) + @u_committer = create_global_setting_for(@u_committer, :mention) notification.new_note(note) should_not_email(@u_committer) end @@ -280,10 +298,11 @@ describe NotificationService, services: true do should_not_email(@u_mentioned) should_not_email(@u_participating) should_not_email(@u_disabled) + should_not_email(@u_lazy_participant) end it do - issue.assignee.update_attributes(notification_level: :mention) + create_global_setting_for(issue.assignee, :mention) notification.new_issue(issue, @u_disabled) should_not_email(issue.assignee) @@ -341,6 +360,7 @@ describe NotificationService, services: true do should_not_email(@unsubscriber) should_not_email(@u_participating) should_not_email(@u_disabled) + should_not_email(@u_lazy_participant) end it 'emails previous assignee even if he has the "on mention" notif level' do @@ -356,6 +376,7 @@ describe NotificationService, services: true do should_not_email(@unsubscriber) should_not_email(@u_participating) should_not_email(@u_disabled) + should_not_email(@u_lazy_participant) end it 'emails new assignee even if he has the "on mention" notif level' do @@ -371,6 +392,7 @@ describe NotificationService, services: true do should_not_email(@unsubscriber) should_not_email(@u_participating) should_not_email(@u_disabled) + should_not_email(@u_lazy_participant) end it 'emails new assignee' do @@ -386,6 +408,7 @@ describe NotificationService, services: true do should_not_email(@unsubscriber) should_not_email(@u_participating) should_not_email(@u_disabled) + should_not_email(@u_lazy_participant) end it 'does not email new assignee if they are the current user' do @@ -401,6 +424,35 @@ describe NotificationService, services: true do should_not_email(@unsubscriber) should_not_email(@u_participating) should_not_email(@u_disabled) + should_not_email(@u_lazy_participant) + end + + context 'participating' do + context 'by assignee' do + before do + issue.update_attribute(:assignee, @u_lazy_participant) + notification.reassigned_issue(issue, @u_disabled) + end + + it { should_email(@u_lazy_participant) } + end + + context 'by note' do + let!(:note) { create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: 'anything', author: @u_lazy_participant) } + + before { notification.reassigned_issue(issue, @u_disabled) } + + it { should_email(@u_lazy_participant) } + end + + context 'by author' do + before do + issue.author = @u_lazy_participant + notification.reassigned_issue(issue, @u_disabled) + end + + it { should_email(@u_lazy_participant) } + end end end @@ -479,6 +531,35 @@ describe NotificationService, services: true do should_not_email(@unsubscriber) should_not_email(@u_participating) should_not_email(@u_disabled) + should_not_email(@u_lazy_participant) + end + + context 'participating' do + context 'by assignee' do + before do + issue.update_attribute(:assignee, @u_lazy_participant) + notification.close_issue(issue, @u_disabled) + end + + it { should_email(@u_lazy_participant) } + end + + context 'by note' do + let!(:note) { create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: 'anything', author: @u_lazy_participant) } + + before { notification.close_issue(issue, @u_disabled) } + + it { should_email(@u_lazy_participant) } + end + + context 'by author' do + before do + issue.author = @u_lazy_participant + notification.close_issue(issue, @u_disabled) + end + + it { should_email(@u_lazy_participant) } + end end end @@ -495,6 +576,35 @@ describe NotificationService, services: true do should_email(@watcher_and_subscriber) should_not_email(@unsubscriber) should_not_email(@u_participating) + should_not_email(@u_lazy_participant) + end + + context 'participating' do + context 'by assignee' do + before do + issue.update_attribute(:assignee, @u_lazy_participant) + notification.reopen_issue(issue, @u_disabled) + end + + it { should_email(@u_lazy_participant) } + end + + context 'by note' do + let!(:note) { create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: 'anything', author: @u_lazy_participant) } + + before { notification.reopen_issue(issue, @u_disabled) } + + it { should_email(@u_lazy_participant) } + end + + context 'by author' do + before do + issue.author = @u_lazy_participant + notification.reopen_issue(issue, @u_disabled) + end + + it { should_email(@u_lazy_participant) } + end end end end @@ -520,6 +630,7 @@ describe NotificationService, services: true do should_email(@u_guest_watcher) should_not_email(@u_participating) should_not_email(@u_disabled) + should_not_email(@u_lazy_participant) end it "emails subscribers of the merge request's labels" do @@ -530,6 +641,36 @@ describe NotificationService, services: true do should_email(subscriber) end + + + context 'participating' do + context 'by assignee' do + before do + merge_request.update_attribute(:assignee, @u_lazy_participant) + notification.new_merge_request(merge_request, @u_disabled) + end + + it { should_email(@u_lazy_participant) } + end + + context 'by note' do + let!(:note) { create(:note_on_issue, noteable: merge_request, project_id: project.id, note: 'anything', author: @u_lazy_participant) } + + before { notification.new_merge_request(merge_request, @u_disabled) } + + it { should_email(@u_lazy_participant) } + end + + context 'by author' do + before do + merge_request.author = @u_lazy_participant + merge_request.save + notification.new_merge_request(merge_request, @u_disabled) + end + + it { should_not_email(@u_lazy_participant) } + end + end end describe '#reassigned_merge_request' do @@ -545,6 +686,36 @@ describe NotificationService, services: true do should_not_email(@unsubscriber) should_not_email(@u_participating) should_not_email(@u_disabled) + should_not_email(@u_lazy_participant) + end + + context 'participating' do + context 'by assignee' do + before do + merge_request.update_attribute(:assignee, @u_lazy_participant) + notification.reassigned_merge_request(merge_request, @u_disabled) + end + + it { should_email(@u_lazy_participant) } + end + + context 'by note' do + let!(:note) { create(:note_on_issue, noteable: merge_request, project_id: project.id, note: 'anything', author: @u_lazy_participant) } + + before { notification.reassigned_merge_request(merge_request, @u_disabled) } + + it { should_email(@u_lazy_participant) } + end + + context 'by author' do + before do + merge_request.author = @u_lazy_participant + merge_request.save + notification.reassigned_merge_request(merge_request, @u_disabled) + end + + it { should_email(@u_lazy_participant) } + end end end @@ -572,6 +743,7 @@ describe NotificationService, services: true do should_not_email(@watcher_and_subscriber) should_not_email(@unsubscriber) should_not_email(@u_participating) + should_not_email(@u_lazy_participant) should_not_email(subscriber_to_label) should_email(subscriber_to_label2) end @@ -590,6 +762,36 @@ describe NotificationService, services: true do should_not_email(@unsubscriber) should_not_email(@u_participating) should_not_email(@u_disabled) + should_not_email(@u_lazy_participant) + end + + context 'participating' do + context 'by assignee' do + before do + merge_request.update_attribute(:assignee, @u_lazy_participant) + notification.close_mr(merge_request, @u_disabled) + end + + it { should_email(@u_lazy_participant) } + end + + context 'by note' do + let!(:note) { create(:note_on_issue, noteable: merge_request, project_id: project.id, note: 'anything', author: @u_lazy_participant) } + + before { notification.close_mr(merge_request, @u_disabled) } + + it { should_email(@u_lazy_participant) } + end + + context 'by author' do + before do + merge_request.author = @u_lazy_participant + merge_request.save + notification.close_mr(merge_request, @u_disabled) + end + + it { should_email(@u_lazy_participant) } + end end end @@ -606,6 +808,36 @@ describe NotificationService, services: true do should_not_email(@unsubscriber) should_not_email(@u_participating) should_not_email(@u_disabled) + should_not_email(@u_lazy_participant) + end + + context 'participating' do + context 'by assignee' do + before do + merge_request.update_attribute(:assignee, @u_lazy_participant) + notification.merge_mr(merge_request, @u_disabled) + end + + it { should_email(@u_lazy_participant) } + end + + context 'by note' do + let!(:note) { create(:note_on_issue, noteable: merge_request, project_id: project.id, note: 'anything', author: @u_lazy_participant) } + + before { notification.merge_mr(merge_request, @u_disabled) } + + it { should_email(@u_lazy_participant) } + end + + context 'by author' do + before do + merge_request.author = @u_lazy_participant + merge_request.save + notification.merge_mr(merge_request, @u_disabled) + end + + it { should_email(@u_lazy_participant) } + end end end @@ -622,6 +854,36 @@ describe NotificationService, services: true do should_not_email(@unsubscriber) should_not_email(@u_participating) should_not_email(@u_disabled) + should_not_email(@u_lazy_participant) + end + + context 'participating' do + context 'by assignee' do + before do + merge_request.update_attribute(:assignee, @u_lazy_participant) + notification.reopen_mr(merge_request, @u_disabled) + end + + it { should_email(@u_lazy_participant) } + end + + context 'by note' do + let!(:note) { create(:note_on_issue, noteable: merge_request, project_id: project.id, note: 'anything', author: @u_lazy_participant) } + + before { notification.reopen_mr(merge_request, @u_disabled) } + + it { should_email(@u_lazy_participant) } + end + + context 'by author' do + before do + merge_request.author = @u_lazy_participant + merge_request.save + notification.reopen_mr(merge_request, @u_disabled) + end + + it { should_email(@u_lazy_participant) } + end end end end @@ -640,6 +902,7 @@ describe NotificationService, services: true do should_email(@u_watcher) should_email(@u_participating) + should_email(@u_lazy_participant) should_not_email(@u_guest_watcher) should_not_email(@u_disabled) end @@ -647,14 +910,19 @@ describe NotificationService, services: true do end def build_team(project) - @u_watcher = create(:user, notification_level: :watch) - @u_participating = create(:user, notification_level: :participating) - @u_participant_mentioned = create(:user, username: 'participant', notification_level: :participating) - @u_disabled = create(:user, notification_level: :disabled) - @u_mentioned = create(:user, username: 'mention', notification_level: :mention) - @u_committer = create(:user, username: 'committer') - @u_not_mentioned = create(:user, username: 'regular', notification_level: :participating) - @u_outsider_mentioned = create(:user, username: 'outsider') + @u_watcher = create_global_setting_for(create(:user), :watch) + @u_participating = create_global_setting_for(create(:user), :participating) + @u_participant_mentioned = create_global_setting_for(create(:user, username: 'participant'), :participating) + @u_disabled = create_global_setting_for(create(:user), :disabled) + @u_mentioned = create_global_setting_for(create(:user, username: 'mention'), :mention) + @u_committer = create(:user, username: 'committer') + @u_not_mentioned = create_global_setting_for(create(:user, username: 'regular'), :participating) + @u_outsider_mentioned = create(:user, username: 'outsider') + + # User to be participant by default + # This user does not contain any record in notification settings table + # It should be treated with a :participating notification_level + @u_lazy_participant = create(:user, username: 'lazy-participant') create_guest_watcher @@ -665,6 +933,15 @@ describe NotificationService, services: true do project.team << [@u_mentioned, :master] project.team << [@u_committer, :master] project.team << [@u_not_mentioned, :master] + project.team << [@u_lazy_participant, :master] + end + + def create_global_setting_for(user, level) + setting = user.global_notification_setting + setting.level = level + setting.save + + user end def create_guest_watcher @@ -677,8 +954,8 @@ describe NotificationService, services: true do def add_users_with_subscription(project, issuable) @subscriber = create :user @unsubscriber = create :user - @subscribed_participant = create(:user, username: 'subscribed_participant', notification_level: :participating) - @watcher_and_subscriber = create(:user, notification_level: :watch) + @subscribed_participant = create_global_setting_for(create(:user, username: 'subscribed_participant'), :participating) + @watcher_and_subscriber = create_global_setting_for(create(:user), :watch) project.team << [@subscribed_participant, :master] project.team << [@subscriber, :master] |