From 5f4911dae896b0cd45086c27365939020aaf1c19 Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Wed, 11 Jan 2023 14:39:56 +1000
Subject: [PATCH] FIX: Channel archive N1 when serializing current user
 (#19820)

* FIX: Channel archive N1 when serializing current user

The `ChatChannelSerializer` serializes the archive for the
channel if it is present, however this was causing an N1 for
the current user serializer in the case of DM channels, which
were not doing `includes(:chat_channel_archive)` in the
`ChatChannelFetcher`.

DM channels cannot be archived, so we can just never try to serialize
the archive for DM channels in `ChatChannelSerializer`, which
removes the N1.

* DEV: Add N1 performance spec for latest.html preloading

We modify current user serializer in chat, so it's a good
idea to have some N1 performance specs to avoid regressions
here.
---
 .../serializers/chat_channel_serializer.rb    |  2 +-
 plugins/chat/spec/plugin_spec.rb              |  4 +-
 .../core_ext/latest_performance_spec.rb       | 70 +++++++++++++++++++
 3 files changed, 73 insertions(+), 3 deletions(-)
 create mode 100644 plugins/chat/spec/requests/core_ext/latest_performance_spec.rb

diff --git a/plugins/chat/app/serializers/chat_channel_serializer.rb b/plugins/chat/app/serializers/chat_channel_serializer.rb
index b720d691f2f..e6707acfd6b 100644
--- a/plugins/chat/app/serializers/chat_channel_serializer.rb
+++ b/plugins/chat/app/serializers/chat_channel_serializer.rb
@@ -61,7 +61,7 @@ class ChatChannelSerializer < ApplicationSerializer
   end
 
   def include_archive_status?
-    scope.is_staff? && (object.archived? || archive&.failed?) && archive.present?
+    !object.direct_message_channel? && scope.is_staff? && archive.present?
   end
 
   def archive_completed
diff --git a/plugins/chat/spec/plugin_spec.rb b/plugins/chat/spec/plugin_spec.rb
index f31c4c1ec92..68cdda55f97 100644
--- a/plugins/chat/spec/plugin_spec.rb
+++ b/plugins/chat/spec/plugin_spec.rb
@@ -345,7 +345,7 @@ describe Chat do
       end
     end
 
-    context "when followed public channels exist" do
+    context "when followed direct message channels exist" do
       fab!(:user_2) { Fabricate(:user) }
       fab!(:channel) { Fabricate(:direct_message_channel, users: [user, user_2]) }
 
@@ -356,7 +356,7 @@ describe Chat do
       end
     end
 
-    context "when followed direct message channels exist" do
+    context "when followed public channels exist" do
       fab!(:channel) { Fabricate(:chat_channel) }
 
       before do
diff --git a/plugins/chat/spec/requests/core_ext/latest_performance_spec.rb b/plugins/chat/spec/requests/core_ext/latest_performance_spec.rb
new file mode 100644
index 00000000000..185279e1d41
--- /dev/null
+++ b/plugins/chat/spec/requests/core_ext/latest_performance_spec.rb
@@ -0,0 +1,70 @@
+# frozen_string_literal: true
+
+describe ListController do
+  fab!(:current_user) { Fabricate(:user) }
+
+  before do
+    SiteSetting.chat_enabled = true
+    Group.refresh_automatic_groups!
+    sign_in(current_user)
+  end
+
+  describe "#latest" do
+    it "does not do N+1 chat_channel_archive queries based on the number of public and DM channels" do
+      user_1 = Fabricate(:user)
+      Fabricate(:direct_message_channel, users: [current_user, user_1])
+      public_channel_1 = Fabricate(:chat_channel)
+      public_channel_2 = Fabricate(:chat_channel)
+
+      Fabricate(
+        :user_chat_channel_membership,
+        user: current_user,
+        chat_channel: public_channel_1,
+        following: true,
+      )
+
+      # warm up
+      get "/latest.html"
+      expect(response.status).to eq(200)
+
+      initial_sql_queries_count =
+        track_sql_queries do
+          get "/latest.html"
+          expect(response.status).to eq(200)
+          expect(response.body).to have_tag("div#data-preloaded") do |element|
+            current_user_json =
+              JSON.parse(
+                JSON.parse(element.current_scope.attribute("data-preloaded").value)["currentUser"],
+              )
+            expect(current_user_json["chat_channels"]["direct_message_channels"].count).to eq(1)
+            expect(current_user_json["chat_channels"]["public_channels"].count).to eq(1)
+          end
+        end.count
+
+      Fabricate(
+        :user_chat_channel_membership,
+        user: current_user,
+        chat_channel: public_channel_2,
+        following: true,
+      )
+      user_2 = Fabricate(:user)
+      Fabricate(:direct_message_channel, users: [current_user, user_2])
+
+      new_sql_queries_count =
+        track_sql_queries do
+          get "/latest.html"
+          expect(response.status).to eq(200)
+          expect(response.body).to have_tag("div#data-preloaded") do |element|
+            current_user_json =
+              JSON.parse(
+                JSON.parse(element.current_scope.attribute("data-preloaded").value)["currentUser"],
+              )
+            expect(current_user_json["chat_channels"]["direct_message_channels"].count).to eq(2)
+            expect(current_user_json["chat_channels"]["public_channels"].count).to eq(2)
+          end
+        end.count
+
+      expect(new_sql_queries_count).to be <= initial_sql_queries_count
+    end
+  end
+end