From da77d06ebb12e07c928315e3a01850956cf623c6 Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Tue, 8 Oct 2024 21:13:40 +0200 Subject: [PATCH] DEV: Update internal tracking in pm/topic tracking state (#29120) --- .../app/components/category-list-item.js | 6 +- .../user-private-messages-group.js | 16 +-- .../controllers/user-private-messages-user.js | 15 +-- .../app/controllers/user-topics-list.js | 10 +- .../discourse/app/models/nav-item.js | 22 ++-- .../app/models/topic-tracking-state.js | 105 +++++++++--------- .../build-private-messages-group-route.js | 7 +- .../app/services/pm-topic-tracking-state.js | 63 +++++------ .../acceptance/user-private-messages-test.js | 10 ++ .../tests/unit/models/nav-item-test.js | 2 +- 10 files changed, 120 insertions(+), 136 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/category-list-item.js b/app/assets/javascripts/discourse/app/components/category-list-item.js index 2aef3d62ce4..027869e7474 100644 --- a/app/assets/javascripts/discourse/app/components/category-list-item.js +++ b/app/assets/javascripts/discourse/app/components/category-list-item.js @@ -28,13 +28,11 @@ export default class CategoryListItem extends Component { ); } - @discourseComputed("topicTrackingState.messageCount") - unreadTopicsCount() { + get unreadTopicsCount() { return this.category.unreadTopicsCount; } - @discourseComputed("topicTrackingState.messageCount") - newTopicsCount() { + get newTopicsCount() { return this.category.newTopicsCount; } diff --git a/app/assets/javascripts/discourse/app/controllers/user-private-messages-group.js b/app/assets/javascripts/discourse/app/controllers/user-private-messages-group.js index f89c758d969..382f7a90b2b 100644 --- a/app/assets/javascripts/discourse/app/controllers/user-private-messages-group.js +++ b/app/assets/javascripts/discourse/app/controllers/user-private-messages-group.js @@ -1,28 +1,20 @@ import Controller, { inject as controller } from "@ember/controller"; -import { action, computed } from "@ember/object"; +import { action } from "@ember/object"; +import { service } from "@ember/service"; import I18n from "discourse-i18n"; export default class extends Controller { + @service pmTopicTrackingState; @controller user; get viewingSelf() { - return this.user.viewingSelf; + return this.user.get("viewingSelf"); } - @computed( - "pmTopicTrackingState.newIncoming.[]", - "pmTopicTrackingState.statesModificationCounter", - "pmTopicTrackingState.isTracking" - ) get newLinkText() { return this.#linkText("new"); } - @computed( - "pmTopicTrackingState.newIncoming.[]", - "pmTopicTrackingState.statesModificationCounter", - "pmTopicTrackingState.isTracking" - ) get unreadLinkText() { return this.#linkText("unread"); } diff --git a/app/assets/javascripts/discourse/app/controllers/user-private-messages-user.js b/app/assets/javascripts/discourse/app/controllers/user-private-messages-user.js index 4c53b9930f2..7a7b69933a8 100644 --- a/app/assets/javascripts/discourse/app/controllers/user-private-messages-user.js +++ b/app/assets/javascripts/discourse/app/controllers/user-private-messages-user.js @@ -1,17 +1,16 @@ import Controller, { inject as controller } from "@ember/controller"; -import { computed } from "@ember/object"; import { service } from "@ember/service"; import I18n from "discourse-i18n"; export default class extends Controller { + @service currentUser; @service router; @controller user; get viewingSelf() { - return this.user.viewingSelf; + return this.user.get("viewingSelf"); } - @computed("viewingSelf", "router.currentRoute.name", "currentUser.admin") get showWarningsWarning() { return ( this.router.currentRoute.name === "userPrivateMessages.user.warnings" && @@ -20,20 +19,10 @@ export default class extends Controller { ); } - @computed( - "pmTopicTrackingState.newIncoming.[]", - "pmTopicTrackingState.statesModificationCounter", - "pmTopicTrackingState.isTracking" - ) get newLinkText() { return this.#linkText("new"); } - @computed( - "pmTopicTrackingState.newIncoming.[]", - "pmTopicTrackingState.statesModificationCounter", - "pmTopicTrackingState.isTracking" - ) get unreadLinkText() { return this.#linkText("unread"); } diff --git a/app/assets/javascripts/discourse/app/controllers/user-topics-list.js b/app/assets/javascripts/discourse/app/controllers/user-topics-list.js index acf002909b2..91e28395200 100644 --- a/app/assets/javascripts/discourse/app/controllers/user-topics-list.js +++ b/app/assets/javascripts/discourse/app/controllers/user-topics-list.js @@ -1,7 +1,8 @@ import { tracked } from "@glimmer/tracking"; import Controller from "@ember/controller"; import { action } from "@ember/object"; -import { or, reads } from "@ember/object/computed"; +import { dependentKeyCompat } from "@ember/object/compat"; +import { or } from "@ember/object/computed"; import { isNone } from "@ember/utils"; import { popupAjaxError } from "discourse/lib/ajax-error"; import BulkSelectHelper from "discourse/lib/bulk-select-helper"; @@ -26,8 +27,6 @@ export default class UserTopicsListController extends Controller { bulkSelectHelper = new BulkSelectHelper(this); - @reads("pmTopicTrackingState.newIncoming.length") incomingCount; - @or("currentUser.canManageTopic", "showDismissRead", "showResetNew") canBulkSelect; @@ -39,6 +38,11 @@ export default class UserTopicsListController extends Controller { } } + @dependentKeyCompat + get incomingCount() { + return this.pmTopicTrackingState.newIncoming.length; + } + get bulkSelectEnabled() { return this.bulkSelectHelper.bulkSelectEnabled; } diff --git a/app/assets/javascripts/discourse/app/models/nav-item.js b/app/assets/javascripts/discourse/app/models/nav-item.js index 83b9206b087..665866b478f 100644 --- a/app/assets/javascripts/discourse/app/models/nav-item.js +++ b/app/assets/javascripts/discourse/app/models/nav-item.js @@ -307,19 +307,15 @@ export default class NavItem extends EmberObject { "topicTrackingState.messageCount" ) count(name, category, tagId, noSubcategories, currentRouteQueryParams) { - const state = this.topicTrackingState; - - if (state) { - return state.lookupCount({ - type: name, - category, - tagId, - noSubcategories, - customFilterFn: hasTrackedFilter(currentRouteQueryParams) - ? isTrackedTopic - : undefined, - }); - } + return this.topicTrackingState?.lookupCount({ + type: name, + category, + tagId, + noSubcategories, + customFilterFn: hasTrackedFilter(currentRouteQueryParams) + ? isTrackedTopic + : undefined, + }); } } 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 a89f0d2504b..b1a9284bf7a 100644 --- a/app/assets/javascripts/discourse/app/models/topic-tracking-state.js +++ b/app/assets/javascripts/discourse/app/models/topic-tracking-state.js @@ -1,13 +1,15 @@ +import { tracked } from "@glimmer/tracking"; import EmberObject, { get } from "@ember/object"; import { service } from "@ember/service"; import { isEmpty } from "@ember/utils"; +import { TrackedArray, TrackedMap } from "@ember-compat/tracked-built-ins"; import { NotificationLevels } from "discourse/lib/notification-levels"; import PreloadStore from "discourse/lib/preload-store"; import DiscourseURL from "discourse/lib/url"; import Category from "discourse/models/category"; import Site from "discourse/models/site"; import { deepEqual, deepMerge } from "discourse-common/lib/object"; -import discourseComputed, { bind } from "discourse-common/utils/decorators"; +import { bind } from "discourse-common/utils/decorators"; function isNew(topic) { return ( @@ -52,15 +54,15 @@ export default class TopicTrackingState extends EmberObject { @service messageBus; @service siteSettings; - messageCount = 0; - - init() { - super.init(...arguments); - - this.states = new Map(); - this.stateChangeCallbacks = {}; - this._trackedTopicLimit = 4000; - } + @tracked messageCount = 0; + @tracked incomingCount = 0; + @tracked newIncoming; + @tracked filterCategory; + @tracked filterTag; + @tracked filter; + states = new TrackedMap(); + stateChangeCallbacks = {}; + _trackedTopicLimit = 4000; willDestroy() { super.willDestroy(...arguments); @@ -143,18 +145,18 @@ export default class TopicTrackingState extends EmberObject { @bind onDeleteMessage(msg) { this.modifyStateProp(msg, "deleted", true); - this.incrementMessageCount(); + this.messageCount++; } @bind onRecoverMessage(msg) { this.modifyStateProp(msg, "deleted", false); - this.incrementMessageCount(); + this.messageCount++; } @bind onDestroyMessage(msg) { - this.incrementMessageCount(); + this.messageCount++; const currentRoute = DiscourseURL.router.currentRoute.parent; if ( @@ -165,49 +167,50 @@ export default class TopicTrackingState extends EmberObject { } } - mutedTopics() { - return (this.currentUser && this.currentUser.muted_topics) || []; + get mutedTopics() { + return this.currentUser?.muted_topics || []; } - unmutedTopics() { - return (this.currentUser && this.currentUser.unmuted_topics) || []; + get unmutedTopics() { + return this.currentUser?.unmuted_topics || []; } trackMutedOrUnmutedTopic(data) { let topics, key; if (data.message_type === "muted") { key = "muted_topics"; - topics = this.mutedTopics(); + topics = this.mutedTopics; } else { key = "unmuted_topics"; - topics = this.unmutedTopics(); + topics = this.unmutedTopics; } + topics = topics.concat({ topicId: data.topic_id, createdAt: Date.now(), }); - this.currentUser && this.currentUser.set(key, topics); + this.currentUser?.set(key, topics); } pruneOldMutedAndUnmutedTopics() { const now = Date.now(); - let mutedTopics = this.mutedTopics().filter( + let mutedTopics = this.mutedTopics.filter( (mutedTopic) => now - mutedTopic.createdAt < 60000 ); - let unmutedTopics = this.unmutedTopics().filter( + let unmutedTopics = this.unmutedTopics.filter( (unmutedTopic) => now - unmutedTopic.createdAt < 60000 ); - this.currentUser && - this.currentUser.set("muted_topics", mutedTopics) && - this.currentUser.set("unmuted_topics", unmutedTopics); + + this.currentUser?.set("muted_topics", mutedTopics); + this.currentUser?.set("unmuted_topics", unmutedTopics); } isMutedTopic(topicId) { - return !!this.mutedTopics().findBy("topicId", topicId); + return !!this.mutedTopics.findBy("topicId", topicId); } isUnmutedTopic(topicId) { - return !!this.unmutedTopics().findBy("topicId", topicId); + return !!this.unmutedTopics.findBy("topicId", topicId); } /** @@ -235,7 +238,7 @@ export default class TopicTrackingState extends EmberObject { state.last_read_post_number < highestSeen ) { this.modifyStateProp(topicId, "last_read_post_number", highestSeen); - this.incrementMessageCount(); + this.messageCount++; } } @@ -324,7 +327,7 @@ export default class TopicTrackingState extends EmberObject { } // hasIncoming relies on this count - this.set("incomingCount", this.newIncoming.length); + this.incomingCount = this.newIncoming.length; } /** @@ -335,8 +338,8 @@ export default class TopicTrackingState extends EmberObject { * @method resetTracking */ resetTracking() { - this.newIncoming = []; - this.set("incomingCount", 0); + this.newIncoming = new TrackedArray(); + this.incomingCount = 0; } /** @@ -346,10 +349,10 @@ export default class TopicTrackingState extends EmberObject { */ clearIncoming(topicIds) { const toRemove = new Set(topicIds); - this.newIncoming = this.newIncoming.filter( - (topicId) => !toRemove.has(topicId) + this.newIncoming = new TrackedArray( + this.newIncoming.filter((topicId) => !toRemove.has(topicId)) ); - this.set("incomingCount", this.newIncoming.length); + this.incomingCount = this.newIncoming.length; } /** @@ -366,7 +369,7 @@ export default class TopicTrackingState extends EmberObject { * c/cat/sub-cat/6/l/latest or tags/c/cat/sub-cat/6/test/l/latest. */ trackIncoming(filter) { - this.newIncoming = []; + this.newIncoming = new TrackedArray(); let category, tag; @@ -388,21 +391,20 @@ export default class TopicTrackingState extends EmberObject { tag = split[1]; } - this.set("filterCategory", category); - this.set("filterTag", tag); - this.set("filter", filter); - this.set("incomingCount", 0); + this.filterCategory = category; + this.filterTag = tag; + this.filter = filter; + this.incomingCount = 0; } /** * Used to determine whether to show the message at the top of the topic list * e.g. "see 1 new or updated topic" * - * @method incomingCount + * @method hasIncoming */ - @discourseComputed("incomingCount") - hasIncoming(incomingCount) { - return incomingCount && incomingCount > 0; + get hasIncoming() { + return this.incomingCount > 0; } /** @@ -430,7 +432,7 @@ export default class TopicTrackingState extends EmberObject { */ removeTopics(topicIds) { topicIds.forEach((topicId) => this.removeTopic(topicId)); - this.incrementMessageCount(); + this.messageCount++; this._afterStateChange(); } @@ -525,11 +527,7 @@ export default class TopicTrackingState extends EmberObject { this._correctMissingState(list, filter); } - this.incrementMessageCount(); - } - - incrementMessageCount() { - this.incrementProperty("messageCount"); + this.messageCount++; } _generateCallbackId() { @@ -1029,7 +1027,7 @@ export default class TopicTrackingState extends EmberObject { if (old.tags !== data.payload.tags) { this.modifyStateProp(data, "tags", data.payload.tags); - this.incrementMessageCount(); + this.messageCount++; } } @@ -1071,7 +1069,7 @@ export default class TopicTrackingState extends EmberObject { } this.modifyState(data, payload); - this.incrementMessageCount(); + this.messageCount++; } } } @@ -1081,7 +1079,7 @@ export default class TopicTrackingState extends EmberObject { this.modifyStateProp(topicId, "is_seen", true); }); - this.incrementMessageCount(); + this.messageCount++; } _dismissNewPosts(topicIds) { @@ -1097,7 +1095,7 @@ export default class TopicTrackingState extends EmberObject { } }); - this.incrementMessageCount(); + this.messageCount++; } _addIncoming(topicId) { @@ -1129,7 +1127,6 @@ export default class TopicTrackingState extends EmberObject { } _afterStateChange() { - this.notifyPropertyChange("states"); Object.values(this.stateChangeCallbacks).forEach((cb) => cb()); } diff --git a/app/assets/javascripts/discourse/app/routes/build-private-messages-group-route.js b/app/assets/javascripts/discourse/app/routes/build-private-messages-group-route.js index d9164e8a3ce..d141f655fdc 100644 --- a/app/assets/javascripts/discourse/app/routes/build-private-messages-group-route.js +++ b/app/assets/javascripts/discourse/app/routes/build-private-messages-group-route.js @@ -1,3 +1,4 @@ +import { getOwner } from "@ember/owner"; import { capitalize } from "@ember/string"; import { findOrResetCachedTopicList } from "discourse/lib/cached-topic-list"; import createPMRoute from "discourse/routes/build-private-messages-route"; @@ -80,10 +81,10 @@ export default (inboxType, filter) => { const userTopicsListController = this.controllerFor("user-topics-list"); userTopicsListController.set("group", this.group); - userTopicsListController.set( - "pmTopicTrackingState.activeGroup", - this.group + const pmTopicTrackingState = getOwner(this).lookup( + "service:pm-topic-tracking-state" ); + pmTopicTrackingState.activeGroup = this.group; this.controllerFor("user-private-messages").set("group", this.group); } diff --git a/app/assets/javascripts/discourse/app/services/pm-topic-tracking-state.js b/app/assets/javascripts/discourse/app/services/pm-topic-tracking-state.js index c07bbba5a0a..ea5e4d0bd3b 100644 --- a/app/assets/javascripts/discourse/app/services/pm-topic-tracking-state.js +++ b/app/assets/javascripts/discourse/app/services/pm-topic-tracking-state.js @@ -1,4 +1,6 @@ -import Service from "@ember/service"; +import { tracked } from "@glimmer/tracking"; +import Service, { service } from "@ember/service"; +import { TrackedArray, TrackedMap } from "@ember-compat/tracked-built-ins"; import { Promise } from "rsvp"; import { ajax } from "discourse/lib/ajax"; import { popupAjaxError } from "discourse/lib/ajax-error"; @@ -12,22 +14,22 @@ import { import { deepEqual, deepMerge } from "discourse-common/lib/object"; import { bind } from "discourse-common/utils/decorators"; +const CHANNEL_PREFIX = "/private-message-topic-tracking-state"; + // See private_message_topic_tracking_state.rb for documentation class PrivateMessageTopicTrackingState extends Service { - CHANNEL_PREFIX = "/private-message-topic-tracking-state"; - inbox = null; - filter = null; - activeGroup = null; + @service currentUser; + @service messageBus; - init() { - super.init(...arguments); - - this.states = new Map(); - this.statesModificationCounter = 0; - this.isTracking = false; - this.newIncoming = []; - this.stateChangeCallbacks = new Map(); - } + @tracked isTracking = false; + @tracked isTrackingIncoming = false; + @tracked statesModificationCounter = 0; + @tracked inbox = null; + @tracked filter = null; + @tracked activeGroup = null; + @tracked newIncoming = new TrackedArray(); + states = new TrackedMap(); + stateChangeCallbacks = new Map(); willDestroy() { super.willDestroy(...arguments); @@ -62,7 +64,7 @@ class PrivateMessageTopicTrackingState extends Service { }); return this._loadInitialState().finally(() => { - this.set("isTracking", true); + this.isTracking = true; }); } @@ -82,13 +84,11 @@ class PrivateMessageTopicTrackingState extends Service { }).length; } - trackIncoming(inbox, filter, group) { - this.setProperties({ - inbox, - filter, - activeGroup: group, - isTrackingIncoming: true, - }); + trackIncoming(inbox, filter, activeGroup) { + this.inbox = inbox; + this.filter = filter; + this.activeGroup = activeGroup; + this.isTrackingIncoming = true; } resetIncomingTracking(topicIds) { @@ -98,21 +98,18 @@ class PrivateMessageTopicTrackingState extends Service { if (topicIds) { const topicIdSet = new Set(topicIds); - this.set( - "newIncoming", + this.newIncoming = new TrackedArray( this.newIncoming.filter((id) => !topicIdSet.has(id)) ); } else { - this.set("newIncoming", []); + this.newIncoming = new TrackedArray(); } } stopIncomingTracking() { if (this.isTrackingIncoming) { - this.setProperties({ - isTrackingIncoming: false, - newIncoming: [], - }); + this.isTrackingIncoming = false; + this.newIncoming = new TrackedArray(); } } @@ -130,11 +127,11 @@ class PrivateMessageTopicTrackingState extends Service { } userChannel() { - return `${this.CHANNEL_PREFIX}/user/${this.currentUser.id}`; + return `${CHANNEL_PREFIX}/user/${this.currentUser.id}`; } groupChannel(groupId) { - return `${this.CHANNEL_PREFIX}/group/${groupId}`; + return `${CHANNEL_PREFIX}/group/${groupId}`; } _isNew(topic) { @@ -248,7 +245,7 @@ class PrivateMessageTopicTrackingState extends Service { _notifyIncoming(topicId) { if (this.isTrackingIncoming && !this.newIncoming.includes(topicId)) { - this.newIncoming.pushObject(topicId); + this.newIncoming.push(topicId); } } @@ -280,7 +277,7 @@ class PrivateMessageTopicTrackingState extends Service { } _afterStateChange() { - this.incrementProperty("statesModificationCounter"); + this.statesModificationCounter++; this.stateChangeCallbacks.forEach((callback) => callback()); } } diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-private-messages-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-private-messages-test.js index f22b484b8eb..9e77e852f1a 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/user-private-messages-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/user-private-messages-test.js @@ -393,6 +393,16 @@ acceptance( ); assert.ok(exists(".show-mores"), "displays the topic incoming info"); + + await publishNewToMessageBus({ topicId: 2 }); + + assert.strictEqual( + query(".messages-nav .user-nav__messages-new").innerText.trim(), + I18n.t("user.messages.new_with_count", { count: 2 }), + "displays the right count" + ); + + assert.ok(exists(".show-mores"), "displays the topic incoming info"); }); test("incoming unread messages while viewing unread", async function (assert) { diff --git a/app/assets/javascripts/discourse/tests/unit/models/nav-item-test.js b/app/assets/javascripts/discourse/tests/unit/models/nav-item-test.js index c00eb588d85..c8a6d0f01eb 100644 --- a/app/assets/javascripts/discourse/tests/unit/models/nav-item-test.js +++ b/app/assets/javascripts/discourse/tests/unit/models/nav-item-test.js @@ -44,7 +44,7 @@ module("Unit | Model | nav-item", function (hooks) { last_read_post_number: null, created_in_new_period: true, }); - navItem.topicTrackingState.incrementMessageCount(); + navItem.topicTrackingState.messageCount++; assert.strictEqual( navItem.count,