Allow users to set custom notifications in projects they don't own and several fixes to code

12 files changed, 24 insertions, 59 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 48c418a8beb..1f85142a78b 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -54,7 +54,7 @@ v 8.9.0 (unreleased)
   - Remove 'main language' feature
   - Pipelines can be canceled only when there are running builds
   - Use downcased path to container repository as this is expected path by Docker
-  - Customized notification settings for projects
+  - Custom notification settings
   - Projects pending deletion will render a 404 page
   - Measure queue duration between gitlab-workhorse and Rails
   - Make Omniauth providers specs to not modify global configuration
diff --git a/app/assets/javascripts/notifications_dropdown.js.coffee b/app/assets/javascripts/notifications_dropdown.js.coffee
index c13010f56ec..74d2298c1fa 100644
--- a/app/assets/javascripts/notifications_dropdown.js.coffee
+++ b/app/assets/javascripts/notifications_dropdown.js.coffee
@@ -9,14 +9,14 @@ class @NotificationsDropdown
 
         notificationLevel = $(@).data 'notification-level'
         label = $(@).data 'notification-title'
-        form = $(this).parents('form:first')
+        form = $(this).parents('.notification-form:first')
         form.find('.js-notification-loading').toggleClass 'fa-bell fa-spin fa-spinner'
         form.find('#notification_setting_level').val(notificationLevel)
-        form.submit();
+        form.submit()
 
     $(document)
-      .off 'ajax:success', '#notification-form'
-      .on 'ajax:success', '#notification-form', (e, data) ->
+      .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)
diff --git a/app/controllers/notification_settings_controller.rb b/app/controllers/notification_settings_controller.rb
index 5d425ad8420..acda174c229 100644
--- a/app/controllers/notification_settings_controller.rb
+++ b/app/controllers/notification_settings_controller.rb
@@ -2,12 +2,16 @@ class NotificationSettingsController < ApplicationController
   before_action :authenticate_user!
 
   def create
-    project = current_user.projects.find(params[:project][:id])
+    project = Project.find(params[:project][:id])
 
-    @notification_setting = current_user.notification_settings_for(project)
-    @saved = @notification_setting.update_attributes(notification_setting_params)
+    if can?(current_user, :read_project, project)
+      @notification_setting = current_user.notification_settings_for(project)
+      @saved = @notification_setting.update_attributes(notification_setting_params)
 
-    render_response
+      render_response
+    else
+      render_404
+    end
   end
 
   def update
@@ -21,7 +25,7 @@ class NotificationSettingsController < ApplicationController
 
   def render_response
     render json: {
-      html: view_to_html_string("notifications/buttons/_notifications", notification_setting: @notification_setting),
+      html: view_to_html_string("shared/notifications/buttons/_button", notification_setting: @notification_setting),
       saved: @saved
     }
   end
diff --git a/app/views/profiles/notifications/_group_settings.html.haml b/app/views/profiles/notifications/_group_settings.html.haml
index 04becd5d860..80c165db01b 100644
--- a/app/views/profiles/notifications/_group_settings.html.haml
+++ b/app/views/profiles/notifications/_group_settings.html.haml
@@ -9,4 +9,4 @@
     = link_to group.name, group_path(group)
 
   .pull-right
-    = render 'notifications/buttons/notifications', notification_setting: setting
+    = render '/shared/notifications/buttons/button', notification_setting: setting
diff --git a/app/views/profiles/notifications/_project_settings.html.haml b/app/views/profiles/notifications/_project_settings.html.haml
index 664a17418db..e1a06a75e0c 100644
--- a/app/views/profiles/notifications/_project_settings.html.haml
+++ b/app/views/profiles/notifications/_project_settings.html.haml
@@ -9,4 +9,4 @@
     = link_to_project(project)
 
   .pull-right
-    = render 'notifications/buttons/notifications', notification_setting: setting
+    = render '/shared/notifications/buttons/button', notification_setting: setting
diff --git a/app/views/profiles/notifications/show.html.haml b/app/views/profiles/notifications/show.html.haml
index cfbb4d79da2..cd0112d6278 100644
--- a/app/views/profiles/notifications/show.html.haml
+++ b/app/views/profiles/notifications/show.html.haml
@@ -25,11 +25,11 @@
           = f.label :notification_email, class: "label-light"
           = f.select :notification_email, @user.all_emails, { include_blank: false }, class: "select2"
 
-        = label_tag :global_notification_level, "Global notification level", class: "label-light"
-        %br
-        .clearfix
-        .form-group.pull-left
-          = render 'notifications/buttons/notifications', notification_setting: @global_notification_setting, left_align: true
+      = label_tag :global_notification_level, "Global notification level", class: "label-light"
+      %br
+      .clearfix
+      .form-group.pull-left
+        = render '/shared/notifications/buttons/button', notification_setting: @global_notification_setting, left_align: true
 
       .clearfix
 
diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml
index df725d81a39..7dc762486e4 100644
--- a/app/views/projects/_home_panel.html.haml
+++ b/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 'notifications/buttons/notifications', notification_setting: @notification_setting
+        = render '/shared/notifications/buttons/button', notification_setting: @notification_setting
 
 :javascript
   new Star();
diff --git a/app/views/notifications/buttons/_notifications.html.haml b/app/views/shared/notifications/buttons/_button.html.haml
index f97cde0ae0c..ff1cf966a9b 100644
--- a/app/views/notifications/buttons/_notifications.html.haml
+++ b/app/views/shared/notifications/buttons/_button.html.haml
@@ -1,7 +1,7 @@
 - left_align = local_assigns[:left_align]
 - if notification_setting
   .dropdown.notification-dropdown.pull-right
-    = form_for notification_setting, remote: true, html: { class: "inline", id: "notification-form" } do |f|
+    = form_for notification_setting, remote: true, html: { class: "inline notification-form" } do |f|
       = hidden_setting_source_input(notification_setting)
       = f.hidden_field :level, class: "notification_setting_level"
       .js-notification-toggle-btns
diff --git a/app/views/shared/projects/_custom_notifications.html.haml b/app/views/shared/projects/_custom_notifications.html.haml
deleted file mode 100644
index ec4bad4ba3a..00000000000
--- a/app/views/shared/projects/_custom_notifications.html.haml
+++ /dev/null
@@ -1,26 +0,0 @@
-#custom-notifications-modal.modal.fade{ tabindex: "-1", role: "dialog", aria: { labelledby: "custom-notifications-title" } }
-  .modal-dialog
-    .modal-content
-      .modal-header
-        %button.close{ type: "button", data: { dismiss: "modal" }, aria: { label: "close" } }
-          %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|
-            .row
-              .col-lg-3
-                %h4.prepend-top-0
-                  Notification events
-              .col-lg-9
-                - NotificationSetting::EMAIL_EVENTS.each do |event, index|
-                  = index
-                  .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"})
-
-                        %strong
-                          = event.to_s.humanize
-                          = icon("spinner spin", class: "custom-notification-event-loading")
diff --git a/app/views/shared/projects/_notification_dropdown.html.haml b/app/views/shared/projects/_notification_dropdown.html.haml
deleted file mode 100644
index 103d5f12f5c..00000000000
--- a/app/views/shared/projects/_notification_dropdown.html.haml
+++ /dev/null
@@ -1,9 +0,0 @@
-%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)
-  %li.divider
-  %li
-    %a.update-notification{ href: "#", role: "button", data: { toggle: "modal", target: "#custom-notifications-modal", notification_level: "custom", notification_title: "Custom" } }
-      %strong.dropdown-menu-inner-title Custom
-      %span.dropdown-menu-inner-content= notification_description("custom")
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 7818fc290f8..cc29c7ef428 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -272,8 +272,7 @@ module API
       expose :access_level
       expose :notification_level do |member, options|
         if member.notification_setting
-          setting = member.notification_setting
-          { level: NotificationSetting.levels[setting.level], events: setting.events }
+          NotificationSetting.levels[member.notification_setting.level]
         end
       end
     end
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index c4edb8c3fd0..776a6ab5edb 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -146,9 +146,6 @@ describe NotificationService, services: true do
       let(:note) { create(:note_on_issue, noteable: confidential_issue, project: project, note: "#{author.to_reference} #{assignee.to_reference} #{non_member.to_reference} #{member.to_reference} #{admin.to_reference}") }
       let(:guest_watcher) { create_user_with_notification(:watch, "guest-watcher-confidential") }
 
-      before do
-      end
-
       it 'filters out users that can not read the issue' do
         project.team << [member, :developer]
         project.team << [guest, :guest]