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.
This commit is contained in:
Martin Brennan 2023-01-11 14:39:56 +10:00 committed by GitHub
parent 3ee0a49254
commit 5f4911dae8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 73 additions and 3 deletions

View File

@ -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

View File

@ -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

View File

@ -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