diff options
author | Felipe Artur <felipefac@gmail.com> | 2016-06-09 16:33:31 -0300 |
---|---|---|
committer | Felipe Artur <felipefac@gmail.com> | 2016-06-10 11:49:30 -0300 |
commit | 39ead205de72461e86db07525922f2fab5fff2a9 (patch) | |
tree | 5336487bad3b89b5db461788aba9eb7d4b9c3b0f | |
parent | 8f6d43e0fea3ce62ec2e8e211755e557f19c51fd (diff) | |
download | gitlab-ce-39ead205de72461e86db07525922f2fab5fff2a9.tar.gz |
Remove notification level fild from users, improve migrations and specsissue_3359_2
-rw-r--r-- | app/controllers/profiles/notifications_controller.rb | 13 | ||||
-rw-r--r-- | app/helpers/notifications_helper.rb | 17 | ||||
-rw-r--r-- | app/models/user.rb | 9 | ||||
-rw-r--r-- | app/services/notification_service.rb | 8 | ||||
-rw-r--r-- | db/migrate/20160607201627_migrate_users_notification_level.rb | 24 | ||||
-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.rb | 21 | ||||
-rw-r--r-- | db/migrate/20160610301627_remove_notification_level_from_users.rb | 7 | ||||
-rw-r--r-- | spec/services/notification_service_spec.rb | 7 |
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] |