FIX: Do not add mailing list headers to group SMTP emails (#13431)

FIX: Do not add mailing list headers to group SMTP emails (#13431)

When we are emailing people from a group inbox, we are having a PM conversation with them, as a support account would. In this case mailing list headers do not make sense. It is not like a forum topic where you may have tens or hundreds of participants – it is a conversation between the group and a small handful of people directly contacting the group, often just one person.

The only header left in tact was List-Unsubsribe which is important for letting people opt out to notifications.

diff --git a/lib/email/sender.rb b/lib/email/sender.rb
index 6b63420..489af2f 100644
--- a/lib/email/sender.rb
+++ b/lib/email/sender.rb
@@ -98,6 +98,9 @@ module Email
       topic_id  = header_value('X-Discourse-Topic-Id')
       reply_key = set_reply_key(post_id, user_id)
       from_address = @message.from&.first
+      smtp_group_id = from_address.blank? ? nil : Group.where(
+        email_username: from_address, smtp_enabled: true
+      ).pluck_first(:id)
 
       # always set a default Message ID from the host
       @message.header['Message-ID'] = "<#{SecureRandom.uuid}@#{host}>"
@@ -161,20 +164,29 @@ module Email
           list_id = "#{SiteSetting.title} <#{host}>"
         end
 
-        # https://www.ietf.org/rfc/rfc3834.txt
-        @message.header['Precedence'] = 'list'
-        @message.header['List-ID']    = list_id
-
-        if topic
-          if SiteSetting.private_email?
-            @message.header['List-Archive'] = "#{Discourse.base_url}#{topic.slugless_url}"
-          else
-            @message.header['List-Archive'] = topic.url
+        # When we are emailing people from a group inbox, we are having a PM
+        # conversation with them, as a support account would. In this case
+        # mailing list headers do not make sense. It is not like a forum topic
+        # where you may have tens or hundreds of participants -- it is a
+        # conversation between the group and a small handful of people
+        # directly contacting the group, often just one person.
+        if !smtp_group_id
+
+          # https://www.ietf.org/rfc/rfc3834.txt
+          @message.header['Precedence'] = 'list'
+          @message.header['List-ID']    = list_id
+
+          if topic
+            if SiteSetting.private_email?
+              @message.header['List-Archive'] = "#{Discourse.base_url}#{topic.slugless_url}"
+            else
+              @message.header['List-Archive'] = topic.url
+            end
           end
         end
       end
 
-      if reply_key.present? && @message.header['Reply-To'].to_s =~ /\<([^\>]+)\>/
+      if reply_key.present? && @message.header['Reply-To'].to_s =~ /\<([^\>]+)\>/ && !smtp_group_id
         email = Regexp.last_match[1]
         @message.header['List-Post'] = "<mailto:#{email}>"
       end
@@ -231,9 +243,7 @@ module Email
 
       # Log when a message is being sent from a group SMTP address, so we
       # can debug deliverability issues.
-      if from_address && smtp_group_id = Group.where(email_username: from_address, smtp_enabled: true).pluck_first(:id)
-        email_log.smtp_group_id = smtp_group_id
-      end
+      email_log.smtp_group_id = smtp_group_id
 
       DiscourseEvent.trigger(:before_email_send, @message, @email_type)
 
diff --git a/spec/components/email/sender_spec.rb b/spec/components/email/sender_spec.rb
index 603dd76..27e9351 100644
--- a/spec/components/email/sender_spec.rb
+++ b/spec/components/email/sender_spec.rb
@@ -366,6 +366,16 @@ describe Email::Sender do
           expect(email_log.user_id).to be_blank
           expect(email_log.smtp_group_id).to eq(group.id)
         end
+
+        it "does not add any of the mailing list headers" do
+          TopicAllowedGroup.create(topic: post.topic, group: group)
+          email_sender.send
+
+          expect(message.header['List-ID']).to eq(nil)
+          expect(message.header['List-Post']).to eq(nil)
+          expect(message.header['List-Archive']).to eq(nil)
+          expect(message.header['Precedence']).to eq(nil)
+        end
       end
     end
 

GitHub sha: ff6114d83f9c6203dee426968850023b77d1e031

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