FIX: Dismissing unread posts did not publish changes to other clients (#22584)

Why this change?

Prior to this change, dismissing unreads posts did not publish the
changes across clients for the same user. As a result, users can end up
seeing an unread count being present but saw no topics being loaded when
visiting the `/unread` route.

app/assets/javascripts/discourse/app/models/topic-tracking-state.js

@@ -1016,6 +1016,10 @@ const TopicTrackingState = EmberObject.extend({
       this._dismissNewTopics(data.payload.topic_ids);
     }
 
+    if (data.message_type === "dismiss_new_posts") {
+      this._dismissNewPosts(data.payload.topic_ids);
+    }
+
     if (["new_topic", "unread", "read"].includes(data.message_type)) {
       this.notifyIncoming(data);
       if (!deepEqual(old, data.payload)) {
@@ -1055,6 +1059,23 @@ const TopicTrackingState = EmberObject.extend({
     topicIds.forEach((topicId) => {
       this.modifyStateProp(topicId, "is_seen", true);
     });
+
+    this.incrementMessageCount();
+  },
+
+  _dismissNewPosts(topicIds) {
+    topicIds.forEach((topicId) => {
+      const state = this.findState(topicId);
+
+      if (state) {
+        this.modifyStateProp(
+          topicId,
+          "last_read_post_number",
+          state.highest_post_number
+        );
+      }
+    });
+
     this.incrementMessageCount();
   },
 

app/controllers/topics_controller.rb

@@ -1100,11 +1100,12 @@ class TopicsController < ApplicationController
 
     dismissed_topic_ids = []
     dismissed_post_topic_ids = []
+
     if !current_user.new_new_view_enabled? || params[:dismiss_topics]
       dismissed_topic_ids =
         TopicsBulkAction.new(current_user, topic_scope.pluck(:id), type: "dismiss_topics").perform!
-      TopicTrackingState.publish_dismiss_new(current_user.id, topic_ids: dismissed_topic_ids)
     end
+
     if params[:dismiss_posts]
       if params[:untrack]
         dismissed_post_topic_ids =

app/models/topic_tracking_state.rb

@@ -29,6 +29,7 @@ class TopicTrackingState
   DESTROY_MESSAGE_TYPE = "destroy"
   READ_MESSAGE_TYPE = "read"
   DISMISS_NEW_MESSAGE_TYPE = "dismiss_new"
+  DISMISS_NEW_POSTS_MESSAGE_TYPE = "dismiss_new_posts"
   MAX_TOPICS = 5000
 
   NEW_MESSAGE_BUS_CHANNEL = "/new"
@@ -231,6 +232,11 @@ class TopicTrackingState
     MessageBus.publish(self.unread_channel_key(user_id), message.as_json, user_ids: [user_id])
   end
 
+  def self.publish_dismiss_new_posts(user_id, topic_ids: [])
+    message = { message_type: DISMISS_NEW_POSTS_MESSAGE_TYPE, payload: { topic_ids: topic_ids } }
+    MessageBus.publish(self.unread_channel_key(user_id), message.as_json, user_ids: [user_id])
+  end
+
   def self.new_filter_sql
     TopicQuery
       .new_filter(Topic, treat_as_new_topic_clause_sql: treat_as_new_topic_clause)

lib/topics_bulk_action.rb

@@ -86,6 +86,7 @@ class TopicsBulkAction
   def dismiss_posts
     highest_number_source_column =
       @user.whisperer? ? "highest_staff_post_number" : "highest_post_number"
+
     sql = <<~SQL
       UPDATE topic_users tu
       SET last_read_post_number = t.#{highest_number_source_column}
@@ -94,6 +95,8 @@ class TopicsBulkAction
     SQL
 
     DB.exec(sql, user_id: @user.id, topic_ids: @topic_ids)
+    TopicTrackingState.publish_dismiss_new_posts(@user.id, topic_ids: @topic_ids.sort)
+
     @changed_ids.concat @topic_ids
   end
 
@@ -115,7 +118,9 @@ class TopicsBulkAction
       now = Time.zone.now
       rows = ids.map { |id| { topic_id: id, user_id: @user.id, created_at: now } }
       DismissedTopicUser.insert_all(rows)
+      TopicTrackingState.publish_dismiss_new(@user.id, topic_ids: ids.sort)
     end
+
     @changed_ids = ids
   end
 

spec/models/topic_tracking_state_spec.rb

@@ -788,4 +788,20 @@ RSpec.describe TopicTrackingState do
     include_examples("publishes message to right groups and users", "/destroy", :publish_destroy)
     include_examples("does not publish message for private topics", :publish_destroy)
   end
+
+  describe "#publish_dismiss_new_posts" do
+    it "publishes the right message to the right users" do
+      messages =
+        MessageBus.track_publish(TopicTrackingState.unread_channel_key(user.id)) do
+          TopicTrackingState.publish_dismiss_new_posts(user.id, topic_ids: [topic.id])
+        end
+
+      expect(messages.size).to eq(1)
+
+      message = messages.first
+
+      expect(message.data["payload"]["topic_ids"]).to contain_exactly(topic.id)
+      expect(message.user_ids).to eq([user.id])
+    end
+  end
 end

spec/requests/topics_controller_spec.rb

@@ -3854,7 +3854,7 @@ RSpec.describe TopicsController do
 
       context "when tracked is unset" do
         it "updates the `new_since` date" do
-          TopicTrackingState.expects(:publish_dismiss_new)
+          TopicTrackingState.expects(:publish_dismiss_new).never
 
           put "/topics/reset-new.json"
           expect(response.status).to eq(200)
@@ -4052,15 +4052,10 @@ RSpec.describe TopicsController do
             messages =
               MessageBus.track_publish do
                 put "/topics/reset-new.json", params: { category_id: private_category.id }
+                expect(response.status).to eq(200)
               end
-            expect(response.status).to eq(200)
+
-            expect(messages.size).to eq(1)
+            expect(messages.size).to eq(0)
-            expect(messages[0].channel).to eq(TopicTrackingState.unread_channel_key(user.id))
-            expect(messages[0].user_ids).to eq([user.id])
-            expect(messages[0].data["message_type"]).to eq(
-              TopicTrackingState::DISMISS_NEW_MESSAGE_TYPE,
-            )
-            expect(messages[0].data["payload"]["topic_ids"]).to eq([])
             expect(DismissedTopicUser.where(user_id: user.id).count).to eq(0)
           end
 

spec/system/dismiss_topics_spec.rb

@@ -1,30 +0,0 @@
-# frozen_string_literal: true
-
-describe "Filtering topics", type: :system, js: true do
-  fab!(:user) { Fabricate(:user) }
-  fab!(:group) { Fabricate(:group).tap { |g| g.add(user) } }
-  fab!(:topic) { Fabricate(:topic) }
-
-  let(:topic_list) { PageObjects::Components::TopicList.new }
-  let(:dismiss_new_modal) { PageObjects::Modals::DismissNew.new }
-
-  before { SiteSetting.experimental_new_new_view_groups = group.id }
-
-  it "displays confirmation modal with preselected options" do
-    sign_in(user)
-
-    visit("/new")
-
-    expect(topic_list).to have_topic(topic)
-
-    find(".dismiss-read", text: "Dismiss…").click
-
-    expect(dismiss_new_modal).to have_dismiss_topics_checked
-    expect(dismiss_new_modal).to have_dismiss_posts_checked
-    expect(dismiss_new_modal).to have_untrack_unchecked
-
-    dismiss_new_modal.click_dismiss
-
-    expect(topic_list).to have_no_topics
-  end
-end

spec/system/dismissing_new_spec.rb

@@ -0,0 +1,114 @@
+# frozen_string_literal: true
+
+RSpec.describe "Dismissing New", type: :system do
+  fab!(:user) { Fabricate(:user) }
+
+  let(:topic_list_controls) { PageObjects::Components::TopicListControls.new }
+  let(:topic_list) { PageObjects::Components::TopicList.new }
+  let(:dismiss_new_modal) { PageObjects::Modals::DismissNew.new }
+
+  describe "when a user has an unread post" do
+    fab!(:topic) { Fabricate(:topic, user: user) }
+    fab!(:post1) { create_post(user: user, topic: topic) }
+    fab!(:post2) { create_post(topic: topic) }
+
+    it "should remove the unread post across sessions after the user dismisses it" do
+      sign_in(user)
+
+      visit("/unread")
+
+      expect(topic_list_controls).to have_unread(count: 1)
+
+      using_session(:tab_1) do
+        sign_in(user)
+
+        visit("/unread")
+
+        expect(topic_list_controls).to have_unread(count: 1)
+      end
+
+      topic_list_controls.dismiss_unread
+
+      expect(topic_list_controls).to have_unread(count: 0)
+
+      using_session(:tab_1) { expect(topic_list_controls).to have_unread(count: 0) }
+    end
+  end
+
+  describe "when a user has a new topic" do
+    fab!(:topic) { Fabricate(:topic) }
+
+    it "should remove the new topic across sessions after the user dismisses it" do
+      sign_in(user)
+
+      visit("/new")
+
+      expect(topic_list_controls).to have_new(count: 1)
+
+      using_session(:tab_1) do
+        sign_in(user)
+
+        visit("/new")
+
+        expect(topic_list_controls).to have_new(count: 1)
+      end
+
+      topic_list_controls.dismiss_new
+
+      expect(topic_list_controls).to have_new(count: 0)
+
+      using_session(:tab_1) { expect(topic_list_controls).to have_new(count: 0) }
+    end
+  end
+
+  describe "when the `experimental_new_new_view_groups` site setting is enabled" do
+    fab!(:group) { Fabricate(:group).tap { |g| g.add(user) } }
+    fab!(:new_topic) { Fabricate(:topic, user: user) }
+    fab!(:post1) { create_post(user: user) }
+    fab!(:post2) { create_post(topic: post1.topic) }
+
+    before { SiteSetting.experimental_new_new_view_groups = group.name }
+
+    it "should remove the new topic and post across sessions after the user dismisses it" do
+      sign_in(user)
+
+      visit("/new")
+
+      expect(topic_list_controls).to have_new(count: 2)
+
+      using_session(:tab_1) do
+        sign_in(user)
+
+        visit("/new")
+
+        expect(topic_list_controls).to have_new(count: 2)
+      end
+
+      topic_list_controls.dismiss_new
+      dismiss_new_modal.click_dismiss
+
+      expect(topic_list_controls).to have_new(count: 0)
+
+      using_session(:tab_1) { expect(topic_list_controls).to have_new(count: 0) }
+    end
+
+    it "displays confirmation modal with preselected options" do
+      sign_in(user)
+
+      visit("/new")
+
+      expect(topic_list).to have_topic(new_topic)
+      expect(topic_list).to have_topic(post1.topic)
+
+      topic_list_controls.dismiss_new
+
+      expect(dismiss_new_modal).to have_dismiss_topics_checked
+      expect(dismiss_new_modal).to have_dismiss_posts_checked
+      expect(dismiss_new_modal).to have_untrack_unchecked
+
+      dismiss_new_modal.click_dismiss
+
+      expect(topic_list).to have_no_topics
+    end
+  end
+end

spec/system/page_objects/components/topic_list_controls.rb

@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+module PageObjects
+  module Components
+    class TopicListControls < PageObjects::Components::Base
+      def has_new?(count:)
+        text =
+          if count == 0
+            I18n.t("js.filters.new.title")
+          else
+            I18n.t("js.filters.new.title_with_count", count: count)
+          end
+
+        has_css?(".nav-item_new", text: text)
+      end
+
+      def has_unread?(count:)
+        text =
+          if count == 0
+            I18n.t("js.filters.unread.title")
+          else
+            I18n.t("js.filters.unread.title_with_count", count: count)
+          end
+
+        has_css?(".nav-item_unread", text: text)
+      end
+
+      def dismiss_unread
+        click_button("dismiss-topics-bottom")
+        click_button("dismiss-read-confirm")
+        self
+      end
+
+      def dismiss_new
+        click_button("dismiss-new-bottom")
+        self
+      end
+    end
+  end
+end