[HOLD for payment 2023-10-13] [$500] The phone number is displayed twice when searching for it after sending a Money Request.

[kavimuru]

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

---

Action Performed:

1. Log in to any account.
2. Send a Money Request to a phone number, for example: +15005550006.
3. Click on the Search icon in the header and search for that phone number - FULL SEARCH (+15005550006) - IMPORTANT.

Expected Result:

The phone number is displayed ONCE.

Actual Result:

The phone number is displayed TWICE.
Note: This issue actually occurs on all types of search pages: Request Money, New Chat, and Inviting a Member to the Workspace.

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.73-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
Notes/Photos/Videos: Any additional supporting documentation

phone-number-show-twice.1.mp4

bandicam.2023-09-22.16-19-43-803.mp4

Expensify/Expensify Issue URL:
Issue reported by: @tranvantoan-qn
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1695047570703519

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01f217b41585e6a18d
• Upwork Job ID: 1705249513796894720
• Last Price Increase: 2023-09-22
• Automatic offers:
• dukenv0307 | Contributor | 26935042
• tranvantoan-qn | Reporter | 26935044

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @kadiealexander (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

Triggered auto assignment to @garrettmknight (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot]

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

[pradeepmdk]

Proposal

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

The phone number is displayed twice when searching for it after sending a Money Request.

What is the root cause of that problem?

personal details login don't have the domain
but when we in the search personal details we are searching with

App/src/libs/OptionsListUtils.js

Line 1154 in 71db96d

const noOptionsMatchExactly = !_.find(personalDetailsOptions.concat(recentReportOptions), (option) => option.login === addSMSDomainIfPhoneNumber(searchValue).toLowerCase());

addSMSDomainIfPhoneNumber

this will happen when new user we are storing without domain in the login but once api give personal details it will update with domain. But requestmoney api personal details does not have the login key.

App/src/libs/actions/IOU.js

Line 539 in 71db96d

login: participant.login,

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

we need change like this

 const noOptionsMatchExactly = !_.find(personalDetailsOptions.concat(recentReportOptions), (option) => addSMSDomainIfPhoneNumber(option.login) === addSMSDomainIfPhoneNumber(searchValue).toLowerCase());

Result

Alternative solution

we need to update all Onyx optimistic data (Request Money, New Chat, and Inviting a Member to the Workspace.) login with domain

[rojiphil]

Isn't this intentional behavior?

[tranvantoan-qn]

I dont think it is intended, because:

• It only happens if we had a money request to that number
• It only happens with full-keyword search - +15005550006

(Below attached image show the number only once - when there is no Money Request)

[dukenv0307]

Proposal

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

The phone number is displayed twice when searching for it after sending a Money Request

What is the root cause of that problem?

We add optimistic personal detail in optimisticData when we request money from the new user.

App/src/libs/actions/IOU.js

Lines 195 to 200 in 71db96d

	if (!_.isEmpty(optimisticPersonalDetailListAction)) {
	optimisticData.push({
	onyxMethod: Onyx.METHOD.MERGE,
	key: ONYXKEYS.PERSONAL_DETAILS_LIST,
	value: optimisticPersonalDetailListAction,
	});

But we don't remove it in successData and failureData and then we open the search page and search with this phone number, we have two option, one is the recent report that is created above, one is the optimistic personal detail that we don't clear when the API returns data

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

If we request for the new user, we should remove optimistic personal detail in successData and failureData

{
  onyxMethod: Onyx.METHOD.MERGE,
  key: ONYXKEYS.PERSONAL_DETAILS_LIST,
  value: {
      [chatReport.participantAccountIDs[0]]: null,
  }
}

App/src/libs/actions/IOU.js

Line 204 in 71db96d

...(isNewChatReport

App/src/libs/actions/IOU.js

Line 287 in 71db96d

...(isNewIOUReport

What alternative solutions did you explore? (Optional)

[pradeepmdk]

@dukenv0307 I think if we remove the personal details from success or failure it will create the regression.

[kadiealexander]

@narefyev91 please check the proposals here!

[narefyev91]

Coming from here #21706 - it was the same issue which we see here - after creating a request money - we store optimistic data in onyx - but do not clean it after getting correct data from API
Proposal from @dukenv0307 looks good to me #28028 (comment)
🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[pradeepmdk]

@narefyev91 did you able to check my comment. it will create regression if we removed the data from onyx

[dukenv0307]

@pradeepmdk What is the regression? Can you share the step?

[dukenv0307]

I don't think this will cause the regression because after API returns data, the current user accountID of the user will be updated and we will not use the optimistic personal data anymore.

[pradeepmdk]

@dukenv0307 already shared the screenshot. In the failure cause we don't have personal information from api(for all the scanrio request money, new chat, workspace invite ). And also requestmoney api not return full data of person al details here.

[dukenv0307]

@pradeepmdk Let's see the error that is returned, the phone number is invalid so the error appears it's expected, not a bug.
#21706 We do the same way in this issue and there is on regression.

[dukenv0307]

1. All screenshots above are not bugs

if you removed from Onyx data on failure how personal details will be there? and also this issue will occur when we create a new chat with a person, not an existing one.

2. Invite flow has nothing to do with removing personal details when requesting money

and this issue #21706 is fixing only the conflict of accountId. not completely remove the onyx data when we stored the data on optimistic

3. This issue removes optimistic personal detail data by setting it as null. Please see the PR for more details.

[pradeepmdk]

This issue removes optimistic personal detail data by setting it as null. Please see the PR for more details.

in this final PR of that ticket https://github.com/Expensify/App/pull/24355/files
they are only setting null personal details is not there and they are doing the open report api.
settledPersonalDetails[accountID] = allPersonalDetails[accountID] || null;

Invite flow has nothing to do with removing personal details when requesting money

I thought we were going to invite the scenario also because in this issue actual result is this:

The phone number is displayed TWICE.
Note: This issue actually occurs on all types of search pages: Request Money, New Chat, and Inviting a Member to the Workspace.

I am sharing the screenshot current scenario only. they all are working based on personal details. if we removed the personal details this will break.

Let we can wait internal team review.

[dukenv0307]

Just a note here

I think you have misunderstood the meaning of this line of code. This line is used to remove optimistic personal detail in successData and failureData because when we call openReport function, the personal detail of fake accountID doesn't present in allPersonalDetails and settledPersonalDetails is used to reset temporary personal detail that we added in optimistic data

settledPersonalDetails[accountID] = allPersonalDetails[accountID] || null;

[kadiealexander]

@madmax330 bump on this!

[melvin-bot]

📣 @dukenv0307 🎉 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 📖

[melvin-bot]

📣 @tranvantoan-qn 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[dukenv0307]

@narefyev91 The PR is ready for review.

[melvin-bot]

Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:

• when @dukenv0307 got assigned: 2023-09-29 02:58:43 Z
• when the PR got merged: 2023-10-04 16:18:45 UTC
• days elapsed: 3

On to the next one 🚀

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.78-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Remove optimistic personal detail after requesting money #28426

If no regressions arise, payment will be issued on 2023-10-13. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

• @narefyev91 does not require payment (Contractor)
• @dukenv0307 requires payment offer (Contributor)
• @tranvantoan-qn requires payment offer (Reporter)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@narefyev91] The PR that introduced the bug has been identified. Link to the PR:
• [@narefyev91] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@narefyev91] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@narefyev91] Determine if we should create a regression test for this bug.
• [@narefyev91] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@kadiealexander] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[kadiealexander]

Payouts due:

• Issue Reporter: $50 @tranvantoan-qn
• Contributor: $500 @dukenv0307

Eligible for 50% #urgency bonus? No

Upwork job is here.

[kadiealexander]

@narefyev91 please don't forget the checklist!

[narefyev91]

• The PR that introduced the bug has been identified. Link to the PR - it not a regression just missing functionality
• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment - N/A
• A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion - N/A
• Determine if we should create a regression test for this bug. - N/A