[$250] Hybrid iOS app –IOU –Approval button is missing after payment of the report with hold expense

[IuliiaHerets]

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.49-0
Reproducible in staging?: N
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y
Issue was repro when executing this PR: #49910
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

1. Go to Hybrid app
2. Log in with your Approver account
3. As employee Go to the room with the two expenses
4. As employee: hold one expense
5. As Approver: Approve just the pending amount
6. As Approver: back to room by click to the header

Expected Result:

The approval button should load after payment of the report with hold expense

Actual Result:

The approval button is missing after payment of the report with hold expense

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6636810_1729099904332.Ap.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021848434780732463293
• Upwork Job ID: 1848434780732463293
• Last Price Increase: 2024-10-21
• Automatic offers:
• jjcoffee | Reviewer | 104595665
• ikevin127 | Contributor | 104595666

Issue Owner

Current Issue Owner: @jjcoffee

[melvin-bot]

Triggered auto assignment to @sonialiap (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]

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

[NJ-2020]

Proposal

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

Hybrid iOS app –IOU –Approval button is missing after payment of the report with hold expense

What is the root cause of that problem?

Right here we check if the unheld total amount is not equal to 0 then we will show the approve button

App/src/libs/actions/IOU.ts

Lines 6990 to 6992 in 66cf824

	const unheldTotalIsZero = iouReport && iouReport.unheldTotal === 0;
	return isCurrentUserManager && !isOpenExpenseReport && !isApproved && !iouSettled && !isArchivedReport && !unheldTotalIsZero;

Since we hold the expense the unheld total amount become 0 which will not show the approve button

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

We should remove the unheldTotalIsZero variable and remove the return check condition

App/src/libs/actions/IOU.ts

Lines 6990 to 6992 in 66cf824

	const unheldTotalIsZero = iouReport && iouReport.unheldTotal === 0;
	return isCurrentUserManager && !isOpenExpenseReport && !isApproved && !iouSettled && !isArchivedReport && !unheldTotalIsZero;

[melvin-bot]

@sonialiap Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

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

[melvin-bot]

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

[ikevin127]

Proposal

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

Approval button is missing after payment of a report with two expenses, one on hold and the other not, once Approving the expense that's not on hold -> the Approve button is missing for the held expense once Submitted.

What is the root cause of that problem?

• The logic which causes this behaviour was added by PR Fix Scanning expense can be approved as 0.00 amount and can not be edited #50817 on purpose in order to solve issue [$250] Hold - Scanning expense can be approved as 0.00 amount and can not be edited #48590 where the Approve button was showing scan expense while still scanning.

This is what is causing our issue because the logic added by the above mentioned PR is deliberately hiding Approve in our case too, even though the steps in our issue don't mention scan expenses but rather manual expenses.

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

Given the RCA, we cannot simply remove the logic that the above mentioned PR added because it would revert the fix for which the logic was added as fix for this issue:

Approver should not be able to approve scanning expense

Here's what only removing the mentioned logic would look like for the other issue vs how what I'm proposing would look:

Only remove logic	Replace logic (below proposal)
proposal-1.mov	proposal-2.mov

Therefore here's my proposal:

1. Remove the unheldTotalIsZero logic implementation from here:

App/src/libs/actions/IOU.ts

Lines 6991 to 6993 in 179d0f0

	const unheldTotalIsZero = iouReport && iouReport.unheldTotal === 0;
	return isCurrentUserManager && !isOpenExpenseReport && !isApproved && !iouSettled && !isArchivedReport && !unheldTotalIsZero;

2. Replace the above implementation with:

    let isTransactionBeingScanned = false;
    const reportTransactions = TransactionUtils.getAllReportTransactions(iouReport?.reportID);
    for (const transaction of reportTransactions) {
        const hasReceipt = TransactionUtils.hasReceipt(transaction);
        const isReceiptBeingScanned = TransactionUtils.isReceiptBeingScanned(transaction);

        // If a transaction has receipt (scan) and its receipt is being scanned, flag it
        if (hasReceipt && isReceiptBeingScanned) {
            isTransactionBeingScanned = true;
        }
    }

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

This ensures we don't show Approve for a scan expense that's still scanning, while fixing our issue of the Approve button not showing at all.

[jjcoffee]

@ikevin127's proposal LGTM! I agree that we can't just remove the unheldTotalIsZero as the other proposal suggests. Also, I'm guessing this isn't limited to the hybrid app.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[M00rish]

Hi guys, I just want to note that this PR was just pushed to production few hours ago and the issue was opened a few days ago, so maybe it has to do with something else I think..., also are not we supposed to unhold the expense before approving or paying?

thanks

[melvin-bot]

📣 @M00rish! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[ikevin127]

@marcochavezf Looks like @M00rish's PR is still within the regression period as it was deployed to staging only 4 days ago - so they might want to fix this one as a follow-up.

Edit: They are also right that their PR got merged on staging 4 days ago whereas this issue was opened 6 days ago so it could not be a regression of their PR - but the solution I proposed and was selected by C+ keeps the other PRs functionality while fixing this issue so I guess we're good to go here ?

More context in #48590 (comment) (the other issue) which introduced the logic which I propose to change.

[jjcoffee]

@ikevin127 I think the RCA in your proposal goes back to #49910 since there they optimistically added unHeldTotal = 0, which would then trigger the unheldTotalIsZero logic added in the other PR.

App/src/libs/actions/IOU.ts

Lines 6527 to 6533 in d82a253

	onyxMethod: Onyx.METHOD.MERGE,
	key: `${ONYXKEYS.COLLECTION.REPORT}${optimisticExpenseReport.reportID}`,
	value: {
	...optimisticExpenseReport,
	unheldTotal: 0,
	},
	},

Is it possible to reproduce without the changes in #50817? I'm wondering if the RCA might go a bit deeper.

[ikevin127]

@jjcoffee I tried reproducing this issue with both #49910 & #50817 changes reverted and with that, our issue is not reproducible anymore:

Screen.Recording.2024-10-23.at.17.59.07.mov

You are right about the offending PR, looks like the unHeldTotal = 0 addition is the root cause for our issue.

[jjcoffee]

@marcochavezf I think we're good to proceed with @ikevin127's proposal still.

[marcochavezf]

Ok sounds good, thanks for the resolution. Assigning @ikevin127 🚀

[melvin-bot]

📣 @jjcoffee 🎉 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]

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

[ikevin127]

PR will be ready for review in ~ 1 hour.
Edit: ♻️ PR is ready for review.