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-12-30] [$250] 'Approve' Button Incorrectly Displayed for Pending Expensify Card Transactions #53101

Open
1 of 8 tasks
m-natarajan opened this issue Nov 25, 2024 · 30 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 Needs Reproduction Reproducible steps needed Weekly KSv2

Comments

@m-natarajan
Copy link

m-natarajan commented Nov 25, 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:
Reproducible in staging?: Needs Reproduction
Reproducible in production?: Needs Reproduction
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?:
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: @shawnborton
Slack conversation (hyperlinked to channel name): ts_external_expensify_expense

Action Performed:

  1. Navigate to the Search page.
  2. Search for an expense associated with an Expensify Card transaction that is still in a pending state.
  3. Observe the expense row in the search results.

Expected Result:

The "Approve" button should not be displayed on the expense row for transactions in a pending state, The button should only appear when the transaction is actionable

Actual Result:

An "Approve" button is displayed for the pending transaction, even though no approval action can be taken at this stage and clicking the "Approve" button results in no action or feedback, as expected for a pending transaction.

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: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence
CleanShot.2024-11-25.at.17.43.29.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021863621928114678264
  • Upwork Job ID: 1863621928114678264
  • Last Price Increase: 2024-12-09
  • Automatic offers:
    • paultsimura | Reviewer | 105276803
    • cretadn22 | Contributor | 105276806
Issue OwnerCurrent Issue Owner: @sakluger
@m-natarajan m-natarajan added Daily KSv2 Needs Reproduction Reproducible steps needed Bug Something is broken. Auto assigns a BugZero manager. labels Nov 25, 2024
Copy link

melvin-bot bot commented Nov 25, 2024

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.

@MelvinBot
Copy link

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

@Shahidullah-Muffakir
Copy link
Contributor

Shahidullah-Muffakir commented Nov 26, 2024

Edited by proposal-police: This proposal was edited at 2024-11-26 01:04:55 UTC.

Proposal

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

When navigating to the Search page and searching for an expense associated with an Expensify Card transaction in a pending state, the "Approve" button is incorrectly displayed for pending transaction.

What is the root cause of that problem?

It seems we did not consider the pending status of transaction while showing approve button.

if (IOU.canApproveIOU(report, policy) && ReportUtils.isAllowedToApproveExpenseReport(report, undefined, policy)) {

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

we can add this check here: isTransaction && transaction?.status !== CONST.TRANSACTION.STATUS.PENDING as:

if (
    (isTransaction && 
        transaction?.status !== CONST.TRANSACTION.STATUS.PENDING && 
        IOU.canApproveIOU(report, policy) && 
        ReportUtils.isAllowedToApproveExpenseReport(report, undefined, policy)
    ) || 
    (!isTransaction && 
        IOU.canApproveIOU(report, policy) && 
        ReportUtils.isAllowedToApproveExpenseReport(report, undefined, policy)
    )
) {
    return CONST.SEARCH.ACTION_TYPES.APPROVE;
}

can be simplified to early return view if the transaction is pending

What alternative solutions did you explore? (Optional)

or use !isTransactionPending
const isTransactionPending = TransactionUtils.isPending(transaction)

in :

return isCurrentUserManager && !isOpenExpenseReport && !isApproved && !iouSettled && !isArchivedReport && !isTransactionBeingScanned && !hasViolations;

@cretadn22
Copy link
Contributor

Proposal

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

When navigating to the Search page and searching for an expense associated with a pending Expensify Card transaction, the "Approve" button is incorrectly displayed for the pending transaction

What is the root cause of that problem?

In the condition for displaying the Approve button on the Search page, we have not included checks for pending card transactions.

if (IOU.canApproveIOU(report, policy) && ReportUtils.isAllowedToApproveExpenseReport(report, undefined, policy)) {

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

In here, We need to include conditions to check if all transactions are pending, similar to what we implemented here

    const transactions = TransactionUtils.getAllReportTransactions(report?.reportID);
    const hasOnlyPendingTransactions = transactions.length > 0 && transactions.every((t) => TransactionUtils.isExpensifyCardTransaction(t) && TransactionUtils.isPending(t));
    if (IOU.canApproveIOU(report, policy) && ReportUtils.isAllowedToApproveExpenseReport(report, undefined, policy) && !hasOnlyPendingTransactions) { 

What alternative solutions did you explore? (Optional)

@melvin-bot melvin-bot bot added the Overdue label Nov 28, 2024
Copy link

melvin-bot bot commented Nov 29, 2024

@sakluger Whoops! This issue is 2 days overdue. Let's get this updated quick!

@sakluger sakluger moved this to Bugs and Follow Up Issues in [#whatsnext] #expense Dec 2, 2024
@sakluger sakluger added the External Added to denote the issue can be worked on by a contributor label Dec 2, 2024
Copy link

melvin-bot bot commented Dec 2, 2024

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

@melvin-bot melvin-bot bot changed the title 'Approve' Button Incorrectly Displayed for Pending Expensify Card Transactions [$250] 'Approve' Button Incorrectly Displayed for Pending Expensify Card Transactions Dec 2, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 2, 2024
Copy link

melvin-bot bot commented Dec 2, 2024

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

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

melvin-bot bot commented Dec 6, 2024

@sakluger, @fedirjh Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@melvin-bot melvin-bot bot added the Overdue label Dec 6, 2024
Copy link

melvin-bot bot commented Dec 9, 2024

@sakluger @fedirjh 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!

Copy link

melvin-bot bot commented Dec 9, 2024

📣 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 Overdue label Dec 9, 2024
@sakluger sakluger added External Added to denote the issue can be worked on by a contributor and removed External Added to denote the issue can be worked on by a contributor labels Dec 9, 2024
Copy link

melvin-bot bot commented Dec 9, 2024

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

@sakluger
Copy link
Contributor

sakluger commented Dec 9, 2024

Hey @paultsimura - we have a few proposals pending review. Could you please take a look when you have a chance? Thanks!

@luacmartins
Copy link
Contributor

luacmartins commented Dec 9, 2024

I'm not sure if we have any way of doing that in staging. Maybe we can mock the data by manually merging it into Onyx, e.g.

Onyx.merge('cardList',
{
  4: {
    "bank": "Expensify Card",
    "lastFourPAN": 3458,
  }
}
);

Onyx.merge('transactions_<TRANSACTIONID>', {amount: 1000, currency: 'USD', cardID: 4,  modifiedMerchant: null, merchant: "Test ECard Transaction", mccGroup: "Airlines", status:"Pending", hasEReceipt: true, receipt: {}, reimbursable: false });

I'm not sure if that'd help

@jjcoffee
Copy link
Contributor

I can take this over, since I can replicate with dummy transaction data similar to what @luacmartins suggests above, but we also need to add data for the search under the relevant snapshot key.

Onyx.merge('transactions_<TRANSACTIONID>', {bank: "Expensify Card", cardId: 4, status: "Pending"})
Onyx.merge('snapshot_<SNAPSHOT_ID>', {data: {transactions_<TRANSACTIONID>: {bank: "Expensify Card", cardId: 4, status: "Pending"}}})
53101repro-2024-12-10_14.18.36.mp4

@jjcoffee
Copy link
Contributor

Let's go with @cretadn22's proposal, as they handle the additional case of multiple pending transactions.

🎀👀🎀 C+ reviewed

Copy link

melvin-bot bot commented Dec 10, 2024

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

@paultsimura
Copy link
Contributor

paultsimura commented Dec 10, 2024

@mountiny please assign @jjcoffee here following this thread.

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

melvin-bot bot commented Dec 10, 2024

📣 @paultsimura 🎉 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 Dec 10, 2024

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

@paultsimura
Copy link
Contributor

Thx @mountiny, please also assign @jjcoffee instead of me.

@luacmartins luacmartins assigned jjcoffee and unassigned paultsimura Dec 10, 2024
@sakluger
Copy link
Contributor

Hey @paultsimura and @jjcoffee - for C+ payment, should we split this between you both or should it go fully to @jjcoffee?

@paultsimura
Copy link
Contributor

Fully to @jjcoffee

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Dec 11, 2024
@jjcoffee
Copy link
Contributor

Update: Just waiting for @cretadn22 to add the remaining screenshots.

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Dec 23, 2024
@melvin-bot melvin-bot bot changed the title [$250] 'Approve' Button Incorrectly Displayed for Pending Expensify Card Transactions [HOLD for payment 2024-12-30] [$250] 'Approve' Button Incorrectly Displayed for Pending Expensify Card Transactions Dec 23, 2024
Copy link

melvin-bot bot commented Dec 23, 2024

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

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Dec 23, 2024
Copy link

melvin-bot bot commented Dec 23, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.77-6 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-12-30. 🎊

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

Copy link

melvin-bot bot commented Dec 23, 2024

@jjcoffee @sakluger @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]

@garrettmknight garrettmknight moved this from Bugs and Follow Up Issues to Hold for Payment in [#whatsnext] #expense Dec 23, 2024
@sakluger
Copy link
Contributor

Hey @jjcoffee, could you please complete the BZ checklist before payment is due next Monday? Thanks!

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 Needs Reproduction Reproducible steps needed Weekly KSv2
Projects
Status: Hold for Payment
Development

No branches or pull requests