Merge branch 'mv-to-service' into 'master'

Move release notification from after_commit to service

Closes #199067

See merge request gitlab-org/gitlab!29853

app/models/release.rb

@@ -34,8 +34,6 @@ class Release < ApplicationRecord
 
   delegate :repository, to: :project
 
-  after_commit :notify_new_release, on: :create, unless: :importing?
-
   MAX_NUMBER_TO_DISPLAY = 3
 
   def to_param
@@ -92,10 +90,6 @@ class Release < ApplicationRecord
       repository.find_tag(tag)
     end
   end
-
-  def notify_new_release
-    NewReleaseWorker.perform_async(id)
-  end
 end
 
 Release.prepend_if_ee('EE::Release')

app/services/releases/create_service.rb

@@ -47,11 +47,17 @@ module Releases
 
       release.save!
 
+      notify_create_release(release)
+
       success(tag: tag, release: release)
     rescue => e
       error(e.message, 400)
     end
 
+    def notify_create_release(release)
+      NotificationService.new.async.send_new_release_notifications(release)
+    end
+
     def build_release(tag)
       project.releases.build(
         name: name,

app/workers/new_release_worker.rb

 # frozen_string_literal: true
 
+# TODO: Worker can be removed in 13.2:
+# https://gitlab.com/gitlab-org/gitlab/-/issues/218231
 class NewReleaseWorker # rubocop:disable Scalability/IdempotentWorker
   include ApplicationWorker
 

changelogs/unreleased/mv-to-service.yml

+---
+title: Move release notification from model callbacks to service
+merge_request: 29853
+author: Ravishankar
+type: performance

spec/models/release_spec.rb

@@ -111,26 +111,6 @@ RSpec.describe Release do
     end
   end
 
-  describe '#notify_new_release' do
-    context 'when a release is created' do
-      it 'instantiates NewReleaseWorker to send notifications' do
-        expect(NewReleaseWorker).to receive(:perform_async)
-
-        create(:release)
-      end
-    end
-
-    context 'when a release is updated' do
-      let!(:release) { create(:release) }
-
-      it 'does not send any new notification' do
-        expect(NewReleaseWorker).not_to receive(:perform_async)
-
-        release.update!(description: 'new description')
-      end
-    end
-  end
-
   describe '#name' do
     context 'name is nil' do
       before do

spec/services/notification_service_spec.rb

@@ -762,7 +762,7 @@ describe NotificationService, :mailer do
     end
   end
 
-  describe '#send_new_release_notifications', :deliver_mails_inline, :sidekiq_inline do
+  describe '#send_new_release_notifications', :deliver_mails_inline do
     context 'when recipients for a new release exist' do
       let(:release) { create(:release) }
 
@@ -774,7 +774,7 @@ describe NotificationService, :mailer do
         recipient_2 = NotificationRecipient.new(user_2, :custom, custom_action: :new_release)
         allow(NotificationRecipients::BuildService).to receive(:build_new_release_recipients).and_return([recipient_1, recipient_2])
 
-        release
+        notification.send_new_release_notifications(release)
 
         should_email(user_1)
         should_email(user_2)

spec/services/releases/create_service_spec.rb

@@ -20,6 +20,8 @@ describe Releases::CreateService do
   describe '#execute' do
     shared_examples 'a successful release creation' do
       it 'creates a new release' do
+        expected_job_count = MailScheduler::NotificationServiceWorker.jobs.size + 1
+
         result = service.execute
 
         expect(project.releases.count).to eq(1)
@@ -30,6 +32,7 @@ describe Releases::CreateService do
         expect(result[:release].name).to eq(name)
         expect(result[:release].author).to eq(user)
         expect(result[:release].sha).to eq(tag_sha)
+        expect(MailScheduler::NotificationServiceWorker.jobs.size).to eq(expected_job_count)
       end
     end
 

spec/workers/new_release_worker_spec.rb

 # frozen_string_literal: true
 
+# TODO: Worker can be removed in 13.2:
+# https://gitlab.com/gitlab-org/gitlab/-/issues/218231
 require 'spec_helper'
 
 describe NewReleaseWorker do