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.

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

[HOLD for payment 2025-01-30] [$250] Invoice - "Hidden" name shown when creating invoice for non existing user #54211

Closed
1 of 8 tasks
vincdargento opened this issue Dec 16, 2024 · 27 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@vincdargento
Copy link

vincdargento commented Dec 16, 2024

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: v9.0.76-6
Reproducible in staging?: Yes
Reproducible in production?: No
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team
Device used: Windows 10 / Chrome
App Component: Other

Action Performed:

  1. Navigate to https://staging.new.expensify.com
  2. Create WS> more features> enable invoice
  3. Navigate to FAB> Send invoice
  4. Enter amount and insert non existing user and finish the flow
  5. Navigate to invoice detail and navigate back

Expected Result:

After creating the invoice, The user name should be the email as the account is non existing

Actual Result:

The user name shows "Hidden" at first
After navigating to the invoice and go back, the "Hidden" changed to random user name

Workaround:

Unknown

Platforms:

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

bug.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021869525665327494178
  • Upwork Job ID: 1869525665327494178
  • Last Price Increase: 2025-01-01
Issue OwnerCurrent Issue Owner: @twisterdotcom
@vincdargento vincdargento added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 16, 2024
Copy link

melvin-bot bot commented Dec 16, 2024

Triggered auto assignment to @twisterdotcom (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.

@bernhardoj
Copy link
Contributor

Proposal

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

Hidden user is shown when sending an invoice to a user we never chatted before.

What is the root cause of that problem?

I can't fully reproduce the issue. In my testing, the Hidden only shows on the header name. Based on the QA recording, looks like there is some BE problem too.

When we send a new invoice to a user we never chatted before, we will optimistically create a new personal details for that user. Then, we get the invoice room name from the receiver account ID and the personal details list.

App/src/libs/ReportUtils.ts

Lines 3928 to 3930 in d86024e

if (isIndividual) {
return LocalePhoneNumber.formatPhoneNumber(PersonalDetailsUtils.getDisplayNameOrDefault(allPersonalDetails?.[invoiceReceiverAccountID]));
}

However, the issue happens when the header is rendered before the personal details list optimistic data is merged. The allPersonalDetails is coming from the lib Onyx.connect which won't trigger a re-render when the personal detail is updated.

But actually, on the HeaderView, we already subscribed to the personal details list Onyx using useOnyx and passed it to getReportName, so whenever the personal details list is updated, the getReportName will be called again.

const [personalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST);

const title = ReportUtils.getReportName(reportHeaderData, policy, parentReportAction, personalDetails, invoiceReceiverPolicy);

So, it should re-trigger getInvoicesChatName when the personal details are updated. However, the allPersonalDetails list here is updated later after the useOnyx hook.

App/src/libs/ReportUtils.ts

Lines 3928 to 3930 in d86024e

if (isIndividual) {
return LocalePhoneNumber.formatPhoneNumber(PersonalDetailsUtils.getDisplayNameOrDefault(allPersonalDetails?.[invoiceReceiverAccountID]));
}

So, even though the function is re-triggered, the allPersonalDetails still holds the old value.

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

Pass the personal details from the component to getInvoicesChatName

App/src/libs/ReportUtils.ts

Lines 4084 to 4086 in d86024e

if (isInvoiceRoom(report)) {
formattedName = getInvoicesChatName(report, invoiceReceiverPolicy);
}

formattedName = getInvoicesChatName(report, invoiceReceiverPolicy, personalDetails);

And use it here

App/src/libs/ReportUtils.ts

Lines 3916 to 3930 in d86024e

function getInvoicesChatName(report: OnyxEntry<Report>, receiverPolicy: OnyxEntry<Policy>): string {
const invoiceReceiver = report?.invoiceReceiver;
const isIndividual = invoiceReceiver?.type === CONST.REPORT.INVOICE_RECEIVER_TYPE.INDIVIDUAL;
const invoiceReceiverAccountID = isIndividual ? invoiceReceiver.accountID : -1;
const invoiceReceiverPolicyID = isIndividual ? '' : invoiceReceiver?.policyID ?? '-1';
const invoiceReceiverPolicy = receiverPolicy ?? getPolicy(invoiceReceiverPolicyID);
const isCurrentUserReceiver = (isIndividual && invoiceReceiverAccountID === currentUserAccountID) || (!isIndividual && PolicyUtils.isPolicyAdmin(invoiceReceiverPolicy));
if (isCurrentUserReceiver) {
return getPolicyName(report);
}
if (isIndividual) {
return LocalePhoneNumber.formatPhoneNumber(PersonalDetailsUtils.getDisplayNameOrDefault(allPersonalDetails?.[invoiceReceiverAccountID]));
}

if (isIndividual) {
    return LocalePhoneNumber.formatPhoneNumber(PersonalDetailsUtils.getDisplayNameOrDefault((personalDetails ?? allPersonalDetails)?.[invoiceReceiverAccountID]));
}

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

This requires a UI test where we need to assert that the HeaderView of invoice room title is updated when the personal detail is updated.

@twisterdotcom twisterdotcom added the External Added to denote the issue can be worked on by a contributor label Dec 18, 2024
@melvin-bot melvin-bot bot changed the title Invoice - Random user name generates when creating invoice for non existing user [$250] Invoice - Random user name generates when creating invoice for non existing user Dec 18, 2024
Copy link

melvin-bot bot commented Dec 18, 2024

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

@melvin-bot melvin-bot bot added Overdue Help Wanted Apply this label when an issue is open to proposals by contributors labels Dec 18, 2024
Copy link

melvin-bot bot commented Dec 18, 2024

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

@melvin-bot melvin-bot bot removed the Overdue label Dec 18, 2024
@twisterdotcom twisterdotcom changed the title [$250] Invoice - Random user name generates when creating invoice for non existing user [$250] Invoice - "Hidden" name shown when creating invoice for non existing user Dec 18, 2024
@twisterdotcom twisterdotcom added Weekly KSv2 and removed Daily KSv2 labels Dec 18, 2024
@twisterdotcom
Copy link
Contributor

Going Weekly over the holidays.

@jjcoffee
Copy link
Contributor

@bernhardoj's proposal LGTM! I also can't replicate the random user showing up, so I think we can just focus on the optimistic part here.

🎀👀🎀 C+ reviewed

Copy link

melvin-bot bot commented Dec 19, 2024

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

Copy link

melvin-bot bot commented Dec 25, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

Copy link

melvin-bot bot commented Dec 30, 2024

@justinpersaud @twisterdotcom @jjcoffee this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@melvin-bot melvin-bot bot added the Overdue label Dec 30, 2024
@jjcoffee
Copy link
Contributor

Not overdue, just waiting for @justinpersaud to return from OOO.

@melvin-bot melvin-bot bot removed the Overdue label Dec 30, 2024
Copy link

melvin-bot bot commented Jan 1, 2025

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jan 6, 2025
@melvin-bot melvin-bot bot added the Weekly KSv2 label Jan 17, 2025
@bernhardoj
Copy link
Contributor

The PR is reverted because the unit test is flaky. I have opened a new PR reverting the revert and fix the unit test.

cc: @jjcoffee

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jan 21, 2025
@melvin-bot melvin-bot bot changed the title [$250] Invoice - "Hidden" name shown when creating invoice for non existing user [HOLD for payment 2025-01-28] [$250] Invoice - "Hidden" name shown when creating invoice for non existing user Jan 21, 2025
Copy link

melvin-bot bot commented Jan 21, 2025

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

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jan 21, 2025
Copy link

melvin-bot bot commented Jan 21, 2025

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

If no regressions arise, payment will be issued on 2025-01-28. 🎊

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

  • @jjcoffee requires payment through NewDot Manual Requests
  • @bernhardoj requires payment through NewDot Manual Requests

Copy link

melvin-bot bot commented Jan 21, 2025

@jjcoffee @twisterdotcom @jjcoffee The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Jan 23, 2025
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2025-01-28] [$250] Invoice - "Hidden" name shown when creating invoice for non existing user [HOLD for payment 2025-01-30] [HOLD for payment 2025-01-28] [$250] Invoice - "Hidden" name shown when creating invoice for non existing user Jan 23, 2025
Copy link

melvin-bot bot commented Jan 23, 2025

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

If no regressions arise, payment will be issued on 2025-01-30. 🎊

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

  • @jjcoffee requires payment through NewDot Manual Requests
  • @bernhardoj requires payment through NewDot Manual Requests

Copy link

melvin-bot bot commented Jan 23, 2025

@jjcoffee @twisterdotcom @jjcoffee The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jan 27, 2025
@twisterdotcom twisterdotcom changed the title [HOLD for payment 2025-01-30] [HOLD for payment 2025-01-28] [$250] Invoice - "Hidden" name shown when creating invoice for non existing user [HOLD for payment 2025-01-30] [$250] Invoice - "Hidden" name shown when creating invoice for non existing user Jan 28, 2025
@melvin-bot melvin-bot bot added the Overdue label Jan 29, 2025
@twisterdotcom
Copy link
Contributor

Waiting for tomorrow and also the BZ checklist.

@jjcoffee
Copy link
Contributor

jjcoffee commented Jan 29, 2025

BugZero Checklist:

  • [Contributor] Classify the bug:
Bug classification

Source of bug:

  • 1a. Result of the original design (eg. a case wasn't considered)
  • 1b. Mistake during implementation
  • 1c. Backend bug
  • 1z. Other:

Where bug was reported:

  • 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
  • 2b. Reported on staging (eg. found during regression or PR testing)
  • 2d. Reported on a PR
  • 2z. Other:

Who reported the bug:

  • 3a. Expensify user
  • 3b. Expensify employee
  • 3c. Contributor
  • 3d. QA
  • 3z. Other:
  • [Contributor] 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: https://github.com/Expensify/App/pull/44970/files#r1934051853

  • [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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

  • [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

  • [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

    Link to issue:

Regression Test Proposal

Precondition:

  • Have a workspace with invoices enabled

Test:

  1. Send an invoice to a new user you have never chatted with before
  2. Verify the header and report preview doesn't show Hidden

Do we agree 👍 or 👎

Copy link

melvin-bot bot commented Jan 30, 2025

Payment Summary

Upwork Job

BugZero Checklist (@twisterdotcom)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1869525665327494178/hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

@twisterdotcom
Copy link
Contributor

Payment Summary:

@bernhardoj
Copy link
Contributor

Requested in ND.

@JmillsExpensify
Copy link

$250 approved for @bernhardoj

@garrettmknight
Copy link
Contributor

$250 approved for @jjcoffee

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
Status: Done
Development

No branches or pull requests

7 participants