[$500] HIGH: [Reliability] Unable to search and start chat with a new user

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.4.64-0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @quinthar
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1713813518935569

Action Performed:

1. Open chat finder clicking search icon
2. Type any email address of a new user who does not have expensify account

Expected Result:

Should be able to start chat with the new user

Actual Result:

No results found error message appears

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Recording.3020.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01c732e2d542ec4ad2
• Upwork Job ID: 1782814314054311936
• Last Price Increase: 2024-04-23
• Automatic offers:
• nkdengineer | Contributor | 0
• DylanDylann | Contributor | 0

[melvin-bot]

Triggered auto assignment to @stephanieelliott (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[melvin-bot]

Triggered auto assignment to @sakluger (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[stephanieelliott]

Reapplying the Bug label to get another BZ member on this while I am OOO til May 2. Thanks @sakluger, I'll grab this back from you when I return!

[dragnoir]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Extra bottom margin below green button in Personal info page

What is the root cause of that problem?

on ChatFinderPage, what ever the search inside the input, we turn results only from existing reports.
We always turn Null to userToInvite

App/src/pages/ChatFinderPage/index.tsx

Line 109 in 944751f

userToInvite: null,

What changes do you think we should make in order to solve the problem?

as we do on NewChatPage,

App/src/pages/NewChatPage.tsx

Line 55 in 944751f

const filteredOptions = OptionsListUtils.getFilteredOptions(

We need to use getFilteredOptions with the debouncedSearchTerm to get back the new userToInvite

update here to:

const filteredOptions = useMemo(() => {
        if (debouncedSearchValue.trim() === '') {
            return {
                recentReports: [],
                personalDetails: [],
                userToInvite: null,
                headerMessage: '',
            };
        }

        const optionsWithSearch = OptionsListUtils.getSearchOptions(options, debouncedSearchValue, betas ?? []);

        const newOptions = OptionsListUtils.filterOptions(optionsWithSearch, debouncedSearchValue);

        const header = OptionsListUtils.getHeaderMessage(newOptions.recentReports.length > 0, false, debouncedSearchValue);
        return {
            recentReports: newOptions.recentReports,
            personalDetails: newOptions.personalDetails,
            userToInvite: optionsWithSearch.userToInvite,
            headerMessage: header,
        };
    }, [betas, debouncedSearchValue, options]);

We can also combine optionsWithSearch with newOptions to use one function

POC:

20240423_140801.mp4

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01c732e2d542ec4ad2

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @ntdiary (External)

[melvin-bot]

Upwork job price has been updated to $500

[nkdengineer]

Proposal

Please re-state the problem that we are trying to solve in this issue.

No results found error message appears

What is the root cause of that problem?

We always return userToInvite option as null in ChatFinderPage here. So this page will never show the new user option when searching.

App/src/pages/ChatFinderPage/index.tsx

Line 109 in 5e54d5a

userToInvite: null,

What changes do you think we should make in order to solve the problem?

1. In filterOptions function, if both recentReports and personalDetails are empty, we can create a userToInvite option. We can do this as the same way we do in getOptions function here and return this option as userToInvite here.

2. After that on ChatFinderPage, we will return userToInvite as newOptions. userToInvite instead of return null here

App/src/pages/ChatFinderPage/index.tsx

Line 109 in 5e54d5a

userToInvite: null,

*Note: We should not calling getSearchOptions again in this case because we only want to use searchOptions which contain all options to filter the option via search input value.

App/src/pages/ChatFinderPage/index.tsx

Line 76 in 3a3cef6

const searchOptions = useMemo(() => {

What alternative solutions did you explore? (Optional)

NA

[kmbcook]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Unable to start chat with new user from new chat finder page.

What is the root cause of that problem?

The changes to implement filtering in search page #37909, involved creation of a new function, libs/OptionsListUtils.filterOptions, replicating/replacing some logic previously performed in libs/OptionsListUtils.getOptions. It looks like the logic to create an option to invite a new user simply was not considered in the new function.

What changes do you think we should make in order to solve the problem?

The data needed for the option to invite a new user is currently created in getOptions, and is named userToInvite. Pull the functionality for creating that data from getOptions into its own new function. Then have both getOptions and filterOptions call the new function. Then, filterOptions can return the data, and the chat finder page can use it.

The condition for creating that data is complicated (shown below). Make sure that each aspect of this condition is considered.

App/src/libs/OptionsListUtils.ts

Lines 1849 to 1859 in 6b18ee9

	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
	) {

[ntdiary]

@DylanDylann will take over as c+. :)

[DylanDylann]

I will review and give an update today

[DylanDylann]

I don't think @dragnoir's proposal is good, we shouldn't call getSearchOptions with the searchValue because of performance (we only should call this function with searchValue is empty).

@kmbcook's proposal and @nkdengineer's proposal have the same idea to create userToInvite in
filterOptions. Let's go with @nkdengineer's proposal because they are first

🎀👀🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @marcaaron, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[DylanDylann]

We can also combine optionsWithSearch with newOptions to use one function

Oh I see, in the next time, it will be clearer if you can split it to alternative solution and give more detail

[dragnoir]

@DylanDylann ALternative means another solution. I described my solutions in steps. And this is a proposal, not the final PR.

I mentioned a brief description about the way to solve the issue then I said, for the performance:

We can also combine optionsWithSearch with newOptions to use one function

I can't create the new function in the proposal, but I described it as you mentioned here #40731 (comment)

[DylanDylann]

Yeah, I see your idea. But for me, it does not make sense to accept a proposal with one line like this. It would be better if you could detail it more

We can also combine optionsWithSearch with newOptions to use one function

Anyway, It is only my decision. Everything is clear and let's wait for the thought from the internal engineer

[dragnoir]

Yeah, I see your idea. But for me, it does not make sense to accept a proposal with one line like this. It would be better if you could detail it more

How many lines do you see on the proposal you picked? It's also one line about the idea we are discussing here

[dragnoir]

@marcaaron, could you please review the proposals once more? I believe the decision made was not fair.

The C+ mentioned a preference for the 'idea to create userToInvite in filterOptions' to enhance performance, which is precisely what I proposed and detailed here.

Additionally, when I discussed this with the C+, he acknowledged that it was included in my proposal but suggested it should be in the alternative solutions section.

I explained that it was the same idea and that an alternative solution was unnecessary. However, he stated that he couldn't select a solution with one line. Please review my proposal, which includes multiple lines, detailed demo code, and a proof of concept video. In contrast, the selected proposal consists of just one line, with no code, demo, or video provided.

I urge you to thoroughly investigate this matter and ensure that decisions within the contributor program are made with fairness and integrity. I have spent countless hours searching for and crafting meaningful proposals, and I expect the decision-making process to reflect this commitment.

[DylanDylann]

As said before

we shouldn't call getSearchOptions with the searchValue because of performance (we only should call this function with searchValue is empty).

---

1. @dragnoir I don't agree with this point

Please review my proposal, which includes multiple lines, detailed demo code, and a proof of concept video. In contrast, the selected proposal consists of just one line, with no code, demo, or video provided.

It isn't correct. In your proposal, you gave the demo code and detailed implementation of the wrong direction and there is only one line is close to my direction

We can also combine optionsWithSearch with newOptions to use one function

It is not necessary to always need demo code and video provision. I expect your ideas are correct and clear enough

---

2. In @nkdengineer's proposal, their solution is similar to my idea and their details also look good to me. It is the reason why I chose their proposal

---

I have spent countless hours searching for and crafting meaningful proposals, and I expect the decision-making process to reflect this commitment.

Regarding this, everyone spends time preparing the proposal, not just you, and I try my best to be as fair as possible. @dragnoir we should minimize discussion here and wait for @marcaaron to give the final decision

[dragnoir]

@DylanDylann you repeat telling "one line", nkdengineer's idea is also just one line and it's the same as mine.

I expect your ideas are correct and clear enough

Yes it's correct, and clear. I created a quick demo and I mentioned that it will be enhanced. I don't have to create the final PR now.

This is what nkdengineer said:

We can do this as the same way we do in getOptions

I also mentioned this here:

I you did more, it's the same getOptions

Also when nkdengineer said:

return this option as userToInvite

I also mentioned:

to get back the new userToInvite

And about your idea, I said:

We can also combine optionsWithSearch with newOptions to use one function

I think this is clear now!!

Sorry, but this not fair and now I had a feeling that you just try to avoid the truth.

[DylanDylann]

Sorry, but this not fair and now I had a feeling that you just try to avoid the truth.

App/CODE_OF_CONDUCT.md

Line 18 in 917c5cf

- Don’t Ruin it for Everyone Else

The conversation goes far, let's stop here and wait for the final decision from the internal engineer

[dragnoir]

@DylanDylann I apologize if my previous message came across as impolite or upset you in any way—that was not my intention. I want to clarify that my concerns stem from the belief that the solution I proposed earlier was similar to the one selected, including the performance aspects I had also addressed. I hope we can resolve this amicably.

Here's a visual comparison to help you understand what I mentioned and you can see how the proposal picked is similar to mine and copy all my ideas.

You can see how similar colors are about similar notes.

Sorry again.

[marcaaron]

@nkdengineer is assigned to 53 issues at the moment. I am having trouble imagining how they could prioritize more work.

I'd rather work with @kmbcook on this one as they are assigned to 0.

@nkdengineer do you have any problem with that?

[marcaaron]

@dragnoir I have not read most of the comments here beyond this one. My feedback from the other issue might also be useful to you in this situation. If we screw up or get something wrong then please feel free to make a claim outside the issue. Someone will do their best to sort it out. Please keep the issue comments related to the problem we are trying to solve if possible 🙇

[nkdengineer]

@nkdengineer is assigned to 53 issues at the moment. I am having trouble imagining how they could prioritize more work.

@marcaaron Hi, most of them were actually merged/on HOLD. I'm not sure why after merged many issues were not updated with "Awaiting payment", maybe something wrong with the automation.

There's only around 16 open PRs from me and I had no trouble keeping them up to date every day if something is needed from me.

So I'm comfortable taking this one and can open the PR within today.

[marcaaron]

Thanks, see you in the reviews.

[melvin-bot]

📣 @nkdengineer 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[nkdengineer]

@DylanDylann The PR is here.

[stephanieelliott]

Thanks for watching this while I was OOO, grabbing this back from you now @sakluger!

Looks like the PR is awaiting some changes requested of @nkdengineer

[stephanieelliott]

PR is merged, undergoing QA and should go to staging on next deploy

[melvin-bot]

📣 @DylanDylann 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[stephanieelliott]

This was deployed to prod on 5/8, for some reason the automation didn't work on this one. 7-day hold has already passed, so payment is due now:

Summarizing payment on this issue:

• Contributor: @nkdengineer $500 via Upwork - PAID
• Contributor+: @DylanDylann $500 via Upwork - PAID

Upwork job is here: https://www.upwork.com/jobs/~01c732e2d542ec4ad2

[DylanDylann]

@stephanieelliott I accepted the offer

[stephanieelliott]

All paid, thanks!