Skip to content

Commit

Permalink
[MM-62193] Fix search in group channels (mattermost#29916)
Browse files Browse the repository at this point in the history 
* 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 <[email protected]>
  • Loading branch information
@JulienTant @mattermost-build
JulienTant and mattermost-build authored Jan 27, 2025
1 parent 6cc85c9 commit dbf52d6
Show file tree
Hide file tree
Showing 6 changed files with 169 additions and 67 deletions.
10 changes: 9 additions & 1 deletion webapp/channels/src/components/new_search/new_search.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) || '';
Expand Down Expand Up @@ -260,7 +268,7 @@ const NewSearch = (): JSX.Element => {
/>
</SearchTypeBadge>
)}
{searchTerms && <span tabIndex={0}>{searchTerms}</span>}
{searchTerms && <NewSearchTerms tabIndex={0}>{searchTerms}</NewSearchTerms>}
{searchTerms && (
<CloseIcon
data-testid='input-clear'
Expand Down
10 changes: 9 additions & 1 deletion webapp/channels/src/components/new_search/search_box.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,17 @@ const SearchBox = forwardRef(
const escapedMatchedPretext = escapeRegex(matchedPretext);
const caretPosition = getCaretPosition();
const extraSpace = caretPosition === searchTerms.length ? ' ' : '';
const existing = searchTerms.slice(0, caretPosition).replace(new RegExp(escapedMatchedPretext + '$', 'i'), '');

// if existing ends with @ and value starts with one, remove it.
let val = value;
if (existing.endsWith('@') && value.startsWith('@')) {
val = value.slice(1);
}

setSearchTerms(
searchTerms.slice(0, caretPosition).replace(new RegExp(escapedMatchedPretext + '$', 'i'), '') +
value +
val +
extraSpace +
searchTerms.slice(caretPosition),
);
Expand Down
107 changes: 55 additions & 52 deletions webapp/channels/src/components/suggestion/search_channel_provider.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@

import type {ServerError} from '@mattermost/types/errors';

import {getChannelNameForSearchShortcut} from 'mattermost-redux/selectors/entities/channels';
import {isDirectChannel, isGroupChannel, sortChannelsByTypeListAndDisplayName} from 'mattermost-redux/utils/channel_utils';

import {loadProfilesForGroupChannels} from 'actions/user_actions';
import {getCurrentLocale} from 'selectors/i18n';
import store from 'stores/redux_store';

Expand All @@ -16,20 +18,7 @@ import type {ResultsCallback} from './provider';
import SearchChannelSuggestion from './search_channel_suggestion';

const getState = store.getState;

function itemToTerm(isAtSearch: boolean, item: { type: string; display_name: string; name: string }) {
const prefix = isAtSearch ? '' : '@';
if (item.type === Constants.DM_CHANNEL) {
return prefix + item.display_name;
}
if (item.type === Constants.GM_CHANNEL) {
return prefix + item.display_name.replace(/ /g, '');
}
if (item.type === Constants.OPEN_CHANNEL || item.type === Constants.PRIVATE_CHANNEL) {
return item.name;
}
return item.name;
}
const dispatch = store.dispatch;

type SearchChannelAutocomplete = (term: string, success?: (channels: Channel[]) => void, error?: (err: ServerError) => void) => void;

Expand All @@ -43,45 +32,59 @@ export default class SearchChannelProvider extends Provider {

handlePretextChanged(pretext: string, resultsCallback: ResultsCallback<Channel>) {
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;
}
}
44 changes: 31 additions & 13 deletions webapp/channels/src/packages/mattermost-redux/src/selectors/entities/channels.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,27 +238,45 @@ 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,
getCurrentChannelId,
(state: GlobalState): UsersState => state.entities.users,
(allChannels: IDMappedObjects<Channel>, 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<Channel>, users: UsersState, channelId: string): string | undefined => {
const channel = allChannels[channelId];
return getChannelNameForSearch(channel, users);
},
);

Expand Down
60 changes: 60 additions & 0 deletions webapp/channels/src/packages/mattermost-redux/src/utils/channel_utils.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@

import type {ChannelNotifyProps} from '@mattermost/types/channels';
import type {Post} from '@mattermost/types/posts';
import type {UsersState} from '@mattermost/types/users';

import {
areChannelMentionsIgnored,
filterChannelsMatchingTerm,
sortChannelsByRecency,
sortChannelsByDisplayName,
sortChannelsByTypeListAndDisplayName,
completeDirectGroupInfo,
} from 'mattermost-redux/utils/channel_utils';

import TestHelper from '../../test/test_helper';
Expand Down Expand Up @@ -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');
});
});
});
5 changes: 5 additions & 0 deletions webapp/channels/src/packages/mattermost-redux/src/utils/channel_utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down

0 comments on commit dbf52d6

Please sign in to comment.