From 0525455ef69f4482b0f7b1610c5ccade40f88dd7 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan <gxtan1990@gmail.com> Date: Fri, 15 Jul 2022 12:48:51 +0800 Subject: [PATCH] FIX: Unread count badge shown for topics that user is not tracking (#17506) When navigating to a topic and directly back to a topic list, an unread count may be shown for the topic even if the user is not tracking it. --- .../discourse/app/lib/topic-list-tracker.js | 25 ++++--- .../discourse/app/services/screen-track.js | 8 ++- .../acceptance/topic-list-tracker-test.js | 70 ++++++++++++++++++- 3 files changed, 90 insertions(+), 13 deletions(-) diff --git a/app/assets/javascripts/discourse/app/lib/topic-list-tracker.js b/app/assets/javascripts/discourse/app/lib/topic-list-tracker.js index 06f2888da10..8a88f6617e2 100644 --- a/app/assets/javascripts/discourse/app/lib/topic-list-tracker.js +++ b/app/assets/javascripts/discourse/app/lib/topic-list-tracker.js @@ -1,4 +1,6 @@ import { Promise } from "rsvp"; +import { NotificationLevels } from "discourse/lib/notification-levels"; + let model, currentTopicId; let lastTopicId, lastHighestRead; @@ -7,14 +9,21 @@ export function setTopicList(incomingModel) { model = incomingModel; model?.topics?.forEach((topic) => { - let highestRead = getHighestReadCache(topic.id); - if (highestRead && highestRead >= topic.last_read_post_number) { - let count = Math.max(topic.highest_post_number - highestRead, 0); - topic.setProperties({ - unread_posts: count, - new_posts: count, - }); - resetHighestReadCache(); + // Only update unread counts for tracked topics + + if (topic.notification_level >= NotificationLevels.TRACKING) { + const highestRead = getHighestReadCache(topic.id); + + if (highestRead && highestRead >= topic.last_read_post_number) { + const count = Math.max(topic.highest_post_number - highestRead, 0); + + topic.setProperties({ + unread_posts: count, + new_posts: count, + }); + + resetHighestReadCache(); + } } }); currentTopicId = null; diff --git a/app/assets/javascripts/discourse/app/services/screen-track.js b/app/assets/javascripts/discourse/app/services/screen-track.js index 9b2ae7f740f..ae799f26245 100644 --- a/app/assets/javascripts/discourse/app/services/screen-track.js +++ b/app/assets/javascripts/discourse/app/services/screen-track.js @@ -38,6 +38,7 @@ export default class ScreenTrack extends Service { start(topicId, topicController) { const currentTopicId = this._topicId; + if (currentTopicId && currentTopicId !== topicId) { this.tick(); this.flush(); @@ -277,9 +278,12 @@ export default class ScreenTrack extends Service { this.topicTrackingState.updateSeen(topicId, highestSeen); if (newTimingsKeys.length > 0) { - if (this.currentUser && !isTesting()) { + if (this.currentUser) { this.consolidateTimings(newTimings, this._topicTime, topicId); - this.sendNextConsolidatedTiming(); + + if (!isTesting()) { + this.sendNextConsolidatedTiming(); + } } else if (this._anonCallback) { // Anonymous viewer - save to localStorage const storage = this.keyValueStore; diff --git a/app/assets/javascripts/discourse/tests/acceptance/topic-list-tracker-test.js b/app/assets/javascripts/discourse/tests/acceptance/topic-list-tracker-test.js index 920b97de3d1..5f4de952209 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/topic-list-tracker-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/topic-list-tracker-test.js @@ -3,11 +3,45 @@ import { previousTopicUrl, setTopicId, } from "discourse/lib/topic-list-tracker"; -import { acceptance } from "discourse/tests/helpers/qunit-helpers"; +import { acceptance, exists } from "discourse/tests/helpers/qunit-helpers"; import { test } from "qunit"; -import { visit } from "@ember/test-helpers"; +import { click, visit } from "@ember/test-helpers"; +import topicFixtures from "discourse/tests/fixtures/topic"; +import discoveryFixtures from "discourse/tests/fixtures/discovery-fixtures"; +import { cloneJSON } from "discourse-common/lib/object"; +import { NotificationLevels } from "discourse/lib/notification-levels"; + +acceptance("Topic list tracking", function (needs) { + let notificationLevel; + + needs.hooks.afterEach(() => { + notificationLevel = null; + }); + + needs.user(); + + needs.pretender((server, helper) => { + server.get("/latest.json", () => { + const fixture = cloneJSON(discoveryFixtures["/latest.json"]); + + if (notificationLevel) { + fixture["topic_list"]["topics"].find((t) => { + if (t.id === 11557) { + t.notification_level = notificationLevel; + } + }); + } + + return helper.response(cloneJSON(fixture)); + }); + + server.get("/t/11557.json", () => { + const topicFixture = topicFixtures["/t/130.json"]; + topicFixture.id = 11557; + return helper.response(cloneJSON(topicFixture)); + }); + }); -acceptance("Topic list tracking", function () { test("Navigation", async function (assert) { await visit("/"); let url = await nextTopicUrl(); @@ -21,4 +55,34 @@ acceptance("Topic list tracking", function () { url = await previousTopicUrl(); assert.strictEqual(url, "/t/error-after-upgrade-to-0-9-7-9/11557"); }); + + test("unread count is set on topic that user is tracking", async function (assert) { + notificationLevel = NotificationLevels.TRACKING; + + await visit("/"); + + await click(".raw-topic-link[data-topic-id='11557']"); + + await visit("/"); + + assert.ok( + exists("tr[data-topic-id='11557'] .unread-posts"), + "unread count for topic is shown" + ); + }); + + test("unread count is not set on topic that user is not tracking", async function (assert) { + notificationLevel = NotificationLevels.REGULAR; + + await visit("/"); + + await click(".raw-topic-link[data-topic-id='11557']"); + + await visit("/"); + + assert.notOk( + exists("tr[data-topic-id='11557'] .unread-posts"), + "unread count for topic is not shown" + ); + }); });