FIX: Use correct group out of multiple for SMTP sender (#14957)

FIX: Use correct group out of multiple for SMTP sender (#14957)

When there are multiple groups on a topic, we were selecting the first from the topic allowed groups to act as the sender email address when sending group SMTP replies via PostAlerter. However, this was not ordered, and since there is no created_at column on TopicAllowedGroup we cannot order this nicely, which caused just a random group to be used (based on whatever postgres decided it felt like that morning).

This commit changes the group used for SMTP sending to be the group using the email_username of the to address of the first incoming email for the topic, if there are more than one allowed groups on the topic. Otherwise it just uses the only SMTP enabled group.

diff --git a/app/models/topic.rb b/app/models/topic.rb
index 3519a5e..db9dd21 100644
--- a/app/models/topic.rb
+++ b/app/models/topic.rb
@@ -1777,6 +1777,10 @@ class Topic < ActiveRecord::Base
+  def first_smtp_enabled_group
+    self.allowed_groups.where(smtp_enabled: true).first
+  end
   def invite_to_private_message(invited_by, target_user, guardian)
diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb
index 7b49d0d..5e394e9 100644
--- a/app/services/post_alerter.rb
+++ b/app/services/post_alerter.rb
@@ -640,7 +640,15 @@ class PostAlerter
   def group_notifying_via_smtp(post)
     return nil if !SiteSetting.enable_smtp || post.post_type != Post.types[:regular]
-    post.topic.allowed_groups.where(smtp_enabled: true).first
+    if post.topic.allowed_groups.count == 1
+      return post.topic.first_smtp_enabled_group
+    end
+    group = Group.find_by_email(post.topic.incoming_email.first.to_addresses)
+    if !group&.smtp_enabled
+      return post.topic.first_smtp_enabled_group
+    end
+    group
   def email_using_group_smtp_if_configured(post)
diff --git a/spec/services/post_alerter_spec.rb b/spec/services/post_alerter_spec.rb
index 5633886..877db1a 100644
--- a/spec/services/post_alerter_spec.rb
+++ b/spec/services/post_alerter_spec.rb
@@ -1374,6 +1374,23 @@ describe PostAlerter do
       expect(email.subject).to eq("Re: #{topic.title}")
+    it "sends a group smtp email when the original group has had SMTP disabled and there is an additional topic allowed group" do
+      incoming_email_post = create_post_with_incoming
+      topic = incoming_email_post.topic
+      other_allowed_group = Fabricate(:smtp_group)
+      TopicAllowedGroup.create(group: other_allowed_group, topic: topic)
+      post = Fabricate(:post, topic: topic)
+      group.update!(smtp_enabled: false)
+      expect {, true) }.to change { ActionMailer::Base.deliveries.size }.by(1)
+      email = ActionMailer::Base.deliveries.last
+      expect(email.from).to include(other_allowed_group.email_username)
+      expect( contain_exactly(topic.reload.topic_allowed_users.order(:created_at)
+      expect( match_array(["", ""])
+      expect(email.subject).to eq("Re: #{topic.title}")
+    end
     it "does not send a group smtp email if smtp is not enabled for the group" do
       group.update!(smtp_enabled: false)
       incoming_email_post = create_post_with_incoming

GitHub sha: 31035010afc4770d81fca577a055b9abacb48d95

This commit appears in #14957 which was approved by CvX. It was merged by martin.