Skip to content

Commit

Permalink
Revert "Merge pull request #46409 from callstack-internal/perf/getsea…
Browse files Browse the repository at this point in the history
…rchtext"

This reverts commit eb16bd6, reversing
changes made to 412fc60.
  • Loading branch information
marcochavezf committed Aug 7, 2024
1 parent d8a5aaa commit 212864d
Show file tree
Hide file tree
Showing 6 changed files with 282 additions and 238 deletions.
5 changes: 0 additions & 5 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1129,11 +1129,6 @@ const CONST = {
// It's copied here so that the same regex pattern can be used in form validations to be consistent with the server.
VALIDATE_FOR_HTML_TAG_REGEX: /<([^>\s]+)(?:[^>]*?)>/g,

// The regex below is used to remove dots only from the local part of the user email (local-part@domain)
// so when we are using search, we can match emails that have dots without explicitly writing the dots (e.g: fistlast@domain will match first.last@domain)
// More info https://github.com/Expensify/App/issues/8007
EMAIL_SEARCH_REGEX: /\.(?=[^\s@]*@)/g,

VALIDATE_FOR_LEADINGSPACES_HTML_TAG_REGEX: /<([\s]+.+[\s]*)>/g,

WHITELISTED_TAGS: [/<>/, /< >/, /<->/, /<-->/, /<br>/, /<br\/>/],
Expand Down
106 changes: 89 additions & 17 deletions src/libs/OptionsListUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,50 @@ function uniqFast(items: string[]): string[] {
return result;
}

/**
* Returns a string with all relevant search terms.
*
* This method must be incredibly performant. It was found to be a big performance bottleneck
* when dealing with accounts that have thousands of reports. For loops are more efficient than _.each
* Array.prototype.push.apply is faster than using the spread operator.
*/
function getSearchText(
report: OnyxInputOrEntry<Report>,
reportName: string,
personalDetailList: Array<Partial<PersonalDetails>>,
isChatRoomOrPolicyExpenseChat: boolean,
isThread: boolean,
): string {
const searchTerms: string[] = [];

for (const personalDetail of personalDetailList) {
if (personalDetail.login) {
// The regex below is used to remove dots only from the local part of the user email (local-part@domain)
// so that we can match emails that have dots without explicitly writing the dots (e.g: fistlast@domain will match first.last@domain)
// More info https://github.com/Expensify/App/issues/8007
searchTerms.push(PersonalDetailsUtils.getDisplayNameOrDefault(personalDetail, '', false), personalDetail.login, personalDetail.login.replace(/\.(?=[^\s@]*@)/g, ''));
}
}

if (report) {
Array.prototype.push.apply(searchTerms, reportName.split(/[,\s]/));

if (isThread) {
const title = ReportUtils.getReportName(report);
const chatRoomSubtitle = ReportUtils.getChatRoomSubtitle(report);

Array.prototype.push.apply(searchTerms, title.split(/[,\s]/));
Array.prototype.push.apply(searchTerms, chatRoomSubtitle?.split(/[,\s]/) ?? ['']);
} else if (isChatRoomOrPolicyExpenseChat) {
const chatRoomSubtitle = ReportUtils.getChatRoomSubtitle(report);

Array.prototype.push.apply(searchTerms, chatRoomSubtitle?.split(/[,\s]/) ?? ['']);
}
}

return uniqFast(searchTerms).join(' ');
}

/**
* Get an object of error messages keyed by microtime by combining all error objects related to the report.
*/
Expand Down Expand Up @@ -745,6 +789,7 @@ function createOption(
phoneNumber: undefined,
hasDraftComment: false,
keyForList: undefined,
searchText: undefined,
isDefaultRoom: false,
isPinned: false,
isWaitingOnBankAccount: false,
Expand Down Expand Up @@ -841,6 +886,9 @@ function createOption(
}

result.text = reportName;
// Disabling this line for safeness as nullish coalescing works only if the value is undefined or null
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
result.searchText = getSearchText(report, reportName, personalDetailList, !!result.isChatRoom || !!result.isPolicyExpenseChat, !!result.isThread);
result.icons = ReportUtils.getIcons(report, personalDetails, personalDetail?.avatar, personalDetail?.login, personalDetail?.accountID, null);
result.subtitle = subtitle;

Expand Down Expand Up @@ -2003,6 +2051,22 @@ function getOptions(
continue;
}

// Finally check to see if this option is a match for the provided search string if we have one
const {searchText, participantsList, isChatRoom} = reportOption;
const participantNames = getParticipantNames(participantsList);

if (searchValue) {
// Determine if the search is happening within a chat room and starts with the report ID
const isReportIdSearch = isChatRoom && Str.startsWith(reportOption.reportID ?? '-1', searchValue);

// Check if the search string matches the search text or participant names considering the type of the room
const isSearchMatch = isSearchStringMatch(searchValue, searchText, participantNames, isChatRoom);

if (!isReportIdSearch && !isSearchMatch) {
continue;
}
}

reportOption.isSelected = isReportSelected(reportOption, selectedOptions);

if (action === CONST.IOU.ACTION.CATEGORIZE) {
Expand All @@ -2027,11 +2091,19 @@ function getOptions(
if (personalDetailsOptionsToExclude.some((optionToExclude) => optionToExclude.login === personalDetailOption.login)) {
return;
}
const {searchText, participantsList, isChatRoom} = personalDetailOption;
const participantNames = getParticipantNames(participantsList);
if (searchValue && !isSearchStringMatch(searchValue, searchText, participantNames, isChatRoom)) {
return;
}

personalDetailsOptions.push(personalDetailOption);
});

const currentUserOption = allPersonalDetailsOptions.find((personalDetailsOption) => personalDetailsOption.login === currentUserLogin);
let currentUserOption = allPersonalDetailsOptions.find((personalDetailsOption) => personalDetailsOption.login === currentUserLogin);
if (searchValue && currentUserOption && !isSearchStringMatch(searchValue, currentUserOption.searchText)) {
currentUserOption = undefined;
}

let userToInvite: ReportUtils.OptionData | null = null;
if (
Expand Down Expand Up @@ -2364,12 +2436,11 @@ function formatSectionsFromSearchTerm(
};
}

const cleanSearchTerm = searchTerm.trim().toLowerCase();
// If you select a new user you don't have a contact for, they won't get returned as part of a recent report or personal details
// This will add them to the list of options, deduping them if they already exist in the other lists
const selectedParticipantsWithoutDetails = selectedOptions.filter((participant) => {
const accountID = participant.accountID ?? null;
const isPartOfSearchTerm = getPersonalDetailSearchTerms(participant).join(' ').toLowerCase().includes(cleanSearchTerm);
const isPartOfSearchTerm = participant.searchText?.toLowerCase().includes(searchTerm.trim().toLowerCase());
const isReportInRecentReports = filteredRecentReports.some((report) => report.accountID === accountID);
const isReportInPersonalDetails = filteredPersonalDetails.some((personalDetail) => personalDetail.accountID === accountID);
return isPartOfSearchTerm && !isReportInRecentReports && !isReportInPersonalDetails;
Expand Down Expand Up @@ -2401,14 +2472,6 @@ function getFirstKeyForList(data?: Option[] | null) {

return firstNonEmptyDataObj.keyForList ? firstNonEmptyDataObj.keyForList : '';
}

function getPersonalDetailSearchTerms(item: Partial<ReportUtils.OptionData>) {
return [item.participantsList?.[0]?.displayName ?? '', item.login ?? '', item.login?.replace(CONST.EMAIL_SEARCH_REGEX, '') ?? ''];
}

function getCurrentUserSearchTerms(item: ReportUtils.OptionData) {
return [item.text ?? '', item.login ?? '', item.login?.replace(CONST.EMAIL_SEARCH_REGEX, '') ?? ''];
}
/**
* Filters options based on the search input value
*/
Expand All @@ -2430,6 +2493,10 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt
const searchValue = parsedPhoneNumber.possible && parsedPhoneNumber.number?.e164 ? parsedPhoneNumber.number.e164 : searchInputValue.toLowerCase();
const searchTerms = searchValue ? searchValue.split(' ') : [];

// The regex below is used to remove dots only from the local part of the user email (local-part@domain)
// so that we can match emails that have dots without explicitly writing the dots (e.g: fistlast@domain will match first.last@domain)
const emailRegex = /\.(?=[^\s@]*@)/g;

const optionsToExclude: Option[] = [{login: CONST.EMAIL.NOTIFICATIONS}];

excludeLogins.forEach((login) => {
Expand All @@ -2449,7 +2516,7 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt

if (login) {
keys.push(login);
keys.push(login.replace(CONST.EMAIL_SEARCH_REGEX, ''));
keys.push(login.replace(emailRegex, ''));
}
});
}
Expand All @@ -2465,7 +2532,7 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt

if (item.login) {
values.push(item.login);
values.push(item.login.replace(CONST.EMAIL_SEARCH_REGEX, ''));
values.push(item.login.replace(emailRegex, ''));
}

if (item.isThread) {
Expand All @@ -2491,9 +2558,15 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt

return uniqFast(values);
});
const personalDetails = filterArrayByMatch(items.personalDetails, term, (item) => uniqFast(getPersonalDetailSearchTerms(item)));
const personalDetails = filterArrayByMatch(items.personalDetails, term, (item) =>
uniqFast([item.participantsList?.[0]?.displayName ?? '', item.login ?? '', item.login?.replace(emailRegex, '') ?? '']),
);

const currentUserOptionSearchText = items.currentUserOption ? uniqFast(getCurrentUserSearchTerms(items.currentUserOption)).join(' ') : '';
const currentUserOptionSearchText = uniqFast([
items.currentUserOption?.text ?? '',
items.currentUserOption?.login ?? '',
items.currentUserOption?.login?.replace(emailRegex, '') ?? '',
]).join(' ');

const currentUserOption = isSearchStringMatch(term, currentUserOptionSearchText) ? items.currentUserOption : null;

Expand Down Expand Up @@ -2560,6 +2633,7 @@ export {
getSearchValueForPhoneOrEmail,
getPersonalDetailsForAccountIDs,
getIOUConfirmationOptionsFromPayeePersonalDetail,
getSearchText,
isSearchStringMatchUserDetails,
getAllReportErrors,
getPolicyExpenseReportOption,
Expand Down Expand Up @@ -2588,8 +2662,6 @@ export {
canCreateOptimisticPersonalDetailOption,
getUserToInviteOption,
shouldShowViolations,
getPersonalDetailSearchTerms,
getCurrentUserSearchTerms,
};

export type {MemberForList, CategorySection, CategoryTreeSection, Options, OptionList, SearchOption, PayeePersonalDetails, Category, Tax, TaxRatesOption, Option, OptionTree};
1 change: 1 addition & 0 deletions src/libs/SidebarUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,7 @@ function getOptionData({
result.participantsList = participantPersonalDetailList;

result.icons = ReportUtils.getIcons(report, personalDetails, personalDetail?.avatar, personalDetail?.login, personalDetail?.accountID, policy, invoiceReceiverPolicy);
result.searchText = OptionsListUtils.getSearchText(report, reportName, participantPersonalDetailList, result.isChatRoom || result.isPolicyExpenseChat, result.isThread);
result.displayNamesWithTooltips = displayNamesWithTooltips;

if (status) {
Expand Down
20 changes: 11 additions & 9 deletions src/pages/NewChatPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,17 @@ function useOptions({isGroupChat}: NewChatPageProps) {

return filteredOptions;
}, [debouncedSearchTerm, defaultOptions, isGroupChat, selectedOptions]);
const cleanSearchTerm = useMemo(() => debouncedSearchTerm.trim().toLowerCase(), [debouncedSearchTerm]);
const headerMessage = useMemo(() => {
return OptionsListUtils.getHeaderMessage(
options.personalDetails.length + options.recentReports.length !== 0,
!!options.userToInvite,
debouncedSearchTerm.trim(),
selectedOptions.some((participant) => OptionsListUtils.getPersonalDetailSearchTerms(participant).join(' ').toLowerCase?.().includes(cleanSearchTerm)),
);
}, [cleanSearchTerm, debouncedSearchTerm, options.personalDetails.length, options.recentReports.length, options.userToInvite, selectedOptions]);

const headerMessage = useMemo(
() =>
OptionsListUtils.getHeaderMessage(
options.personalDetails.length + options.recentReports.length !== 0,
!!options.userToInvite,
debouncedSearchTerm.trim(),
selectedOptions.some((participant) => participant?.searchText?.toLowerCase?.().includes(debouncedSearchTerm.trim().toLowerCase())),
),
[debouncedSearchTerm, options.personalDetails.length, options.recentReports.length, options.userToInvite, selectedOptions],
);

useEffect(() => {
if (!debouncedSearchTerm.length) {
Expand Down
4 changes: 1 addition & 3 deletions src/pages/iou/request/MoneyRequestParticipantsSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ function MoneyRequestParticipantsSelector({participants = CONST.EMPTY_ARRAY, onF
shouldInitialize: didScreenTransitionEnd,
});

const cleanSearchTerm = useMemo(() => debouncedSearchTerm.trim().toLowerCase(), [debouncedSearchTerm]);
const offlineMessage: string = isOffline ? `${translate('common.youAppearToBeOffline')} ${translate('search.resultsAreLimited')}` : '';

const isIOUSplit = iouType === CONST.IOU.TYPE.SPLIT;
Expand Down Expand Up @@ -217,7 +216,7 @@ function MoneyRequestParticipantsSelector({participants = CONST.EMPTY_ARRAY, onF
(chatOptions.personalDetails ?? []).length + (chatOptions.recentReports ?? []).length !== 0,
!!chatOptions?.userToInvite,
debouncedSearchTerm.trim(),
participants.some((participant) => OptionsListUtils.getPersonalDetailSearchTerms(participant).join(' ').toLowerCase().includes(cleanSearchTerm)),
participants.some((participant) => participant?.searchText?.toLowerCase().includes(debouncedSearchTerm.trim().toLowerCase())),
);

return [newSections, headerMessage];
Expand All @@ -231,7 +230,6 @@ function MoneyRequestParticipantsSelector({participants = CONST.EMPTY_ARRAY, onF
chatOptions.userToInvite,
personalDetails,
translate,
cleanSearchTerm,
]);

/**
Expand Down
Loading

0 comments on commit 212864d

Please sign in to comment.