Backport 5480-epic-notifications from EE

7 files changed, 57 insertions, 26 deletions

diff --git a/app/controllers/sent_notifications_controller.rb b/app/controllers/sent_notifications_controller.rb
index 04c36b3ebfe..93a71103a09 100644
--- a/app/controllers/sent_notifications_controller.rb
+++ b/app/controllers/sent_notifications_controller.rb
@@ -17,16 +17,20 @@ class SentNotificationsController < ApplicationController
     flash[:notice] = "You have been unsubscribed from this thread."
 
     if current_user
-      case noteable
-      when Issue
-        redirect_to issue_path(noteable)
-      when MergeRequest
-        redirect_to merge_request_path(noteable)
-      else
-        redirect_to root_path
-      end
+      redirect_to noteable_path(noteable)
     else
       redirect_to new_user_session_path
     end
   end
+
+  def noteable_path(noteable)
+    case noteable
+    when Issue
+      issue_path(noteable)
+    when MergeRequest
+      merge_request_path(noteable)
+    else
+      root_path
+    end
+  end
 end
diff --git a/app/mailers/emails/notes.rb b/app/mailers/emails/notes.rb
index 50e17fe7717..d9a6fe2a41e 100644
--- a/app/mailers/emails/notes.rb
+++ b/app/mailers/emails/notes.rb
@@ -43,7 +43,7 @@ module Emails
     private
 
     def note_target_url_options
-      [@project, @note.noteable, anchor: "note_#{@note.id}"]
+      [@project || @group, @note.noteable, anchor: "note_#{@note.id}"]
     end
 
     def note_thread_options(recipient_id)
@@ -58,8 +58,9 @@ module Emails
       # `note_id` is a `Note` when originating in `NotifyPreview`
       @note = note_id.is_a?(Note) ? note_id : Note.find(note_id)
       @project = @note.project
+      @group = @note.noteable.try(:group)
 
-      if @project && @note.persisted?
+      if (@project || @group) && @note.persisted?
         @sent_notification = SentNotification.record_note(@note, recipient_id, reply_key)
       end
     end
diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb
index 3646e08a15f..1db1482d6b7 100644
--- a/app/mailers/notify.rb
+++ b/app/mailers/notify.rb
@@ -94,6 +94,7 @@ class Notify < BaseMailer
   def subject(*extra)
     subject = ""
     subject << "#{@project.name} | " if @project
+    subject << "#{@group.name} | " if @group
     subject << extra.join(' | ') if extra.present?
     subject << " | #{Gitlab.config.gitlab.email_subject_suffix}" if Gitlab.config.gitlab.email_subject_suffix.present?
     subject
@@ -117,10 +118,9 @@ class Notify < BaseMailer
     @reason = headers['X-GitLab-NotificationReason']
 
     if Gitlab::IncomingEmail.enabled? && @sent_notification
-      address = Mail::Address.new(Gitlab::IncomingEmail.reply_address(reply_key))
-      address.display_name = @project.full_name
-
-      headers['Reply-To'] = address
+      headers['Reply-To'] = Mail::Address.new(Gitlab::IncomingEmail.reply_address(reply_key)).tap do |address|
+        address.display_name = reply_display_name(model)
+      end
 
       fallback_reply_message_id = "<reply-#{reply_key}@#{Gitlab.config.gitlab.host}>".freeze
       headers['References'] ||= []
@@ -132,6 +132,11 @@ class Notify < BaseMailer
     mail(headers)
   end
 
+  # `model` is used on EE code
+  def reply_display_name(_model)
+    @project.full_name
+  end
+
   # Send an email that starts a new conversation thread,
   # with headers suitable for grouping by thread in email clients.
   #
diff --git a/app/models/note.rb b/app/models/note.rb
index e426f84832b..109405d3f17 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -317,10 +317,6 @@ class Note < ActiveRecord::Base
     !system? && !for_snippet?
   end
 
-  def can_create_notification?
-    true
-  end
-
   def discussion_class(noteable = nil)
     # When commit notes are rendered on an MR's Discussion page, they are
     # displayed in one discussion instead of individually.
diff --git a/app/models/sent_notification.rb b/app/models/sent_notification.rb
index 6e311806be1..3da7c301d28 100644
--- a/app/models/sent_notification.rb
+++ b/app/models/sent_notification.rb
@@ -5,14 +5,14 @@ class SentNotification < ActiveRecord::Base
   belongs_to :noteable, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
   belongs_to :recipient, class_name: "User"
 
-  validates :project, :recipient, presence: true
+  validates :recipient, presence: true
   validates :reply_key, presence: true, uniqueness: true
   validates :noteable_id, presence: true, unless: :for_commit?
   validates :commit_id, presence: true, if: :for_commit?
   validates :in_reply_to_discussion_id, format: { with: /\A\h{40}\z/, allow_nil: true }
   validate :note_valid
 
-  after_save :keep_around_commit
+  after_save :keep_around_commit, if: :for_commit?
 
   class << self
     def reply_key
diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb
index 83e59a649b6..5658699664d 100644
--- a/app/services/notification_recipient_service.rb
+++ b/app/services/notification_recipient_service.rb
@@ -45,6 +45,10 @@ module NotificationRecipientService
         target.project
       end
 
+      def group
+        project&.group || target.try(:group)
+      end
+
       def recipients
         @recipients ||= []
       end
@@ -67,6 +71,7 @@ module NotificationRecipientService
           user, type,
           reason: reason,
           project: project,
+          group: group,
           custom_action: custom_action,
           target: target,
           acting_user: acting_user
@@ -107,11 +112,11 @@ module NotificationRecipientService
 
         # Users with a notification setting on group or project
         user_ids += user_ids_notifiable_on(project, :custom)
-        user_ids += user_ids_notifiable_on(project.group, :custom)
+        user_ids += user_ids_notifiable_on(group, :custom)
 
         # Users with global level custom
         user_ids_with_project_level_global = user_ids_notifiable_on(project, :global)
-        user_ids_with_group_level_global   = user_ids_notifiable_on(project.group, :global)
+        user_ids_with_group_level_global   = user_ids_notifiable_on(group, :global)
 
         global_users_ids = user_ids_with_project_level_global.concat(user_ids_with_group_level_global)
         user_ids += user_ids_with_global_level_custom(global_users_ids, custom_action)
@@ -123,6 +128,10 @@ module NotificationRecipientService
         add_recipients(project_watchers, :watch, nil)
       end
 
+      def add_group_watchers
+        add_recipients(group_watchers, :watch, nil)
+      end
+
       # Get project users with WATCH notification level
       def project_watchers
         project_members_ids = user_ids_notifiable_on(project)
@@ -138,6 +147,14 @@ module NotificationRecipientService
         user_scope.where(id: user_ids_with_project_setting.concat(user_ids_with_group_setting).uniq)
       end
 
+      def group_watchers
+        user_ids_with_group_global = user_ids_notifiable_on(group, :global)
+        user_ids = user_ids_with_global_level_watch(user_ids_with_group_global)
+        user_ids_with_group_setting = select_group_members_ids(group, [], user_ids_with_group_global, user_ids)
+
+        user_scope.where(id: user_ids_with_group_setting)
+      end
+
       def add_subscribed_users
         return unless target.respond_to? :subscribers
 
@@ -281,6 +298,14 @@ module NotificationRecipientService
         note.project
       end
 
+      def group
+        if note.for_project_noteable?
+          project.group
+        else
+          target.try(:group)
+        end
+      end
+
       def build!
         # Add all users participating in the thread (author, assignee, comment authors)
         add_participants(note.author)
@@ -289,11 +314,11 @@ module NotificationRecipientService
         if note.for_project_noteable?
           # Merge project watchers
           add_project_watchers
-
-          # Merge project with custom notification
-          add_custom_notifications
+        else
+          add_group_watchers
         end
 
+        add_custom_notifications
         add_subscribed_users
       end
 
diff --git a/app/workers/new_note_worker.rb b/app/workers/new_note_worker.rb
index b925741934a..67c54fbf10e 100644
--- a/app/workers/new_note_worker.rb
+++ b/app/workers/new_note_worker.rb
@@ -5,7 +5,7 @@ class NewNoteWorker
   # old `NewNoteWorker` jobs (can remove later)
   def perform(note_id, _params = {})
     if note = Note.find_by(id: note_id)
-      NotificationService.new.new_note(note) if note.can_create_notification?
+      NotificationService.new.new_note(note)
       Notes::PostProcessService.new(note).execute
     else
       Rails.logger.error("NewNoteWorker: couldn't find note with ID=#{note_id}, skipping job")