Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow searching new user in chat finder page #40962

Merged
merged 10 commits into from
May 3, 2024
Merged

Allow searching new user in chat finder page #40962

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a3809cb
Allow searching new user in chat finder page
nkdengineer Apr 25, 2024
ddac1ef
fix lint
nkdengineer Apr 25, 2024
00c1f3e
Merge branch 'main' into fix/40731
nkdengineer Apr 26, 2024
2b37da2
dry create user to invite option
nkdengineer Apr 26, 2024
a2c2f8e
merge main
nkdengineer Apr 30, 2024
d9f71f1
fix ts check
nkdengineer Apr 30, 2024
73fe334
add shouldGetOptionToInvite param
nkdengineer Apr 30, 2024
2bd5401
Update src/libs/OptionsListUtils.ts
nkdengineer May 2, 2024
9cac55c
Merge branch 'main' into fix/40731
nkdengineer May 2, 2024
23960ac
refactor getUserToInviteOption function
nkdengineer May 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 90 additions & 38 deletions src/libs/OptionsListUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,16 @@ type GetOptionsConfig = {
transactionViolations?: OnyxCollection<TransactionViolation[]>;
};

type GetUserToInviteConfig = {
searchValue: string;
excludeUnknownUsers?: boolean;
optionsToExclude?: Array<Partial<ReportUtils.OptionData>>;
selectedOptions?: Array<Partial<ReportUtils.OptionData>>;
betas: OnyxEntry<Beta[]>;
reportActions?: ReportActions;
showChatPreviewLine?: boolean;
};

type MemberForList = {
text: string;
alternateText: string;
Expand Down Expand Up @@ -1521,6 +1531,60 @@ function orderOptions(options: ReportUtils.OptionData[], searchValue: string | u
);
}

function getUserToInviteOption({
searchValue,
excludeUnknownUsers = false,
optionsToExclude = [],
selectedOptions = [],
betas,
reportActions = {},
showChatPreviewLine = false,
}: GetUserToInviteConfig): ReportUtils.OptionData | null {
let userToInvite: ReportUtils.OptionData | null = null;
const parsedPhoneNumber = PhoneNumber.parsePhoneNumber(LoginUtils.appendCountryCode(Str.removeSMSDomain(searchValue)));

if (
searchValue &&
!isCurrentUser({login: searchValue} as PersonalDetails) &&
selectedOptions.every((option) => 'login' in option && option.login !== searchValue) &&
((Str.isValidEmail(searchValue) && !Str.isDomainEmail(searchValue) && !Str.endsWith(searchValue, CONST.SMS.DOMAIN)) ||
(parsedPhoneNumber.possible && Str.isValidE164Phone(LoginUtils.getPhoneNumberWithoutSpecialChars(parsedPhoneNumber.number?.input ?? '')))) &&
!optionsToExclude.find((optionToExclude) => 'login' in optionToExclude && optionToExclude.login === PhoneNumber.addSMSDomainIfPhoneNumber(searchValue).toLowerCase()) &&
(searchValue !== CONST.EMAIL.CHRONOS || Permissions.canUseChronos(betas)) &&
!excludeUnknownUsers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we clean this up a bit more? This condition is hard to read as is. And we could avoid the comment block entirely by creating some variables that are a bit easier to parse.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marcaaron I updated, please help to check again.

) {
// Generates an optimistic account ID for new users not yet saved in Onyx
const optimisticAccountID = UserUtils.generateAccountID(searchValue);
const personalDetailsExtended = {
...allPersonalDetails,
[optimisticAccountID]: {
accountID: optimisticAccountID,
login: searchValue,
},
};
userToInvite = createOption([optimisticAccountID], personalDetailsExtended, null, reportActions, {
showChatPreviewLine,
});
userToInvite.isOptimisticAccount = true;
userToInvite.login = searchValue;
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
userToInvite.text = userToInvite.text || searchValue;
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
userToInvite.alternateText = userToInvite.alternateText || searchValue;

// If user doesn't exist, use a fallback avatar
userToInvite.icons = [
{
source: FallbackAvatar,
name: searchValue,
type: CONST.ICON_TYPE_AVATAR,
},
];
}

return userToInvite;
}

/**
* filter options based on specific conditions
*/
Expand Down Expand Up @@ -1825,44 +1889,16 @@ function getOptions(
.concat(recentReportOptions)
.find((option) => option.login === PhoneNumber.addSMSDomainIfPhoneNumber(searchValue ?? '').toLowerCase() || option.login === searchValue?.toLowerCase());

if (
searchValue &&
(noOptions || noOptionsMatchExactly) &&
!isCurrentUser({login: searchValue} as PersonalDetails) &&
selectedOptions.every((option) => 'login' in option && option.login !== searchValue) &&
((Str.isValidEmail(searchValue) && !Str.isDomainEmail(searchValue) && !Str.endsWith(searchValue, CONST.SMS.DOMAIN)) ||
(parsedPhoneNumber.possible && Str.isValidE164Phone(LoginUtils.getPhoneNumberWithoutSpecialChars(parsedPhoneNumber.number?.input ?? '')))) &&
!optionsToExclude.find((optionToExclude) => 'login' in optionToExclude && optionToExclude.login === PhoneNumber.addSMSDomainIfPhoneNumber(searchValue).toLowerCase()) &&
(searchValue !== CONST.EMAIL.CHRONOS || Permissions.canUseChronos(betas)) &&
!excludeUnknownUsers
) {
// Generates an optimistic account ID for new users not yet saved in Onyx
const optimisticAccountID = UserUtils.generateAccountID(searchValue);
const personalDetailsExtended = {
...allPersonalDetails,
[optimisticAccountID]: {
accountID: optimisticAccountID,
login: searchValue,
},
};
userToInvite = createOption([optimisticAccountID], personalDetailsExtended, null, reportActions, {
if (noOptions || noOptionsMatchExactly) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nkdengineer Should we move this check to getUserToInviteOption?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we shouldn't because this check is not the same in filterOptions function.

Copy link
Contributor

@DylanDylann DylanDylann Apr 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nkdengineer
We can pass one more param like

shouldGetOptionToInvite={noOptions || noOptionsMatchExactly}

similar to another place

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DylanDylann Agree, updated to add this param.

userToInvite = getUserToInviteOption({
searchValue,
excludeUnknownUsers,
optionsToExclude,
selectedOptions,
betas,
reportActions,
showChatPreviewLine,
});
userToInvite.isOptimisticAccount = true;
userToInvite.login = searchValue;
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
userToInvite.text = userToInvite.text || searchValue;
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
userToInvite.alternateText = userToInvite.alternateText || searchValue;

// If user doesn't exist, use a fallback avatar
userToInvite.icons = [
{
source: FallbackAvatar,
name: searchValue,
type: CONST.ICON_TYPE_AVATAR,
},
];
}

// If we are prioritizing 1:1 chats in search, do it only once we started searching
Expand Down Expand Up @@ -2223,7 +2259,7 @@ function getFirstKeyForList(data?: Option[] | null) {
/**
* Filters options based on the search input value
*/
function filterOptions(options: Options, searchInputValue: string): Options {
function filterOptions(options: Options, searchInputValue: string, betas: OnyxEntry<Beta[]> = []): Options {
const searchValue = getSearchValueForPhoneOrEmail(searchInputValue);
const searchTerms = searchValue ? searchValue.split(' ') : [];

Expand Down Expand Up @@ -2293,10 +2329,26 @@ function filterOptions(options: Options, searchInputValue: string): Options {

const recentReports = matchResults.recentReports.concat(matchResults.personalDetails);

let userToInvite: ReportUtils.OptionData | null = null;

/**
* We create a new user option if the following conditions are satisfied:
* - there's no match recent report and personal detail option
* - The searchValue is a valid email or phone number
* - The searchValue isn't the current personal detail login
* - We can use chronos or the search value is not the chronos email
*/
if (recentReports.length === 0) {
userToInvite = getUserToInviteOption({
searchValue,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why other params is unnecessary here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unnecessary here because when we get the search option via getSearchOptions, the other params is default value.

betas,
});
}

return {
personalDetails: [],
recentReports: orderOptions(recentReports, searchValue),
userToInvite: null,
userToInvite,
currentUserOption: null,
categoryOptions: [],
tagOptions: [],
Expand Down
8 changes: 4 additions & 4 deletions src/pages/ChatFinderPage/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,15 @@ function ChatFinderPage({betas, isSearchingForReports, navigation}: ChatFinderPa
};
}

const newOptions = OptionsListUtils.filterOptions(searchOptions, debouncedSearchValue);
const header = OptionsListUtils.getHeaderMessage(newOptions.recentReports.length > 0, false, debouncedSearchValue);
const newOptions = OptionsListUtils.filterOptions(searchOptions, debouncedSearchValue, betas);
const header = OptionsListUtils.getHeaderMessage(newOptions.recentReports.length + Number(!!newOptions.userToInvite) > 0, false, debouncedSearchValue);
return {
recentReports: newOptions.recentReports,
personalDetails: newOptions.personalDetails,
userToInvite: null,
userToInvite: newOptions.userToInvite,
headerMessage: header,
};
}, [debouncedSearchValue, searchOptions]);
}, [debouncedSearchValue, searchOptions, betas]);

const {recentReports, personalDetails: localPersonalDetails, userToInvite, headerMessage} = debouncedSearchValue.trim() !== '' ? filteredOptions : searchOptions;

Expand Down
Loading