Commit 52109f9c authored by Luke Duncalfe's avatar Luke Duncalfe

Merge branch '15332-notify-when-merge-request-draft-status-changes' into 'master'

Send notifications to subscribers when draft status removed

See merge request gitlab-org/gitlab!55444
parents 9fb9481e d24ace0c
......@@ -23,6 +23,14 @@ module Emails
mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason))
end
def change_in_merge_request_draft_status_email(recipient_id, merge_request_id, updated_by_user_id, reason = nil)
setup_merge_request_mail(merge_request_id, recipient_id)
@updated_by_user = User.find(updated_by_user_id)
mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason))
end
# rubocop: disable CodeReuse/ActiveRecord
def reassigned_merge_request_email(recipient_id, merge_request_id, previous_assignee_ids, updated_by_user_id, reason = nil)
setup_merge_request_mail(merge_request_id, recipient_id)
......
......@@ -154,11 +154,20 @@ module MergeRequests
elsif old_title_wip && !new_title_wip
# Unmarked as Draft/WIP
#
notify_draft_status_changed(merge_request)
merge_request_activity_counter
.track_unmarked_as_draft_action(user: current_user)
end
end
def notify_draft_status_changed(merge_request)
notification_service.async.change_in_merge_request_draft_status(
merge_request,
current_user
)
end
def handle_milestone_change(merge_request)
return if skip_milestone_email
......
......@@ -189,6 +189,20 @@ class NotificationService
end
end
def change_in_merge_request_draft_status(merge_request, current_user)
recipients = NotificationRecipients::BuildService.build_recipients(merge_request, current_user, action: "draft_status_change")
recipients.each do |recipient|
mailer.send(
:change_in_merge_request_draft_status_email,
recipient.user.id,
merge_request.id,
current_user.id,
recipient.reason
).deliver_later
end
end
# When a merge request is found to be unmergeable, we should send an email to:
#
# * mr author
......
---
title: Send notifications to subscribers when merge request draft status removed
merge_request: 55444
author:
type: changed
......@@ -656,6 +656,48 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
end
end
context 'when the draft status is changed' do
let!(:non_subscriber) { create(:user) }
let!(:subscriber) do
create(:user) { |u| merge_request.toggle_subscription(u, project) }
end
before do
project.add_developer(non_subscriber)
project.add_developer(subscriber)
end
context 'removing draft status' do
before do
merge_request.update_attribute(:title, 'Draft: New Title')
end
it 'sends notifications for subscribers', :sidekiq_might_not_need_inline do
opts = { title: 'New title' }
perform_enqueued_jobs do
@merge_request = described_class.new(project, user, opts).execute(merge_request)
end
should_email(subscriber)
should_not_email(non_subscriber)
end
end
context 'adding draft status' do
it 'does not send notifications', :sidekiq_might_not_need_inline do
opts = { title: 'Draft: New title' }
perform_enqueued_jobs do
@merge_request = described_class.new(project, user, opts).execute(merge_request)
end
should_not_email(subscriber)
should_not_email(non_subscriber)
end
end
end
context 'when the merge request is relabeled' do
let!(:non_subscriber) { create(:user) }
let!(:subscriber) { create(:user) { |u| label.toggle_subscription(u, project) } }
......
......@@ -1868,6 +1868,42 @@ RSpec.describe NotificationService, :mailer do
end
end
describe '#change_in_merge_request_draft_status' do
let(:merge_request) { create(:merge_request, author: author, source_project: project) }
let_it_be(:current_user) { create(:user) }
it 'sends emails to relevant users only', :aggregate_failures do
notification.change_in_merge_request_draft_status(merge_request, current_user)
merge_request.reviewers.each { |reviewer| should_email(reviewer) }
merge_request.assignees.each { |assignee| should_email(assignee) }
should_email(merge_request.author)
should_email(@u_watcher)
should_email(@subscriber)
should_email(@watcher_and_subscriber)
should_email(@u_guest_watcher)
should_not_email(@u_participant_mentioned)
should_not_email(@u_guest_custom)
should_not_email(@u_custom_global)
should_not_email(@unsubscriber)
should_not_email(@u_participating)
should_not_email(@u_disabled)
should_not_email(@u_lazy_participant)
end
it_behaves_like 'participating notifications' do
let(:participant) { create(:user, username: 'user-participant') }
let(:issuable) { merge_request }
let(:notification_trigger) { notification.change_in_merge_request_draft_status(merge_request, @u_disabled) }
end
it_behaves_like 'project emails are disabled' do
let(:notification_target) { merge_request }
let(:notification_trigger) { notification.change_in_merge_request_draft_status(merge_request, @u_disabled) }
end
end
describe '#push_to_merge_request' do
before do
update_custom_notification(:push_to_merge_request, @u_guest_custom, resource: project)
......
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