Commit f097e4db authored by Lin Jen-Shin's avatar Lin Jen-Shin

Don't send rejection mails for all auto-generated mails

Also make it easier to have mailer helper
parent 8ec089f3
...@@ -31,8 +31,6 @@ class EmailReceiverWorker ...@@ -31,8 +31,6 @@ class EmailReceiverWorker
when Gitlab::Email::EmptyEmailError when Gitlab::Email::EmptyEmailError
can_retry = true can_retry = true
"It appears that the email is blank. Make sure your reply is at the top of the email, we can't process inline replies." "It appears that the email is blank. Make sure your reply is at the top of the email, we can't process inline replies."
when Gitlab::Email::AutoGeneratedEmailError
"The email was marked as 'auto generated', which we can't accept. Please create your comment through the web interface."
when Gitlab::Email::UserNotFoundError when Gitlab::Email::UserNotFoundError
"We couldn't figure out what user corresponds to the email. Please create your comment through the web interface." "We couldn't figure out what user corresponds to the email. Please create your comment through the web interface."
when Gitlab::Email::UserBlockedError when Gitlab::Email::UserBlockedError
......
---
title: Don't send rejection mails for all auto-generated mails
merge_request: 13254
author:
...@@ -15,7 +15,6 @@ module Gitlab ...@@ -15,7 +15,6 @@ module Gitlab
def execute def execute
raise SentNotificationNotFoundError unless sent_notification raise SentNotificationNotFoundError unless sent_notification
raise AutoGeneratedEmailError if mail.header.to_s =~ /auto-(generated|replied)/
validate_permission!(:create_note) validate_permission!(:create_note)
......
...@@ -26,6 +26,10 @@ module Gitlab ...@@ -26,6 +26,10 @@ module Gitlab
raise EmptyEmailError if @raw.blank? raise EmptyEmailError if @raw.blank?
mail = build_mail mail = build_mail
raise AutoGeneratedEmailError if
mail.header.to_s =~ /auto-(generated|replied)/
mail_key = extract_mail_key(mail) mail_key = extract_mail_key(mail)
handler = Handler.for(mail, mail_key) handler = Handler.for(mail, mail_key)
......
...@@ -36,15 +36,6 @@ describe Gitlab::Email::Handler::CreateNoteHandler do ...@@ -36,15 +36,6 @@ describe Gitlab::Email::Handler::CreateNoteHandler do
end end
end end
context "when the email was auto generated" do
let!(:mail_key) { '636ca428858779856c226bb145ef4fad' }
let!(:email_raw) { fixture_file("emails/auto_reply.eml") }
it "raises an AutoGeneratedEmailError" do
expect { receiver.execute }.to raise_error(Gitlab::Email::AutoGeneratedEmailError)
end
end
context "when the noteable could not be found" do context "when the noteable could not be found" do
before do before do
noteable.destroy noteable.destroy
......
...@@ -28,14 +28,6 @@ describe Gitlab::Email::Receiver do ...@@ -28,14 +28,6 @@ describe Gitlab::Email::Receiver do
it "raises an UnknownIncomingEmail error" do it "raises an UnknownIncomingEmail error" do
expect { receiver.execute }.to raise_error(Gitlab::Email::UnknownIncomingEmail) expect { receiver.execute }.to raise_error(Gitlab::Email::UnknownIncomingEmail)
end end
context "and the email contains no references header" do
let(:email_raw) { fixture_file("emails/auto_reply.eml").gsub(mail_key, "!!!") }
it "raises an UnknownIncomingEmail error" do
expect { receiver.execute }.to raise_error(Gitlab::Email::UnknownIncomingEmail)
end
end
end end
context "when the email is blank" do context "when the email is blank" do
...@@ -45,4 +37,12 @@ describe Gitlab::Email::Receiver do ...@@ -45,4 +37,12 @@ describe Gitlab::Email::Receiver do
expect { receiver.execute }.to raise_error(Gitlab::Email::EmptyEmailError) expect { receiver.execute }.to raise_error(Gitlab::Email::EmptyEmailError)
end end
end end
context "when the email was auto generated" do
let(:email_raw) { fixture_file("emails/auto_reply.eml") }
it "raises an AutoGeneratedEmailError" do
expect { receiver.execute }.to raise_error(Gitlab::Email::AutoGeneratedEmailError)
end
end
end end
...@@ -49,7 +49,7 @@ RSpec.configure do |config| ...@@ -49,7 +49,7 @@ RSpec.configure do |config|
config.include SearchHelpers, type: :feature config.include SearchHelpers, type: :feature
config.include WaitForRequests, :js config.include WaitForRequests, :js
config.include StubConfiguration config.include StubConfiguration
config.include EmailHelpers, type: :mailer config.include EmailHelpers, :mailer, type: :mailer
config.include TestEnv config.include TestEnv
config.include ActiveJob::TestHelper config.include ActiveJob::TestHelper
config.include ActiveSupport::Testing::TimeHelpers config.include ActiveSupport::Testing::TimeHelpers
...@@ -93,6 +93,10 @@ RSpec.configure do |config| ...@@ -93,6 +93,10 @@ RSpec.configure do |config|
RequestStore.clear! RequestStore.clear!
end end
config.before(:example, :mailer) do
reset_delivered_emails!
end
if ENV['CI'] if ENV['CI']
config.around(:each) do |ex| config.around(:each) do |ex|
ex.run_with_retry retry: 2 ex.run_with_retry retry: 2
......
require "spec_helper" require "spec_helper"
describe EmailReceiverWorker do describe EmailReceiverWorker, :mailer do
let(:raw_message) { fixture_file('emails/valid_reply.eml') } let(:raw_message) { fixture_file('emails/valid_reply.eml') }
context "when reply by email is enabled" do context "when reply by email is enabled" do
...@@ -17,12 +17,16 @@ describe EmailReceiverWorker do ...@@ -17,12 +17,16 @@ describe EmailReceiverWorker do
context "when an error occurs" do context "when an error occurs" do
before do before do
allow_any_instance_of(Gitlab::Email::Receiver).to receive(:execute).and_raise(Gitlab::Email::EmptyEmailError) allow_any_instance_of(Gitlab::Email::Receiver).to receive(:execute).and_raise(error)
end end
it "sends out a rejection email" do context 'when the error is Gitlab::Email::EmptyEmailError' do
perform_enqueued_jobs do let(:error) { Gitlab::Email::EmptyEmailError }
described_class.new.perform(raw_message)
it 'sends out a rejection email' do
perform_enqueued_jobs do
described_class.new.perform(raw_message)
end
email = ActionMailer::Base.deliveries.last email = ActionMailer::Base.deliveries.last
expect(email).not_to be_nil expect(email).not_to be_nil
...@@ -30,6 +34,18 @@ describe EmailReceiverWorker do ...@@ -30,6 +34,18 @@ describe EmailReceiverWorker do
expect(email.subject).to include("Rejected") expect(email.subject).to include("Rejected")
end end
end end
context 'when the error is Gitlab::Email::AutoGeneratedEmailError' do
let(:error) { Gitlab::Email::AutoGeneratedEmailError }
it 'does not send out any rejection email' do
perform_enqueued_jobs do
described_class.new.perform(raw_message)
end
should_not_email_anyone
end
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