FIX: Dismiss new with better migration (#12062)

FIX: Dismiss new with better migration (#12062)

Original PR was reverted because of broken migration Revert "FEATURE: Ability to dismiss all new topics (#12018)" by lis2 · Pull Request #12058 · discourse/discourse · GitHub

I fixed it by adding this line

          AND topics.id IN(SELECT id FROM topics ORDER BY created_at DESC LIMIT :max_new_topics)

This time it is left joining a limited amount of topics. I tested it on few databases and it worked quite smooth

diff --git a/app/assets/javascripts/discourse/app/models/topic-tracking-state.js b/app/assets/javascripts/discourse/app/models/topic-tracking-state.js
index 5909ed6..b00e842 100644
--- a/app/assets/javascripts/discourse/app/models/topic-tracking-state.js
+++ b/app/assets/javascripts/discourse/app/models/topic-tracking-state.js
@@ -182,17 +182,11 @@ const TopicTrackingState = EmberObject.extend({
   },
 
   dismissNewTopic(data) {
-    Object.keys(this.states).forEach((k) => {
-      const topic = this.states[k];
-      if (
-        (!data.payload.category_id ||
-          topic.category_id === parseInt(data.payload.category_id, 10)) &&
-        (!data.payload.tag_id || topic.tags.includes(data.payload.tag_id))
-      ) {
-        this.states[k] = Object.assign({}, topic, {
-          is_seen: true,
-        });
-      }
+    data.payload.topic_ids.forEach((k) => {
+      const topic = this.states[`t${k}`];
+      this.states[`t${k}`] = Object.assign({}, topic, {
+        is_seen: true,
+      });
     });
     this.notifyPropertyChange("states");
     this.incrementMessageCount();
diff --git a/app/assets/javascripts/discourse/tests/unit/models/topic-tracking-state-test.js b/app/assets/javascripts/discourse/tests/unit/models/topic-tracking-state-test.js
index 8e5a0e7..56542df 100644
--- a/app/assets/javascripts/discourse/tests/unit/models/topic-tracking-state-test.js
+++ b/app/assets/javascripts/discourse/tests/unit/models/topic-tracking-state-test.js
@@ -346,36 +346,7 @@ module("Unit | Model | topic-tracking-state", function (hooks) {
 
     state.dismissNewTopic({
       message_type: "dismiss_new",
-      topic_id: 112,
-      payload: { category_id: 2 },
-    });
-    assert.equal(state.states["t112"].is_seen, false);
-    state.dismissNewTopic({
-      message_type: "dismiss_new",
-      topic_id: 112,
-      payload: { category_id: 1 },
-    });
-    assert.equal(state.states["t112"].is_seen, true);
-
-    state.states["t112"].is_seen = false;
-    state.dismissNewTopic({
-      message_type: "dismiss_new",
-      topic_id: 112,
-      payload: { tag_id: "bar" },
-    });
-    assert.equal(state.states["t112"].is_seen, false);
-    state.dismissNewTopic({
-      message_type: "dismiss_new",
-      topic_id: 112,
-      payload: { tag_id: "foo" },
-    });
-    assert.equal(state.states["t112"].is_seen, true);
-
-    state.states["t112"].is_seen = false;
-    state.dismissNewTopic({
-      message_type: "dismiss_new",
-      topic_id: 112,
-      payload: {},
+      payload: { topic_ids: [112] },
     });
     assert.equal(state.states["t112"].is_seen, true);
   });
diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb
index eaf6789..7fa7cf9 100644
--- a/app/controllers/topics_controller.rb
+++ b/app/controllers/topics_controller.rb
@@ -914,36 +914,30 @@ class TopicsController < ApplicationController
   end
 
   def reset_new
-    if params[:category_id].present?
-      category_ids = [params[:category_id]]
-      if params[:include_subcategories] == 'true'
-        category_ids = category_ids.concat(Category.where(parent_category_id: params[:category_id]).pluck(:id))
-      end
-
-      topic_scope =
-        if params[:tag_id]
-          Topic.where(category_id: category_ids).joins(:tags).where(tags: { name: params[:tag_id] })
-        else
-          Topic.where(category_id: category_ids)
+    topic_scope =
+      if params[:category_id].present?
+        category_ids = [params[:category_id]]
+        if params[:include_subcategories] == 'true'
+          category_ids = category_ids.concat(Category.where(parent_category_id: params[:category_id]).pluck(:id))
         end
 
-      DismissTopics.new(current_user, topic_scope).perform!
-      category_ids.each do |category_id|
-        TopicTrackingState.publish_dismiss_new(current_user.id, category_id: category_id, tag_id: params[:tag_id])
-      end
-    elsif params[:tag_id].present?
-      DismissTopics.new(current_user, Topic.joins(:tags).where(tags: { name: params[:tag_id] })).perform!
-      TopicTrackingState.publish_dismiss_new(current_user.id, tag_id: params[:tag_id])
-    else
-      if params[:tracked].to_s == "true"
-        topics = TopicQuery.tracked_filter(TopicQuery.new(current_user).new_results, current_user.id)
-        topic_users = topics.map { |topic| { topic_id: topic.id, user_id: current_user.id, last_read_post_number: 0 } }
-        TopicUser.insert_all(topic_users) if !topic_users.empty?
+        scope = Topic.where(category_id: category_ids)
+        scope = scope.joins(:tags).where(tags: { name: params[:tag_id] }) if params[:tag_id]
+        scope
+      elsif params[:tag_id].present?
+        Topic.joins(:tags).where(tags: { name: params[:tag_id] })
       else
-        current_user.user_stat.update_column(:new_since, Time.zone.now)
-        TopicTrackingState.publish_dismiss_new(current_user.id)
+        if params[:tracked].to_s == "true"
+          TopicQuery.tracked_filter(TopicQuery.new(current_user).new_results, current_user.id)
+        else
+          current_user.user_stat.update_column(:new_since, Time.zone.now)
+          Topic
+        end
       end
-    end
+
+    dismissed_topic_ids = DismissTopics.new(current_user, topic_scope).perform!
+    TopicTrackingState.publish_dismiss_new(current_user.id, topic_ids: dismissed_topic_ids)
+
     render body: nil
   end
 
diff --git a/app/jobs/scheduled/clean_dismissed_topic_users.rb b/app/jobs/scheduled/clean_dismissed_topic_users.rb
index c3190a5..72109ff 100644
--- a/app/jobs/scheduled/clean_dismissed_topic_users.rb
+++ b/app/jobs/scheduled/clean_dismissed_topic_users.rb
@@ -24,7 +24,7 @@ module Jobs
                     WHEN COALESCE(user_options.new_topic_duration_minutes, :default_duration) = :always THEN users.created_at
                     WHEN COALESCE(user_options.new_topic_duration_minutes, :default_duration) = :last_visit THEN COALESCE(users.previous_visit_at,users.created_at)
                     ELSE (:now::timestamp - INTERVAL '1 MINUTE' * COALESCE(user_options.new_topic_duration_minutes, :default_duration))
-                 END, user_stats.new_since, :min_date)
+                 END, users.created_at, :min_date)
         AND dtu1.id = dtu2.id
       SQL
       sql = DB.sql_fragment(sql,
diff --git a/app/models/topic_tracking_state.rb b/app/models/topic_tracking_state.rb
index 78e4c70..045ad34 100644
--- a/app/models/topic_tracking_state.rb
+++ b/app/models/topic_tracking_state.rb
@@ -215,13 +215,12 @@ class TopicTrackingState
     MessageBus.publish(self.unread_channel_key(user_id), message.as_json, user_ids: [user_id])
   end
 
-  def self.publish_dismiss_new(user_id, category_id: nil, tag_id: nil)
-    payload = {}
-    payload[:category_id] = category_id if category_id
-    payload[:tag_id] = tag_id if tag_id
+  def self.publish_dismiss_new(user_id, topic_ids: [])
     message = {
       message_type: "dismiss_new",
-      payload: payload
+      payload: {
+        topic_ids: topic_ids
+      }
     }
     MessageBus.publish(self.unread_channel_key(user_id), message.as_json, user_ids: [user_id])
   end
@@ -231,7 +230,7 @@ class TopicTrackingState
                   WHEN COALESCE(uo.new_topic_duration_minutes, :default_duration) = :always THEN u.created_at
                   WHEN COALESCE(uo.new_topic_duration_minutes, :default_duration) = :last_visit THEN COALESCE(u.previous_visit_at,u.created_at)
                   ELSE (:now::timestamp - INTERVAL '1 MINUTE' * COALESCE(uo.new_topic_duration_minutes, :default_duration))
-               END, us.new_since, :min_date)",
+               END, u.created_at, :min_date)",
                 now: DateTime.now,
                 last_visit: User::NewTopicDuration::LAST_VISIT,
                 always: User::NewTopicDuration::ALWAYS,
diff --git a/app/models/user_option.rb b/app/models/user_option.rb
index 1c0cfbb..8ba26ac 100644
--- a/app/models/user_option.rb
+++ b/app/models/user_option.rb

[... diff too long, it was truncated ...]

GitHub sha: ad3ec580

This commit appears in #12062 which was approved by martin. It was merged by lis2.