From c0a086a9887d1a3b0e9b04ba761c91c3f685a387 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Thu, 16 Feb 2023 10:00:40 +1000 Subject: [PATCH] DEV: Move ChatThreadsManager to channel (#20304) This commit changes the ChatThreadsManager into a native class instead of an ember service, and initializes it for every ChatChannel model. This way each channel has its own thread manager and cache that we can load/unload as needed, and we also move activeThread to the channel since it makes more sense to keep it there, not inside the chat service. The pattern of calling setOwner with the passed in owner from ChatChannel is adapted from the latest ember docs, and is needed to avoid the error below when calling services from the native class: > Attempting to lookup an injected property on an object without a container, ensure that the object was instantiated via a container It works well _only_ if we use our own getOwner wrapper from addon/lib/get-owner, which is for backwards compat. c.f. https://guides.emberjs.com/release/in-depth-topics/native-classes-in-depth/ --- .../discourse/components/chat-thread.js | 2 +- .../{services => lib}/chat-threads-manager.js | 18 +++++++++--------- .../discourse/models/chat-channel.js | 5 +++++ .../discourse/routes/chat-channel-thread.js | 9 +++------ .../discourse/routes/chat-channel.js | 8 ++------ .../javascripts/discourse/services/chat-api.js | 4 ++-- .../javascripts/discourse/services/chat.js | 1 - 7 files changed, 22 insertions(+), 25 deletions(-) rename plugins/chat/assets/javascripts/discourse/{services => lib}/chat-threads-manager.js (81%) diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-thread.js b/plugins/chat/assets/javascripts/discourse/components/chat-thread.js index 26bf057456a..1c07f0c538b 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-thread.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-thread.js @@ -9,7 +9,7 @@ export default class ChatThreadPanel extends Component { @service router; get thread() { - return this.chat.activeThread; + return this.chat.activeChannel.activeThread; } get title() { diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-threads-manager.js b/plugins/chat/assets/javascripts/discourse/lib/chat-threads-manager.js similarity index 81% rename from plugins/chat/assets/javascripts/discourse/services/chat-threads-manager.js rename to plugins/chat/assets/javascripts/discourse/lib/chat-threads-manager.js index 4bcd79b8af6..a19aa7492e5 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-threads-manager.js +++ b/plugins/chat/assets/javascripts/discourse/lib/chat-threads-manager.js @@ -1,4 +1,5 @@ -import Service, { inject as service } from "@ember/service"; +import { inject as service } from "@ember/service"; +import { setOwner } from "@ember/application"; import Promise from "rsvp"; import ChatThread from "discourse/plugins/chat/discourse/models/chat-thread"; import { tracked } from "@glimmer/tracking"; @@ -6,19 +7,23 @@ import { TrackedObject } from "@ember-compat/tracked-built-ins"; import { popupAjaxError } from "discourse/lib/ajax-error"; /* - The ChatThreadsManager service is responsible for managing the loaded chat threads - for the current chat channel. + The ChatThreadsManager is responsible for managing the loaded chat threads + for a ChatChannel model. It provides helpers to facilitate using and managing loaded threads instead of constantly fetching them from the server. */ -export default class ChatThreadsManager extends Service { +export default class ChatThreadsManager { @service chatSubscriptionsManager; @service chatApi; @service currentUser; @tracked _cached = new TrackedObject(); + constructor(owner) { + setOwner(this, owner); + } + async find(channelId, threadId, options = { fetchIfNotFound: true }) { const existingThread = this.#findStale(threadId); if (existingThread) { @@ -30,11 +35,6 @@ export default class ChatThreadsManager extends Service { } } - // whenever the active channel changes, do this - resetCache() { - this._cached = new TrackedObject(); - } - get threads() { return Object.values(this._cached); } diff --git a/plugins/chat/assets/javascripts/discourse/models/chat-channel.js b/plugins/chat/assets/javascripts/discourse/models/chat-channel.js index 04a4e97a52d..c38b83d5650 100644 --- a/plugins/chat/assets/javascripts/discourse/models/chat-channel.js +++ b/plugins/chat/assets/javascripts/discourse/models/chat-channel.js @@ -6,6 +6,8 @@ import { ajax } from "discourse/lib/ajax"; import { escapeExpression } from "discourse/lib/utilities"; import { tracked } from "@glimmer/tracking"; import slugifyChannel from "discourse/plugins/chat/discourse/lib/slugify-channel"; +import ChatThreadsManager from "discourse/plugins/chat/discourse/lib/chat-threads-manager"; +import { getOwner } from "discourse-common/lib/get-owner"; export const CHATABLE_TYPES = { directMessageChannel: "DirectMessage", @@ -65,6 +67,9 @@ export default class ChatChannel extends RestModel { @tracked description; @tracked chatableType; @tracked status; + @tracked activeThread; + + threadsManager = new ChatThreadsManager(getOwner(this)); get escapedTitle() { return escapeExpression(this.title); diff --git a/plugins/chat/assets/javascripts/discourse/routes/chat-channel-thread.js b/plugins/chat/assets/javascripts/discourse/routes/chat-channel-thread.js index 8621dc311da..3aa4e2b8cea 100644 --- a/plugins/chat/assets/javascripts/discourse/routes/chat-channel-thread.js +++ b/plugins/chat/assets/javascripts/discourse/routes/chat-channel-thread.js @@ -3,19 +3,16 @@ import { inject as service } from "@ember/service"; export default class ChatChannelThread extends DiscourseRoute { @service router; - @service chatThreadsManager; @service chatStateManager; @service chat; async model(params) { - return this.chatThreadsManager.find( - this.modelFor("chat.channel").id, - params.threadId - ); + const channel = this.modelFor("chat.channel"); + return channel.threadsManager.find(channel.id, params.threadId); } afterModel(model) { - this.chat.activeThread = model; + this.chat.activeChannel.activeThread = model; this.chatStateManager.openSidePanel(); } } diff --git a/plugins/chat/assets/javascripts/discourse/routes/chat-channel.js b/plugins/chat/assets/javascripts/discourse/routes/chat-channel.js index e34f57c9ff1..69ea1c3b3f8 100644 --- a/plugins/chat/assets/javascripts/discourse/routes/chat-channel.js +++ b/plugins/chat/assets/javascripts/discourse/routes/chat-channel.js @@ -5,12 +5,12 @@ import { action } from "@ember/object"; @withChatChannel export default class ChatChannelRoute extends DiscourseRoute { - @service chatThreadsManager; + @service chat; @service chatStateManager; @action willTransition(transition) { - this.chat.activeThread = null; + this.chat.activeChannel.activeThread = null; this.chatStateManager.closeSidePanel(); if (!transition?.to?.name?.startsWith("chat.")) { @@ -19,8 +19,4 @@ export default class ChatChannelRoute extends DiscourseRoute { this.chat.updatePresence(); } } - - beforeModel() { - this.chatThreadsManager.resetCache(); - } } diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-api.js b/plugins/chat/assets/javascripts/discourse/services/chat-api.js index c379bc1ff64..35e680031b3 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-api.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-api.js @@ -10,8 +10,8 @@ import Collection from "../lib/collection"; * @implements {@ember/service} */ export default class ChatApi extends Service { + @service chat; @service chatChannelsManager; - @service chatThreadsManager; /** * Get a channel by its ID. @@ -40,7 +40,7 @@ export default class ChatApi extends Service { */ thread(channelId, threadId) { return this.#getRequest(`/channels/${channelId}/threads/${threadId}`).then( - (result) => this.chatThreadsManager.store(result.thread) + (result) => this.chat.activeChannel.threadsManager.store(result.thread) ); } diff --git a/plugins/chat/assets/javascripts/discourse/services/chat.js b/plugins/chat/assets/javascripts/discourse/services/chat.js index d5d1cbcd74e..e500e84f754 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat.js @@ -36,7 +36,6 @@ export default class Chat extends Service { @service site; @service chatChannelsManager; @tracked activeChannel = null; - @tracked activeThread = null; cook = null; presenceChannel = null; sidebarActive = false;