From 908add79de5248fe7e1c57f68788cabc0d5906c9 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Fri, 30 Dec 2022 15:30:36 +0100 Subject: [PATCH] DEV: moves channel-archive-status to channels subscriptions (#19567) It was quite an oddball because its a global subscription created on each channel. channels manager now allows us to elegantly solve this case. --- .../serializers/chat_channel_serializer.rb | 1 - .../structured_channel_serializer.rb | 3 ++ plugins/chat/app/services/chat_publisher.rb | 4 +- .../components/chat-channel-archive-status.js | 38 +---------------- .../services/chat-channels-manager.js | 20 ++++++--- .../services/chat-subscriptions-manager.js | 41 +++++++++++++++++++ .../api/chat_current_user_channels_spec.rb | 1 + 7 files changed, 63 insertions(+), 45 deletions(-) diff --git a/plugins/chat/app/serializers/chat_channel_serializer.rb b/plugins/chat/app/serializers/chat_channel_serializer.rb index 26f8da8fc04..b720d691f2f 100644 --- a/plugins/chat/app/serializers/chat_channel_serializer.rb +++ b/plugins/chat/app/serializers/chat_channel_serializer.rb @@ -111,7 +111,6 @@ class ChatChannelSerializer < ApplicationSerializer new_mentions: @opts[:new_mentions_message_bus_last_id] || MessageBus.last_id(ChatPublisher.new_mentions_message_bus_channel(object.id)), - archive_status: MessageBus.last_id("/chat/channel-archive-status"), }, } end diff --git a/plugins/chat/app/serializers/structured_channel_serializer.rb b/plugins/chat/app/serializers/structured_channel_serializer.rb index a197c36c62e..e88b9a00b5d 100644 --- a/plugins/chat/app/serializers/structured_channel_serializer.rb +++ b/plugins/chat/app/serializers/structured_channel_serializer.rb @@ -45,6 +45,8 @@ class StructuredChannelSerializer < ApplicationSerializer channel_edits: chat_message_bus_last_ids[ChatPublisher::CHANNEL_EDITS_MESSAGE_BUS_CHANNEL], channel_status: chat_message_bus_last_ids[ChatPublisher::CHANNEL_STATUS_MESSAGE_BUS_CHANNEL], new_channel: chat_message_bus_last_ids[ChatPublisher::NEW_CHANNEL_MESSAGE_BUS_CHANNEL], + archive_status: + chat_message_bus_last_ids[ChatPublisher::CHANNEL_ARCHIVE_STATUS_MESSAGE_BUS_CHANNEL], } if id = @@ -67,6 +69,7 @@ class StructuredChannelSerializer < ApplicationSerializer ChatPublisher::CHANNEL_EDITS_MESSAGE_BUS_CHANNEL, ChatPublisher::CHANNEL_STATUS_MESSAGE_BUS_CHANNEL, ChatPublisher::NEW_CHANNEL_MESSAGE_BUS_CHANNEL, + ChatPublisher::CHANNEL_ARCHIVE_STATUS_MESSAGE_BUS_CHANNEL, ] if !scope.anonymous? diff --git a/plugins/chat/app/services/chat_publisher.rb b/plugins/chat/app/services/chat_publisher.rb index 6dff51e6566..f628cf5b674 100644 --- a/plugins/chat/app/services/chat_publisher.rb +++ b/plugins/chat/app/services/chat_publisher.rb @@ -230,6 +230,8 @@ module ChatPublisher ) end + CHANNEL_ARCHIVE_STATUS_MESSAGE_BUS_CHANNEL = "/chat/channel-archive-status" + def self.publish_archive_status( chat_channel, archive_status:, @@ -238,7 +240,7 @@ module ChatPublisher total_messages: ) MessageBus.publish( - "/chat/channel-archive-status", + CHANNEL_ARCHIVE_STATUS_MESSAGE_BUS_CHANNEL, { chat_channel_id: chat_channel.id, archive_failed: archive_status == :failed, diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-channel-archive-status.js b/plugins/chat/assets/javascripts/discourse/components/chat-channel-archive-status.js index 7f76393cdcc..94d27ed5a66 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-channel-archive-status.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-channel-archive-status.js @@ -4,7 +4,7 @@ import I18n from "I18n"; import { popupAjaxError } from "discourse/lib/ajax-error"; import getURL from "discourse-common/lib/get-url"; import { action } from "@ember/object"; -import discourseComputed, { bind } from "discourse-common/utils/decorators"; +import discourseComputed from "discourse-common/utils/decorators"; import { inject as service } from "@ember/service"; export default Component.extend({ @@ -49,43 +49,7 @@ export default Component.extend({ .catch(popupAjaxError); }, - didInsertElement() { - this._super(...arguments); - - if (this.currentUser.admin) { - this.messageBus.subscribe( - "/chat/channel-archive-status", - this.onMessage, - this.channel.meta.message_bus_last_ids.archive_status - ); - } - }, - - willDestroyElement() { - this._super(...arguments); - - if (this.currentUser.admin) { - this.messageBus.unsubscribe( - "/chat/channel-archive-status", - this.onMessage - ); - } - }, - _getTopicUrl() { return getURL(`/t/-/${this.channel.archive_topic_id}`); }, - - @bind - onMessage(busData) { - if (busData.chat_channel_id === this.channel.id) { - this.channel.setProperties({ - archive_failed: busData.archive_failed, - archive_completed: busData.archive_completed, - archived_messages: busData.archived_messages, - archive_topic_id: busData.archive_topic_id, - total_messages: busData.total_messages, - }); - } - }, }); diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js index ab556c1cf6e..1a73752a07c 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js @@ -7,25 +7,33 @@ import { popupAjaxError } from "discourse/lib/ajax-error"; const DIRECT_MESSAGE_CHANNELS_LIMIT = 20; +/* + The ChatChannelsManager service is responsible for managing the loaded chat channels. + It provides helpers to facilitate using and managing laoded channels instead of constantly + fetching them from the server. +*/ + export default class ChatChannelsManager extends Service { @service chatSubscriptionsManager; @service chatApi; @service currentUser; @tracked _cached = new TrackedObject(); - get channels() { - return Object.values(this._cached); - } - - async find(id) { + async find(id, options = { fetchIfNotFound: true }) { const existingChannel = this.#findStale(id); if (existingChannel) { return Promise.resolve(existingChannel); - } else { + } else if (options.fetchIfNotFound) { return this.#find(id); + } else { + return Promise.resolve(); } } + get channels() { + return Object.values(this._cached); + } + store(channelObject) { let model = this.#findStale(channelObject.id); diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-subscriptions-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-subscriptions-manager.js index 8feb3edd217..4b58c3153b9 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-subscriptions-manager.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-subscriptions-manager.js @@ -44,6 +44,7 @@ export default class ChatSubscriptionsManager extends Service { startChannelsSubscriptions(messageBusIds) { this._startNewChannelSubscription(messageBusIds.new_channel); + this._startChannelArchiveStatusSubscription(messageBusIds.archive_status); this._startUserTrackingStateSubscription(messageBusIds.user_tracking_state); this._startChannelsEditsSubscription(messageBusIds.channel_edits); this._startChannelsStatusChangesSubscription(messageBusIds.channel_status); @@ -54,6 +55,7 @@ export default class ChatSubscriptionsManager extends Service { stopChannelsSubscriptions() { this._stopNewChannelSubscription(); + this._stopChannelArchiveStatusSubscription(); this._stopUserTrackingStateSubscription(); this._stopChannelsEditsSubscription(); this._stopChannelsStatusChangesSubscription(); @@ -64,6 +66,25 @@ export default class ChatSubscriptionsManager extends Service { }); } + _startChannelArchiveStatusSubscription(lastId) { + if (this.currentUser.admin) { + this.messageBus.subscribe( + "/chat/channel-archive-status", + this._onChannelArchiveStatusUpdate, + lastId + ); + } + } + + _stopChannelArchiveStatusSubscription() { + if (this.currentUser.admin) { + this.messageBus.unsubscribe( + "/chat/channel-archive-status", + this._onChannelArchiveStatusUpdate + ); + } + } + _startChannelMentionsSubscription(channel) { this.messageBus.subscribe( `/chat/${channel.id}/new-mentions`, @@ -72,6 +93,26 @@ export default class ChatSubscriptionsManager extends Service { ); } + @bind + _onChannelArchiveStatusUpdate(busData) { + // we don't want to fetch a channel we don't have locally because archive status changed + this.chatChannelsManager + .find(busData.chat_channel_id, { fetchIfNotFound: false }) + .then((channel) => { + if (!channel) { + return; + } + + channel.setProperties({ + archive_failed: busData.archive_failed, + archive_completed: busData.archive_completed, + archived_messages: busData.archived_messages, + archive_topic_id: busData.archive_topic_id, + total_messages: busData.total_messages, + }); + }); + } + @bind _onNewMentions(busData) { this.chatChannelsManager.find(busData.channel_id).then((channel) => { diff --git a/plugins/chat/spec/requests/api/chat_current_user_channels_spec.rb b/plugins/chat/spec/requests/api/chat_current_user_channels_spec.rb index 6b7a21bb264..c1b2327c2a1 100644 --- a/plugins/chat/spec/requests/api/chat_current_user_channels_spec.rb +++ b/plugins/chat/spec/requests/api/chat_current_user_channels_spec.rb @@ -90,6 +90,7 @@ describe Chat::Api::ChatCurrentUserChannelsController do expect(ids["channel_edits"]).not_to eq(nil) expect(ids["channel_status"]).not_to eq(nil) expect(ids["new_channel"]).not_to eq(nil) + expect(ids["archive_status"]).not_to eq(nil) end response.parsed_body["public_channels"][0]["meta"]["message_bus_last_ids"].tap do |ids|