App icon indicating copy to clipboard operation
App copied to clipboard

[$250] 'Approve' Button Incorrectly Displayed for Pending Expensify Card Transactions

Open m-natarajan opened this issue 1 year ago β€’ 25 comments

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
  • [x] MacOS: Chrome / Safari
  • [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/f6e4db94-d054-4461-9e26-df4bd8bbb697

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: @paultsimura

m-natarajan avatar Nov 25 '24 23:11 m-natarajan

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.

melvin-bot[bot] avatar Nov 25 '24 23:11 melvin-bot[bot]

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

MelvinBot avatar Nov 25 '24 23:11 MelvinBot

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. https://github.com/Expensify/App/blob/2ce0764f8e8faf9a814072f4445f53822e840b5a/src/libs/SearchUIUtils.ts#L298

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 : https://github.com/Expensify/App/blob/2ce0764f8e8faf9a814072f4445f53822e840b5a/src/libs/actions/IOU.ts#L6850

Shahidullah-Muffakir avatar Nov 26 '24 01:11 Shahidullah-Muffakir

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.

https://github.com/Expensify/App/blob/2ce0764f8e8faf9a814072f4445f53822e840b5a/src/libs/SearchUIUtils.ts#L298

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)

cretadn22 avatar Nov 26 '24 01:11 cretadn22

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

melvin-bot[bot] avatar Nov 29 '24 09:11 melvin-bot[bot]

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

melvin-bot[bot] avatar Dec 02 '24 16:12 melvin-bot[bot]

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

melvin-bot[bot] avatar Dec 02 '24 16:12 melvin-bot[bot]

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

melvin-bot[bot] avatar Dec 06 '24 09:12 melvin-bot[bot]

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

melvin-bot[bot] avatar Dec 09 '24 09:12 melvin-bot[bot]

πŸ“£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πŸ’Έ

melvin-bot[bot] avatar Dec 09 '24 16:12 melvin-bot[bot]

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

melvin-bot[bot] avatar Dec 09 '24 16:12 melvin-bot[bot]

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

sakluger avatar Dec 09 '24 16:12 sakluger

Hey @shawnborton, is there a way to create a Pending Expensify Card Transaction by a Contributor / C+?

paultsimura avatar Dec 09 '24 16:12 paultsimura

@Shahidullah-Muffakir / @cretadn22 did you manage to reproduce this issue? If yes – please share the steps to create such a transaction πŸ™

paultsimura avatar Dec 09 '24 16:12 paultsimura

is there a way to create a Pending Expensify Card Transaction by a Contributor / C+?

@paultsimura I'm not entirely sure, @grgia or @luacmartins any idea?

shawnborton avatar Dec 09 '24 19:12 shawnborton

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

luacmartins avatar Dec 09 '24 23:12 luacmartins

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"}}})

https://github.com/user-attachments/assets/3d0f2656-549f-41ac-8e85-f31f57cfa915

jjcoffee avatar Dec 10 '24 13:12 jjcoffee

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

:ribbon::eyes::ribbon: C+ reviewed

jjcoffee avatar Dec 10 '24 13:12 jjcoffee

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

melvin-bot[bot] avatar Dec 10 '24 13:12 melvin-bot[bot]

@mountiny please assign @jjcoffee here following this thread.

paultsimura avatar Dec 10 '24 14:12 paultsimura

πŸ“£ @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

melvin-bot[bot] avatar Dec 10 '24 16:12 melvin-bot[bot]

πŸ“£ @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 πŸ“–

melvin-bot[bot] avatar Dec 10 '24 16:12 melvin-bot[bot]

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

paultsimura avatar Dec 10 '24 16:12 paultsimura

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

sakluger avatar Dec 10 '24 21:12 sakluger

Fully to @jjcoffee

paultsimura avatar Dec 10 '24 22:12 paultsimura

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

jjcoffee avatar Dec 16 '24 15:12 jjcoffee

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

melvin-bot[bot] avatar Dec 23 '24 21:12 melvin-bot[bot]

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

  • https://github.com/Expensify/App/pull/53955

If no regressions arise, payment will be issued on 2024-12-30. :confetti_ball:

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

  • @jjcoffee requires payment through NewDot Manual Requests
  • @cretadn22 requires payment automatic offer (Contributor)

melvin-bot[bot] avatar Dec 23 '24 21:12 melvin-bot[bot]

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

melvin-bot[bot] avatar Dec 23 '24 21:12 melvin-bot[bot]

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

sakluger avatar Dec 27 '24 22:12 sakluger