From dbf52d670d3ac15494222cca58d3105e8a6aea5f Mon Sep 17 00:00:00 2001 From: Julien Tant <785518+JulienTant@users.noreply.github.com> Date: Mon, 27 Jan 2025 09:44:42 -0700 Subject: [PATCH] [MM-62193] Fix search in group channels (#29916) * feat: Add profile loading for group channels in search autocomplete * refactor: Use getChannelNameForSearchShortcut for channel name formatting * refactor: Improve channel search term generation logic * refactor: Simplify search channel provider logic and improve code readability * fix: Improve channel search shortcut name handling with fallback * refactor: Avoid overriding channels parameter in search channel provider * refactor: Revert variable names to original style in search channel provider * feat: Add getChannelNameForSearchShortcut selector for channel name retrieval * refactor: Extract common channel name search logic into helper function * fix text overflowing from search bar * test: Add tests for completeDirectGroupInfo function * test: Add type assertion for UsersState in channel_utils test * test: add test case for completeDirectGroupInfo with omitCurrentUser false * improve name composition for channels in search * eslint * lint --------- Co-authored-by: Mattermost Build --- .../src/components/new_search/new_search.tsx | 10 +- .../src/components/new_search/search_box.tsx | 10 +- .../suggestion/search_channel_provider.tsx | 107 +++++++++--------- .../src/selectors/entities/channels.ts | 44 ++++--- .../src/utils/channel_utils.test.ts | 60 ++++++++++ .../src/utils/channel_utils.ts | 5 + 6 files changed, 169 insertions(+), 67 deletions(-) diff --git a/webapp/channels/src/components/new_search/new_search.tsx b/webapp/channels/src/components/new_search/new_search.tsx index 34ad61e8e9..9810e4be8e 100644 --- a/webapp/channels/src/components/new_search/new_search.tsx +++ b/webapp/channels/src/components/new_search/new_search.tsx @@ -94,6 +94,14 @@ const NewSearchContainer = styled.div` } `; +const NewSearchTerms = styled.span` + overflow: hidden; + text-overflow: ellipsis; + min-width: 0; + margin-right: 32px; + white-space: nowrap; +`; + const NewSearch = (): JSX.Element => { const currentChannelName = useSelector(getCurrentChannelNameForSearchShortcut); const searchTerms = useSelector(getSearchTerms) || ''; @@ -260,7 +268,7 @@ const NewSearch = (): JSX.Element => { /> )} - {searchTerms && {searchTerms}} + {searchTerms && {searchTerms}} {searchTerms && ( void, error?: (err: ServerError) => void) => void; @@ -43,45 +32,59 @@ export default class SearchChannelProvider extends Provider { handlePretextChanged(pretext: string, resultsCallback: ResultsCallback) { const captured = (/\b(?:in|channel):\s*(\S*)$/i).exec(pretext.toLowerCase()); - if (captured) { - let channelPrefix = captured[1]; - const isAtSearch = channelPrefix.startsWith('@'); - if (isAtSearch) { - channelPrefix = channelPrefix.replace(/^@/, ''); - } - const isTildeSearch = channelPrefix.startsWith('~'); - if (isTildeSearch) { - channelPrefix = channelPrefix.replace(/^~/, ''); - } - this.startNewRequest(channelPrefix); - - this.autocompleteChannelsForSearch( - channelPrefix, - (data: Channel[]) => { - if (this.shouldCancelDispatch(channelPrefix)) { - return; - } - - let channels = data; - if (isAtSearch) { - channels = channels.filter((ch: Channel) => isDirectChannel(ch) || isGroupChannel(ch)); - } - - const locale = getCurrentLocale(getState()); - - channels = channels.sort(sortChannelsByTypeListAndDisplayName.bind(null, locale, [Constants.OPEN_CHANNEL, Constants.PRIVATE_CHANNEL, Constants.DM_CHANNEL, Constants.GM_CHANNEL])); - const channelNames = channels.map(itemToTerm.bind(null, isAtSearch)); - - resultsCallback({ - matchedPretext: channelPrefix, - terms: channelNames, - items: channels, - component: SearchChannelSuggestion, - }); - }, - ); + if (!captured) { + return false; } - return Boolean(captured); + const prefix = captured[1].replace(/^[@~]/, ''); + const isAtSearch = captured[1].startsWith('@'); + + this.startNewRequest(prefix); + + this.autocompleteChannelsForSearch( + prefix, + async (data: Channel[]) => { + if (this.shouldCancelDispatch(prefix)) { + return; + } + + let channels = data; + if (isAtSearch) { + channels = data.filter((ch: Channel) => + isDirectChannel(ch) || isGroupChannel(ch), + ); + } + + // Load profiles for group channels if needed + const groupChannels = channels.filter(isGroupChannel); + if (groupChannels.length > 0) { + await dispatch(loadProfilesForGroupChannels(groupChannels)); + } + + // Sort channels + const locale = getCurrentLocale(getState()); + channels.sort(sortChannelsByTypeListAndDisplayName.bind(null, locale, [ + Constants.OPEN_CHANNEL, + Constants.PRIVATE_CHANNEL, + Constants.DM_CHANNEL, + Constants.GM_CHANNEL, + ])); + + // Get channel names using the selector + const channelNames = channels.map((channel) => { + const name = getChannelNameForSearchShortcut(getState(), channel.id) || channel.name; + return isAtSearch && !name.startsWith('@') ? `@${name}` : name; + }); + + resultsCallback({ + matchedPretext: prefix, + terms: channelNames, + items: channels, + component: SearchChannelSuggestion, + }); + }, + ); + + return true; } } diff --git a/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/channels.ts b/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/channels.ts index 690689d02c..551e1e5b61 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/channels.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/channels.ts @@ -238,6 +238,26 @@ export const getCurrentChannel: (state: GlobalState) => Channel | undefined = cr }, ); +const getChannelNameForSearch = (channel: Channel | undefined, users: UsersState): string | undefined => { + if (!channel) { + return undefined; + } + + // Only get the extra info from users if we need it + if (channel.type === General.DM_CHANNEL) { + const dmChannelWithInfo = completeDirectChannelInfo(users, Preferences.DISPLAY_PREFER_USERNAME, channel); + return `@${dmChannelWithInfo.display_name}`; + } + + // Replace spaces in GM channel names + if (channel.type === General.GM_CHANNEL) { + const gmChannelWithInfo = completeDirectGroupInfo(users, Preferences.DISPLAY_PREFER_USERNAME, channel, false); + return `@${gmChannelWithInfo.display_name.replace(/\s/g, '')}`; + } + + return channel.name; +}; + export const getCurrentChannelNameForSearchShortcut: (state: GlobalState) => string | undefined = createSelector( 'getCurrentChannelNameForSearchShortcut', getAllChannels, @@ -245,20 +265,18 @@ export const getCurrentChannelNameForSearchShortcut: (state: GlobalState) => str (state: GlobalState): UsersState => state.entities.users, (allChannels: IDMappedObjects, currentChannelId: string, users: UsersState): string | undefined => { const channel = allChannels[currentChannelId]; + return getChannelNameForSearch(channel, users); + }, +); - // Only get the extra info from users if we need it - if (channel?.type === General.DM_CHANNEL) { - const dmChannelWithInfo = completeDirectChannelInfo(users, Preferences.DISPLAY_PREFER_USERNAME, channel); - return `@${dmChannelWithInfo.display_name}`; - } - - // Replace spaces in GM channel names - if (channel?.type === General.GM_CHANNEL) { - const gmChannelWithInfo = completeDirectGroupInfo(users, Preferences.DISPLAY_PREFER_USERNAME, channel, false); - return `@${gmChannelWithInfo.display_name.replace(/\s/g, '')}`; - } - - return channel?.name; +export const getChannelNameForSearchShortcut: (state: GlobalState, channelId: string) => string | undefined = createSelector( + 'getChannelNameForSearchShortcut', + getAllChannels, + (state: GlobalState): UsersState => state.entities.users, + (state: GlobalState, channelId: string): string => channelId, + (allChannels: IDMappedObjects, users: UsersState, channelId: string): string | undefined => { + const channel = allChannels[channelId]; + return getChannelNameForSearch(channel, users); }, ); diff --git a/webapp/channels/src/packages/mattermost-redux/src/utils/channel_utils.test.ts b/webapp/channels/src/packages/mattermost-redux/src/utils/channel_utils.test.ts index 2c89b946dc..2d40901392 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/utils/channel_utils.test.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/utils/channel_utils.test.ts @@ -3,6 +3,7 @@ import type {ChannelNotifyProps} from '@mattermost/types/channels'; import type {Post} from '@mattermost/types/posts'; +import type {UsersState} from '@mattermost/types/users'; import { areChannelMentionsIgnored, @@ -10,6 +11,7 @@ import { sortChannelsByRecency, sortChannelsByDisplayName, sortChannelsByTypeListAndDisplayName, + completeDirectGroupInfo, } from 'mattermost-redux/utils/channel_utils'; import TestHelper from '../../test/test_helper'; @@ -179,4 +181,62 @@ describe('ChannelUtils', () => { const expectedOutput = JSON.stringify([channelDM, channelGM, channelPrivate, channelOpen1, channelOpen2]); expect(actualOutput).toEqual(expectedOutput); }); + + describe('completeDirectGroupInfo', () => { + const currentUserId = 'current_user_id'; + const currentUser = {id: currentUserId, username: 'current_user_id', first_name: '', last_name: ''}; + const user1 = {id: 'user1', username: 'user1', first_name: '', last_name: ''}; + const user2 = {id: 'user2', username: 'user2', first_name: '', last_name: ''}; + const user3 = {id: 'user3', username: 'user3', first_name: '', last_name: ''}; + + const usersState = { + currentUserId, + profiles: { + [user1.id]: user1, + [user2.id]: user2, + [user3.id]: user3, + [currentUserId]: currentUser, + }, + profilesInChannel: { + channel1: new Set([user1.id, user2.id]), + channel2: new Set([user1.id, user2.id, user3.id]), + }, + } as any as UsersState; + + const baseChannel = TestHelper.fakeChannelOverride({ + id: 'channel1', + type: General.GM_CHANNEL, + display_name: '', + }); + + it('should set display name from profilesInChannel', () => { + const channel = {...baseChannel, id: 'channel1'}; + const result = completeDirectGroupInfo(usersState, 'username', channel); + expect(result.display_name).toBe('user1, user2'); + }); + + it('should set display name for larger group', () => { + const channel = {...baseChannel, id: 'channel2'}; + const result = completeDirectGroupInfo(usersState, 'username', channel); + expect(result.display_name).toBe('user1, user2, user3'); + }); + + it('should use existing display_name when no profilesInChannel', () => { + const channel = {...baseChannel, id: 'channel3', display_name: 'user1, user2'}; + const result = completeDirectGroupInfo(usersState, 'username', channel); + expect(result.display_name).toBe('user1, user2'); + }); + + it('should return original channel when usernames not found', () => { + const channel = {...baseChannel, id: 'channel3', display_name: 'unknown1, unknown2'}; + const result = completeDirectGroupInfo(usersState, 'username', channel); + expect(result).toBe(channel); + }); + + it('should include current user when omitCurrentUser is false', () => { + const channel = {...baseChannel, id: 'channel1'}; + const result = completeDirectGroupInfo(usersState, 'username', channel, false); + expect(result.display_name).toBe('current_user_id, user1, user2'); + }); + }); }); diff --git a/webapp/channels/src/packages/mattermost-redux/src/utils/channel_utils.ts b/webapp/channels/src/packages/mattermost-redux/src/utils/channel_utils.ts index b533986b2f..779217dae2 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/utils/channel_utils.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/utils/channel_utils.ts @@ -161,6 +161,11 @@ export function completeDirectGroupInfo(usersState: UsersState, teammateNameDisp const gm = {...channel}; if (profilesIds) { + // sometimes the current user is not part of the profilesInChannel + if (!omitCurrentUser) { + profilesIds.add(currentUserId); + } + gm.display_name = getGroupDisplayNameFromUserIds(profilesIds, profiles, currentUserId, teammateNameDisplay, omitCurrentUser); return gm; }