DEV: Add SMTP group ID to EmailLog (#13381)

Adds a new `smtp_group_id` column to `EmailLog` which is filled in if the mail `from_address` matches a group's `email_username`. This is for easier debugging, so we know which emails have been sent via group SMTP.

app/mailers/user_notifications.rb

@@ -546,9 +546,6 @@ class UserNotifications < ActionMailer::Base
         group.smtp_server, group.smtp_port, group.smtp_ssl, group.smtp_ssl
       )
 
-      # TODO (martin): Remove this once testing is over and this is more stable.
-      Rails.logger.warn("Using SMTP settings from group #{group.name} (#{group.id}) to send user notification for topic #{post.topic.id} and user #{user.id} (#{user.email})")
-
       delivery_method_options = {
         address: group.smtp_server,
         port: port,

app/models/email_log.rb

@@ -16,6 +16,8 @@ class EmailLog < ActiveRecord::Base
 
   belongs_to :user
   belongs_to :post
+  belongs_to :smtp_group, class_name: 'Group'
+
   has_one :topic, through: :post
 
   validates :email_type, :to_address, presence: true
@@ -89,23 +91,25 @@ end
 #
 # Table name: email_logs
 #
-#  id         :integer          not null, primary key
-#  to_address :string           not null
-#  email_type :string           not null
-#  user_id    :integer
-#  created_at :datetime         not null
-#  updated_at :datetime         not null
-#  post_id    :integer
-#  bounce_key :uuid
-#  bounced    :boolean          default(FALSE), not null
-#  message_id :string
+#  id            :integer          not null, primary key
+#  to_address    :string           not null
+#  email_type    :string           not null
+#  user_id       :integer
+#  created_at    :datetime         not null
+#  updated_at    :datetime         not null
+#  post_id       :integer
+#  bounce_key    :uuid
+#  bounced       :boolean          default(FALSE), not null
+#  message_id    :string
+#  smtp_group_id :integer
 #
 # Indexes
 #
-#  index_email_logs_on_bounce_key  (bounce_key) UNIQUE WHERE (bounce_key IS NOT NULL)
-#  index_email_logs_on_bounced     (bounced)
-#  index_email_logs_on_created_at  (created_at)
-#  index_email_logs_on_message_id  (message_id)
-#  index_email_logs_on_post_id     (post_id)
-#  index_email_logs_on_user_id     (user_id)
+#  idx_email_logs_on_smtp_group_id  (smtp_group_id)
+#  index_email_logs_on_bounce_key   (bounce_key) UNIQUE WHERE (bounce_key IS NOT NULL)
+#  index_email_logs_on_bounced      (bounced)
+#  index_email_logs_on_created_at   (created_at)
+#  index_email_logs_on_message_id   (message_id)
+#  index_email_logs_on_post_id      (post_id)
+#  index_email_logs_on_user_id      (user_id)
 #

db/migrate/20210614232334_add_smtp_group_id_to_email_log.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class AddSmtpGroupIdToEmailLog < ActiveRecord::Migration[6.1]
+  def change
+    add_column :email_logs, :smtp_group_id, :integer, null: true, index: true
+  end
+end

lib/email/sender.rb

@@ -97,6 +97,7 @@ module Email
       post_id   = header_value('X-Discourse-Post-Id')
       topic_id  = header_value('X-Discourse-Topic-Id')
       reply_key = set_reply_key(post_id, user_id)
+      from_address = @message.from&.first
 
       # always set a default Message ID from the host
       @message.header['Message-ID'] = "<#{SecureRandom.uuid}@#{host}>"
@@ -228,6 +229,12 @@ module Email
 
       email_log.message_id = @message.message_id
 
+      # 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
+
       DiscourseEvent.trigger(:before_email_send, @message, @email_type)
 
       begin

spec/components/email/sender_spec.rb

@@ -337,6 +337,36 @@ describe Email::Sender do
         expect(email_log.to_address).to eq('eviltrout@test.domain')
         expect(email_log.user_id).to be_blank
       end
+
+      context 'when the email is sent using group SMTP credentials' do
+        let(:reply) { Fabricate(:post, topic: post.topic, reply_to_user: post.user, reply_to_post_number: post.post_number) }
+        let(:notification) { Fabricate(:posted_notification, user: post.user, post: reply) }
+        let(:message) do
+          UserNotifications.user_private_message(
+            post.user,
+            post: reply,
+            notification_type: notification.notification_type,
+            notification_data_hash: notification.data_hash
+          )
+        end
+        let(:group) { Fabricate(:smtp_group) }
+
+        before do
+          SiteSetting.enable_smtp = true
+        end
+
+        it 'adds the group id to the email log' do
+          TopicAllowedGroup.create(topic: post.topic, group: group)
+
+          email_sender.send
+
+          expect(email_log).to be_present
+          expect(email_log.email_type).to eq('valid_type')
+          expect(email_log.to_address).to eq(post.user.email)
+          expect(email_log.user_id).to be_blank
+          expect(email_log.smtp_group_id).to eq(group.id)
+        end
+      end
     end
 
     context "email log with a post id and topic id" do

spec/fabricators/group_fabricator.rb

@@ -24,3 +24,12 @@ Fabricator(:imap_group, from: :group) do
   email_username "discourseteam@ponyexpress.com"
   email_password "test"
 end
+
+Fabricator(:smtp_group, from: :group) do
+  smtp_server "smtp.ponyexpress.com"
+  smtp_port 587
+  smtp_ssl true
+  smtp_enabled true
+  email_username "discourseteam@ponyexpress.com"
+  email_password "test"
+end