FIX: Email threads sometimes not grouping for group SMTP (#13727)

This PR fixes a couple of issues related to group SMTP:

1. When running the group SMTP job, we were exiting early if the email was for the OP because of an IMAP race condition. However this causes issues when replying as a new topic for an existing SMTP topic, as the recipient does not get the OP email which can cause threading problems.
2. When sending emails for a new topic spun out like the issue in 1., we are not maintaining the original subject/topic title because that is based on the incoming email record, which we were not doing because the group SMTP email was never sent because of issue 1.
This commit is contained in:
Martin Brennan 2021-07-14 14:23:14 +10:00 committed by GitHub
parent d11fe6fde5
commit 068889cb5f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 26 additions and 4 deletions

View File

@ -52,7 +52,11 @@ module Jobs
#
# Basically, we should never be sending this notification for the first
# post in a topic.
if post.is_first_post?
#
# If the group does not have IMAP enabled then this could be legitimate,
# for example in cases where we are creating a new topic to reply to another
# group PM and we need to send the participants the group OP email.
if post.is_first_post? && group.imap_enabled
ImapSyncLog.warn("Aborting SMTP email for post #{post.id} in topic #{post.topic_id} to #{email}, the post is the OP and should not send an email.", group)
return
end

View File

@ -171,9 +171,27 @@ RSpec.describe Jobs::GroupSmtpEmail do
context "when the post in the argument is the OP" do
let(:post_id) { post.topic.posts.first.id }
it "aborts and does not send a group SMTP email; the OP is the one that sent the email in the first place" do
expect { subject.execute(args) }.not_to(change { EmailLog.count })
expect(ActionMailer::Base.deliveries.count).to eq(0)
context "when the group has imap enabled" do
before do
group.update!(imap_enabled: true)
end
it "aborts and does not send a group SMTP email; the OP is the one that sent the email in the first place" do
expect { subject.execute(args) }.not_to(change { EmailLog.count })
expect(ActionMailer::Base.deliveries.count).to eq(0)
end
end
context "when the group does not have imap enabled" do
before do
group.update!(imap_enabled: false)
end
it "sends the email as expected" do
subject.execute(args)
expect(ActionMailer::Base.deliveries.count).to eq(1)
end
end
end