summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFelipe Artur <felipefac@gmail.com>2016-06-09 16:33:31 -0300
committerFelipe Artur <felipefac@gmail.com>2016-06-10 11:49:30 -0300
commit39ead205de72461e86db07525922f2fab5fff2a9 (patch)
tree5336487bad3b89b5db461788aba9eb7d4b9c3b0f
parent8f6d43e0fea3ce62ec2e8e211755e557f19c51fd (diff)
downloadgitlab-ce-39ead205de72461e86db07525922f2fab5fff2a9.tar.gz
Remove notification level fild from users, improve migrations and specsissue_3359_2
-rw-r--r--app/controllers/profiles/notifications_controller.rb13
-rw-r--r--app/helpers/notifications_helper.rb17
-rw-r--r--app/models/user.rb9
-rw-r--r--app/services/notification_service.rb8
-rw-r--r--db/migrate/20160607201627_migrate_users_notification_level.rb24
-rw-r--r--db/migrate/20160610140403_remove_notification_setting_not_null_constraints.rb (renamed from db/migrate/20160606192159_remove_notification_setting_not_null_constraints.rb)6
-rw-r--r--db/migrate/20160610201627_migrate_users_notification_level.rb21
-rw-r--r--db/migrate/20160610301627_remove_notification_level_from_users.rb7
-rw-r--r--spec/services/notification_service_spec.rb7
9 files changed, 55 insertions, 57 deletions
diff --git a/app/controllers/profiles/notifications_controller.rb b/app/controllers/profiles/notifications_controller.rb
index 1e9ceb87857..40d1906a53f 100644
--- a/app/controllers/profiles/notifications_controller.rb
+++ b/app/controllers/profiles/notifications_controller.rb
@@ -17,21 +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 notification_params
- params.require(:notification_level)
+ def global_notification_setting_params
+ params.require(:global_notification_setting).permit(:level)
end
private
def update_notification_settings
- return true unless notification_params
+ return true unless global_notification_setting_params
- notification_setting = current_user.global_notification_setting
- notification_setting.level = notification_params
-
- notification_setting.save
+ 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 9769458f79e..50c21fc0d49 100644
--- a/app/helpers/notifications_helper.rb
+++ b/app/helpers/notifications_helper.rb
@@ -71,26 +71,13 @@ module NotificationsHelper
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) +
+ 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_level_description(level))
+ content_tag(:p, notification_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/user.rb b/app/models/user.rb
index d2da83ab4b3..7afbfbf112a 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -797,9 +797,12 @@ class User < ActiveRecord::Base
# 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
+ 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)
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index af7bbe37439..875a3f4fab6 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -353,7 +353,9 @@ class NotificationService
users = users.reject(&:blocked?)
users.reject do |user|
- next user.global_notification_setting.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)
@@ -362,13 +364,13 @@ class NotificationService
end
# reject users who globally set mention notification and has no setting per project/group
- next user.global_notification_setting.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.global_notification_setting.level == level
+ setting.global? && global_notification_setting.level == level
end
end
diff --git a/db/migrate/20160607201627_migrate_users_notification_level.rb b/db/migrate/20160607201627_migrate_users_notification_level.rb
deleted file mode 100644
index 7417d66fef7..00000000000
--- a/db/migrate/20160607201627_migrate_users_notification_level.rb
+++ /dev/null
@@ -1,24 +0,0 @@
-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/db/migrate/20160606192159_remove_notification_setting_not_null_constraints.rb b/db/migrate/20160610140403_remove_notification_setting_not_null_constraints.rb
index c20ac9acdc2..259abb08e47 100644
--- a/db/migrate/20160606192159_remove_notification_setting_not_null_constraints.rb
+++ b/db/migrate/20160610140403_remove_notification_setting_not_null_constraints.rb
@@ -2,6 +2,10 @@ 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
+
+ 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/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 5a9a9d62a15..b99e02ba678 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -85,13 +85,14 @@ describe NotificationService, services: true do
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_email(@u_lazy_participant) }
+ it { should_not_email(@u_lazy_participant) }
end
end
end
@@ -953,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]