From 359157c097993a9b917ca590e128e85cf358d95d Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Mon, 28 Mar 2016 18:17:42 +0200
Subject: [PATCH] Introduce NotificationSetting to user interface

* visiting project will create notification setting if missing
* change notification setting per project even without membership
* use notification settings instead of membership on profile page

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
---
 app/assets/javascripts/project.js.coffee      |  8 +-
 .../profiles/notifications_controller.rb      | 14 ++--
 app/controllers/projects_controller.rb        | 17 ++++-
 app/helpers/notifications_helper.rb           | 74 +++++++++++--------
 app/models/notification.rb                    |  4 +-
 app/models/notification_setting.rb            |  7 ++
 app/models/user.rb                            |  7 +-
 .../notifications/_settings.html.haml         | 18 ++---
 .../profiles/notifications/show.html.haml     | 30 ++++----
 .../projects/buttons/_notifications.html.haml | 20 ++---
 10 files changed, 106 insertions(+), 93 deletions(-)

diff --git a/app/assets/javascripts/project.js.coffee b/app/assets/javascripts/project.js.coffee
index 87d313ed67c..f171442d05a 100644
--- a/app/assets/javascripts/project.js.coffee
+++ b/app/assets/javascripts/project.js.coffee
@@ -37,15 +37,9 @@ class @Project
     $('.update-notification').on 'click', (e) ->
       e.preventDefault()
       notification_level = $(@).data 'notification-level'
+      label = $(@).data 'notification-title'
       $('#notification_level').val(notification_level)
       $('#notification-form').submit()
-      label = null
-      switch notification_level
-        when 0 then label = ' Disabled '
-        when 1 then label = ' Participating '
-        when 2 then label = ' Watching '
-        when 3 then label = ' Global '
-        when 4 then label = ' On Mention '
       $('#notifications-button').empty().append("<i class='fa fa-bell'></i>" + label + "<i class='fa fa-angle-down'></i>")
       $(@).parents('ul').find('li.active').removeClass 'active'
       $(@).parent().addClass 'active'
diff --git a/app/controllers/profiles/notifications_controller.rb b/app/controllers/profiles/notifications_controller.rb
index 1fd1d6882df..6ca7537300f 100644
--- a/app/controllers/profiles/notifications_controller.rb
+++ b/app/controllers/profiles/notifications_controller.rb
@@ -2,8 +2,8 @@ class Profiles::NotificationsController < Profiles::ApplicationController
   def show
     @user = current_user
     @notification = current_user.notification
-    @project_members = current_user.project_members
-    @group_members = current_user.group_members
+    @group_notifications = current_user.notification_settings.for_groups
+    @project_notifications = current_user.notification_settings.for_projects
   end
 
   def update
@@ -11,14 +11,10 @@ class Profiles::NotificationsController < Profiles::ApplicationController
 
     @saved = if type == 'global'
                current_user.update_attributes(user_params)
-             elsif type == 'group'
-               group_member = current_user.group_members.find(params[:notification_id])
-               group_member.notification_level = params[:notification_level]
-               group_member.save
              else
-               project_member = current_user.project_members.find(params[:notification_id])
-               project_member.notification_level = params[:notification_level]
-               project_member.save
+               notification_setting = current_user.notification_settings.find(params[:notification_id])
+               notification_setting.level = params[:notification_level]
+               notification_setting.save
              end
 
     respond_to do |format|
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 928817ba811..77122f59128 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -98,14 +98,23 @@ class ProjectsController < Projects::ApplicationController
 
     respond_to do |format|
       format.html do
+        if current_user
+          @membership = @project.team.find_member(current_user.id)
+
+          if @membership
+            @notification_setting = current_user.notification_settings.find_or_initialize_by(source: @project)
+
+            unless @notification_setting.persisted?
+              @notification_setting.set_defaults
+              @notification_setting.save
+            end
+          end
+        end
+
         if @project.repository_exists?
           if @project.empty_repo?
             render 'projects/empty'
           else
-            if current_user
-              @membership = @project.team.find_member(current_user.id)
-            end
-
             render :show
           end
         else
diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb
index 499c655d2bf..a0e91a63d2d 100644
--- a/app/helpers/notifications_helper.rb
+++ b/app/helpers/notifications_helper.rb
@@ -1,48 +1,60 @@
 module NotificationsHelper
   include IconsHelper
 
-  def notification_icon(notification)
-    if notification.disabled?
-      icon('volume-off', class: 'ns-mute')
-    elsif notification.participating?
-      icon('volume-down', class: 'ns-part')
-    elsif notification.watch?
-      icon('volume-up', class: 'ns-watch')
-    else
-      icon('circle-o', class: 'ns-default')
+  def notification_icon_class(level)
+    case level.to_sym
+    when :disabled
+      'microphone-slash'
+    when :participating
+      'volume-up'
+    when :watch
+      'eye'
+    when :mention
+      'at'
+    when :global
+      'globe'
     end
   end
 
-  def notification_list_item(notification_level, user_membership)
-    case notification_level
-    when Notification::N_DISABLED
-      update_notification_link(Notification::N_DISABLED, user_membership, 'Disabled', 'microphone-slash')
-    when Notification::N_PARTICIPATING
-      update_notification_link(Notification::N_PARTICIPATING, user_membership, 'Participate', 'volume-up')
-    when Notification::N_WATCH
-      update_notification_link(Notification::N_WATCH, user_membership, 'Watch', 'eye')
-    when Notification::N_MENTION
-      update_notification_link(Notification::N_MENTION, user_membership, 'On mention', 'at')
-    when Notification::N_GLOBAL
-      update_notification_link(Notification::N_GLOBAL, user_membership, 'Global', 'globe')
-    else
-      # do nothing
+  def notification_icon(level)
+    icon("#{notification_icon_class(level)} fw")
+  end
+
+  def notification_title(level)
+    case level.to_sym
+    when :disabled
+      'Disabled'
+    when :participating
+      'Participate'
+    when :watch
+      'Watch'
+    when :mention
+      'On mention'
+    when :global
+      'Global'
     end
   end
 
-  def update_notification_link(notification_level, user_membership, title, icon)
-    content_tag(:li, class: active_level_for(user_membership, notification_level)) do
-      link_to '#', class: 'update-notification', data: { notification_level: notification_level } do
-        icon("#{icon} fw", text: title)
+  def notification_list_item(level, setting)
+    title = notification_title(level)
+
+    data = {
+      notification_level: level,
+      notification_title: title
+    }
+
+    content_tag(:li, class: active_level_for(setting, level)) do
+      link_to '#', class: 'update-notification', data: data do
+        icon("#{notification_icon_class(level)} fw", text: title)
       end
     end
   end
 
-  def notification_label(user_membership)
-    Notification.new(user_membership).to_s
+  def notification_label(setting)
+    notification_title(setting.level)
   end
 
-  def active_level_for(user_membership, level)
-    'active' if user_membership.notification_level == level
+  def active_level_for(setting, level)
+    'active' if setting.level == level
   end
 end
diff --git a/app/models/notification.rb b/app/models/notification.rb
index 171b8df45c2..379f041969b 100644
--- a/app/models/notification.rb
+++ b/app/models/notification.rb
@@ -57,7 +57,7 @@ class Notification
   def level
     target.notification_level
   end
-  
+
   def to_s
     case level
     when N_DISABLED
@@ -71,7 +71,7 @@ class Notification
     when N_GLOBAL
       'Global'
     else
-      # do nothing      
+      # do nothing
     end
   end
 end
diff --git a/app/models/notification_setting.rb b/app/models/notification_setting.rb
index 0dce146b7a9..287862a01bc 100644
--- a/app/models/notification_setting.rb
+++ b/app/models/notification_setting.rb
@@ -11,4 +11,11 @@ class NotificationSetting < ActiveRecord::Base
   # Notification level
   # Note: When adding an option, it MUST go on the end of the array.
   enum level: [:disabled, :participating, :watch, :global, :mention]
+
+  scope :for_groups, -> { where(source_type: 'Namespace') }
+  scope :for_projects, -> { where(source_type: 'Project') }
+
+  def set_defaults
+    self.level = :global
+  end
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index 128ddc2a694..59493e6f90c 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -143,6 +143,7 @@ class User < ActiveRecord::Base
   has_many :spam_logs,                dependent: :destroy
   has_many :builds,                   dependent: :nullify, class_name: 'Ci::Build'
   has_many :todos,                    dependent: :destroy
+  has_many :notification_settings,    dependent: :destroy
 
   #
   # Validations
@@ -157,7 +158,7 @@ class User < ActiveRecord::Base
     presence: true,
     uniqueness: { case_sensitive: false }
 
-  validates :notification_level, inclusion: { in: Notification.notification_levels }, presence: true
+  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? }
@@ -190,6 +191,10 @@ 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.
+  enum notification_level: [:disabled, :participating, :watch, :global, :mention]
+
   alias_attribute :private_token, :authentication_token
 
   delegate :path, to: :namespace, allow_nil: true, prefix: true
diff --git a/app/views/profiles/notifications/_settings.html.haml b/app/views/profiles/notifications/_settings.html.haml
index d0d044136f6..c32de0b9925 100644
--- a/app/views/profiles/notifications/_settings.html.haml
+++ b/app/views/profiles/notifications/_settings.html.haml
@@ -1,17 +1,17 @@
 %li.notification-list-item
   %span.notification.fa.fa-holder.append-right-5
-    - if notification.global?
-      = notification_icon(@notification)
+    - if setting.global?
+      = notification_icon(current_user.notification_level)
     - else
-      = notification_icon(notification)
+      = notification_icon(setting.level)
 
   %span.str-truncated
-    - if membership.kind_of? GroupMember
-      = link_to membership.group.name, membership.group
+    - if setting.source.kind_of? Project
+      = link_to_project(setting.source)
     - else
-      = link_to_project(membership.project)
+      = link_to setting.source.name, group_path(setting.source)
   .pull-right
     = form_tag profile_notifications_path, method: :put, remote: true, class: 'update-notifications' do
-      = hidden_field_tag :notification_type, type, id: dom_id(membership, 'notification_type')
-      = hidden_field_tag :notification_id, membership.id, id: dom_id(membership, 'notification_id')
-      = select_tag :notification_level, options_for_select(Notification.options_with_labels, notification.level), class: 'form-control trigger-submit'
+      = hidden_field_tag :notification_id, setting.id
+      = hidden_field_tag :notification_level, setting.level
+      = select_tag :notification_level, options_for_select(User.notification_levels.keys, setting.level), class: 'form-control trigger-submit'
diff --git a/app/views/profiles/notifications/show.html.haml b/app/views/profiles/notifications/show.html.haml
index de80abd7f4d..f6900f61b2d 100644
--- a/app/views/profiles/notifications/show.html.haml
+++ b/app/views/profiles/notifications/show.html.haml
@@ -26,29 +26,29 @@
       .form-group
         = f.label :notification_level, class: 'label-light'
         .radio
-          = f.label :notification_level, value: Notification::N_DISABLED do
-            = f.radio_button :notification_level, Notification::N_DISABLED
+          = 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: Notification::N_MENTION do
-            = f.radio_button :notification_level, Notification::N_MENTION
+          = 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: Notification::N_PARTICIPATING do
-            = f.radio_button :notification_level, Notification::N_PARTICIPATING
+          = 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: Notification::N_WATCH do
-            = f.radio_button :notification_level, Notification::N_WATCH
+          = f.label :notification_level, value: :watch do
+            = f.radio_button :notification_level, :watch
             .level-title
               Watch
             %p You will receive notifications for any activity
@@ -57,18 +57,16 @@
         = f.submit 'Update settings', class: "btn btn-create"
       %hr
       %h5
-        Groups (#{@group_members.count})
+        Groups (#{@group_notifications.count})
       %div
         %ul.bordered-list
-          - @group_members.each do |group_member|
-            - notification = Notification.new(group_member)
-            = render 'settings', type: 'group', membership: group_member, notification: notification
+          - @group_notifications.each do |setting|
+            = render 'settings', setting: setting
       %h5
-        Projects (#{@project_members.count})
+        Projects (#{@project_notifications.count})
       %p.account-well
         To specify the notification level per project of a group you belong to, you need to be a member of the project itself, not only its group.
       .append-bottom-default
         %ul.bordered-list
-          - @project_members.each do |project_member|
-            - notification = Notification.new(project_member)
-            = render 'settings', type: 'project', membership: project_member, notification: notification
+          - @project_notifications.each do |setting|
+            = render 'settings', setting: setting
diff --git a/app/views/projects/buttons/_notifications.html.haml b/app/views/projects/buttons/_notifications.html.haml
index a3786c35a1f..4b8a10f0819 100644
--- a/app/views/projects/buttons/_notifications.html.haml
+++ b/app/views/projects/buttons/_notifications.html.haml
@@ -1,20 +1,12 @@
-- case @membership
-- when ProjectMember
+- if @notification_setting
   = form_tag profile_notifications_path, method: :put, remote: true, class: 'inline', id: 'notification-form' do
-    = hidden_field_tag :notification_type, 'project'
-    = hidden_field_tag :notification_id, @membership.id
-    = hidden_field_tag :notification_level
+    = hidden_field_tag :notification_id, @notification_setting.id
+    = hidden_field_tag :notification_level, @notification_setting.level
     %span.dropdown
       %a.dropdown-new.btn.notifications-btn#notifications-button{href: '#', "data-toggle" => "dropdown"}
         = icon('bell')
-        = notification_label(@membership)
+        = notification_title(@notification_setting.level)
         = icon('angle-down')
       %ul.dropdown-menu.dropdown-menu-right.project-home-dropdown
-        - Notification.project_notification_levels.each do |level|
-          = notification_list_item(level, @membership)
-
-- when GroupMember
-  .btn.disabled.notifications-btn.has-tooltip{title: "To change the notification level, you need to be a member of the project itself, not only its group."}
-    = icon('bell')
-    = notification_label(@membership)
-    = icon('angle-down')
+        - NotificationSetting.levels.each do |level|
+          = notification_list_item(level.first, @notification_setting)
-- 
GitLab