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

[$250] Scan - Replace receipt option is present when request is still scanning in one transaction view #39494

Closed
6 tasks done
kbecciv opened this issue Apr 3, 2024 · 17 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@kbecciv
Copy link

kbecciv commented Apr 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: 1.4.59-3
Reproducible in staging?: y
Reproducible in production?: n
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4468886
Issue reported by: Applause - Internal Team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to workspace chat that has no unsettled request.
  3. Create a scan request
  4. While the scanning is ongoing, click on the receipt.
  5. Click 3-dot menu.
  6. Note that Replace option is present.
  7. Create a second scan request.
  8. Click 3-dot menu.
  9. Note that Replace option is actually not present for request that is still scanning.

Expected Result:

In Step 5, there should not be Replace option for the request that is still scanning.

Actual Result:

In Step 5, Replace receipt option is available for the request that is still scanning. Clicking on it leads to not here page. This only happens when there is one transaction.

Workaround:

n/a

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

Bug6436635_1712143969929.bandicam_2024-04-03_19-28-07-637.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01d008a7eab4bb0c77
  • Upwork Job ID: 1775595658777747456
  • Last Price Increase: 2024-04-10
  • Automatic offers:
    • shubham1206agra | Contributor | 0
@kbecciv kbecciv added the DeployBlockerCash This issue or pull request should block deployment label Apr 3, 2024
Copy link

melvin-bot bot commented Apr 3, 2024

Triggered auto assignment to @amyevans (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

@kbecciv
Copy link
Author

kbecciv commented Apr 3, 2024

We think that this bug might be related to #wave-collect - Release 1

@amyevans
Copy link
Contributor

amyevans commented Apr 3, 2024

This seems like a rather minor bug coming out of the One Transaction View feature, so going to demote it to not block deploy and open it up for proposals. cc @NikkiWines though for visibility, in case you disagree or have any context to add!

@amyevans amyevans added Daily KSv2 External Added to denote the issue can be worked on by a contributor Bug Something is broken. Auto assigns a BugZero manager. and removed DeployBlockerCash This issue or pull request should block deployment labels Apr 3, 2024
@melvin-bot melvin-bot bot changed the title Scan - Replace receipt option is present when request is still scanning in one transaction view [$500] Scan - Replace receipt option is present when request is still scanning in one transaction view Apr 3, 2024
Copy link

melvin-bot bot commented Apr 3, 2024

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

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

melvin-bot bot commented Apr 3, 2024

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

Copy link

melvin-bot bot commented Apr 3, 2024

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

@amyevans amyevans changed the title [$500] Scan - Replace receipt option is present when request is still scanning in one transaction view [$250] Scan - Replace receipt option is present when request is still scanning in one transaction view Apr 3, 2024
Copy link

melvin-bot bot commented Apr 3, 2024

Upwork job price has been updated to $250

@abzokhattab
Copy link
Contributor

abzokhattab commented Apr 3, 2024

Proposal

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

User can replace receipt while the request is scanning

What is the root cause of that problem?

The receipt replace action is not disabled if the request is scanning in the canEditFieldOfMoneyRequest function

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

we need to move these vars on top in order to use the transaction and check whether the transaction is being scanned or not

App/src/libs/ReportUtils.ts

Lines 2391 to 2393 in 338aeff

const iouMessage = reportAction?.originalMessage as IOUMessage;
const moneyRequestReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${iouMessage?.IOUReportID}`] ?? ({} as Report);
const transaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${iouMessage?.IOUTransactionID}`] ?? ({} as Transaction);

then add the following condition to return false if the transaction is being scanned:

    if (TransactionUtils.isReceiptBeingScanned(transaction)) {
        return false;
    }

POC

Screen.Recording.2024-04-03.at.9.00.31.PM.mov

@nkdengineer
Copy link
Contributor

nkdengineer commented Apr 4, 2024

Proposal

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

In Step 5, Replace receipt option is available for the request that is still scanning. Clicking on it leads to not here page. This only happens when there is one transaction.

What is the root cause of that problem?

When there's one transaction in the IOU report, the MoneyRequestView that's being shown is actually the transactionThreadReport (can be seen here), this is because when there's only 1 transaction, we'll show the "single transaction view" in the IOU report.

However, when we're passing the contextValue down here, we're still passing the report as the iouReport, not the transactionThreadReport (see here).

This leads to this scanning check condition checking on an empty transaction, because it's getting transaction on the wrong report (the IOU report rather than transactionThreadReport)

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

In contextValue here, use the transactionThreadReport as the report

<ShowContextMenuContext.Provider value={{
    ...contextValue,
    report: transactionThreadReport
}}>

The transactionThreadReport is the same report we passed to the wrapped MoneyRequestView here

What alternative solutions did you explore? (Optional)

We should also check other values in contextValue to make sure they are related to the correct report being shown (transactionThreadReport).

@melvin-bot melvin-bot bot added the Overdue label Apr 5, 2024
@twisterdotcom
Copy link
Contributor

@Santhosh-Sellavel any of these proposals good to go?

@melvin-bot melvin-bot bot removed the Overdue label Apr 8, 2024
@Santhosh-Sellavel
Copy link
Collaborator

Santhosh-Sellavel commented Apr 9, 2024

@twisterdotcom This some how got missed from my radar. Can you reassign, I can't get to this quicker. Again sorry for the delay

@Santhosh-Sellavel Santhosh-Sellavel removed their assignment Apr 9, 2024
@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 10, 2024
Copy link

melvin-bot bot commented Apr 10, 2024

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

@twisterdotcom twisterdotcom added the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 10, 2024
@shubham1206agra
Copy link
Contributor

@twisterdotcom Can you ask QA to retest this?
This may have been fixed already.

@shubham1206agra
Copy link
Contributor

But @nkdengineer solution is correct (in context passing sense) in some sense. We need to find the bug created if we don't apply the solution.

Copy link

melvin-bot bot commented Apr 10, 2024

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

@izarutskaya
Copy link

izarutskaya commented Apr 11, 2024

Not reproduced in latest build
Build v1.4.62-0

bandicam.2024-04-11.13-37-45-106.mp4

@shubham1206agra shubham1206agra removed their assignment Apr 11, 2024
@shubham1206agra
Copy link
Contributor

Thanks for the confirmation.
@twisterdotcom We can close this.

@github-project-automation github-project-automation bot moved this from MEDIUM to CRITICAL in [#whatsnext] #vip-vsb Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
No open projects
Status: CRITICAL
Development

No branches or pull requests

8 participants