Merge branch 'revert-2fb64612' into 'master'

Revert "Merge branch '226988-notify-email_participants-instead-of-external_author' into 'master'"

See merge request gitlab-org/gitlab!50699

app/mailers/emails/service_desk.rb

@@ -17,18 +17,18 @@ module Emails
         send_from_user_email: false,
         sender_name: @project.service_desk_setting&.outgoing_name
       )
-      options = service_desk_options(email_sender, 'thank_you', @issue.external_author)
+      options = service_desk_options(email_sender, 'thank_you')
                   .merge(subject: "Re: #{subject_base}")
 
       mail_new_thread(@issue, options)
     end
 
-    def service_desk_new_note_email(issue_id, note_id, recipient)
+    def service_desk_new_note_email(issue_id, note_id)
       @note = Note.find(note_id)
       setup_service_desk_mail(issue_id)
 
       email_sender = sender(@note.author_id)
-      options = service_desk_options(email_sender, 'new_note', recipient)
+      options = service_desk_options(email_sender, 'new_note')
                   .merge(subject: subject_base)
 
       mail_answer_thread(@issue, options)
@@ -44,10 +44,10 @@ module Emails
       @sent_notification = SentNotification.record(@issue, @support_bot.id, reply_key)
     end
 
-    def service_desk_options(email_sender, email_type, recipient)
+    def service_desk_options(email_sender, email_type)
       {
         from: email_sender,
-        to: recipient
+        to: @issue.external_author
       }.tap do |options|
         next unless template_body = template_content(email_type)
 

app/mailers/previews/notify_preview.rb

@@ -169,7 +169,7 @@ class NotifyPreview < ActionMailer::Preview
     cleanup do
       note = create_note(noteable_type: 'Issue', noteable_id: issue.id, note: 'Issue note content')
 
-      Notify.service_desk_new_note_email(issue.id, note.id, 'someone@gitlab.com').message
+      Notify.service_desk_new_note_email(issue.id, note.id).message
     end
   end
 

app/models/issue.rb

@@ -434,10 +434,6 @@ class Issue < ApplicationRecord
     moved_to || duplicated_to
   end
 
-  def email_participants_emails
-    issue_email_participants.pluck(:email)
-  end
-
   private
 
   def ensure_metrics

app/services/notification_service.rb

@@ -347,19 +347,18 @@ class NotificationService
   end
 
   def send_service_desk_notification(note)
+    return unless Gitlab::ServiceDesk.supported?
     return unless note.noteable_type == 'Issue'
 
     issue = note.noteable
-
-    return unless issue.issue_email_participants.any?
-
-    recipients = issue.email_participants_emails
     support_bot = User.support_bot
-    recipients.delete(issue.external_author) if note.author == support_bot
 
-    recipients.each do |recipient|
-      mailer.service_desk_new_note_email(issue.id, note.id, recipient).deliver_later
-    end
+    return unless issue.external_author.present?
+    return unless issue.project.service_desk_enabled?
+    return if note.author == support_bot
+    return unless issue.subscribed?(support_bot, issue.project)
+
+    mailer.service_desk_new_note_email(issue.id, note.id).deliver_later
   end
 
   # Notify users when a new release is created

changelogs/unreleased/226988-notify-email_participants-instead-of-external_author.yml

----
-title: Notify issue_email_participants instead of external_author
-merge_request: 48823
-author: Lee Tickett @leetickett
-type: other

spec/mailers/emails/service_desk_spec.rb

@@ -13,13 +13,8 @@ RSpec.describe Emails::ServiceDesk do
   let_it_be(:user) { create(:user) }
   let_it_be(:project) { create(:project) }
   let_it_be(:issue) { create(:issue, project: project) }
-  let_it_be(:email) { 'someone@gitlab.com' }
   let(:template) { double(content: template_content) }
 
-  before_all do
-    issue.issue_email_participants.create!(email: email)
-  end
-
   before do
     stub_const('ServiceEmailClass', Class.new(ApplicationMailer))
 
@@ -77,10 +72,6 @@ RSpec.describe Emails::ServiceDesk do
     let(:template_content) { 'custom text' }
     let(:issue) { create(:issue, project: project)}
 
-    before do
-      issue.issue_email_participants.create!(email: email)
-    end
-
     context 'when a template is in the repository' do
       let(:project) { create(:project, :custom_repo, files: { ".gitlab/service_desk_templates/#{template_key}.md" => template_content }) }
 
@@ -160,7 +151,7 @@ RSpec.describe Emails::ServiceDesk do
     let_it_be(:note) { create(:note_on_issue, noteable: issue, project: project) }
     let_it_be(:default_text) { note.note }
 
-    subject { ServiceEmailClass.service_desk_new_note_email(issue.id, note.id, email) }
+    subject { ServiceEmailClass.service_desk_new_note_email(issue.id, note.id) }
 
     it_behaves_like 'read template from repository', 'new_note'
 

spec/mailers/notify_spec.rb

@@ -1285,7 +1285,6 @@ RSpec.describe Notify do
     context 'for service desk issues' do
       before do
         issue.update!(external_author: 'service.desk@example.com')
-        issue.issue_email_participants.create!(email: 'service.desk@example.com')
       end
 
       def expect_sender(username)
@@ -1334,7 +1333,7 @@ RSpec.describe Notify do
       describe 'new note email' do
         let_it_be(:first_note) { create(:discussion_note_on_issue, note: 'Hello world') }
 
-        subject { described_class.service_desk_new_note_email(issue.id, first_note.id, 'service.desk@example.com') }
+        subject { described_class.service_desk_new_note_email(issue.id, first_note.id) }
 
         it_behaves_like 'an unsubscribeable thread'
 

spec/models/issue_spec.rb

@@ -1260,15 +1260,4 @@ RSpec.describe Issue do
       expect { issue.issue_type_supports?(:unkown_feature) }.to raise_error(ArgumentError)
     end
   end
-
-  describe '#email_participants_emails' do
-    let_it_be(:issue) { create(:issue) }
-
-    it 'returns a list of emails' do
-      participant1 = issue.issue_email_participants.create(email: 'a@gitlab.com')
-      participant2 = issue.issue_email_participants.create(email: 'b@gitlab.com')
-
-      expect(issue.email_participants_emails).to contain_exactly(participant1.email, participant2.email)
-    end
-  end
 end

spec/services/notification_service_spec.rb

@@ -297,17 +297,17 @@ RSpec.describe NotificationService, :mailer do
   describe 'Notes' do
     context 'issue note' do
       let_it_be(:project) { create(:project, :private) }
-      let_it_be_with_reload(:issue) { create(:issue, project: project, assignees: [assignee]) }
+      let_it_be(:issue) { create(:issue, project: project, assignees: [assignee]) }
       let_it_be(:mentioned_issue) { create(:issue, assignees: issue.assignees) }
       let_it_be_with_reload(:author) { create(:user) }
       let(:note) { create(:note_on_issue, author: author, noteable: issue, project_id: issue.project_id, note: '@mention referenced, @unsubscribed_mentioned and @outsider also') }
 
       subject { notification.new_note(note) }
 
-      context 'issue_email_participants' do
+      context 'on service desk issue' do
         before do
           allow(Notify).to receive(:service_desk_new_note_email)
-                             .with(Integer, Integer, String).and_return(mailer)
+                             .with(Integer, Integer).and_return(mailer)
 
           allow(::Gitlab::IncomingEmail).to receive(:enabled?) { true }
           allow(::Gitlab::IncomingEmail).to receive(:supports_wildcard?) { true }
@@ -318,7 +318,7 @@ RSpec.describe NotificationService, :mailer do
 
         def should_email!
           expect(Notify).to receive(:service_desk_new_note_email)
-            .with(issue.id, note.id, issue.external_author)
+            .with(issue.id, note.id)
         end
 
         def should_not_email!
@@ -347,19 +347,33 @@ RSpec.describe NotificationService, :mailer do
         let(:project) { issue.project }
         let(:note) { create(:note, noteable: issue, project: project) }
 
-        context 'do not exist' do
+        context 'a non-service-desk issue' do
           it_should_not_email!
         end
 
-        context 'do exist' do
-          let!(:issue_email_participant) { issue.issue_email_participants.create!(email: 'service.desk@example.com') }
-
+        context 'a service-desk issue' do
           before do
             issue.update!(external_author: 'service.desk@example.com')
             project.update!(service_desk_enabled: true)
           end
 
           it_should_email!
+
+          context 'where the project has disabled the feature' do
+            before do
+              project.update!(service_desk_enabled: false)
+            end
+
+            it_should_not_email!
+          end
+
+          context 'when the support bot has unsubscribed' do
+            before do
+              issue.unsubscribe(User.support_bot, project)
+            end
+
+            it_should_not_email!
+          end
         end
       end