From f0ea57e30eaf1e8babf7ac124f1faba7487b6b2f Mon Sep 17 00:00:00 2001
From: David Battersby <info@davidbattersby.com>
Date: Mon, 9 Dec 2024 15:36:41 +0400
Subject: [PATCH] FIX: chat channel sort order consistency in sidebar (#30180)

This change creates a shallow copy of the public message channels so we don't change the original array while sorting.

Without this change the publicMessageChannels getter cache gets invalidated and cached again with the sorted channels array instead, which causes a bug where the sidebar channel list sorting order is updated by activity when user interacts with the chat drawer.
---
 .../components/channels-list-public.gjs       |  4 +---
 .../chat/drawer-routes/channels.gjs           |  2 +-
 .../components/chat/routes/channels.gjs       |  2 +-
 .../services/chat-channels-manager.js         |  2 +-
 .../spec/system/list_channels/sidebar_spec.rb | 19 +++++++++++++++++++
 5 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/plugins/chat/assets/javascripts/discourse/components/channels-list-public.gjs b/plugins/chat/assets/javascripts/discourse/components/channels-list-public.gjs
index 8459d28d63e..f2886e3d3e0 100644
--- a/plugins/chat/assets/javascripts/discourse/components/channels-list-public.gjs
+++ b/plugins/chat/assets/javascripts/discourse/components/channels-list-public.gjs
@@ -34,9 +34,7 @@ export default class ChannelsListPublic extends Component {
   }
 
   get channelList() {
-    return this.args.sortByActivity === true
-      ? this.chatChannelsManager.publicMessageChannelsByActivity
-      : this.chatChannelsManager.publicMessageChannels;
+    return this.chatChannelsManager.publicMessageChannelsByActivity;
   }
 
   @action
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/drawer-routes/channels.gjs b/plugins/chat/assets/javascripts/discourse/components/chat/drawer-routes/channels.gjs
index 34c5ac63c22..b66bf1a8b8c 100644
--- a/plugins/chat/assets/javascripts/discourse/components/chat/drawer-routes/channels.gjs
+++ b/plugins/chat/assets/javascripts/discourse/components/chat/drawer-routes/channels.gjs
@@ -22,7 +22,7 @@ export default class ChatDrawerRoutesChannels extends Component {
 
       {{#if this.chatStateManager.isDrawerExpanded}}
         <div class="chat-drawer-content">
-          <ChannelsListPublic @sortByActivity={{true}} />
+          <ChannelsListPublic />
         </div>
 
         <ChatFooter />
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/routes/channels.gjs b/plugins/chat/assets/javascripts/discourse/components/chat/routes/channels.gjs
index 120d2ac1575..bc7de3a402e 100644
--- a/plugins/chat/assets/javascripts/discourse/components/chat/routes/channels.gjs
+++ b/plugins/chat/assets/javascripts/discourse/components/chat/routes/channels.gjs
@@ -17,7 +17,7 @@ export default class ChatRoutesChannels extends Component {
         </navbar.Actions>
       </Navbar>
 
-      <ChannelsListPublic @sortByActivity={{true}} />
+      <ChannelsListPublic />
     </div>
   </template>
 }
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 8825dd76c22..cba2db1707c 100644
--- a/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js
+++ b/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js
@@ -136,7 +136,7 @@ export default class ChatChannelsManager extends Service {
   }
 
   get publicMessageChannelsByActivity() {
-    return this.#sortChannelsByActivity(this.publicMessageChannels);
+    return this.#sortChannelsByActivity([...this.publicMessageChannels]);
   }
 
   @cached
diff --git a/plugins/chat/spec/system/list_channels/sidebar_spec.rb b/plugins/chat/spec/system/list_channels/sidebar_spec.rb
index 6405021c806..b45a227e691 100644
--- a/plugins/chat/spec/system/list_channels/sidebar_spec.rb
+++ b/plugins/chat/spec/system/list_channels/sidebar_spec.rb
@@ -4,6 +4,7 @@ RSpec.describe "List channels | sidebar", type: :system do
   fab!(:current_user) { Fabricate(:user) }
 
   let(:chat) { PageObjects::Pages::Chat.new }
+  let(:drawer_page) { PageObjects::Pages::ChatDrawer.new }
 
   before do
     chat_system_bootstrap
@@ -72,6 +73,24 @@ RSpec.describe "List channels | sidebar", type: :system do
           ".channel-#{channel_1.id}",
         )
       end
+
+      it "does not change sorting order when using drawer" do
+        Fabricate(:chat_message, chat_channel: channel_1)
+        visit("/")
+
+        expect(page.find("#sidebar-section-content-chat-channels li:nth-child(1)")).to have_css(
+          ".channel-#{channel_2.id}",
+        )
+
+        drawer_page.visit_index
+        drawer_page.click_channels
+
+        expect(drawer_page).to have_channel_at_position(channel_1, 1)
+
+        expect(page.find("#sidebar-section-content-chat-channels li:nth-child(1)")).to have_css(
+          ".channel-#{channel_2.id}",
+        )
+      end
     end
 
     context "when direct message channels" do