Remove notification level fild from users, improve migrations and specsissue_3359_2

4 files changed, 18 insertions, 29 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