github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2024-11-25 09:42:07 +08:00
Code Issues Actions 6 Packages Projects Releases Wiki Activity

FIX: Topic list nav items count not respecting tracked filter. (#16935)

Browse Source 
This commit seeks to only handle the `f=tracked` and `filter=tracked`
query params for a topic list. There are other "hidden" filters for a
topic list which can be activated by passing the right query param to
the request. However, they are hidden because there is no way to
activate those filters via the UI. We are handling the `f=tracked`
filter because we will soon be adding a link that allows a user to
quickly view their tracked topics.
This commit is contained in:
Alan Guo Xiang Tan 2022-06-01 14:54:42 +08:00 committed by GitHub
parent 098bea19de
commit 1e9f132b15
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 582 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
app/assets/javascripts/discourse/app/components/navigation-item.js
View File

@ -9,6 +9,7 @@ export default Component.extend(FilterModeMixin, {
"content.hasIcon:has-icon",
"content.classNames",
"isHidden:hidden",
"content.name",
],
attributeBindings: ["content.title:title"],
hidden: false,

58
app/assets/javascripts/discourse/app/lib/topic-list-tracked-filter.js Normal file
View File

@ -0,0 +1,58 @@
import Site from "discourse/models/site";
import User from "discourse/models/user";
export function hasTrackedFilter(queryParams) {
if (!queryParams) {
return false;
}
return queryParams.f === "tracked" || queryParams.filter === "tracked";
}
/**
* Logic here needs to be in sync with `TopicQuery#tracked_filter` on the server side. See `TopicQuery#tracked_filter`
* for the rational behind this decision.
*/
export function isTrackedTopic(topic) {
if (topic.category_id) {
const categories = Site.current().trackedCategoriesList;
for (const category of categories) {
if (category.id === topic.category_id) {
return true;
}
if (
category.subcategories &&
category.subcategories.some((subCategory) => {
if (subCategory.id === topic.category_id) {
return true;
}
if (
subCategory.subcategories &&
subCategory.subcategories.some((c) => {
return c.id === topic.category_id;
})
) {
return true;
}
})
) {
return true;
}
}
}
if (topic.tags) {
const tags = User.current().trackedTags;
for (const tag of tags) {
if (topic.tags.indexOf(tag) > -1) {
return true;
}
}
}
return false;
}

4
app/assets/javascripts/discourse/app/models/category.js
View File

@ -298,12 +298,12 @@ const Category = RestModel.extend({
@discourseComputed("id", "topicTrackingState.messageCount")
unreadTopics(id) {
return this.topicTrackingState.countUnread(id);
return this.topicTrackingState.countUnread({ categoryId: id });
},
@discourseComputed("id", "topicTrackingState.messageCount")
newTopics(id) {
return this.topicTrackingState.countNew(id);
return this.topicTrackingState.countNew({ categoryId: id });
},
setNotification(notification_level) {

18
app/assets/javascripts/discourse/app/models/nav-item.js
View File

@ -8,6 +8,10 @@ import deprecated from "discourse-common/lib/deprecated";
import discourseComputed from "discourse-common/utils/decorators";
import { emojiUnescape } from "discourse/lib/text";
import { getOwner } from "discourse-common/lib/get-owner";
import {
hasTrackedFilter,
isTrackedTopic,
} from "discourse/lib/topic-list-tracked-filter";
import getURL from "discourse-common/lib/get-url";
import { reads } from "@ember/object/computed";
@ -76,12 +80,22 @@ const NavItem = EmberObject.extend({
"category",
"tagId",
"noSubcategories",
"currentRouteQueryParams",
"topicTrackingState.messageCount"
)
count(name, category, tagId, noSubcategories) {
count(name, category, tagId, noSubcategories, currentRouteQueryParams) {
const state = this.topicTrackingState;
if (state) {
return state.lookupCount(name, category, tagId, noSubcategories);
return state.lookupCount({
type: name,
category,
tagId,
noSubcategories,
customFilterFn: hasTrackedFilter(currentRouteQueryParams)
? isTrackedTopic
: undefined,
});
}
},
});

14
app/assets/javascripts/discourse/app/models/site.js
View File

@ -11,6 +11,7 @@ import discourseComputed from "discourse-common/utils/decorators";
import { getOwner } from "discourse-common/lib/get-owner";
import { isEmpty } from "@ember/utils";
import { htmlSafe } from "@ember/template";
import { NotificationLevels } from "discourse/lib/notification-levels";
const Site = RestModel.extend({
isReadOnly: alias("is_readonly"),
@ -83,6 +84,19 @@ const Site = RestModel.extend({
: this.sortedCategories;
},
@discourseComputed("categories.[]")
trackedCategoriesList(categories) {
const trackedCategories = [];
for (const category of categories) {
if (category.notification_level >= NotificationLevels.TRACKING) {
trackedCategories.push(category);
}
}
return trackedCategories;
},
postActionTypeById(id) {
return this.get("postActionByIdLookup.action" + id);
},

109
app/assets/javascripts/discourse/app/models/topic-tracking-state.js
View File

@ -476,7 +476,13 @@ const TopicTrackingState = EmberObject.extend({
return new Set(result);
},
countCategoryByState(type, categoryId, tagId, noSubcategories) {
countCategoryByState({
type,
categoryId,
tagId,
noSubcategories,
customFilterFn,
}) {
const subcategoryIds = noSubcategories
? new Set([categoryId])
: this.getSubCategoryIds(categoryId);
@ -486,28 +492,53 @@ const TopicTrackingState = EmberObject.extend({
);
let filterFn = type === "new" ? isNew : isUnread;
return Array.from(this.states.values()).filter(
(topic) =>
filterFn(topic) &&
(!categoryId || subcategoryIds.has(topic.category_id)) &&
(!tagId || (topic.tags && topic.tags.indexOf(tagId) > -1)) &&
(type !== "new" ||
!mutedCategoryIds ||
mutedCategoryIds.indexOf(topic.category_id) === -1)
).length;
return Array.from(this.states.values()).filter((topic) => {
if (!filterFn(topic)) {
return false;
}
if (categoryId && !subcategoryIds.has(topic.category_id)) {
return false;
}
if (tagId && !(topic.tags && topic.tags.indexOf(tagId) > -1)) {
return false;
}
if (
type === "new" &&
mutedCategoryIds &&
mutedCategoryIds.indexOf(topic.category_id) !== -1
) {
return false;
}
if (customFilterFn && !customFilterFn.call(this, topic)) {
return false;
}
return true;
}).length;
},
countNew(categoryId, tagId, noSubcategories) {
return this.countCategoryByState("new", categoryId, tagId, noSubcategories);
},
countUnread(categoryId, tagId, noSubcategories) {
return this.countCategoryByState(
"unread",
countNew({ categoryId, tagId, noSubcategories, customFilterFn } = {}) {
return this.countCategoryByState({
type: "new",
categoryId,
tagId,
noSubcategories
);
noSubcategories,
customFilterFn,
});
},
countUnread({ categoryId, tagId, noSubcategories, customFilterFn } = {}) {
return this.countCategoryByState({
type: "unread",
categoryId,
tagId,
noSubcategories,
customFilterFn,
});
},
/**
@ -597,22 +628,44 @@ const TopicTrackingState = EmberObject.extend({
return sum;
},
lookupCount(name, category, tagId, noSubcategories) {
if (name === "latest") {
lookupCount({ type, category, tagId, noSubcategories, customFilterFn } = {}) {
if (type === "latest") {
return (
this.lookupCount("new", category, tagId, noSubcategories) +
this.lookupCount("unread", category, tagId, noSubcategories)
this.lookupCount({
type: "new",
category,
tagId,
noSubcategories,
customFilterFn,
}) +
this.lookupCount({
type: "unread",
category,
tagId,
noSubcategories,
customFilterFn,
})
);
}
let categoryId = category ? get(category, "id") : null;
if (name === "new") {
return this.countNew(categoryId, tagId, noSubcategories);
} else if (name === "unread") {
return this.countUnread(categoryId, tagId, noSubcategories);
if (type === "new") {
return this.countNew({
categoryId,
tagId,
noSubcategories,
customFilterFn,
});
} else if (type === "unread") {
return this.countUnread({
categoryId,
tagId,
noSubcategories,
customFilterFn,
});
} else {
const categoryName = name.split("/")[1];
const categoryName = type.split("/")[1];
if (categoryName) {
return this.countCategory(categoryId, tagId);
}

9
app/assets/javascripts/discourse/app/models/user.js
View File

@ -1008,6 +1008,15 @@ const User = RestModel.extend({
new Date(this.do_not_disturb_until) >= new Date()
);
},
@discourseComputed(
"tracked_tags.[]",
"watched_tags.[]",
"watching_first_post_tags.[]"
)
trackedTags(trackedTags, watchedTags, watchingFirstPostTags) {
return [...trackedTags, ...watchedTags, ...watchingFirstPostTags];
},
});
User.reopenClass(Singleton, {

2
app/assets/javascripts/discourse/app/widgets/hamburger-menu.js
View File

@ -96,7 +96,7 @@ export default createWidget("hamburger-menu", {
lookupCount(type) {
const tts = this.register.lookup("topic-tracking-state:main");
return tts ? tts.lookupCount(type) : 0;
return tts ? tts.lookupCount({ type }) : 0;
},
generalLinks() {

307
app/assets/javascripts/discourse/tests/acceptance/topic-discovery-tracked-test.js Normal file
View File

@ -0,0 +1,307 @@
import { settled, visit } from "@ember/test-helpers";
import I18n from "I18n";
import { test } from "qunit";
import {
acceptance,
publishToMessageBus,
query,
} from "discourse/tests/helpers/qunit-helpers";
import Site from "discourse/models/site";
import { NotificationLevels } from "discourse/lib/notification-levels";
import topicFixtures from "discourse/tests/fixtures/discovery-fixtures";
import { cloneJSON } from "discourse-common/lib/object";
acceptance("Topic Discovery Tracked", function (needs) {
needs.user({
tracked_tags: ["tag1"],
watched_tags: ["tag2"],
watching_first_post_tags: ["tag3"],
});
needs.pretender((server, helper) => {
server.get("/c/:category-slug/:category-id/l/latest.json", () => {
return helper.response(cloneJSON(topicFixtures["/latest.json"]));
});
server.get("/tag/:tag_name/notifications", () => {
return helper.response({
tag_notification: {
id: "test",
notification_level: NotificationLevels.TRACKING,
},
});
});
server.get("/tag/:tag_name/l/latest.json", () => {
return helper.response(cloneJSON(topicFixtures["/latest.json"]));
});
});
test("visit discovery pages with tracked filter", async function (assert) {
const categories = Site.current().categories;
// Category id 1001 has two subcategories
const category = categories.find((c) => c.id === 1001);
category.set("notification_level", NotificationLevels.TRACKING);
this.container.lookup("topic-tracking-state:main").loadStates([
{
topic_id: 1,
highest_post_number: 1,
last_read_post_number: null,
created_at: "2022-05-11T03:09:31.959Z",
category_id: category.id,
notification_level: NotificationLevels.TRACKING,
created_in_new_period: true,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
topic_id: 2,
highest_post_number: 12,
last_read_post_number: 11,
created_at: "2020-02-09T09:40:02.672Z",
category_id: category.subcategories[0].id,
notification_level: NotificationLevels.TRACKING,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
topic_id: 3,
highest_post_number: 12,
last_read_post_number: 11,
created_at: "2020-02-09T09:40:02.672Z",
category_id: category.subcategories[0].subcategories[0].id,
notification_level: NotificationLevels.TRACKING,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
topic_id: 4,
highest_post_number: 15,
last_read_post_number: 14,
created_at: "2021-06-14T12:41:02.477Z",
category_id: 3,
notification_level: NotificationLevels.TRACKING,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
topic_id: 5,
highest_post_number: 1,
last_read_post_number: null,
created_at: "2021-06-14T12:41:02.477Z",
category_id: 3,
notification_level: null,
created_in_new_period: true,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
topic_id: 6,
highest_post_number: 17,
last_read_post_number: 16,
created_at: "2020-10-31T03:41:42.257Z",
category_id: 1234,
notification_level: NotificationLevels.TRACKING,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
tags: ["tag3"],
},
]);
await visit("/");
assert.strictEqual(
query("#navigation-bar li.unread").textContent.trim(),
I18n.t("filters.unread.title_with_count", { count: 4 }),
"displays the right content on unread link"
);
assert.strictEqual(
query("#navigation-bar li.new").textContent.trim(),
I18n.t("filters.new.title_with_count", { count: 2 }),
"displays the right content on new link"
);
await visit("/?f=tracked");
assert.strictEqual(
query("#navigation-bar li.unread").textContent.trim(),
I18n.t("filters.unread.title_with_count", { count: 3 }),
"displays the right content on unread link"
);
assert.strictEqual(
query("#navigation-bar li.new").textContent.trim(),
I18n.t("filters.new.title_with_count", { count: 1 }),
"displays the right content on new link"
);
// simulate reading topic id 1
publishToMessageBus("/unread", {
topic_id: 1,
message_type: "read",
payload: {
last_read_post_number: 1,
highest_post_number: 1,
},
});
// simulate reading topic id 3
publishToMessageBus("/unread", {
topic_id: 3,
message_type: "read",
payload: {
last_read_post_number: 12,
highest_post_number: 12,
},
});
await settled();
assert.strictEqual(
query("#navigation-bar li.unread").textContent.trim(),
I18n.t("filters.unread.title_with_count", { count: 2 }),
"displays the right content on unread link"
);
assert.strictEqual(
query("#navigation-bar li.new").textContent.trim(),
I18n.t("filters.new.title"),
"displays the right content on new link"
);
});
test("visit discovery page filtered by tag with tracked filter", async function (assert) {
this.container.lookup("topic-tracking-state:main").loadStates([
{
topic_id: 1,
highest_post_number: 1,
last_read_post_number: null,
created_at: "2022-05-11T03:09:31.959Z",
category_id: 1,
notification_level: null,
created_in_new_period: true,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
tags: ["someothertag"],
},
]);
await visit("/tag/someothertag");
assert.strictEqual(
query("#navigation-bar li.unread").textContent.trim(),
I18n.t("filters.unread.title"),
"displays the right content on unread link"
);
assert.strictEqual(
query("#navigation-bar li.new").textContent.trim(),
I18n.t("filters.new.title_with_count", { count: 1 }),
"displays the right content on new link"
);
await visit("/tag/someothertag?f=tracked");
assert.strictEqual(
query("#navigation-bar li.unread").textContent.trim(),
I18n.t("filters.unread.title"),
"displays the right content on unread link"
);
assert.strictEqual(
query("#navigation-bar li.new").textContent.trim(),
I18n.t("filters.new.title"),
"displays the right content on new link"
);
});
test("visit discovery page filtered by category with tracked filter", async function (assert) {
const categories = Site.current().categories;
const category = categories.at(-1);
category.set("notification_level", NotificationLevels.TRACKING);
this.container.lookup("topic-tracking-state:main").loadStates([
{
topic_id: 1,
highest_post_number: 1,
last_read_post_number: null,
created_at: "2022-05-11T03:09:31.959Z",
category_id: category.id,
notification_level: null,
created_in_new_period: true,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
topic_id: 2,
highest_post_number: 12,
last_read_post_number: 11,
created_at: "2020-02-09T09:40:02.672Z",
category_id: category.id,
notification_level: NotificationLevels.TRACKING,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
topic_id: 3,
highest_post_number: 15,
last_read_post_number: 14,
created_at: "2021-06-14T12:41:02.477Z",
category_id: 3,
notification_level: NotificationLevels.TRACKING,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
topic_id: 4,
highest_post_number: 17,
last_read_post_number: 16,
created_at: "2020-10-31T03:41:42.257Z",
category_id: 1234,
notification_level: NotificationLevels.TRACKING,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
tags: ["tag3"],
},
]);
await visit(`/c/3`);
assert.strictEqual(
query("#navigation-bar li.unread").textContent.trim(),
I18n.t("filters.unread.title_with_count", { count: 1 }),
"displays the right content on unread link"
);
assert.strictEqual(
query("#navigation-bar li.new").textContent.trim(),
I18n.t("filters.new.title"),
"displays the right content on new link"
);
await visit(`/c/3?f=tracked`);
assert.strictEqual(
query("#navigation-bar li.unread").textContent.trim(),
I18n.t("filters.unread.title"),
"displays the right content on unread link"
);
assert.strictEqual(
query("#navigation-bar li.new").textContent.trim(),
I18n.t("filters.new.title"),
"displays the right content on new link"
);
});
});

52
app/assets/javascripts/discourse/tests/fixtures/site-fixtures.js vendored
View File

@ -500,6 +500,58 @@ export default {
default_view: "latest",
subcategory_list_style: "boxes",
},
{
id: 1001,
name: "Parent Category",
color: "27AA5B",
text_color: "FFFFFF",
slug: "parent-category",
topic_count: 95,
post_count: 827,
description: "some description",
description_text: "some description",
topic_url: "/t/some-url",
read_restricted: false,
permission: 1,
parent_category_id: null,
notification_level: null,
background_url: null,
},
{
id: 1002,
name: "Sub Category",
color: "27AA5B",
text_color: "FFFFFF",
slug: "sub-category",
topic_count: 95,
post_count: 827,
description: "some description",
description_text: "some description",
topic_url: "/t/some-url",
read_restricted: false,
permission: 1,
parent_category_id: 1001,
notification_level: null,
background_url: null,
},
{
id: 1003,
name: "Sub Sub Category",
color: "27AA5B",
text_color: "FFFFFF",
slug: "sub-sub-category",
topic_count: 95,
post_count: 827,
description: "some description",
description_text: "some description",
topic_url: "/t/some-url",
read_restricted: false,
permission: 1,
parent_category_id: 1002,
notification_level: null,
background_url: null,
},
],
post_action_types: [
{

2
app/assets/javascripts/discourse/tests/integration/components/select-kit/category-chooser-test.js
View File

@ -107,7 +107,7 @@ discourseModule(
assert.strictEqual(
this.subject.rows().length,
22,
25,
"all categories are visible"
);

54
app/assets/javascripts/discourse/tests/unit/models/topic-tracking-state-test.js
View File

@ -1025,9 +1025,9 @@ discourseModule("Unit | Model | topic-tracking-state", function (hooks) {
const trackingState = TopicTrackingState.create({ currentUser });
assert.strictEqual(trackingState.countNew(1), 0);
assert.strictEqual(trackingState.countNew(2), 0);
assert.strictEqual(trackingState.countNew(3), 0);
assert.strictEqual(trackingState.countNew({ categoryId: 1 }), 0);
assert.strictEqual(trackingState.countNew({ categoryId: 2 }), 0);
assert.strictEqual(trackingState.countNew({ categoryId: 3 }), 0);
trackingState.states.set("t112", {
last_read_post_number: null,
@ -1037,11 +1037,18 @@ discourseModule("Unit | Model | topic-tracking-state", function (hooks) {
created_in_new_period: true,
});
assert.strictEqual(trackingState.countNew(1), 1);
assert.strictEqual(trackingState.countNew(1, undefined, true), 0);
assert.strictEqual(trackingState.countNew(1, "missing-tag"), 0);
assert.strictEqual(trackingState.countNew(2), 1);
assert.strictEqual(trackingState.countNew(3), 0);
assert.strictEqual(trackingState.countNew({ categoryId: 1 }), 1);
assert.strictEqual(
trackingState.countNew({ categoryId: 1, noSubcategories: true }),
0
);
assert.strictEqual(
trackingState.countNew({ categoryId: 1, tagId: "missing-tag" }),
0
);
assert.strictEqual(trackingState.countNew({ categoryId: 2 }), 1);
assert.strictEqual(trackingState.countNew({ categoryId: 3 }), 0);
trackingState.states.set("t113", {
last_read_post_number: null,
@ -1052,11 +1059,23 @@ discourseModule("Unit | Model | topic-tracking-state", function (hooks) {
created_in_new_period: true,
});
assert.strictEqual(trackingState.countNew(1), 2);
assert.strictEqual(trackingState.countNew(2), 2);
assert.strictEqual(trackingState.countNew(3), 1);
assert.strictEqual(trackingState.countNew(3, "amazing"), 1);
assert.strictEqual(trackingState.countNew(3, "missing"), 0);
assert.strictEqual(trackingState.countNew({ categoryId: 1 }), 2);
assert.strictEqual(trackingState.countNew({ categoryId: 2 }), 2);
assert.strictEqual(trackingState.countNew({ categoryId: 3 }), 1);
assert.strictEqual(
trackingState.countNew({
categoryId: 3,
tagId: "amazing",
}),
1
);
assert.strictEqual(
trackingState.countNew({
categoryId: 3,
tagId: "missing",
}),
0
);
trackingState.states.set("t111", {
last_read_post_number: null,
@ -1066,16 +1085,17 @@ discourseModule("Unit | Model | topic-tracking-state", function (hooks) {
created_in_new_period: true,
});
assert.strictEqual(trackingState.countNew(1), 3);
assert.strictEqual(trackingState.countNew(2), 2);
assert.strictEqual(trackingState.countNew(3), 1);
assert.strictEqual(trackingState.countNew({ categoryId: 1 }), 3);
assert.strictEqual(trackingState.countNew({ categoryId: 2 }), 2);
assert.strictEqual(trackingState.countNew({ categoryId: 3 }), 1);
trackingState.states.set("t115", {
last_read_post_number: null,
id: 115,
category_id: 4,
});
assert.strictEqual(trackingState.countNew(4), 0);
assert.strictEqual(trackingState.countNew({ categoryId: 4 }), 0);
});
test("mute and unmute topic", function (assert) {

3
lib/topic_query.rb
View File

@ -345,6 +345,9 @@ class TopicQuery
regular: TopicUser.notification_levels[:regular], tracking: TopicUser.notification_levels[:tracking])
end
# Any changes here will need to be reflected in `lib/topic-list-tracked-filter.js` for the `isTrackedTopic` function on
# the client side. The `f=tracked` query param is not heavily used so we do not want to be querying for a topic's
# tracked status by default. Instead, the client will handle the filtering when the `f=tracked` query params is present.
def self.tracked_filter(list, user_id)
tracked_category_ids_sql = <<~SQL
SELECT cd.category_id FROM category_users cd