Merge branch 'ee-5863-customize-notifications-for-new-epic' into 'master'

Backport of EE changes "Customize notifications for new epic - Web and API"

See merge request gitlab-org/gitlab-ce!20793

12 files changed, 87 insertions, 31 deletions

diff --git a/.rubocop.yml b/.rubocop.yml
index a586190319b..9858bbe0ddd 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -48,6 +48,8 @@ Naming/FileName:
     - 'qa/bin/*'
     - 'config/**/*'
     - 'lib/generators/**/*'
+    - 'locale/unfound_translations.rb'
+    - 'ee/locale/unfound_translations.rb'
     - 'ee/lib/generators/**/*'
   IgnoreExecutableScripts: true
   AllowedAcronyms:
diff --git a/app/controllers/notification_settings_controller.rb b/app/controllers/notification_settings_controller.rb
index ed20302487c..461f26561f1 100644
--- a/app/controllers/notification_settings_controller.rb
+++ b/app/controllers/notification_settings_controller.rb
@@ -5,14 +5,14 @@ class NotificationSettingsController < ApplicationController
     return render_404 unless can_read?(resource)
 
     @notification_setting = current_user.notification_settings_for(resource)
-    @saved = @notification_setting.update(notification_setting_params)
+    @saved = @notification_setting.update(notification_setting_params_for(resource))
 
     render_response
   end
 
   def update
     @notification_setting = current_user.notification_settings.find(params[:id])
-    @saved = @notification_setting.update(notification_setting_params)
+    @saved = @notification_setting.update(notification_setting_params_for(@notification_setting.source))
 
     render_response
   end
@@ -42,8 +42,8 @@ class NotificationSettingsController < ApplicationController
     }
   end
 
-  def notification_setting_params
-    allowed_fields = NotificationSetting::EMAIL_EVENTS.dup
+  def notification_setting_params_for(source)
+    allowed_fields = NotificationSetting.email_events(source).dup
     allowed_fields << :level
     params.require(:notification_setting).permit(allowed_fields)
   end
diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb
index 3e42063224e..a185f2916d4 100644
--- a/app/helpers/notifications_helper.rb
+++ b/app/helpers/notifications_helper.rb
@@ -83,21 +83,11 @@ module NotificationsHelper
   end
 
   def notification_event_name(event)
-    # All values from NotificationSetting::EMAIL_EVENTS
+    # All values from NotificationSetting.email_events
     case event
     when :success_pipeline
       s_('NotificationEvent|Successful pipeline')
     else
-      N_('NotificationEvent|New note')
-      N_('NotificationEvent|New issue')
-      N_('NotificationEvent|Reopen issue')
-      N_('NotificationEvent|Close issue')
-      N_('NotificationEvent|Reassign issue')
-      N_('NotificationEvent|New merge request')
-      N_('NotificationEvent|Close merge request')
-      N_('NotificationEvent|Reassign merge request')
-      N_('NotificationEvent|Merge merge request')
-      N_('NotificationEvent|Failed pipeline')
       s_(event.to_s.humanize)
     end
   end
diff --git a/app/models/notification_setting.rb b/app/models/notification_setting.rb
index 1df3a51a7fc..1600acfc575 100644
--- a/app/models/notification_setting.rb
+++ b/app/models/notification_setting.rb
@@ -45,6 +45,15 @@ class NotificationSetting < ActiveRecord::Base
     :success_pipeline
   ].freeze
 
+  # Update unfound_translations.rb when events are changed
+  def self.email_events(source = nil)
+    EMAIL_EVENTS
+  end
+
+  def email_events
+    self.class.email_events(source)
+  end
+
   EXCLUDED_PARTICIPATING_EVENTS = [
     :success_pipeline
   ].freeze
diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb
index 4389fd89538..5c0e8a35cb0 100644
--- a/app/services/notification_recipient_service.rb
+++ b/app/services/notification_recipient_service.rb
@@ -130,7 +130,7 @@ module NotificationRecipientService
       end
 
       def add_project_watchers
-        add_recipients(project_watchers, :watch, nil)
+        add_recipients(project_watchers, :watch, nil) if project
       end
 
       def add_group_watchers
@@ -220,6 +220,8 @@ module NotificationRecipientService
     end
 
     class Default < Base
+      MENTION_TYPE_ACTIONS = [:new_issue, :new_merge_request].freeze
+
       attr_reader :target
       attr_reader :current_user
       attr_reader :action
@@ -252,7 +254,7 @@ module NotificationRecipientService
 
         add_subscribed_users
 
-        if [:new_issue, :new_merge_request].include?(custom_action)
+        if self.class.mention_type_actions.include?(custom_action)
           # These will all be participants as well, but adding with the :mention
           # type ensures that users with the mention notification level will
           # receive them, too.
@@ -279,10 +281,14 @@ module NotificationRecipientService
       end
 
       # Build event key to search on custom notification level
-      # Check NotificationSetting::EMAIL_EVENTS
+      # Check NotificationSetting.email_events
       def custom_action
         @custom_action ||= "#{action}_#{target.class.model_name.name.underscore}".to_sym
       end
+
+      def self.mention_type_actions
+        MENTION_TYPE_ACTIONS.dup
+      end
     end
 
     class NewNote < Base
diff --git a/app/views/shared/notifications/_custom_notifications.html.haml b/app/views/shared/notifications/_custom_notifications.html.haml
index 1f6e8f98bbb..43a87fd8397 100644
--- a/app/views/shared/notifications/_custom_notifications.html.haml
+++ b/app/views/shared/notifications/_custom_notifications.html.haml
@@ -19,7 +19,7 @@
                   - paragraph = _('Custom notification levels are the same as participating levels. With custom notification levels you will also receive notifications for select events. To find out more, check out %{notification_link}.') % { notification_link: notification_link.html_safe }
                   #{ paragraph.html_safe }
               .col-lg-8
-                - NotificationSetting::EMAIL_EVENTS.each_with_index do |event, index|
+                - notification_setting.email_events.each_with_index do |event, index|
                   - field_id = "#{notifications_menu_identifier("modal", notification_setting)}_notification_setting[#{event}]"
                   .form-group
                     .form-check{ class: ("prepend-top-0" if index == 0) }
diff --git a/doc/api/notification_settings.md b/doc/api/notification_settings.md
index 682b90361bd..165b9a11c7a 100644
--- a/doc/api/notification_settings.md
+++ b/doc/api/notification_settings.md
@@ -15,7 +15,7 @@ mention
 custom
 ```
 
-If the `custom` level is used, specific email events can be controlled. Notification email events are defined in the `NotificationSetting::EMAIL_EVENTS` model variable. Currently, these events are recognized:
+If the `custom` level is used, specific email events can be controlled. Available events are returned by `NotificationSetting.email_events`. Currently, these events are recognized:
 
 ```
 new_note
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 27f28e1df93..453ebb9c669 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -856,7 +856,7 @@ module API
     class NotificationSetting < Grape::Entity
       expose :level
       expose :events, if: ->(notification_setting, _) { notification_setting.custom? } do
-        ::NotificationSetting::EMAIL_EVENTS.each do |event|
+        ::NotificationSetting.email_events.each do |event|
           expose event
         end
       end
diff --git a/lib/api/notification_settings.rb b/lib/api/notification_settings.rb
index 0266bf2f717..bf0d6b9e434 100644
--- a/lib/api/notification_settings.rb
+++ b/lib/api/notification_settings.rb
@@ -23,7 +23,7 @@ module API
       params do
         optional :level, type: String, desc: 'The global notification level'
         optional :notification_email, type: String, desc: 'The email address to send notifications'
-        NotificationSetting::EMAIL_EVENTS.each do |event|
+        NotificationSetting.email_events.each do |event|
           optional event, type: Boolean, desc: 'Enable/disable this notification'
         end
       end
@@ -50,7 +50,9 @@ module API
       end
     end
 
-    %w[group project].each do |source_type|
+    [Group, Project].each do |source_class|
+      source_type = source_class.name.underscore
+
       params do
         requires :id, type: String, desc: "The #{source_type} ID"
       end
@@ -73,7 +75,7 @@ module API
         end
         params do
           optional :level, type: String, desc: "The #{source_type} notification level"
-          NotificationSetting::EMAIL_EVENTS.each do |event|
+          NotificationSetting.email_events(source_class).each do |event|
             optional event, type: Boolean, desc: 'Enable/disable this notification'
           end
         end
diff --git a/locale/unfound_translations.rb b/locale/unfound_translations.rb
new file mode 100644
index 00000000000..0826d64049b
--- /dev/null
+++ b/locale/unfound_translations.rb
@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+# Dynamic translations which needs to be marked by `N_` so they can be found by `rake gettext:find`, see:
+# https://github.com/grosser/gettext_i18n_rails#unfound-translations-with-rake-gettextfind
+
+# NotificationSetting.email_events
+N_('NotificationEvent|New note')
+N_('NotificationEvent|New issue')
+N_('NotificationEvent|Reopen issue')
+N_('NotificationEvent|Close issue')
+N_('NotificationEvent|Reassign issue')
+N_('NotificationEvent|New merge request')
+N_('NotificationEvent|Close merge request')
+N_('NotificationEvent|Reassign merge request')
+N_('NotificationEvent|Merge merge request')
+N_('NotificationEvent|Failed pipeline')
diff --git a/spec/controllers/notification_settings_controller_spec.rb b/spec/controllers/notification_settings_controller_spec.rb
index e133950e684..a3356a86d4b 100644
--- a/spec/controllers/notification_settings_controller_spec.rb
+++ b/spec/controllers/notification_settings_controller_spec.rb
@@ -21,10 +21,11 @@ describe NotificationSettingsController do
     end
 
     context 'when authorized' do
+      let(:notification_setting) { user.notification_settings_for(source) }
       let(:custom_events) do
         events = {}
 
-        NotificationSetting::EMAIL_EVENTS.each do |event|
+        NotificationSetting.email_events(source).each do |event|
           events[event.to_s] = true
         end
 
@@ -36,7 +37,7 @@ describe NotificationSettingsController do
       end
 
       context 'for projects' do
-        let(:notification_setting) { user.notification_settings_for(project) }
+        let(:source) { project }
 
         it 'creates notification setting' do
           post :create,
@@ -67,7 +68,7 @@ describe NotificationSettingsController do
       end
 
       context 'for groups' do
-        let(:notification_setting) { user.notification_settings_for(group) }
+        let(:source) { group }
 
         it 'creates notification setting' do
           post :create,
@@ -145,7 +146,7 @@ describe NotificationSettingsController do
         let(:custom_events) do
           events = {}
 
-          NotificationSetting::EMAIL_EVENTS.each do |event|
+          notification_setting.email_events.each do |event|
             events[event] = "true"
           end
         end
diff --git a/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb
index 77c475b9f52..e545b674b4f 100644
--- a/spec/models/notification_setting_spec.rb
+++ b/spec/models/notification_setting_spec.rb
@@ -94,9 +94,39 @@ RSpec.describe NotificationSetting do
     end
   end
 
-  context 'email events' do
-    it 'includes EXCLUDED_WATCHER_EVENTS in EMAIL_EVENTS' do
-      expect(described_class::EMAIL_EVENTS).to include(*described_class::EXCLUDED_WATCHER_EVENTS)
+  describe '.email_events' do
+    subject { described_class.email_events }
+
+    it 'returns email events' do
+      expect(subject).to include(
+        :new_note,
+        :new_issue,
+        :reopen_issue,
+        :close_issue,
+        :reassign_issue,
+        :new_merge_request,
+        :reopen_merge_request,
+        :close_merge_request,
+        :reassign_merge_request,
+        :merge_merge_request,
+        :failed_pipeline,
+        :success_pipeline
+      )
+    end
+
+    it 'includes EXCLUDED_WATCHER_EVENTS' do
+      expect(subject).to include(*described_class::EXCLUDED_WATCHER_EVENTS)
+    end
+  end
+
+  describe '#email_events' do
+    let(:source) { build(:group) }
+
+    subject { build(:notification_setting, source: source) }
+
+    it 'calls email_events' do
+      expect(described_class).to receive(:email_events).with(source)
+      subject.email_events
     end
   end
 end