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 2024-11-11] [$250] Invoice - In the first invoice created, the text " Expense " is not shown #48465

Closed
3 of 6 tasks
IuliiaHerets opened this issue Sep 3, 2024 · 66 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor retest-weekly Apply this label if you want this issue tested on a Weekly basis by Applause Weekly KSv2

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Sep 3, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 9.0.28
Reproducible in staging?: Y
Reproducible in production?: Y
Issue reported by: Applause Internal Team

Action Performed:

  1. Launch app
  2. Tap fab -- new workspace
  3. Navigate to LHN
  4. Tap fab -- Send invoice
  5. Enter an amount and tap next
  6. Select a user
    7.Tap send invoice
  7. Via plus icon -- send invoice, create 2 more invoice
  8. Note preview of all invoice created

Expected Result:

In the first invoice created, the text " Expense " is not shown

Actual Result:

In the first invoice created, the text " Expense ut it is displayed in other invoice created.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6591669_1725366285458.invoice.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021837147636277169515
  • Upwork Job ID: 1837147636277169515
  • Last Price Increase: 2024-10-18
  • Automatic offers:
    • aimane-chnaif | Reviewer | 104521419
    • nkdengineer | Contributor | 104521423
Issue OwnerCurrent Issue Owner: @puneetlath
@IuliiaHerets IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Sep 3, 2024
Copy link

melvin-bot bot commented Sep 3, 2024

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

@IuliiaHerets
Copy link
Author

@puneetlath FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

@nkdengineer
Copy link
Contributor

The same RCA with #48238

@puneetlath puneetlath changed the title Invoice - In the first invoice created, the text " Expense " is not shown [HOLD #48238] Invoice - In the first invoice created, the text " Expense " is not shown Sep 4, 2024
@puneetlath
Copy link
Contributor

Thanks @nkdengineer. Going to put on hold for that issue.

@puneetlath puneetlath added Weekly KSv2 and removed Daily KSv2 labels Sep 4, 2024
@melvin-bot melvin-bot bot added the Overdue label Sep 13, 2024
@puneetlath puneetlath added the External Added to denote the issue can be worked on by a contributor label Sep 20, 2024
@melvin-bot melvin-bot bot changed the title [HOLD #48238] Invoice - In the first invoice created, the text " Expense " is not shown [$250] [HOLD #48238] Invoice - In the first invoice created, the text " Expense " is not shown Sep 20, 2024
Copy link

melvin-bot bot commented Sep 20, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 20, 2024
Copy link

melvin-bot bot commented Sep 20, 2024

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

Copy link

melvin-bot bot commented Sep 23, 2024

@puneetlath, @aimane-chnaif Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot added the Overdue label Sep 23, 2024
@aimane-chnaif
Copy link
Contributor

Is this still on hold?

@melvin-bot melvin-bot bot removed the Overdue label Sep 23, 2024
@puneetlath puneetlath changed the title [$250] [HOLD #48238] Invoice - In the first invoice created, the text " Expense " is not shown [$250] Invoice - In the first invoice created, the text " Expense " is not shown Sep 23, 2024
@puneetlath
Copy link
Contributor

Looks like it doesn't need to be anymore. I removed the hold and I'll ask QA to re-test.

@puneetlath puneetlath added the retest-weekly Apply this label if you want this issue tested on a Weekly basis by Applause label Sep 23, 2024
@m-natarajan
Copy link

Able to reproduce

Screenrecorder-2024-09-24-10-02-25-910_compress_1.mp4

@nkdengineer
Copy link
Contributor

nkdengineer commented Sep 24, 2024

Edited by proposal-police: This proposal was edited at 2024-10-04 05:37:38 UTC.

Proposal

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

Some send invoices display Expense although we don't type the merchant

What is the root cause of that problem?

  1. The first send invoice is created with merchant as (none) then it works as expected. After that, the transaction draft still exists because this logic

useEffect(() => {
if (transaction?.reportID === reportID) {
return;
}
IOU.initMoneyRequest(reportID, policy, isFromGlobalCreate, transactionRequestType.current);
}, [transaction, policy, reportID, iouType, isFromGlobalCreate]);

  1. When we start the second send invoice, we clear the transactionDraft first and then open the IOURequestStartPage which will initiate the money request. That problem starts here, the draft transaction is updated after we call initMoneyRequest (let's see the log below, the transaction change log is logged here).

App/src/libs/actions/IOU.ts

Lines 382 to 385 in b3675da

function startMoneyRequest(iouType: ValueOf<typeof CONST.IOU.TYPE>, reportID: string, requestType?: IOURequestType, skipConfirmation = false) {
clearMoneyRequest(CONST.IOU.OPTIMISTIC_TRANSACTION_ID, skipConfirmation);
switch (requestType) {
case CONST.IOU.REQUEST_TYPE.MANUAL:

Screen.Recording.2024-09-25.at.01.17.59.mov

We move to this case because the transaction still exists as mentioned in point 1. Then the transaction draft is init with merchant as undefined because the current transaction value in the storage is null then we send merchant as an empty string to SendInvoice API. As expected, BE returns the merchant as Expense.

if (currentTransaction?.iouRequestType === iouRequestType) {

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

We have some options to solve this issue

  1. We can add Promise.all to ensure the draft transaction is cleared before we start another money request
function startMoneyRequest(iouType: ValueOf<typeof CONST.IOU.TYPE>, reportID: string, requestType?: IOURequestType, skipConfirmation = false) {
    Promise.all([clearMoneyRequest(CONST.IOU.OPTIMISTIC_TRANSACTION_ID, skipConfirmation)]).then(() => {
        switch (requestType) {
            case CONST.IOU.REQUEST_TYPE.MANUAL:
                Navigation.navigate(ROUTES.MONEY_REQUEST_CREATE_TAB_MANUAL.getRoute(CONST.IOU.ACTION.CREATE, iouType, CONST.IOU.OPTIMISTIC_TRANSACTION_ID, reportID));
                return;
            case CONST.IOU.REQUEST_TYPE.SCAN:
                Navigation.navigate(ROUTES.MONEY_REQUEST_CREATE_TAB_SCAN.getRoute(CONST.IOU.ACTION.CREATE, iouType, CONST.IOU.OPTIMISTIC_TRANSACTION_ID, reportID));
                return;
            case CONST.IOU.REQUEST_TYPE.DISTANCE:
                Navigation.navigate(ROUTES.MONEY_REQUEST_CREATE_TAB_DISTANCE.getRoute(CONST.IOU.ACTION.CREATE, iouType, CONST.IOU.OPTIMISTIC_TRANSACTION_ID, reportID));
                return;
            default:
                Navigation.navigate(ROUTES.MONEY_REQUEST_CREATE.getRoute(CONST.IOU.ACTION.CREATE, iouType, CONST.IOU.OPTIMISTIC_TRANSACTION_ID, reportID));
        }
    });
}

App/src/libs/actions/IOU.ts

Lines 382 to 385 in b3675da

function startMoneyRequest(iouType: ValueOf<typeof CONST.IOU.TYPE>, reportID: string, requestType?: IOURequestType, skipConfirmation = false) {
clearMoneyRequest(CONST.IOU.OPTIMISTIC_TRANSACTION_ID, skipConfirmation);
switch (requestType) {
case CONST.IOU.REQUEST_TYPE.MANUAL:

  1. Pass the transaction value from the component to initMoneyRequest instead of getting it from allTransactionDrafts

useEffect(() => {
if (transaction?.reportID === reportID) {
return;
}
IOU.initMoneyRequest(reportID, policy, isFromGlobalCreate, transactionRequestType.current);
}, [transaction, policy, reportID, iouType, isFromGlobalCreate]);

  1. Fallback the merchant here to CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT instead of empty string

const {amount = 0, currency = '', created = '', merchant = '', category = '', tag = '', taxCode = '', taxAmount = 0, billable, comment, participants} = transaction ?? {};

  1. Add another check to not display the merchant if it's the default merchant as we do here
if (formattedMerchant && formattedMerchant !== CONST.TRANSACTION.DEFAULT_MERCHANT && formattedMerchant !== CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT && formattedMerchant !== CONST.TRANSACTION.DEFAULT_MERCHANT) { 

if (formattedMerchant && formattedMerchant !== CONST.TRANSACTION.DEFAULT_MERCHANT && formattedMerchant !== CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT) {

What alternative solutions did you explore? (Optional)

  1. We can add a useEffect with a cleanup function in IOURequestStartPage to clear the transaction draft when this page is unmounted if the action param is create

@aimane-chnaif
Copy link
Contributor

When submit expense with merchant = "Expense" (manually typed), "Expense" will be hidden. Won't this be a regression?

image

main:

image

@nkdengineer
Copy link
Contributor

@aimane-chnaif I think the current behavior on the main branch is also a bug because you can see if we submit a normal expense with the merchant as Expense, this merchant doesn't show in the money request preview.

Screen.Recording.2024-10-17.at.15.10.26.mov

@aimane-chnaif
Copy link
Contributor

@puneetlath do you agree to show nothing on report preview when submit expense with merchant as Expense manually (same as default merchant)? This already happens on money request preview.

In that case, we can go with @nkdengineer's solution 4. Otherwise, solution 3 also works.
Link to proposal: #48465 (comment)
Test branch: main...nkdengineer:App:fix/48465

🎀👀🎀 C+ reviewed

Copy link

melvin-bot bot commented Oct 17, 2024

Current assignee @puneetlath is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

Copy link

melvin-bot bot commented Oct 18, 2024

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

@puneetlath
Copy link
Contributor

That seems fine to me.

@melvin-bot melvin-bot bot added the Overdue label Oct 21, 2024
@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 21, 2024
Copy link

melvin-bot bot commented Oct 21, 2024

📣 @aimane-chnaif 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

Copy link

melvin-bot bot commented Oct 21, 2024

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

@melvin-bot melvin-bot bot removed the Overdue label Oct 21, 2024
@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Oct 22, 2024
@nkdengineer
Copy link
Contributor

@aimane-chnaif this PR is ready for review

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Nov 4, 2024
@melvin-bot melvin-bot bot changed the title [$250] Invoice - In the first invoice created, the text " Expense " is not shown [HOLD for payment 2024-11-11] [$250] Invoice - In the first invoice created, the text " Expense " is not shown Nov 4, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Nov 4, 2024
Copy link

melvin-bot bot commented Nov 4, 2024

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

Copy link

melvin-bot bot commented Nov 4, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.56-9 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 2024-11-11. 🎊

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

Copy link

melvin-bot bot commented Nov 4, 2024

@aimane-chnaif @puneetlath 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]

@aimane-chnaif
Copy link
Contributor

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
  • 2b. Reported on staging (deploy blocker)
  • 2c. 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: This was not constantly reproducible and I was not able to pinpoint the offending PR which caused this.

  • [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.

Regression Test Proposal Template
  • [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:

N/A

Test:

  1. Create new workspace
  2. Navigate to LHN
  3. Tap FAB > Send invoice
  4. Enter an amount and tap next
  5. Select a user
  6. Tap send invoice
  7. Via FAB, create 2 more invoices
  8. Note preview of all invoices are created
  9. Verify that the text "Expense" is not shown on all previews
  10. Edit an invoice expense with merchant as Expense
  11. Verify that the text "Expense" is not shown on all previews

Do we agree 👍 or 👎

@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

@puneetlath
Copy link
Contributor

All paid. Thanks y'all!

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. External Added to denote the issue can be worked on by a contributor retest-weekly Apply this label if you want this issue tested on a Weekly basis by Applause Weekly KSv2
Projects
Status: Done
Development

No branches or pull requests

6 participants