summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFelipe Artur <felipefac@gmail.com>2016-06-06 12:50:54 -0300
committerFelipe Artur <felipefac@gmail.com>2016-06-10 10:58:13 -0300
commit8f6d43e0fea3ce62ec2e8e211755e557f19c51fd (patch)
tree72930d029dd905d7630ec07dd65acb0ac4b0739c
parentf29fd65cdde1d769fc89f0cc57ea989765b5068f (diff)
downloadgitlab-ce-8f6d43e0fea3ce62ec2e8e211755e557f19c51fd.tar.gz
Remove notification level from user model
-rw-r--r--CHANGELOG1
-rw-r--r--app/controllers/profiles/notifications_controller.rb24
-rw-r--r--app/helpers/notifications_helper.rb32
-rw-r--r--app/models/notification_setting.rb1
-rw-r--r--app/models/user.rb18
-rw-r--r--app/services/notification_service.rb16
-rw-r--r--app/views/profiles/notifications/_group_settings.html.haml2
-rw-r--r--app/views/profiles/notifications/_project_settings.html.haml2
-rw-r--r--app/views/profiles/notifications/show.html.haml28
-rw-r--r--db/migrate/20160606192159_remove_notification_setting_not_null_constraints.rb7
-rw-r--r--db/migrate/20160607201627_migrate_users_notification_level.rb24
-rw-r--r--spec/models/notification_setting_spec.rb1
-rw-r--r--spec/services/notification_service_spec.rb296
13 files changed, 391 insertions, 61 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 1c64739f701..bf437e267a4 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -54,6 +54,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.
- An indicator is now displayed at the top of the comment field for confidential issues.
- RepositoryCheck::SingleRepositoryWorker public and private methods are now instrumented
diff --git a/app/controllers/profiles/notifications_controller.rb b/app/controllers/profiles/notifications_controller.rb
index 18ee55c839a..1e9ceb87857 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"
@@ -18,4 +19,19 @@ class Profiles::NotificationsController < Profiles::ApplicationController
def user_params
params.require(:user).permit(:notification_email, :notification_level)
end
+
+ def notification_params
+ params.require(:notification_level)
+ end
+
+ private
+
+ def update_notification_settings
+ return true unless notification_params
+
+ notification_setting = current_user.global_notification_setting
+ notification_setting.level = notification_params
+
+ notification_setting.save
+ end
end
diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb
index b8e64b3890a..9769458f79e 100644
--- a/app/helpers/notifications_helper.rb
+++ b/app/helpers/notifications_helper.rb
@@ -61,4 +61,36 @@ 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(:notification_level, level, @global_notification_setting.level.to_sym == level) +
+ content_tag(:div, level.to_s.capitalize, class: "level-title") +
+ content_tag(:p, notification_level_description(level))
+ end
+ end
+ end
+
+ html.html_safe
+ end
+
+ def notification_level_description(level)
+ case level
+ when :disabled
+ "You will not get any notifications via email"
+ when :mention
+ "You will receive notifications only for comments in which you were @mentioned"
+ when :participating
+ "You will only receive notifications from related resources (e.g. from your commits or assigned issues)"
+ when :watch
+ "You will receive notifications for any activity"
+ end
+ 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..d2da83ab4b3 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,14 @@ 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
+ setting = notification_settings.find_or_initialize_by(source: nil)
+ setting.level = NotificationSetting.levels[DEFAULT_NOTIFICATION_LEVEL] unless setting.persisted?
+ 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..af7bbe37439 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,7 @@ class NotificationService
users = users.reject(&:blocked?)
users.reject do |user|
- next user.notification_level == level unless project
+ next user.global_notification_setting.level == level unless project
setting = user.notification_settings_for(project)
@@ -361,13 +362,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 user.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? && user.global_notification_setting.level == level
end
end
@@ -456,7 +457,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/20160606192159_remove_notification_setting_not_null_constraints.rb b/db/migrate/20160606192159_remove_notification_setting_not_null_constraints.rb
new file mode 100644
index 00000000000..c20ac9acdc2
--- /dev/null
+++ b/db/migrate/20160606192159_remove_notification_setting_not_null_constraints.rb
@@ -0,0 +1,7 @@
+class RemoveNotificationSettingNotNullConstraints < ActiveRecord::Migration
+ def up
+ change_column :notification_settings, :source_type, :string, null: true
+ change_column :notification_settings, :source_id, :integer, null: true
+ change_column :users, :notification_level, :integer, null: true
+ end
+end
diff --git a/db/migrate/20160607201627_migrate_users_notification_level.rb b/db/migrate/20160607201627_migrate_users_notification_level.rb
new file mode 100644
index 00000000000..7417d66fef7
--- /dev/null
+++ b/db/migrate/20160607201627_migrate_users_notification_level.rb
@@ -0,0 +1,24 @@
+class MigrateUsersNotificationLevel < ActiveRecord::Migration
+ # Migrates only users which changes theier default notification level :participating
+ # creating a new record on notification settins table
+
+ def up
+ changed_users = exec_query(%Q{
+ SELECT id, notification_level
+ FROM users
+ WHERE notification_level != 1
+ })
+
+ changed_users.each do |row|
+ uid = row['id']
+ u_notification_level = row['notification_level']
+
+ execute(%Q{
+ INSERT INTO notification_settings
+ (user_id, level, created_at, updated_at)
+ VALUES
+ (#{uid}, #{u_notification_level}, now(), now())
+ })
+ end
+ 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..5a9a9d62a15 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,19 @@ 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
+ note.author = @u_lazy_participant
+ note.save
+ notification.new_note(note)
+ end
+
+
+ it { should_email(@u_lazy_participant) }
+ end
+ end
end
describe 'new note on issue in project that belongs to a group' do
@@ -106,6 +120,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 +250,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 +264,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 +297,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 +359,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 +375,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 +391,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 +407,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 +423,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 +530,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 +575,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 +629,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 +640,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 +685,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 +742,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 +761,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 +807,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 +853,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 +901,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 +909,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 +932,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