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.
This commit is contained in:
Sam Saffron
2020-06-11 16:47:45 +10:00
parent 0061f758bd
commit a26b490047
5 changed files with 45 additions and 19 deletions

View File

@ -71,11 +71,16 @@ const NavItem = EmberObject.extend({
return mode + name.replace(" ", "-"); return mode + name.replace(" ", "-");
}, },
@discourseComputed("name", "category", "topicTrackingState.messageCount") @discourseComputed(
count(name, category) { "name",
"category",
"tagId",
"topicTrackingState.messageCount"
)
count(name, category, tagId) {
const state = this.topicTrackingState; const state = this.topicTrackingState;
if (state) { if (state) {
return state.lookupCount(name, category); return state.lookupCount(name, category, tagId);
} }
} }
}); });

View File

@ -408,7 +408,7 @@ const TopicTrackingState = EmberObject.extend({
return new Set(result); return new Set(result);
}, },
countCategoryByState(fn, categoryId) { countCategoryByState(fn, categoryId, tagId) {
const subcategoryIds = this.getSubCategoryIds(categoryId); const subcategoryIds = this.getSubCategoryIds(categoryId);
return _.chain(this.states) return _.chain(this.states)
.filter(fn) .filter(fn)
@ -416,17 +416,18 @@ const TopicTrackingState = EmberObject.extend({
topic => topic =>
topic.archetype !== "private_message" && topic.archetype !== "private_message" &&
!topic.deleted && !topic.deleted &&
(!categoryId || subcategoryIds.has(topic.category_id)) (!categoryId || subcategoryIds.has(topic.category_id)) &&
(!tagId || (topic.tags && topic.tags.indexOf(tagId) > -1))
) )
.value().length; .value().length;
}, },
countNew(categoryId) { countNew(categoryId, tagId) {
return this.countCategoryByState(isNew, categoryId); return this.countCategoryByState(isNew, categoryId, tagId);
}, },
countUnread(categoryId) { countUnread(categoryId, tagId) {
return this.countCategoryByState(isUnread, categoryId); return this.countCategoryByState(isUnread, categoryId, tagId);
}, },
countTags(tags) { countTags(tags) {
@ -462,10 +463,14 @@ const TopicTrackingState = EmberObject.extend({
return counts; return counts;
}, },
countCategory(category_id) { countCategory(category_id, tagId) {
let sum = 0; let sum = 0;
Object.values(this.states).forEach(topic => { 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 += sum +=
topic.last_read_post_number === null || topic.last_read_post_number === null ||
topic.last_read_post_number < topic.highest_post_number topic.last_read_post_number < topic.highest_post_number
@ -476,23 +481,24 @@ const TopicTrackingState = EmberObject.extend({
return sum; return sum;
}, },
lookupCount(name, category) { lookupCount(name, category, tagId) {
if (name === "latest") { if (name === "latest") {
return ( 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; let categoryId = category ? get(category, "id") : null;
if (name === "new") { if (name === "new") {
return this.countNew(categoryId); return this.countNew(categoryId, tagId);
} else if (name === "unread") { } else if (name === "unread") {
return this.countUnread(categoryId); return this.countUnread(categoryId, tagId);
} else { } else {
const categoryName = name.split("/")[1]; const categoryName = name.split("/")[1];
if (categoryName) { if (categoryName) {
return this.countCategory(categoryId); return this.countCategory(categoryId, tagId);
} }
} }
}, },

View File

@ -186,7 +186,7 @@ class TopicTrackingState
end end
def self.include_tags_in_report? def self.include_tags_in_report?
@include_tags_in_report @include_tags_in_report || SiteSetting.show_filter_by_tag
end end
def self.include_tags_in_report=(v) def self.include_tags_in_report=(v)

View File

@ -585,6 +585,14 @@ describe TopicTrackingState do
expect(row.tags).to contain_exactly("apples", "bananas") expect(row.tags).to contain_exactly("apples", "bananas")
TopicTrackingState.include_tags_in_report = false 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) report = TopicTrackingState.report(user)
expect(report.length).to eq(1) expect(report.length).to eq(1)

View File

@ -175,7 +175,10 @@ QUnit.test("getSubCategoryIds", assert => {
QUnit.test("countNew", assert => { QUnit.test("countNew", assert => {
const store = createStore(); 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", { const bar = store.createRecord("category", {
id: 2, id: 2,
slug: "bar", slug: "bar",
@ -202,6 +205,7 @@ QUnit.test("countNew", assert => {
}; };
assert.equal(state.countNew(1), 1); assert.equal(state.countNew(1), 1);
assert.equal(state.countNew(1, "missing-tag"), 0);
assert.equal(state.countNew(2), 1); assert.equal(state.countNew(2), 1);
assert.equal(state.countNew(3), 0); assert.equal(state.countNew(3), 0);
@ -209,12 +213,15 @@ QUnit.test("countNew", assert => {
last_read_post_number: null, last_read_post_number: null,
id: 113, id: 113,
notification_level: NotificationLevels.TRACKING, notification_level: NotificationLevels.TRACKING,
category_id: 3 category_id: 3,
tags: ["amazing"]
}; };
assert.equal(state.countNew(1), 2); assert.equal(state.countNew(1), 2);
assert.equal(state.countNew(2), 2); assert.equal(state.countNew(2), 2);
assert.equal(state.countNew(3), 1); assert.equal(state.countNew(3), 1);
assert.equal(state.countNew(3, "amazing"), 1);
assert.equal(state.countNew(3, "missing"), 0);
state.states["t111"] = { state.states["t111"] = {
last_read_post_number: null, last_read_post_number: null,