FIX: Do not send emails to mailing_list_mode subscribers for PMs (#14159

)

This bug was introduced by f66007e.

In PostJobsEnqueuer we previously did not fire the after_post_create
event and after_topic_create event for private message topics. This was
changed in the above commit in order to publish message bus messages
for topic tracking state updates. Unfortunately this caused the
NotifyMailingListSubscribers job to be enqueued for all posts including
private messages, and admins and the users involved in the PMs got
emailed the contents of the PMs if they had mailing list mode enabled.

Luckily the impact of this was mitigated by a Guardian#can_see? check
for each mailing list mode user in the NotifyMailingListSubscribers job.
We never want to notify mailing list mode subscribers for private messages
so an early return has been added there, plus the logic in PostJobsEnqueuer
has been fixed, and tests have been added to that class where there were
none before.

app/jobs/regular/notify_mailing_list_subscribers.rb

@@ -28,7 +28,8 @@ def execute(args)
       post_id = args[:post_id]
       post = post_id ? Post.with_deleted.find_by(id: post_id) : nil
 
-      return if !post || post.trashed? || post.user_deleted? || !post.topic || post.raw.blank?
+      return if !post || post.trashed? || post.user_deleted? ||
+                  !post.topic || post.raw.blank? || post.topic.private_message?
 
       users =
           User.activated.not_silenced.not_suspended.real

lib/post_jobs_enqueuer.rb

@@ -55,10 +55,13 @@ def make_visible
   def after_post_create
     Jobs.enqueue(:post_update_topic_tracking_state, post_id: @post.id)
 
-    Jobs.enqueue_in(SiteSetting.email_time_window_mins.minutes,
-      :notify_mailing_list_subscribers,
-      post_id: @post.id,
-    )
+    if !@topic.private_message?
+      Jobs.enqueue_in(
+        SiteSetting.email_time_window_mins.minutes,
+        :notify_mailing_list_subscribers,
+        post_id: @post.id,
+      )
+    end
   end
 
   def after_topic_create

spec/jobs/notify_mailing_list_subscribers_spec.rb

@@ -80,6 +80,15 @@
       include_examples "no emails"
     end
 
+    context "with a private message" do
+      before do
+        post.topic.update!(archetype: Archetype.private_message, category: nil)
+        TopicAllowedUser.create(topic: post.topic, user: mailing_list_user)
+        post.topic.reload
+      end
+      include_examples "no emails"
+    end
+
     context "with a valid post from another user" do
 
       context "to an inactive user" do

spec/lib/post_jobs_enqueuer_spec.rb

@@ -0,0 +1,105 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe PostJobsEnqueuer do
+  let!(:post) { Fabricate(:post, topic: topic) }
+  let!(:topic) { Fabricate(:topic) }
+  let(:new_topic) { false }
+  let(:opts) { { post_alert_options: {} } }
+
+  subject { described_class.new(post, topic, new_topic, opts) }
+
+  context "for regular topics" do
+    it "enqueues the :post_alert job" do
+      expect_enqueued_with(job: :post_alert, args: {
+        post_id: post.id,
+        new_record: true,
+        options: opts[:post_alert_options]
+      }) do
+        subject.enqueue_jobs
+      end
+    end
+
+    it "enqueues the :notify_mailing_list_subscribers job" do
+      expect_enqueued_with(job: :notify_mailing_list_subscribers, args: { post_id: post.id }) do
+        subject.enqueue_jobs
+      end
+    end
+
+    it "enqueues the :post_update_topic_tracking_state job" do
+      expect_enqueued_with(job: :post_update_topic_tracking_state, args: { post_id: post.id }) do
+        subject.enqueue_jobs
+      end
+    end
+
+    it "enqueues the :feature_topic_users job" do
+      expect_enqueued_with(job: :feature_topic_users, args: { topic_id: topic.id }) do
+        subject.enqueue_jobs
+      end
+    end
+
+    context "for new topics" do
+      let(:new_topic) { true }
+
+      it "calls the correct topic tracking state class to publish_new" do
+        TopicTrackingState.expects(:publish_new).with(topic)
+        PrivateMessageTopicTrackingState.expects(:publish_new).never
+        subject.enqueue_jobs
+      end
+    end
+  end
+
+  context "for private messages" do
+    let!(:topic) { Fabricate(:private_message_topic) }
+
+    it "does not enqueue the :notify_mailing_list_subscribers job" do
+      expect_not_enqueued_with(job: :notify_mailing_list_subscribers, args: { post_id: post.id }) do
+        subject.enqueue_jobs
+      end
+    end
+
+    it "enqueues the :post_update_topic_tracking_state job" do
+      expect_enqueued_with(job: :post_update_topic_tracking_state, args: { post_id: post.id }) do
+        subject.enqueue_jobs
+      end
+    end
+
+    it "enqueues the :feature_topic_users job" do
+      expect_enqueued_with(job: :feature_topic_users, args: { topic_id: topic.id }) do
+        subject.enqueue_jobs
+      end
+    end
+
+    context "for new topics" do
+      let(:new_topic) { true }
+
+      it "calls the correct topic tracking state class to publish_new" do
+        TopicTrackingState.expects(:publish_new).never
+        PrivateMessageTopicTrackingState.expects(:publish_new).with(topic)
+        subject.enqueue_jobs
+      end
+    end
+
+    context "for a post > post_number 1" do
+      let!(:post) do
+        Fabricate(:post, topic: topic)
+        Fabricate(:post, topic: topic)
+      end
+
+      context "when there is a topic embed" do
+        before do
+          SiteSetting.embed_unlisted = true
+          topic.update(visible: false)
+          Fabricate(:topic_embed, post: post, embed_url: "http://test.com")
+        end
+
+        it "does not enqueue the :make_embedded_topic_visible job" do
+          expect_not_enqueued_with(job: :make_embedded_topic_visible, args: { topic_id: topic.id }) do
+            subject.enqueue_jobs
+          end
+        end
+      end
+    end
+  end
+end