Re-use notifications dropdown on user profile

app/assets/javascripts/dispatcher.js.coffee

@@ -126,6 +126,8 @@ class Dispatcher
         shortcut_handler = new ShortcutsDashboardNavigation()
       when 'profiles'
         new Profile()
+        new NotificationsForm()
+        new NotificationsDropdown()
       when 'projects'
         new Project()
         new ProjectAvatar()
@@ -139,6 +141,7 @@ class Dispatcher
             new ProjectNew()
           when 'show'
             new ProjectShow()
+            new NotificationsDropdown()
           when 'wikis'
             new Wikis()
             shortcut_handler = new ShortcutsNavigation()

app/assets/javascripts/notifications_dropdown.js.coffee

+class @NotificationsDropdown
+  $ ->
+    $(document)
+      .off 'click', '.update-notification'
+      .on 'click', '.update-notification', (e) ->
+        e.preventDefault()
+        notificationLevel = $(@).data 'notification-level'
+        label = $(@).data 'notification-title'
+        form = $(this).parents('form:first')
+        form.find('.js-notification-loading').toggleClass 'fa-bell fa-spin fa-spinner'
+        form.find('#notification_setting_level').val(notificationLevel)
+        form.submit();
+
+    $(document)
+      .off 'ajax:success', '#notification-form'
+      .on 'ajax:success', '#notification-form', (e, data) ->
+        if data.saved
+          new Flash('Notification settings saved', 'notice')
+          $(e.currentTarget).closest('.notification-dropdown').replaceWith(data.html)
+        else
+          new Flash('Failed to save new settings', 'alert')

app/assets/javascripts/notifications_form.js.coffee

 class @NotificationsForm
   constructor: ->
-    @form = $('.custom-notifications-form')
-
     @removeEventListeners()
     @initEventListeners()
 
@@ -14,7 +12,7 @@ class @NotificationsForm
   toggleCheckbox: (e) =>
     $checkbox = $(e.currentTarget)
     $parent = $checkbox.closest('.checkbox')
-
+    console.log($parent)
     @saveEvent($checkbox, $parent)
 
   showCheckboxLoadingSpinner: ($parent) ->
@@ -26,11 +24,14 @@ class @NotificationsForm
       .removeClass 'is-done'
 
   saveEvent: ($checkbox, $parent) ->
+    form = $parent.parents('form:first')
+    console.log(form)
+
     $.ajax(
-      url: @form.attr('action')
-      method: 'patch'
+      url: form.attr('action')
+      method: form.attr('method')
       dataType: 'json'
-      data: @form.serialize()
+      data: form.serialize()
       beforeSend: =>
         @showCheckboxLoadingSpinner($parent)
     ).done (data) ->

app/assets/javascripts/profile.js.coffee

@@ -8,6 +8,10 @@ class @Profile
     $('.js-preferences-form').on 'change.preference', 'input[type=radio]', ->
       $(this).parents('form').submit()
 
+    # Automatically submit email form when it changes
+    $('#user_notification_email').on 'change', ->
+      $(this).parents('form').submit()
+
     $('.update-username').on 'ajax:before', ->
       $('.loading-username').show()
       $(this).find('.update-success').hide()

app/assets/javascripts/project.js.coffee

@@ -34,27 +34,6 @@ class @Project
       $(@).parents('.no-password-message').remove()
       e.preventDefault()
 
-    $(document)
-      .off 'click', '.update-notification'
-      .on 'click', '.update-notification', (e) ->
-        e.preventDefault()
-        notificationLevel = $(@).data 'notification-level'
-        label = $(@).data 'notification-title'
-        $('.js-notification-loading').toggleClass 'fa-bell fa-spin fa-spinner'
-        $('#notification_setting_level').val(notificationLevel)
-        $('#notification-form').submit()
-
-    $(document)
-      .off 'ajax:success', '#notification-form'
-      .on 'ajax:success', '#notification-form', (e, data) ->
-        if data.saved
-          new Flash('Notification settings saved', 'notice')
-          $('.js-notification-toggle-btns')
-            .closest('.notification-dropdown')
-            .replaceWith(data.html)
-        else
-          new Flash('Failed to save new settings', 'alert')
-
 
     @projectSelectDropdown()
 

app/controllers/groups/notification_settings_controller.rb

-class Groups::NotificationSettingsController < Groups::ApplicationController
-  before_action :authenticate_user!
-
-  def update
-    notification_setting = current_user.notification_settings_for(group)
-    saved = notification_setting.update_attributes(notification_setting_params)
-
-    render json: { saved: saved }
-  end
-
-  private
-
-  def notification_setting_params
-    params.require(:notification_setting).permit(:level)
-  end
-end

app/controllers/projects/notification_settings_controller.rb → app/controllers/notification_settings_controller.rb

-class Projects::NotificationSettingsController < Projects::ApplicationController
+class NotificationSettingsController < ApplicationController
   before_action :authenticate_user!
 
-  def update
+  def create
+    project = current_user.projects.find(params[:project][:id])
+
     @notification_setting = current_user.notification_settings_for(project)
-    saved = @notification_setting.update_attributes(notification_setting_params)
+    @saved = @notification_setting.update_attributes(notification_setting_params)
 
-    render json: {
-      html: view_to_html_string("projects/buttons/_notifications", locals: { project: @project, notification_setting: @notification_setting }),
-      saved: saved
-    }
+    render_response
+  end
+
+  def update
+    @notification_setting = current_user.notification_settings.find(params[:id])
+    @saved = @notification_setting.update_attributes(notification_setting_params)
+
+    render_response
   end
 
   private
 
+  def render_response
+    render json: {
+      html: view_to_html_string("notifications/buttons/_notifications", notification_setting: @notification_setting),
+      saved: @saved
+    }
+  end
+
   def notification_setting_params
     allowed_fields = NotificationSetting::EMAIL_EVENTS.dup
     allowed_fields << :level

app/controllers/profiles/notifications_controller.rb

 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
+    @group_notifications         = current_user.notification_settings.for_groups.order(:id)
+    @project_notifications       = current_user.notification_settings.for_projects.order(:id)
     @global_notification_setting = current_user.global_notification_setting
   end
 
   def update
-    if current_user.update_attributes(user_params) && update_notification_settings
+    if current_user.update_attributes(user_params)
       flash[:notice] = "Notification settings saved"
     else
       flash[:alert] = "Failed to save new settings"
@@ -19,16 +19,4 @@ class Profiles::NotificationsController < Profiles::ApplicationController
   def user_params
     params.require(:user).permit(:notification_email)
   end
-
-  def global_notification_setting_params
-    params.require(:global_notification_setting).permit(:level)
-  end
-
-  private
-
-  def update_notification_settings
-    return true unless global_notification_setting_params
-
-    current_user.global_notification_setting.update_attributes(global_notification_setting_params)
-  end
 end

app/helpers/notifications_helper.rb

@@ -62,22 +62,14 @@ module NotificationsHelper
     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(:"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_description(level))
-        end
-      end
+  # Identifier to trigger individually dropdowns and custom settings modals in the same view
+  def notifications_menu_identifier(type, notification_setting)
+    "#{type}-#{notification_setting.user_id}-#{notification_setting.source_id}-#{notification_setting.source_type}"
   end
 
-    html.html_safe
+  # Create hidden field to send notification setting source to controller
+  def hidden_setting_source_input(notification_setting)
+    return unless notification_setting.source_type
+    hidden_field_tag "#{notification_setting.source_type.downcase}[id]", notification_setting.source_id
   end
 end

app/services/notification_service.rb

@@ -159,7 +159,7 @@ class NotificationService
     recipients = add_project_watchers(recipients, note.project)
 
     # Merge project with custom notification
-    recipients = add_project_custom_notifications(recipients, note.project, :new_note)
+    recipients = add_custom_notifications(recipients, note.project, :new_note)
 
     # Reject users with Mention notification level, except those mentioned in _this_ note.
     recipients = reject_mention_users(recipients - mentioned_users, note.project)
@@ -257,12 +257,20 @@ class NotificationService
   protected
 
   # Get project/group users with CUSTOM notification level
-  def add_project_custom_notifications(recipients, project, action)
+  def add_custom_notifications(recipients, project, action)
     user_ids = []
 
+    # Users with a notification setting on group or project
     user_ids += notification_settings_for(project, :custom, action)
     user_ids += notification_settings_for(project.group, :custom, action)
 
+    # Users with global level custom
+    users_with_project_level_global = notification_settings_for(project, :global)
+    users_with_group_level_global   = notification_settings_for(project.group, :global)
+
+    global_users_ids = users_with_project_level_global.concat(users_with_group_level_global)
+    user_ids += users_with_global_level_custom(global_users_ids, action)
+
     recipients.concat(User.find(user_ids))
   end
 
@@ -271,7 +279,7 @@ class NotificationService
     project_members = notification_settings_for(project)
 
     users_with_project_level_global = notification_settings_for(project, :global)
-    users_with_group_level_global = notification_settings_for(project, :global)
+    users_with_group_level_global   = notification_settings_for(project.group, :global)
     users = users_with_global_level_watch([users_with_project_level_global, users_with_group_level_global].flatten.uniq)
 
     users_with_project_setting = select_project_member_setting(project, users_with_project_level_global, users)
@@ -293,11 +301,21 @@ class NotificationService
   end
 
   def users_with_global_level_watch(ids)
+    settings_with_global_level_of(:watch, ids).pluck(:user_id)
+  end
+
+  def users_with_global_level_custom(ids, action)
+    settings = settings_with_global_level_of(:custom, ids)
+    settings = settings.select { |setting| setting.events[action] }
+    settings.map(&:user_id)
+  end
+
+  def settings_with_global_level_of(level, ids)
     NotificationSetting.where(
       user_id: ids,
       source_type: nil,
-      level: NotificationSetting.levels[:watch]
-    ).pluck(:user_id)
+      level: NotificationSetting.levels[level]
+    )
   end
 
   # Build a list of users based on project notifcation settings
@@ -477,7 +495,7 @@ class NotificationService
 
     recipients = target.participants(current_user)
     recipients = add_project_watchers(recipients, project)
-    recipients = add_project_custom_notifications(recipients, project, custom_action)
+    recipients = add_custom_notifications(recipients, project, custom_action)
     recipients = reject_mention_users(recipients, project)
 
     recipients = recipients.uniq

app/views/notifications/buttons/_notifications.html.haml

+- if notification_setting
+  .dropdown.notification-dropdown.pull-right
+    - url = notification_setting.persisted? ? notification_setting_path(notification_setting) : notification_settings_path(notification_setting)
+
+    = form_for notification_setting, remote: true, html: { class: "inline", id: "notification-form" } do |f|
+      = hidden_setting_source_input(notification_setting)
+      = f.hidden_field :level, class: "notification_setting_level"
+      .js-notification-toggle-btns
+        .btn-group
+          - if notification_setting.level != "custom"
+            %button.dropdown-new.btn.btn-default.notifications-btn#notifications-button{ type: "button", data: { toggle: "dropdown", target: notifications_menu_identifier("dropdown", notification_setting) } }
+              = icon("bell", class: "js-notification-loading")
+              = notification_title(notification_setting.level)
+              = icon("caret-down")
+          - else
+            %button.dropdown-new.btn.btn-default.notifications-btn#notifications-button{ type: "button", data: { toggle: "modal", target: "#" + notifications_menu_identifier("modal", notification_setting) } }
+              = icon("bell", class: "js-notification-loading")
+              = notification_title(notification_setting.level)
+            %button.btn.btn-danger.dropdown-toggle{ data: { toggle: "dropdown", target: notifications_menu_identifier("dropdown", notification_setting) } }
+              %span.caret
+              .sr-only Toggle dropdown
+          = render "shared/notifications/notification_dropdown", notification_setting: notification_setting
+
+          = content_for :scripts_body do
+            = render "shared/notifications/custom_notifications", notification_setting: notification_setting

app/views/profiles/notifications/_group_settings.html.haml

@@ -9,5 +9,4 @@
     = link_to group.name, group_path(group)
 
   .pull-right
-    = form_for [group, setting], remote: true, html: { class: 'update-notifications' } do |f|
-      = f.select :level, NotificationSetting.levels.keys, {}, class: 'form-control trigger-submit'
+    = render 'notifications/buttons/notifications', notification_setting: setting

app/views/profiles/notifications/_project_settings.html.haml

@@ -9,5 +9,4 @@
     = link_to_project(project)
 
   .pull-right
-    = form_for [project.namespace.becomes(Namespace), project, setting], remote: true, html: { class: 'update-notifications' } do |f|
-      = f.select :level, NotificationSetting.levels.keys, {}, class: 'form-control trigger-submit'
+    = render 'notifications/buttons/notifications', notification_setting: setting

app/views/profiles/notifications/show.html.haml

@@ -24,12 +24,12 @@
         .form-group
           = f.label :notification_email, class: "label-light"
           = f.select :notification_email, @user.all_emails, { include_blank: false }, class: "select2"
-        .form-group
-          = f.label :notification_level, class: 'label-light'
-          = notification_level_radio_buttons
 
-        .prepend-top-default
-          = f.submit 'Update settings', class: "btn btn-create"
+      .form-group.pull-left
+        = render 'notifications/buttons/notifications', notification_setting: @global_notification_setting
+
+      .clearfix
+
       %hr
       %h5
         Groups (#{@group_notifications.count})

app/views/projects/_home_panel.html.haml

@@ -32,7 +32,7 @@
       .project-repo-buttons.btn-group.project-right-buttons
         = render "projects/buttons/download"
         = render 'projects/buttons/dropdown'
-        = render 'projects/buttons/notifications'
+        = render 'notifications/buttons/notifications', notification_setting: @notification_setting
 
 :javascript
   new Star();

app/views/projects/buttons/_notifications.html.haml

-- if @notification_setting
-  .dropdown.notification-dropdown.pull-right
-    = form_for @notification_setting, url: namespace_project_notification_setting_path(@project.namespace.becomes(Namespace), @project), method: :patch, remote: true, html: { class: "inline", id: "notification-form" } do |f|
-      = f.hidden_field :level
-      .js-notification-toggle-btns
-        - if @notification_setting.level != "custom"
-          %button.dropdown-new.btn.btn-default.notifications-btn#notifications-button{ type: "button", data: { toggle: "dropdown", target: ".notification-dropdown" } }
-            = icon("bell", class: "js-notification-loading")
-            = notification_title(@notification_setting.level)
-            = icon("caret-down")
-        - else
-          .btn-group
-            %button.dropdown-new.btn.btn-default.notifications-btn#notifications-button{ type: "button", data: { toggle: "modal", target: "#custom-notifications-modal" } }
-              = icon("bell", class: "js-notification-loading")
-              = notification_title(@notification_setting.level)
-            %button.btn.btn-danger.dropdown-toggle{ data: { toggle: "dropdown", target: ".notification-dropdown" } }
-              %span.caret
-              .sr-only Toggle dropdown
-      = render "shared/projects/notification_dropdown"
-  = content_for :scripts_body do
-    = render "shared/projects/custom_notifications"

app/views/shared/projects/_custom_notifications.html.haml → app/views/shared/notifications/_custom_notifications.html.haml

-#custom-notifications-modal.modal.fade{ tabindex: "-1", role: "dialog", aria: { labelledby: "custom-notifications-title" } }
+.modal.fade{ tabindex: "-1", role: "dialog", id: notifications_menu_identifier("modal", notification_setting), aria: { labelledby: "custom-notifications-title" } }
   .modal-dialog
     .modal-content
       .modal-header
@@ -6,10 +6,11 @@
           %span{ aria: { hidden: "true" } } ×
         %h4#custom-notifications-title.modal-title
           Custom notification events
+
       .modal-body
         .container-fluid
-          = form_for @notification_setting, url: namespace_project_notification_setting_path(@project.namespace.becomes(Namespace), @project), method: :patch, html: { class: "custom-notifications-form" } do |f|
-            = f.hidden_field :level
+          = form_for notification_setting, html: { class: "custom-notifications-form" } do |f|
+            = hidden_setting_source_input(notification_setting)
             .row
               .col-lg-3
                 %h4.prepend-top-0
@@ -20,7 +21,7 @@
                   .form-group
                     .checkbox{ class: ("prepend-top-0" if index == 0) }
                       %label{ for: "events_#{event}" }
-                        = check_box("notification_setting", event, {id: "events_#{event}", class: "js-custom-notification-event"})
+                        = check_box("", event, { name: "notification_setting[#{event}]", class: "js-custom-notification-event", checked: notification_setting.events[event] })
 
                         %strong
                           = event.to_s.humanize

app/views/shared/notifications/_notification_dropdown.html.haml

+%ul.dropdown-menu.dropdown-menu-no-wrap.dropdown-menu-align-right.dropdown-menu-selectable.dropdown-menu-large{ role: "menu", class: notifications_menu_identifier("dropdown", notification_setting) }
+  - NotificationSetting.levels.each_key do |level|
+    - next if level == "custom"
+    - next if level == "global" && notification_setting.source.nil?
+
+    = notification_list_item(level, notification_setting)
+
+  - unless notification_setting.custom?
+    %li.divider
+    %li
+      %a.update-notification{ href: "#", role: "button", data: { toggle: "modal", target: "#" + notifications_menu_identifier("modal", notification_setting), notification_level: "custom", notification_title: "Custom" } }
+        Custom

app/views/shared/projects/_notification_dropdown.html.haml

-%ul.dropdown-menu.dropdown-menu-no-wrap.dropdown-menu-align-right.dropdown-menu-selectable.dropdown-menu-large{ role: "menu" }
-  - NotificationSetting.levels.each do |level|
-    - if level.first != "custom"
-      = notification_list_item(level.first, @notification_setting)
-  - if @notification_setting.level != "custom"
-    %li.divider
-    %li
-      %a.update-notification{ href: "#", role: "button", data: { toggle: "modal", target: "#custom-notifications-modal", notification_level: "custom", notification_title: "Custom" } }
-        Custom

config/routes.rb

@@ -118,9 +118,17 @@ Rails.application.routes.draw do
     end
   end
 
+  #
   # Spam reports
+  #
   resources :abuse_reports, only: [:new, :create]
 
+  #
+  # Notification settings
+  #
+  resources :notification_settings, only: [:create, :update]
+
+
   #
   # Import
   #
@@ -416,7 +424,6 @@ Rails.application.routes.draw do
 
       resource :avatar, only: [:destroy]
       resources :milestones, constraints: { id: /[^\/]+/ }, only: [:index, :show, :update, :new, :create]
-      resource :notification_setting, only: [:update]
     end
   end
 
@@ -648,7 +655,6 @@ Rails.application.routes.draw do
 
         resources :forks, only: [:index, :new, :create]
         resource :import, only: [:new, :create, :show]
-        resource :notification_setting, only: [:update]
 
         resources :refs, only: [] do
           collection do

spec/controllers/groups/notification_settings_controller_spec.rb

-require 'spec_helper'
-
-describe Groups::NotificationSettingsController do
-  let(:group) { create(:group) }
-  let(:user) { create(:user) }
-
-  describe '#update' do
-    context 'when not authorized' do
-      it 'redirects to sign in page' do
-        put :update,
-            group_id: group.to_param,
-            notification_setting: { level: :participating }
-
-        expect(response).to redirect_to(new_user_session_path)
-      end
-    end
-
-    context 'when authorized' do
-      before do
-        sign_in(user)
-      end
-
-      it 'returns success' do
-        put :update,
-            group_id: group.to_param,
-            notification_setting: { level: :participating }
-
-        expect(response.status).to eq 200
-      end
-    end
-  end
-end

spec/controllers/projects/notification_settings_controller_spec.rb → spec/controllers/notification_settings_controller_spec.rb

 require 'spec_helper'
 
-describe Projects::NotificationSettingsController do
+describe NotificationSettingsController do
   let(:project) { create(:empty_project) }
   let(:user) { create(:user) }
 
@@ -8,12 +8,11 @@ describe Projects::NotificationSettingsController do
     project.team << [user, :developer]
   end
 
-  describe '#update' do
+  describe '#create' do
     context 'when not authorized' do
       it 'redirects to sign in page' do
-        put :update,
-            namespace_id: project.namespace.to_param,
-            project_id: project.to_param,
+        post :create,
+             project: { id: project.id },
              notification_setting: { level: :participating }
 
         expect(response).to redirect_to(new_user_session_path)
@@ -26,9 +25,8 @@ describe Projects::NotificationSettingsController do
       end
 
       it 'returns success' do
-        put :update,
-            namespace_id: project.namespace.to_param,
-            project_id: project.to_param,
+        post :create,
+             project: { id: project.id },
              notification_setting: { level: :participating }
 
         expect(response.status).to eq 200
@@ -44,9 +42,8 @@ describe Projects::NotificationSettingsController do
         end
 
         it 'returns success' do
-          put :update,
-              namespace_id: project.namespace.to_param,
-              project_id: project.to_param,
+          post :create,
+               project: { id: project.id },
                notification_setting: { level: :participating, events: custom_events }
 
           expect(response.status).to eq 200
@@ -58,10 +55,67 @@ describe Projects::NotificationSettingsController do
       let(:private_project) { create(:project, :private) }
       before { sign_in(user) }
 
+      it 'returns 404' do
+        post :create,
+             project: { id: private_project.id },
+             notification_setting: { level: :participating }
+
+        expect(response.status).to eq(404)
+      end
+    end
+  end
+
+  describe '#update' do
+    let(:notification_setting) { user.global_notification_setting }
+
+    context 'when not authorized' do
+      it 'redirects to sign in page' do
+        put :update,
+            id: notification_setting,
+            notification_setting: { level: :participating }
+
+        expect(response).to redirect_to(new_user_session_path)
+      end
+    end
+
+    context 'when authorized' do
+      before{ sign_in(user) }
+
+      it 'returns success' do
+        put :update,
+            id: notification_setting,
+            notification_setting: { level: :participating }
+
+        expect(response.status).to eq 200
+      end
+
+      context 'and setting custom notification setting' do
+        let(:custom_events) do
+          events = {}
+
+          NotificationSetting::EMAIL_EVENTS.each do |event|
+            events[event] = "true"
+          end
+        end
+
+        it 'returns success' do
+          put :update,
+              id: notification_setting,
+              notification_setting: { level: :participating, events: custom_events }
+
+          expect(response.status).to eq 200
+        end
+      end
+    end
+
+    context 'not authorized' do
+      let(:other_user) { create(:user) }
+
+      before { sign_in(other_user) }
+
       it 'returns 404' do
         put :update,
-            namespace_id: private_project.namespace.to_param,
-            project_id: private_project.to_param,
+            id: notification_setting,
             notification_setting: { level: :participating }
 
         expect(response.status).to eq(404)