FIX: present correct new/unread counts when filtered by tag

FIX: present correct new/unread counts when filtered by tag

Previously we would incorrectly ignore tags.

This ensures tracking state is properly shipped to client if show_filter_by_tag is enabled.

diff --git a/app/assets/javascripts/discourse/app/models/nav-item.js b/app/assets/javascripts/discourse/app/models/nav-item.js
index bf41bdd..49c0ad3 100644
--- a/app/assets/javascripts/discourse/app/models/nav-item.js
+++ b/app/assets/javascripts/discourse/app/models/nav-item.js
@@ -71,11 +71,16 @@ const NavItem = EmberObject.extend({
     return mode + name.replace(" ", "-");
   },
 
-  @discourseComputed("name", "category", "topicTrackingState.messageCount")
-  count(name, category) {
+  @discourseComputed(
+    "name",
+    "category",
+    "tagId",
+    "topicTrackingState.messageCount"
+  )
+  count(name, category, tagId) {
     const state = this.topicTrackingState;
     if (state) {
-      return state.lookupCount(name, category);
+      return state.lookupCount(name, category, tagId);
     }
   }
 });
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 ee18ed4..2875f2a 100644
--- a/app/assets/javascripts/discourse/app/models/topic-tracking-state.js
+++ b/app/assets/javascripts/discourse/app/models/topic-tracking-state.js
@@ -408,7 +408,7 @@ const TopicTrackingState = EmberObject.extend({
     return new Set(result);
   },
 
-  countCategoryByState(fn, categoryId) {
+  countCategoryByState(fn, categoryId, tagId) {
     const subcategoryIds = this.getSubCategoryIds(categoryId);
     return _.chain(this.states)
       .filter(fn)
@@ -416,17 +416,18 @@ const TopicTrackingState = EmberObject.extend({
         topic =>
           topic.archetype !== "private_message" &&
           !topic.deleted &&
-          (!categoryId || subcategoryIds.has(topic.category_id))
+          (!categoryId || subcategoryIds.has(topic.category_id)) &&
+          (!tagId || (topic.tags && topic.tags.indexOf(tagId) > -1))
       )
       .value().length;
   },
 
-  countNew(categoryId) {
-    return this.countCategoryByState(isNew, categoryId);
+  countNew(categoryId, tagId) {
+    return this.countCategoryByState(isNew, categoryId, tagId);
   },
 
-  countUnread(categoryId) {
-    return this.countCategoryByState(isUnread, categoryId);
+  countUnread(categoryId, tagId) {
+    return this.countCategoryByState(isUnread, categoryId, tagId);
   },
 
   countTags(tags) {
@@ -462,10 +463,14 @@ const TopicTrackingState = EmberObject.extend({
     return counts;
   },
 
-  countCategory(category_id) {
+  countCategory(category_id, tagId) {
     let sum = 0;
     Object.values(this.states).forEach(topic => {
-      if (topic.category_id === category_id && !topic.deleted) {
+      if (
+        topic.category_id === category_id &&
+        !topic.deleted &&
+        (!tagId || (topic.tags && topic.tags.indexOf(tagId) > -1))
+      ) {
         sum +=
           topic.last_read_post_number === null ||
           topic.last_read_post_number < topic.highest_post_number
@@ -476,23 +481,24 @@ const TopicTrackingState = EmberObject.extend({
     return sum;
   },
 
-  lookupCount(name, category) {
+  lookupCount(name, category, tagId) {
     if (name === "latest") {
       return (
-        this.lookupCount("new", category) + this.lookupCount("unread", category)
+        this.lookupCount("new", category, tagId) +
+        this.lookupCount("unread", category, tagId)
       );
     }
 
     let categoryId = category ? get(category, "id") : null;
 
     if (name === "new") {
-      return this.countNew(categoryId);
+      return this.countNew(categoryId, tagId);
     } else if (name === "unread") {
-      return this.countUnread(categoryId);
+      return this.countUnread(categoryId, tagId);
     } else {
       const categoryName = name.split("/")[1];
       if (categoryName) {
-        return this.countCategory(categoryId);
+        return this.countCategory(categoryId, tagId);
       }
     }
   },
diff --git a/app/models/topic_tracking_state.rb b/app/models/topic_tracking_state.rb
index dcf1d60..9adec9a 100644
--- a/app/models/topic_tracking_state.rb
+++ b/app/models/topic_tracking_state.rb
@@ -186,7 +186,7 @@ class TopicTrackingState
   end
 
   def self.include_tags_in_report?
-    @include_tags_in_report
+    @include_tags_in_report || SiteSetting.show_filter_by_tag
   end
 
   def self.include_tags_in_report=(v)
diff --git a/spec/models/topic_tracking_state_spec.rb b/spec/models/topic_tracking_state_spec.rb
index ae0f852..7baf794 100644
--- a/spec/models/topic_tracking_state_spec.rb
+++ b/spec/models/topic_tracking_state_spec.rb
@@ -585,6 +585,14 @@ describe TopicTrackingState do
       expect(row.tags).to contain_exactly("apples", "bananas")
 
       TopicTrackingState.include_tags_in_report = false
+      SiteSetting.show_filter_by_tag = true
+
+      report = TopicTrackingState.report(user)
+      expect(report.length).to eq(1)
+      row = report[0]
+      expect(row.tags).to contain_exactly("apples", "bananas")
+
+      SiteSetting.show_filter_by_tag = false
 
       report = TopicTrackingState.report(user)
       expect(report.length).to eq(1)
diff --git a/test/javascripts/models/topic-tracking-state-test.js b/test/javascripts/models/topic-tracking-state-test.js
index ee6c0c0..d9151e4 100644
--- a/test/javascripts/models/topic-tracking-state-test.js
+++ b/test/javascripts/models/topic-tracking-state-test.js
@@ -175,7 +175,10 @@ QUnit.test("getSubCategoryIds", assert => {
 
 QUnit.test("countNew", assert => {
   const store = createStore();
-  const foo = store.createRecord("category", { id: 1, slug: "foo" });
+  const foo = store.createRecord("category", {
+    id: 1,
+    slug: "foo"
+  });
   const bar = store.createRecord("category", {
     id: 2,
     slug: "bar",
@@ -202,6 +205,7 @@ QUnit.test("countNew", assert => {
   };
 
   assert.equal(state.countNew(1), 1);
+  assert.equal(state.countNew(1, "missing-tag"), 0);
   assert.equal(state.countNew(2), 1);
   assert.equal(state.countNew(3), 0);
 
@@ -209,12 +213,15 @@ QUnit.test("countNew", assert => {
     last_read_post_number: null,
     id: 113,
     notification_level: NotificationLevels.TRACKING,
-    category_id: 3
+    category_id: 3,
+    tags: ["amazing"]
   };
 
   assert.equal(state.countNew(1), 2);
   assert.equal(state.countNew(2), 2);
   assert.equal(state.countNew(3), 1);
+  assert.equal(state.countNew(3, "amazing"), 1);
+  assert.equal(state.countNew(3, "missing"), 0);
 
   state.states["t111"] = {
     last_read_post_number: null,

GitHub sha: a26b4900

1 Like

This commit has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/clicking-on-new-in-tag-page-only-lists-new-topics-from-tag-page/90473/3