FIX: Dismiss unread respects tracked query param (#10714)

* WIP:  'dismiss...' respectes tracked query param

* Address review comments

* Dismiss new respects query params

* Remove comment

* Better variable name

* remove self

app/assets/javascripts/discourse/app/controllers/discovery-sortable.js

@@ -39,10 +39,12 @@ export function changeSort(sortBy) {
   }
 }
 
-export function resetParams() {
+export function resetParams(skipParams = []) {
   let { controller } = this;
   controllerOpts.queryParams.forEach((p) => {
-    controller.set(p, queryParams[p].default);
+    if (!skipParams.includes(p)) {
+      controller.set(p, queryParams[p].default);
+    }
   });
 }
 

app/assets/javascripts/discourse/app/controllers/discovery/topics.js

@@ -57,9 +57,9 @@ const controllerOpts = {
       return false;
     },
 
-    refresh() {
+    refresh(options = { skipResettingParams: [] }) {
       const filter = this.get("model.filter");
-      this.send("resetParams");
+      this.send("resetParams", options.skipResettingParams);
 
       // Don't refresh if we're still loading
       if (this.get("discovery.loading")) {
@@ -72,23 +72,36 @@ const controllerOpts = {
       this.set("discovery.loading", true);
 
       this.topicTrackingState.resetTracking();
+
       this.store.findFiltered("topicList", { filter }).then((list) => {
         TopicList.hideUniformCategory(list, this.category);
 
-        this.setProperties({ model: list });
-        this.resetSelected();
-
-        if (this.topicTrackingState) {
-          this.topicTrackingState.sync(list, filter);
+        // If query params are present in the current route, we need still need to sync topic
+        // tracking with the topicList without any query params. Then we set the topic
+        // list to the list filtered with query params in the afterRefresh.
+        const params = this.router.currentRoute.queryParams;
+        if (Object.keys(params).length) {
+          this.store
+            .findFiltered("topicList", { filter, params })
+            .then((listWithParams) => {
+              this.afterRefresh(filter, list, listWithParams);
+            });
+        } else {
+          this.afterRefresh(filter, list);
         }
-
-        this.send("loadingComplete");
       });
     },
 
     resetNew() {
-      Topic.resetNew(this.category, !this.noSubcategories).then(() =>
-        this.send("refresh")
+      const tracked =
+        (this.router.currentRoute.queryParams["f"] ||
+          this.router.currentRoute.queryParams["filter"]) === "tracked";
+
+      Topic.resetNew(this.category, !this.noSubcategories, tracked).then(() =>
+        this.send(
+          "refresh",
+          tracked ? { skipResettingParams: ["filter", "f"] } : {}
+        )
       );
     },
 
@@ -97,6 +110,17 @@ const controllerOpts = {
     },
   },
 
+  afterRefresh(filter, list, listModel = list) {
+    this.setProperties({ model: listModel });
+    this.resetSelected();
+
+    if (this.topicTrackingState) {
+      this.topicTrackingState.sync(list, filter);
+    }
+
+    this.send("loadingComplete");
+  },
+
   isFilterPage: function (filter, filterType) {
     if (!filter) {
       return false;

app/assets/javascripts/discourse/app/mixins/bulk-topic-selection.js

@@ -22,22 +22,21 @@ export default Mixin.create({
     },
 
     dismissRead(operationType, options) {
-      let operation;
-      if (operationType === "posts") {
-        operation = { type: "dismiss_posts" };
-      } else {
-        operation = {
-          type: "change_notification_level",
-          notification_level_id: NotificationLevels.REGULAR,
-        };
-      }
+      const operation =
+        operationType === "posts"
+          ? { type: "dismiss_posts" }
+          : {
+              type: "change_notification_level",
+              notification_level_id: NotificationLevels.REGULAR,
+            };
 
-      let promise;
-      if (this.selected.length > 0) {
-        promise = Topic.bulkOperation(this.selected, operation);
-      } else {
-        promise = Topic.bulkOperationByFilter("unread", operation, options);
-      }
+      const tracked =
+        (this.router.currentRoute.queryParams["f"] ||
+          this.router.currentRoute.queryParams["filter"]) === "tracked";
+
+      const promise = this.selected.length
+        ? Topic.bulkOperation(this.selected, operation, tracked)
+        : Topic.bulkOperationByFilter("unread", operation, options, tracked);
 
       promise.then((result) => {
         if (result && result.topic_ids) {
@@ -47,7 +46,10 @@ export default Mixin.create({
         }
 
         this.send("closeModal");
-        this.send("refresh");
+        this.send(
+          "refresh",
+          tracked ? { skipResettingParams: ["filter", "f"] } : {}
+        );
       });
     },
   },

app/assets/javascripts/discourse/app/models/topic.js

@@ -799,18 +799,21 @@ Topic.reopenClass({
     return promise;
   },
 
-  bulkOperation(topics, operation) {
+  bulkOperation(topics, operation, tracked) {
+    const data = {
+      topic_ids: topics.mapBy("id"),
+      operation,
+      tracked,
+    };
+
     return ajax("/topics/bulk", {
       type: "PUT",
-      data: {
-        topic_ids: topics.map((t) => t.get("id")),
-        operation,
-      },
+      data,
     });
   },
 
-  bulkOperationByFilter(filter, operation, options) {
-    let data = { filter, operation };
+  bulkOperationByFilter(filter, operation, options, tracked) {
+    const data = { filter, operation, tracked };
 
     if (options) {
       if (options.categoryId) {
@@ -830,10 +833,12 @@ Topic.reopenClass({
     });
   },
 
-  resetNew(category, include_subcategories) {
-    const data = category
-      ? { category_id: category.id, include_subcategories }
-      : {};
+  resetNew(category, include_subcategories, tracked = false) {
+    const data = { tracked };
+    if (category) {
+      data.category_id = category.id;
+      data.include_subcategories = include_subcategories;
+    }
     return ajax("/topics/reset-new", { type: "PUT", data });
   },
 

app/controllers/topics_controller.rb

@@ -845,6 +845,7 @@ class TopicsController < ApplicationController
     elsif params[:filter] == 'unread'
       tq = TopicQuery.new(current_user)
       topics = TopicQuery.unread_filter(tq.joined_topic_user, current_user.id, staff: guardian.is_staff?).listable_topics
+      topics = TopicQuery.tracked_filter(topics, current_user.id) if params[:tracked].to_s == "true"
 
       if params[:category_id]
         if params[:include_subcategories]
@@ -892,8 +893,14 @@ class TopicsController < ApplicationController
         TopicTrackingState.publish_dismiss_new(current_user.id, category_id)
       end
     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"
+        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?
+      else
+        current_user.user_stat.update_column(:new_since, Time.zone.now)
+        TopicTrackingState.publish_dismiss_new(current_user.id)
+      end
     end
     render body: nil
   end

lib/topic_query.rb

@@ -394,6 +394,33 @@ class TopicQuery
                regular: TopicUser.notification_levels[:regular], tracking: TopicUser.notification_levels[:tracking])
   end
 
+  def self.tracked_filter(list, user_id)
+    sql = +<<~SQL
+      topics.category_id IN (
+        SELECT cu.category_id FROM category_users cu
+        WHERE cu.user_id = :user_id AND cu.notification_level >= :tracking
+      )
+    SQL
+
+    if SiteSetting.tagging_enabled
+      sql << <<~SQL
+        OR topics.id IN (
+          SELECT tt.topic_id FROM topic_tags tt WHERE tt.tag_id IN (
+            SELECT tu.tag_id
+            FROM tag_users tu
+            WHERE tu.user_id = :user_id AND tu.notification_level >= :tracking
+          )
+        )
+      SQL
+    end
+
+    list.where(
+      sql,
+      user_id: user_id,
+      tracking: NotificationLevels.all[:tracking]
+    )
+  end
+
   def prioritize_pinned_topics(topics, options)
     pinned_clause = if options[:category_id]
       +"topics.category_id = #{options[:category_id].to_i} AND"
@@ -833,30 +860,7 @@ class TopicQuery
       end
 
       if filter == "tracked"
-        sql = +<<~SQL
-          topics.category_id IN (
-            SELECT cu.category_id FROM category_users cu
-            WHERE cu.user_id = :user_id AND cu.notification_level >= :tracking
-          )
-        SQL
-
-        if SiteSetting.tagging_enabled
-          sql << <<~SQL
-            OR topics.id IN (
-              SELECT tt.topic_id FROM topic_tags tt WHERE tt.tag_id IN (
-                SELECT tu.tag_id
-                FROM tag_users tu
-                WHERE tu.user_id = :user_id AND tu.notification_level >= :tracking
-              )
-            )
-          SQL
-        end
-
-        result = result.where(
-          sql,
-          user_id: @user.id,
-          tracking: NotificationLevels.all[:tracking]
-        )
+        result = TopicQuery.tracked_filter(result, @user.id)
       end
     end
 

spec/requests/topics_controller_spec.rb

@@ -2567,6 +2567,36 @@ RSpec.describe TopicsController do
           topic_ids: topic_ids, operation: operation
         }
       end
+
+      it "respects the tracked parameter" do
+        # untracked topic
+        category = Fabricate(:category)
+        CategoryUser.set_notification_level_for_category(user,
+                                                     NotificationLevels.all[:regular],
+                                                     category.id)
+        create_post(user: user, topic_id: topic.id)
+        topic.update!(category_id: category.id)
+        create_post(topic_id: topic.id)
+
+        # tracked topic
+        tracked_category = Fabricate(:category)
+        CategoryUser.set_notification_level_for_category(user,
+                                                     NotificationLevels.all[:tracking],
+                                                     tracked_category.id)
+        tracked_topic = create_post(user: user).topic
+        tracked_topic.update!(category_id: tracked_category.id)
+        create_post(topic_id: tracked_topic.id)
+
+        put "/topics/bulk.json", params: {
+          filter: 'unread',
+          operation: { type: 'dismiss_posts' },
+          tracked: true
+        }
+
+        expect(response.status).to eq(200)
+        expect(TopicUser.get(topic, user).last_read_post_number).to eq(topic.posts.count - 1)
+        expect(TopicUser.get(tracked_topic, user).last_read_post_number).to eq(tracked_topic.posts.count)
+      end
     end
   end
 
@@ -2655,7 +2685,6 @@ RSpec.describe TopicsController do
       sign_in(user)
 
       old_date = 2.years.ago
-
       user.user_stat.update_column(:new_since, old_date)
 
       TopicTrackingState.expects(:publish_dismiss_new).with(user.id)
@@ -2666,6 +2695,35 @@ RSpec.describe TopicsController do
       expect(user.user_stat.new_since.to_date).not_to eq(old_date.to_date)
     end
 
+    describe "when tracked param is true" do
+      it "does not update user_stat.new_since" do
+        sign_in(user)
+
+        old_date = 2.years.ago
+        user.user_stat.update_column(:new_since, old_date)
+
+        put "/topics/reset-new.json?tracked=true"
+        expect(response.status).to eq(200)
+        user.reload
+        expect(user.user_stat.new_since.to_date).to eq(old_date.to_date)
+      end
+
+      it "creates topic user records for each unread topic" do
+        sign_in(user)
+        user.user_stat.update_column(:new_since, 2.years.ago)
+
+        tracked_category = Fabricate(:category)
+        CategoryUser.set_notification_level_for_category(user,
+                                                     NotificationLevels.all[:tracking],
+                                                     tracked_category.id)
+        tracked_topic = create_post.topic
+        tracked_topic.update!(category_id: tracked_category.id)
+
+        create_post # This is a new post, but is not tracked so a record will not be created for it
+        expect { put "/topics/reset-new.json?tracked=true" }.to change { TopicUser.where(user_id: user.id, last_read_post_number: 0).count }.by(1)
+      end
+    end
+
     context 'category' do
       fab!(:category) { Fabricate(:category) }
       fab!(:subcategory) { Fabricate(:category, parent_category_id: category.id) }