Commit 12ca460c authored by charlie ablett's avatar charlie ablett

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

This reverts merge request !48823
parent 879eb7c5
...@@ -17,18 +17,18 @@ module Emails ...@@ -17,18 +17,18 @@ module Emails
send_from_user_email: false, send_from_user_email: false,
sender_name: @project.service_desk_setting&.outgoing_name 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}") .merge(subject: "Re: #{subject_base}")
mail_new_thread(@issue, options) mail_new_thread(@issue, options)
end 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) @note = Note.find(note_id)
setup_service_desk_mail(issue_id) setup_service_desk_mail(issue_id)
email_sender = sender(@note.author_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) .merge(subject: subject_base)
mail_answer_thread(@issue, options) mail_answer_thread(@issue, options)
...@@ -44,10 +44,10 @@ module Emails ...@@ -44,10 +44,10 @@ module Emails
@sent_notification = SentNotification.record(@issue, @support_bot.id, reply_key) @sent_notification = SentNotification.record(@issue, @support_bot.id, reply_key)
end end
def service_desk_options(email_sender, email_type, recipient) def service_desk_options(email_sender, email_type)
{ {
from: email_sender, from: email_sender,
to: recipient to: @issue.external_author
}.tap do |options| }.tap do |options|
next unless template_body = template_content(email_type) next unless template_body = template_content(email_type)
......
...@@ -169,7 +169,7 @@ class NotifyPreview < ActionMailer::Preview ...@@ -169,7 +169,7 @@ class NotifyPreview < ActionMailer::Preview
cleanup do cleanup do
note = create_note(noteable_type: 'Issue', noteable_id: issue.id, note: 'Issue note content') 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
end end
......
...@@ -434,10 +434,6 @@ class Issue < ApplicationRecord ...@@ -434,10 +434,6 @@ class Issue < ApplicationRecord
moved_to || duplicated_to moved_to || duplicated_to
end end
def email_participants_emails
issue_email_participants.pluck(:email)
end
private private
def ensure_metrics def ensure_metrics
......
...@@ -347,19 +347,18 @@ class NotificationService ...@@ -347,19 +347,18 @@ class NotificationService
end end
def send_service_desk_notification(note) def send_service_desk_notification(note)
return unless Gitlab::ServiceDesk.supported?
return unless note.noteable_type == 'Issue' return unless note.noteable_type == 'Issue'
issue = note.noteable issue = note.noteable
return unless issue.issue_email_participants.any?
recipients = issue.email_participants_emails
support_bot = User.support_bot support_bot = User.support_bot
recipients.delete(issue.external_author) if note.author == support_bot
recipients.each do |recipient| return unless issue.external_author.present?
mailer.service_desk_new_note_email(issue.id, note.id, recipient).deliver_later return unless issue.project.service_desk_enabled?
end 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 end
# Notify users when a new release is created # Notify users when a new release is created
......
---
title: Notify issue_email_participants instead of external_author
merge_request: 48823
author: Lee Tickett @leetickett
type: other
...@@ -13,13 +13,8 @@ RSpec.describe Emails::ServiceDesk do ...@@ -13,13 +13,8 @@ RSpec.describe Emails::ServiceDesk do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project) }
let_it_be(:issue) { create(:issue, project: project) } let_it_be(:issue) { create(:issue, project: project) }
let_it_be(:email) { 'someone@gitlab.com' }
let(:template) { double(content: template_content) } let(:template) { double(content: template_content) }
before_all do
issue.issue_email_participants.create!(email: email)
end
before do before do
stub_const('ServiceEmailClass', Class.new(ApplicationMailer)) stub_const('ServiceEmailClass', Class.new(ApplicationMailer))
...@@ -77,10 +72,6 @@ RSpec.describe Emails::ServiceDesk do ...@@ -77,10 +72,6 @@ RSpec.describe Emails::ServiceDesk do
let(:template_content) { 'custom text' } let(:template_content) { 'custom text' }
let(:issue) { create(:issue, project: project)} 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 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 }) } 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 ...@@ -160,7 +151,7 @@ RSpec.describe Emails::ServiceDesk do
let_it_be(:note) { create(:note_on_issue, noteable: issue, project: project) } let_it_be(:note) { create(:note_on_issue, noteable: issue, project: project) }
let_it_be(:default_text) { note.note } 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' it_behaves_like 'read template from repository', 'new_note'
......
...@@ -1285,7 +1285,6 @@ RSpec.describe Notify do ...@@ -1285,7 +1285,6 @@ RSpec.describe Notify do
context 'for service desk issues' do context 'for service desk issues' do
before do before do
issue.update!(external_author: 'service.desk@example.com') issue.update!(external_author: 'service.desk@example.com')
issue.issue_email_participants.create!(email: 'service.desk@example.com')
end end
def expect_sender(username) def expect_sender(username)
...@@ -1334,7 +1333,7 @@ RSpec.describe Notify do ...@@ -1334,7 +1333,7 @@ RSpec.describe Notify do
describe 'new note email' do describe 'new note email' do
let_it_be(:first_note) { create(:discussion_note_on_issue, note: 'Hello world') } 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' it_behaves_like 'an unsubscribeable thread'
......
...@@ -1260,15 +1260,4 @@ RSpec.describe Issue do ...@@ -1260,15 +1260,4 @@ RSpec.describe Issue do
expect { issue.issue_type_supports?(:unkown_feature) }.to raise_error(ArgumentError) expect { issue.issue_type_supports?(:unkown_feature) }.to raise_error(ArgumentError)
end end
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 end
...@@ -297,17 +297,17 @@ RSpec.describe NotificationService, :mailer do ...@@ -297,17 +297,17 @@ RSpec.describe NotificationService, :mailer do
describe 'Notes' do describe 'Notes' do
context 'issue note' do context 'issue note' do
let_it_be(:project) { create(:project, :private) } 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(:mentioned_issue) { create(:issue, assignees: issue.assignees) }
let_it_be_with_reload(:author) { create(:user) } 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') } 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) } subject { notification.new_note(note) }
context 'issue_email_participants' do context 'on service desk issue' do
before do before do
allow(Notify).to receive(:service_desk_new_note_email) 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(:enabled?) { true }
allow(::Gitlab::IncomingEmail).to receive(:supports_wildcard?) { true } allow(::Gitlab::IncomingEmail).to receive(:supports_wildcard?) { true }
...@@ -318,7 +318,7 @@ RSpec.describe NotificationService, :mailer do ...@@ -318,7 +318,7 @@ RSpec.describe NotificationService, :mailer do
def should_email! def should_email!
expect(Notify).to receive(:service_desk_new_note_email) expect(Notify).to receive(:service_desk_new_note_email)
.with(issue.id, note.id, issue.external_author) .with(issue.id, note.id)
end end
def should_not_email! def should_not_email!
...@@ -347,19 +347,33 @@ RSpec.describe NotificationService, :mailer do ...@@ -347,19 +347,33 @@ RSpec.describe NotificationService, :mailer do
let(:project) { issue.project } let(:project) { issue.project }
let(:note) { create(:note, noteable: issue, project: 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! it_should_not_email!
end end
context 'do exist' do context 'a service-desk issue' do
let!(:issue_email_participant) { issue.issue_email_participants.create!(email: 'service.desk@example.com') }
before do before do
issue.update!(external_author: 'service.desk@example.com') issue.update!(external_author: 'service.desk@example.com')
project.update!(service_desk_enabled: true) project.update!(service_desk_enabled: true)
end end
it_should_email! 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
end end
......
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