Skip to content

Commit

Permalink
Merge pull request Expensify#40537 from GandalfGwaihir/issue40512
Browse files Browse the repository at this point in the history
[Fix]: Inconsistency in limitation when adding members via FAB and group details page
  • Loading branch information
marcaaron authored Apr 30, 2024
2 parents d4ad6b1 + 71d0830 commit 1abad69
Show file tree
Hide file tree
Showing 8 changed files with 14 additions and 63 deletions.
1 change: 0 additions & 1 deletion src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,6 @@ const CONST = {
MEMBER: 'member',
},
MAX_COUNT_BEFORE_FOCUS_UPDATE: 30,
MAXIMUM_PARTICIPANTS: 8,
SPLIT_REPORTID: '-2',
ACTIONS: {
LIMIT: 50,
Expand Down
2 changes: 0 additions & 2 deletions src/languages/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import type {
LogSizeParams,
ManagerApprovedAmountParams,
ManagerApprovedParams,
MaxParticipantsReachedParams,
NewFaceEnterMagicCodeParams,
NoLongerHaveAccessParams,
NotAllowedExtensionParams,
Expand Down Expand Up @@ -270,7 +269,6 @@ export default {
youAfterPreposition: 'you',
your: 'your',
conciergeHelp: 'Please reach out to Concierge for help.',
maxParticipantsReached: ({count}: MaxParticipantsReachedParams) => `You've selected the maximum number (${count}) of participants.`,
youAppearToBeOffline: 'You appear to be offline.',
thisFeatureRequiresInternet: 'This feature requires an active internet connection to be used.',
areYouSure: 'Are you sure?',
Expand Down
2 changes: 0 additions & 2 deletions src/languages/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import type {
LogSizeParams,
ManagerApprovedAmountParams,
ManagerApprovedParams,
MaxParticipantsReachedParams,
NewFaceEnterMagicCodeParams,
NoLongerHaveAccessParams,
NotAllowedExtensionParams,
Expand Down Expand Up @@ -260,7 +259,6 @@ export default {
youAfterPreposition: 'ti',
your: 'tu',
conciergeHelp: 'Por favor, contacta con Concierge para obtener ayuda.',
maxParticipantsReached: ({count}: MaxParticipantsReachedParams) => `Has seleccionado el número máximo (${count}) de participantes.`,
youAppearToBeOffline: 'Parece que estás desconectado.',
thisFeatureRequiresInternet: 'Esta función requiere una conexión a Internet activa para ser utilizada.',
areYouSure: '¿Estás seguro?',
Expand Down
5 changes: 0 additions & 5 deletions src/languages/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@ type CharacterLimitParams = {
limit: number;
};

type MaxParticipantsReachedParams = {
count: number;
};

type ZipCodeExampleFormatParams = {
zipSampleFormat: string;
};
Expand Down Expand Up @@ -333,7 +329,6 @@ export type {
LoggedInAsParams,
ManagerApprovedAmountParams,
ManagerApprovedParams,
MaxParticipantsReachedParams,
NewFaceEnterMagicCodeParams,
NoLongerHaveAccessParams,
NotAllowedExtensionParams,
Expand Down
9 changes: 2 additions & 7 deletions src/libs/OptionsListUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2117,11 +2117,7 @@ function getMemberInviteOptions(
/**
* Helper method that returns the text to be used for the header's message and title (if any)
*/
function getHeaderMessage(hasSelectableOptions: boolean, hasUserToInvite: boolean, searchValue: string, maxParticipantsReached = false, hasMatchedParticipant = false): string {
if (maxParticipantsReached) {
return Localize.translate(preferredLocale, 'common.maxParticipantsReached', {count: CONST.REPORT.MAXIMUM_PARTICIPANTS});
}

function getHeaderMessage(hasSelectableOptions: boolean, hasUserToInvite: boolean, searchValue: string, hasMatchedParticipant = false): string {
const isValidPhone = PhoneNumber.parsePhoneNumber(LoginUtils.appendCountryCode(searchValue)).possible;

const isValidEmail = Str.isValidEmail(searchValue);
Expand Down Expand Up @@ -2173,14 +2169,13 @@ function formatSectionsFromSearchTerm(
selectedOptions: ReportUtils.OptionData[],
filteredRecentReports: ReportUtils.OptionData[],
filteredPersonalDetails: ReportUtils.OptionData[],
maxOptionsSelected: boolean,
personalDetails: OnyxEntry<PersonalDetailsList> = {},
shouldGetOptionDetails = false,
): SectionForSearchTerm {
// We show the selected participants at the top of the list when there is no search term or maximum number of participants has already been selected
// However, if there is a search term we remove the selected participants from the top of the list unless they are part of the search results
// This clears up space on mobile views, where if you create a group with 4+ people you can't see the selected participants and the search results at the same time
if (searchTerm === '' || maxOptionsSelected) {
if (searchTerm === '') {
return {
section: {
title: undefined,
Expand Down
35 changes: 9 additions & 26 deletions src/pages/NewChatPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,25 +73,23 @@ function useOptions({isGroupChat}: NewChatPageProps) {
undefined,
true,
);
const maxParticipantsReached = selectedOptions.length === CONST.REPORT.MAXIMUM_PARTICIPANTS;

const headerMessage = OptionsListUtils.getHeaderMessage(
filteredOptions.personalDetails.length + filteredOptions.recentReports.length !== 0,
Boolean(filteredOptions.userToInvite),
debouncedSearchTerm.trim(),
maxParticipantsReached,
selectedOptions.some((participant) => participant?.searchText?.toLowerCase?.().includes(debouncedSearchTerm.trim().toLowerCase())),
);
return {...filteredOptions, headerMessage, maxParticipantsReached};
return {...filteredOptions, headerMessage};
}, [betas, debouncedSearchTerm, isGroupChat, listOptions.personalDetails, listOptions.reports, selectedOptions]);

useEffect(() => {
if (!debouncedSearchTerm.length || options.maxParticipantsReached) {
if (!debouncedSearchTerm.length) {
return;
}

Report.searchInServer(debouncedSearchTerm);
}, [debouncedSearchTerm, options.maxParticipantsReached]);
}, [debouncedSearchTerm]);

useEffect(() => {
if (!newGroupDraft?.participants) {
Expand All @@ -117,37 +115,22 @@ function NewChatPage({isGroupChat}: NewChatPageProps) {
const {insets} = useStyledSafeAreaInsets();
const [isSearchingForReports] = useOnyx(ONYXKEYS.IS_SEARCHING_FOR_REPORTS, {initWithStoredValues: false});

const {
headerMessage,
maxParticipantsReached,
searchTerm,
debouncedSearchTerm,
setSearchTerm,
selectedOptions,
setSelectedOptions,
recentReports,
personalDetails,
userToInvite,
areOptionsInitialized,
} = useOptions({
isGroupChat,
});
const {headerMessage, searchTerm, debouncedSearchTerm, setSearchTerm, selectedOptions, setSelectedOptions, recentReports, personalDetails, userToInvite, areOptionsInitialized} =
useOptions({
isGroupChat,
});

const [sections, firstKeyForList] = useMemo(() => {
const sectionsList: OptionsListUtils.CategorySection[] = [];
let firstKey = '';

const formatResults = OptionsListUtils.formatSectionsFromSearchTerm(debouncedSearchTerm, selectedOptions, recentReports, personalDetails, maxParticipantsReached);
const formatResults = OptionsListUtils.formatSectionsFromSearchTerm(debouncedSearchTerm, selectedOptions, recentReports, personalDetails);
sectionsList.push(formatResults.section);

if (!firstKey) {
firstKey = OptionsListUtils.getFirstKeyForList(formatResults.section.data);
}

if (maxParticipantsReached) {
return [sectionsList, firstKey];
}

sectionsList.push({
title: translate('common.recents'),
data: recentReports,
Expand Down Expand Up @@ -178,7 +161,7 @@ function NewChatPage({isGroupChat}: NewChatPageProps) {
}

return [sectionsList, firstKey];
}, [debouncedSearchTerm, selectedOptions, recentReports, personalDetails, maxParticipantsReached, translate, userToInvite]);
}, [debouncedSearchTerm, selectedOptions, recentReports, personalDetails, translate, userToInvite]);

/**
* Creates a new 1:1 chat with the option and the current user,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,6 @@ function MoneyTemporaryForRefactorRequestParticipantsSelector({participants, onF

const offlineMessage = isOffline ? [`${translate('common.youAppearToBeOffline')} ${translate('search.resultsAreLimited')}`, {isTranslated: true}] : '';

const maxParticipantsReached = participants.length === CONST.REPORT.MAXIMUM_PARTICIPANTS;

const isIOUSplit = iouType === CONST.IOU.TYPE.SPLIT;
const isCategorizeOrShareAction = [CONST.IOU.ACTION.CATEGORIZE, CONST.IOU.ACTION.SHARE].includes(action);

Expand Down Expand Up @@ -132,22 +130,10 @@ function MoneyTemporaryForRefactorRequestParticipantsSelector({participants, onF
isCategorizeOrShareAction ? 0 : undefined,
);

const formatResults = OptionsListUtils.formatSectionsFromSearchTerm(
debouncedSearchTerm,
participants,
chatOptions.recentReports,
chatOptions.personalDetails,
maxParticipantsReached,
personalDetails,
true,
);
const formatResults = OptionsListUtils.formatSectionsFromSearchTerm(debouncedSearchTerm, participants, chatOptions.recentReports, chatOptions.personalDetails, personalDetails, true);

newSections.push(formatResults.section);

if (maxParticipantsReached) {
return [newSections, {}];
}

newSections.push({
title: translate('common.recents'),
data: chatOptions.recentReports,
Expand Down Expand Up @@ -183,7 +169,6 @@ function MoneyTemporaryForRefactorRequestParticipantsSelector({participants, onF
action,
canUseP2PDistanceRequests,
iouRequestType,
maxParticipantsReached,
personalDetails,
translate,
didScreenTransitionEnd,
Expand Down Expand Up @@ -272,10 +257,9 @@ function MoneyTemporaryForRefactorRequestParticipantsSelector({participants, onF
lodashGet(newChatOptions, 'personalDetails', []).length + lodashGet(newChatOptions, 'recentReports', []).length !== 0,
Boolean(newChatOptions.userToInvite),
debouncedSearchTerm.trim(),
maxParticipantsReached,
lodashSome(participants, (participant) => participant.searchText.toLowerCase().includes(debouncedSearchTerm.trim().toLowerCase())),
),
[maxParticipantsReached, newChatOptions, participants, debouncedSearchTerm],
[newChatOptions, participants, debouncedSearchTerm],
);

// Right now you can't split an expense with a workspace and other additional participants
Expand Down
3 changes: 1 addition & 2 deletions tests/perf-test/OptionsListUtils.perf-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,6 @@ describe('OptionsListUtils', () => {
Object.values(selectedOptions),
Object.values(filteredRecentReports),
Object.values(filteredPersonalDetails),
false,
mockedPersonalDetails,
true,
),
Expand All @@ -177,6 +176,6 @@ describe('OptionsListUtils', () => {
const mockedPersonalDetails = getMockedPersonalDetails(PERSONAL_DETAILS_COUNT);

await waitForBatchedUpdates();
await measureFunction(() => formatSectionsFromSearchTerm('', Object.values(selectedOptions), [], [], true, mockedPersonalDetails, true));
await measureFunction(() => formatSectionsFromSearchTerm('', Object.values(selectedOptions), [], [], mockedPersonalDetails, true));
});
});

0 comments on commit 1abad69

Please sign in to comment.