Merge branch 'sdesk_event_update' into 'master'

Move creation of event outside of mailer

See merge request gitlab-org/gitlab!64404

app/mailers/emails/service_desk.rb

@@ -20,9 +20,7 @@ module Emails
       options = service_desk_options(email_sender, 'thank_you', @issue.external_author)
                   .merge(subject: "Re: #{subject_base}")
 
-      mail_new_thread(@issue, options).tap do
-        Gitlab::Metrics::BackgroundTransaction.current&.add_event(:service_desk_thank_you_email)
-      end
+      mail_new_thread(@issue, options)
     end
 
     def service_desk_new_note_email(issue_id, note_id, recipient)
@@ -33,9 +31,7 @@ module Emails
       options = service_desk_options(email_sender, 'new_note', recipient)
                   .merge(subject: subject_base)
 
-      mail_answer_thread(@issue, options).tap do
-        Gitlab::Metrics::BackgroundTransaction.current&.add_event(:service_desk_new_note_email)
-      end
+      mail_answer_thread(@issue, options)
     end
 
     private

app/services/notification_service.rb

@@ -396,6 +396,7 @@ class NotificationService
 
     recipients.each do |recipient|
       mailer.service_desk_new_note_email(issue.id, note.id, recipient).deliver_later
+      Gitlab::Metrics::BackgroundTransaction.current&.add_event(:service_desk_new_note_email)
     end
   end
 

lib/gitlab/email/handler/service_desk_handler.rb

@@ -96,6 +96,7 @@ module Gitlab
 
         def send_thank_you_email
           Notify.service_desk_thank_you_email(@issue.id).deliver_later
+          Gitlab::Metrics::BackgroundTransaction.current&.add_event(:service_desk_thank_you_email)
         end
 
         def message_including_template

spec/lib/gitlab/email/handler/service_desk_handler_spec.rb

@@ -50,6 +50,15 @@ RSpec.describe Gitlab::Email::Handler::ServiceDeskHandler do
       it 'sends thank you email' do
         expect { receiver.execute }.to have_enqueued_job.on_queue('mailers')
       end
+
+      it 'adds metric events for incoming and reply emails' do
+        metric_transaction = double('Gitlab::Metrics::WebTransaction', increment: true, observe: true)
+        allow(::Gitlab::Metrics::BackgroundTransaction).to receive(:current).and_return(metric_transaction)
+        expect(metric_transaction).to receive(:add_event).with(:receive_email_service_desk, anything)
+        expect(metric_transaction).to receive(:add_event).with(:service_desk_thank_you_email)
+
+        receiver.execute
+      end
     end
 
     context 'when everything is fine' do

spec/mailers/emails/service_desk_spec.rb

@@ -115,16 +115,6 @@ RSpec.describe Emails::ServiceDesk do
     end
   end
 
-  shared_examples 'notification with metric event' do |event_type|
-    it 'adds metric event' do
-      metric_transaction = double('Gitlab::Metrics::WebTransaction', increment: true, observe: true)
-      allow(::Gitlab::Metrics::BackgroundTransaction).to receive(:current).and_return(metric_transaction)
-      expect(metric_transaction).to receive(:add_event).with(event_type)
-
-      subject.content_type
-    end
-  end
-
   describe '.service_desk_thank_you_email' do
     let_it_be(:reply_in_subject) { true }
     let_it_be(:default_text) do
@@ -134,7 +124,6 @@ RSpec.describe Emails::ServiceDesk do
     subject { ServiceEmailClass.service_desk_thank_you_email(issue.id) }
 
     it_behaves_like 'read template from repository', 'thank_you'
-    it_behaves_like 'notification with metric event', :service_desk_thank_you_email
 
     context 'handling template markdown' do
       context 'with a simple text' do
@@ -175,7 +164,6 @@ RSpec.describe Emails::ServiceDesk do
     subject { ServiceEmailClass.service_desk_new_note_email(issue.id, note.id, email) }
 
     it_behaves_like 'read template from repository', 'new_note'
-    it_behaves_like 'notification with metric event', :service_desk_new_note_email
 
     context 'handling template markdown' do
       context 'with a simple text' do

spec/services/notification_service_spec.rb

@@ -376,41 +376,31 @@ RSpec.describe NotificationService, :mailer do
 
         let(:subject) { NotificationService.new }
         let(:mailer) { double(deliver_later: true) }
+        let(:issue) { create(:issue, author: User.support_bot) }
+        let(:project) { issue.project }
+        let(:note) { create(:note, noteable: issue, project: project) }
 
-        def should_email!
-          expect(Notify).to receive(:service_desk_new_note_email)
-            .with(issue.id, note.id, issue.external_author)
-        end
+        shared_examples 'notification with exact metric events' do |number_of_events|
+          it 'adds metric event' do
+            metric_transaction = double('Gitlab::Metrics::WebTransaction', increment: true, observe: true)
+            allow(::Gitlab::Metrics::BackgroundTransaction).to receive(:current).and_return(metric_transaction)
+            expect(metric_transaction).to receive(:add_event).with(:service_desk_new_note_email).exactly(number_of_events).times
 
-        def should_not_email!
-          expect(Notify).not_to receive(:service_desk_new_note_email)
+            subject.new_note(note)
+          end
         end
 
-        def execute!
-          subject.new_note(note)
-        end
+        shared_examples 'no participants are notified' do
+          it 'does not send the email' do
+            expect(Notify).not_to receive(:service_desk_new_note_email)
 
-        def self.it_should_email!
-          it 'sends the email' do
-            should_email!
-            execute!
+            subject.new_note(note)
           end
-        end
 
-        def self.it_should_not_email!
-          it 'doesn\'t send the email' do
-            should_not_email!
-            execute!
-          end
+          it_behaves_like 'notification with exact metric events', 0
         end
 
-        let(:issue) { create(:issue, author: User.support_bot) }
-        let(:project) { issue.project }
-        let(:note) { create(:note, noteable: issue, project: project) }
-
-        context 'do not exist' do
-          it_should_not_email!
-        end
+        it_behaves_like 'no participants are notified'
 
         context 'do exist and note not confidential' do
           let!(:issue_email_participant) { issue.issue_email_participants.create!(email: 'service.desk@example.com') }
@@ -420,7 +410,14 @@ RSpec.describe NotificationService, :mailer do
             project.update!(service_desk_enabled: true)
           end
 
-          it_should_email!
+          it 'sends the email' do
+            expect(Notify).to receive(:service_desk_new_note_email)
+              .with(issue.id, note.id, issue.external_author)
+
+            subject.new_note(note)
+          end
+
+          it_behaves_like 'notification with exact metric events', 1
         end
 
         context 'do exist and note is confidential' do
@@ -432,7 +429,7 @@ RSpec.describe NotificationService, :mailer do
             project.update!(service_desk_enabled: true)
           end
 
-          it_should_not_email!
+          it_behaves_like 'no participants are notified'
         end
       end