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

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.
diff --git a/app/jobs/regular/group_smtp_email.rb b/app/jobs/regular/group_smtp_email.rb
index ce113e5..3970eca 100644
--- a/app/jobs/regular/group_smtp_email.rb
+++ b/app/jobs/regular/group_smtp_email.rb
@@ -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
diff --git a/spec/jobs/regular/group_smtp_email_spec.rb b/spec/jobs/regular/group_smtp_email_spec.rb
index 9b792a8..bc8991f 100644
--- a/spec/jobs/regular/group_smtp_email_spec.rb
+++ b/spec/jobs/regular/group_smtp_email_spec.rb
@@ -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
 

GitHub sha: 068889cb5f54fd4b402eeeb77572c33ad261f86b

This commit appears in #13727 which was approved by SamSaffron. It was merged by martin.