fix(app): keep search results in server order instead of per channel (#1777)

This commit is contained in:
Hampus authored and GitHub committed 2026-08-20 13:10:47 +02:00
1 parent ded51b65d3
commit d9bfca66d6
4 files changed
+138 -38

No files matched your search

@@ -28,6 +28,10 @@ import {
SearchIndexingState, SearchIndexingState,
} from '@app/features/channel/components/channel_search_results/SearchResultsStateViews'; } from '@app/features/channel/components/channel_search_results/SearchResultsStateViews';
import type {MessageGroupRenderWrapperProps} from '@app/features/channel/components/MessageGroup'; import type {MessageGroupRenderWrapperProps} from '@app/features/channel/components/MessageGroup';
import {
buildSearchResultGroups,
buildSearchResultGroupsByMessageId,
} from '@app/features/channel/components/SearchResultGrouping';
import {SearchResultMessageList} from '@app/features/channel/components/SearchResultMessageList'; import {SearchResultMessageList} from '@app/features/channel/components/SearchResultMessageList';
import {areSegmentsEqual} from '@app/features/channel/components/SearchResultsUtils'; import {areSegmentsEqual} from '@app/features/channel/components/SearchResultsUtils';
import {DEFAULT_SCOPE_VALUE, getScopeOptionsForChannel} from '@app/features/channel/components/SearchScopeOptions'; import {DEFAULT_SCOPE_VALUE, getScopeOptionsForChannel} from '@app/features/channel/components/SearchScopeOptions';
@@ -134,35 +138,27 @@ export const ChannelSearchResults = observer(
() => new Map(successChannels.map((searchChannel) => [searchChannel.id, searchChannel])), () => new Map(successChannels.map((searchChannel) => [searchChannel.id, searchChannel])),
[successChannels], [successChannels],
); );
const messagesByChannel = useMemo(() => { const resultGroups = useMemo(() => buildSearchResultGroups(successResults), [successResults]);
const grouped = new Map<string, Array<Message>>(); const resultGroupsByMessageId = useMemo(() => buildSearchResultGroupsByMessageId(resultGroups), [resultGroups]);
for (const message of successResults) {
if (!grouped.has(message.channelId)) {
grouped.set(message.channelId, []);
}
grouped.get(message.channelId)!.push(message);
}
return grouped;
}, [successResults]);
const onCopySelectedMessages = useMessageSelectionCopyForMessages<HTMLDivElement>(successResults); const onCopySelectedMessages = useMessageSelectionCopyForMessages<HTMLDivElement>(successResults);
const spammerOverrideVersion = LocalUserSpamOverride.version; const spammerOverrideVersion = LocalUserSpamOverride.version;
const collapsedMessageVisibility = useMemo( const collapsedMessageVisibility = useMemo(
() => ({ () => ({
isMessageRevealed: (message: Message) => { isMessageRevealed: (message: Message) => {
const channelMessages = messagesByChannel.get(message.channelId); const resultGroup = resultGroupsByMessageId.get(message.id);
if (!channelMessages) return false; if (!resultGroup) return false;
const messageChannel = searchChannelsById.get(message.channelId) ?? Channels.getChannel(message.channelId); const messageChannel = searchChannelsById.get(message.channelId) ?? Channels.getChannel(message.channelId);
if (!messageChannel) return false; if (!messageChannel) return false;
const groupKey = getCollapsedMessageGroupKey({ const groupKey = getCollapsedMessageGroupKey({
channel: messageChannel, channel: messageChannel,
messages: channelMessages, messages: resultGroup.messages,
messageId: message.id, messageId: message.id,
treatSpam: true, treatSpam: true,
}); });
return groupKey != null && revealedGroupKeys.has(groupKey); return groupKey != null && revealedGroupKeys.has(groupKey);
}, },
}), }),
[messagesByChannel, revealedGroupKeys, searchChannelsById, spammerOverrideVersion], [resultGroupsByMessageId, revealedGroupKeys, searchChannelsById, spammerOverrideVersion],
); );
const handleCollapsedGroupRevealChange = useCallback((groupKey: string, revealed: boolean) => { const handleCollapsedGroupRevealChange = useCallback((groupKey: string, revealed: boolean) => {
setRevealedGroupKeys((current) => { setRevealedGroupKeys((current) => {
@@ -606,9 +602,9 @@ export const ChannelSearchResults = observer(
data-message-selection-root="true" data-message-selection-root="true"
data-flx="channel.channel-search-results.render-content.results-scroller" data-flx="channel.channel-search-results.render-content.results-scroller"
> >
{Array.from(messagesByChannel.entries()).map(([resultChannelId, messages]) => { {resultGroups.map((resultGroup) => {
const renderData = getSearchResultChannelRenderData( const renderData = getSearchResultChannelRenderData(
resultChannelId, resultGroup.channelId,
searchChannelsById, searchChannelsById,
(activeScope ?? DEFAULT_SCOPE_VALUE) as MessageSearchScope, (activeScope ?? DEFAULT_SCOPE_VALUE) as MessageSearchScope,
); );
@@ -649,7 +645,7 @@ export const ChannelSearchResults = observer(
</div> </div>
); );
return ( return (
<React.Fragment key={resultChannelId}> <React.Fragment key={resultGroup.key}>
<MessageContextPrefix <MessageContextPrefix
channel={messageChannel} channel={messageChannel}
showGuildMeta={showGuildMeta} showGuildMeta={showGuildMeta}
@@ -660,7 +656,7 @@ export const ChannelSearchResults = observer(
/> />
<SearchResultMessageList <SearchResultMessageList
channel={messageChannel} channel={messageChannel}
messages={messages} messages={resultGroup.messages}
revealedGroupKeys={revealedGroupKeys} revealedGroupKeys={revealedGroupKeys}
onGroupRevealChange={handleCollapsedGroupRevealChange} onGroupRevealChange={handleCollapsedGroupRevealChange}
collapsedGroupClassName={styles.collapsedMessageGroup} collapsedGroupClassName={styles.collapsedMessageGroup}
@@ -725,7 +721,7 @@ export const ChannelSearchResults = observer(
handlePaginationJump, handlePaginationJump,
onCopySelectedMessages, onCopySelectedMessages,
collapsedMessageVisibility, collapsedMessageVisibility,
messagesByChannel, resultGroups,
searchChannelsById, searchChannelsById,
revealedGroupKeys, revealedGroupKeys,
handleCollapsedGroupRevealChange, handleCollapsedGroupRevealChange,
@@ -0,0 +1,65 @@
// SPDX-License-Identifier: AGPL-3.0-or-later
import type {Message} from '@app/features/messaging/models/MessagingMessage';
import {describe, expect, it} from 'vitest';
import {
buildSearchResultGroups,
buildSearchResultGroupsByMessageId,
countSearchResultChannels,
} from './SearchResultGrouping';
const message = (id: string, channelId: string): Message => ({id, channelId}) as Message;
describe('SearchResultGrouping', () => {
it('keeps the order the server returned instead of bucketing per channel', () => {
const groups = buildSearchResultGroups([
message('500', 'general'),
message('400', 'random'),
message('300', 'general'),
]);
expect(groups.map((group) => group.messages.map((result) => result.id))).toEqual([['500'], ['400'], ['300']]);
expect(groups.map((group) => group.channelId)).toEqual(['general', 'random', 'general']);
});
it('merges consecutive results from the same channel into one group', () => {
const groups = buildSearchResultGroups([
message('500', 'general'),
message('490', 'general'),
message('400', 'random'),
]);
expect(groups).toHaveLength(2);
expect(groups[0].messages.map((result) => result.id)).toEqual(['500', '490']);
expect(groups[1].messages.map((result) => result.id)).toEqual(['400']);
});
it('gives every group a unique key even when a channel appears more than once', () => {
const groups = buildSearchResultGroups([
message('500', 'general'),
message('400', 'random'),
message('300', 'general'),
]);
expect(new Set(groups.map((group) => group.key)).size).toBe(3);
});
it('indexes each message to the group it renders in', () => {
const groups = buildSearchResultGroups([
message('500', 'general'),
message('400', 'random'),
message('300', 'general'),
]);
const groupsByMessageId = buildSearchResultGroupsByMessageId(groups);
expect(groupsByMessageId.get('500')).toBe(groups[0]);
expect(groupsByMessageId.get('400')).toBe(groups[1]);
expect(groupsByMessageId.get('300')).toBe(groups[2]);
});
it('counts distinct channels rather than groups', () => {
const groups = buildSearchResultGroups([
message('500', 'general'),
message('400', 'random'),
message('300', 'general'),
]);
expect(countSearchResultChannels(groups)).toBe(2);
expect(countSearchResultChannels(buildSearchResultGroups([]))).toBe(0);
});
});
@@ -0,0 +1,38 @@
// SPDX-License-Identifier: AGPL-3.0-or-later
import type {Message} from '@app/features/messaging/models/MessagingMessage';
export interface SearchResultGroup {
key: string;
channelId: string;
messages: Array<Message>;
}
export const buildSearchResultGroups = (messages: Array<Message>): Array<SearchResultGroup> => {
const groups: Array<SearchResultGroup> = [];
let currentGroup: SearchResultGroup | null = null;
for (const message of messages) {
if (!currentGroup || currentGroup.channelId !== message.channelId) {
currentGroup = {key: `${message.channelId}-${message.id}`, channelId: message.channelId, messages: []};
groups.push(currentGroup);
}
currentGroup.messages.push(message);
}
return groups;
};
export const buildSearchResultGroupsByMessageId = (
groups: Array<SearchResultGroup>,
): Map<string, SearchResultGroup> => {
const groupsByMessageId = new Map<string, SearchResultGroup>();
for (const group of groups) {
for (const message of group.messages) {
groupsByMessageId.set(message.id, group);
}
}
return groupsByMessageId;
};
export const countSearchResultChannels = (groups: Array<SearchResultGroup>): number => {
return new Set(groups.map((group) => group.channelId)).size;
};
@@ -8,6 +8,11 @@ import {Message as MessageComponent} from '@app/features/channel/components/Chan
import {CollapsedMessageVisibilityProvider} from '@app/features/channel/components/CollapsedMessageVisibilityContext'; import {CollapsedMessageVisibilityProvider} from '@app/features/channel/components/CollapsedMessageVisibilityContext';
import {MessageActionBottomSheet} from '@app/features/channel/components/MessageActionBottomSheet'; import {MessageActionBottomSheet} from '@app/features/channel/components/MessageActionBottomSheet';
import type {MessageGroupRenderWrapperProps} from '@app/features/channel/components/MessageGroup'; import type {MessageGroupRenderWrapperProps} from '@app/features/channel/components/MessageGroup';
import {
buildSearchResultGroups,
buildSearchResultGroupsByMessageId,
countSearchResultChannels,
} from '@app/features/channel/components/SearchResultGrouping';
import {SearchResultMessageList} from '@app/features/channel/components/SearchResultMessageList'; import {SearchResultMessageList} from '@app/features/channel/components/SearchResultMessageList';
import {type ChannelSearchFilters, useChannelSearch} from '@app/features/channel/hooks/useChannelSearch'; import {type ChannelSearchFilters, useChannelSearch} from '@app/features/channel/hooks/useChannelSearch';
import type {Channel} from '@app/features/channel/models/Channel'; import type {Channel} from '@app/features/channel/models/Channel';
@@ -216,22 +221,17 @@ export const ChannelSearchBottomSheet: React.FC<ChannelSearchBottomSheetProps> =
() => new Map(successChannels.map((searchChannel) => [searchChannel.id, searchChannel])), () => new Map(successChannels.map((searchChannel) => [searchChannel.id, searchChannel])),
[successChannels], [successChannels],
); );
const messagesByChannel = React.useMemo(() => { const resultGroups = React.useMemo(() => buildSearchResultGroups(successResults), [successResults]);
const grouped = new Map<string, Array<Message>>(); const resultGroupsByMessageId = React.useMemo(
for (const message of successResults) { () => buildSearchResultGroupsByMessageId(resultGroups),
if (!grouped.has(message.channelId)) { [resultGroups],
grouped.set(message.channelId, []); );
}
grouped.get(message.channelId)!.push(message);
}
return grouped;
}, [successResults]);
const spammerOverrideVersion = LocalUserSpamOverride.version; const spammerOverrideVersion = LocalUserSpamOverride.version;
const collapsedMessageVisibility = React.useMemo( const collapsedMessageVisibility = React.useMemo(
() => ({ () => ({
isMessageRevealed: (message: Message) => { isMessageRevealed: (message: Message) => {
const channelMessages = messagesByChannel.get(message.channelId); const resultGroup = resultGroupsByMessageId.get(message.id);
if (!channelMessages) { if (!resultGroup) {
return false; return false;
} }
const messageChannel = searchChannelsById.get(message.channelId) ?? Channels.getChannel(message.channelId); const messageChannel = searchChannelsById.get(message.channelId) ?? Channels.getChannel(message.channelId);
@@ -240,14 +240,14 @@ export const ChannelSearchBottomSheet: React.FC<ChannelSearchBottomSheetProps> =
} }
const groupKey = getCollapsedMessageGroupKey({ const groupKey = getCollapsedMessageGroupKey({
channel: messageChannel, channel: messageChannel,
messages: channelMessages, messages: resultGroup.messages,
messageId: message.id, messageId: message.id,
treatSpam: true, treatSpam: true,
}); });
return groupKey != null && revealedGroupKeys.has(groupKey); return groupKey != null && revealedGroupKeys.has(groupKey);
}, },
}), }),
[messagesByChannel, revealedGroupKeys, searchChannelsById, spammerOverrideVersion], [resultGroupsByMessageId, revealedGroupKeys, searchChannelsById, spammerOverrideVersion],
); );
const handleCollapsedGroupRevealChange = useCallback((groupKey: string, revealed: boolean) => { const handleCollapsedGroupRevealChange = useCallback((groupKey: string, revealed: boolean) => {
setRevealedGroupKeys((current) => { setRevealedGroupKeys((current) => {
@@ -598,7 +598,7 @@ export const ChannelSearchBottomSheet: React.FC<ChannelSearchBottomSheetProps> =
); );
} }
const totalPages = Math.max(1, Math.ceil(total / hitsPerPage)); const totalPages = Math.max(1, Math.ceil(total / hitsPerPage));
const hasMultipleChannels = messagesByChannel.size > 1; const hasMultipleChannels = countSearchResultChannels(resultGroups) > 1;
return ( return (
<> <>
<CollapsedMessageVisibilityProvider <CollapsedMessageVisibilityProvider
@@ -613,8 +613,9 @@ export const ChannelSearchBottomSheet: React.FC<ChannelSearchBottomSheetProps> =
data-message-selection-root="true" data-message-selection-root="true"
data-flx="channel.channel-search-bottom-sheet.render-content.results-scroller" data-flx="channel.channel-search-bottom-sheet.render-content.results-scroller"
> >
{Array.from(messagesByChannel.entries()).map(([channelId, messages]) => { {resultGroups.map((resultGroup) => {
const messageChannel = searchChannelsById.get(channelId) ?? Channels.getChannel(channelId); const messageChannel =
searchChannelsById.get(resultGroup.channelId) ?? Channels.getChannel(resultGroup.channelId);
if (!messageChannel) { if (!messageChannel) {
return null; return null;
} }
@@ -648,7 +649,7 @@ export const ChannelSearchBottomSheet: React.FC<ChannelSearchBottomSheetProps> =
</LongPressable> </LongPressable>
); );
return ( return (
<React.Fragment key={channelId}> <React.Fragment key={resultGroup.key}>
{hasMultipleChannels && ( {hasMultipleChannels && (
<div <div
className={styles.channelSection} className={styles.channelSection}
@@ -667,7 +668,7 @@ export const ChannelSearchBottomSheet: React.FC<ChannelSearchBottomSheetProps> =
)} )}
<SearchResultMessageList <SearchResultMessageList
channel={messageChannel} channel={messageChannel}
messages={messages} messages={resultGroup.messages}
revealedGroupKeys={revealedGroupKeys} revealedGroupKeys={revealedGroupKeys}
onGroupRevealChange={handleCollapsedGroupRevealChange} onGroupRevealChange={handleCollapsedGroupRevealChange}
collapsedGroupClassName={styles.collapsedMessageGroup} collapsedGroupClassName={styles.collapsedMessageGroup}