Commit f73c0224 authored by Ravishankar Gnanaprakasam's avatar Ravishankar Gnanaprakasam Committed by Ravishankar

Move notification from after_commit to service

Adding change log

Fixing the test case failures in this MR Pipeline

Adding case to cover the email in service spec

Improving the test with expecting the actual class instead of any args

Review comments addressed

Checking only the job count avoiding sidekiq inline

Adding back the worker code

Adding new line

Adding the queues back

Apply suggestion to app/workers/new_release_worker.rb

Add TODO to remove the code
parent 26e0b8e7
......@@ -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')
......@@ -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,
......
# 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
......
---
title: Move release notification from model callbacks to service
merge_request: 29853
author: Ravishankar
type: performance
......@@ -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
......
......@@ -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)
......
......@@ -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
......
# 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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment