From a26b490047b58a0d75736421b8530ac216727cbe Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Thu, 11 Jun 2020 16:47:45 +1000 Subject: [PATCH] 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. --- .../discourse/app/models/nav-item.js | 11 +++++-- .../app/models/topic-tracking-state.js | 32 +++++++++++-------- app/models/topic_tracking_state.rb | 2 +- spec/models/topic_tracking_state_spec.rb | 8 +++++ .../models/topic-tracking-state-test.js | 11 +++++-- 5 files changed, 45 insertions(+), 19 deletions(-) diff --git a/app/assets/javascripts/discourse/app/models/nav-item.js b/app/assets/javascripts/discourse/app/models/nav-item.js index bf41bdde661..49c0ad37b44 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 ee18ed42874..2875f2a5195 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 dcf1d60400b..9adec9acc43 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 ae0f852d635..7baf794b045 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 ee6c0c0e4e5..d9151e4da1d 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,