FIX: add server-side limit for DM channels (#30300)

When fetching DM channels, we would fetch all the user's DM channels,
and limit the number of DM channels we display on the client-side using
the `DIRECT_MESSAGE_CHANNELS_LIMIT`.

This is obviously not scalable as users are having more and more
discussions with other users.

This commit ensures we always limit the number of DM channels we send to the
client.

It adds the `MAX_DM_CHANNEL_RESULTS` in `ChannelFetcher` and use it to
limit the records returned by the server. This value should always be
lower than `DIRECT_MESSAGE_CHANNELS_LIMIT` on the client-side, so that
when the user has more "unread DMs" they still show up on the client as
they read them.

While adding a spec, I also updated the spec that ensures we have a
limit on public channels to use `stub_const` to temporarily lower the number
of fixtures we create during the test.
This commit is contained in:
Régis Hanol 2024-12-16 18:36:29 +01:00 committed by GitHub
parent ea9cdf7d47
commit 062e4fb4f3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 38 additions and 10 deletions

View File

@ -4,6 +4,9 @@ module Chat
class ChannelFetcher
MAX_PUBLIC_CHANNEL_RESULTS = 50
# NOTE: this should always be > than `DIRECT_MESSAGE_CHANNELS_LIMIT` in `chat-channel-manager.js`
MAX_DM_CHANNEL_RESULTS = 75
def self.structured(guardian, include_threads: false)
memberships = Chat::ChannelMembershipManager.all_for_user(guardian.user)
public_channels = secured_public_channels(guardian, status: :open, following: true)
@ -232,16 +235,19 @@ module Chat
end
query =
query.order("last_message.created_at DESC NULLS LAST").group(
"chat_channels.id",
"last_message.id",
)
query
.limit(MAX_DM_CHANNEL_RESULTS)
.order("last_message.created_at DESC NULLS LAST")
.group("chat_channels.id", "last_message.id")
channels = query.to_a
preload_fields =
User.allowed_user_custom_fields(guardian) +
UserField.all.pluck(:id).map { |fid| "#{User::USER_FIELD_PREFIX}#{fid}" }
User.preload_custom_fields(channels.map { |c| c.chatable.users }.flatten, preload_fields)
User.preload_custom_fields(channels.flat_map { _1.chatable.users }, preload_fields)
channels
end

View File

@ -279,12 +279,14 @@ describe Chat::ChannelFetcher do
end
it "ensures limit has a max value" do
over_limit = Chat::ChannelFetcher::MAX_PUBLIC_CHANNEL_RESULTS + 1
over_limit.times { Fabricate(:category_channel) }
stub_const(Chat::ChannelFetcher, "MAX_PUBLIC_CHANNEL_RESULTS", 5) do
limit = Chat::ChannelFetcher::MAX_PUBLIC_CHANNEL_RESULTS + 1
limit.times { Fabricate(:category_channel) }
expect(described_class.secured_public_channels(guardian, limit: over_limit).length).to eq(
Chat::ChannelFetcher::MAX_PUBLIC_CHANNEL_RESULTS,
)
expect(described_class.secured_public_channels(guardian, limit:).size).to eq(
Chat::ChannelFetcher::MAX_PUBLIC_CHANNEL_RESULTS,
)
end
end
it "does not show the user category channels they cannot access" do
@ -412,6 +414,26 @@ describe Chat::ChannelFetcher do
result = described_class.tracking_state([direct_message_channel1.id], guardian)
expect(result.channel_tracking[target_membership.chat_channel_id][:unread_count]).to eq(0)
end
it "limits the number of results returned" do
stub_const(Chat::ChannelFetcher, "MAX_DM_CHANNEL_RESULTS", 5) do
(Chat::ChannelFetcher::MAX_DM_CHANNEL_RESULTS + 1).times do
chat_channel = Fabricate(:direct_message_channel)
Fabricate(
:user_chat_channel_membership_for_dm,
chat_channel:,
user: user1,
following: true,
)
Chat::DirectMessageUser.create!(direct_message: chat_channel.chatable, user: user1)
Fabricate(:chat_message, chat_channel:, user: user2)
end
expect(described_class.secured_direct_message_channels(user1.id, guardian).size).to eq(
Chat::ChannelFetcher::MAX_DM_CHANNEL_RESULTS,
)
end
end
end
describe ".unreads_total" do