[HOLD for payment 2023-12-20] [$500] Web - Split Scan - Console error shows up when clicking Split on empty split action menu

[kbecciv]

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: v1.4.11-1
Reproducible in staging?: y
Reproducible in production?: n
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: Applause - Internal Team
Slack conversation:

Action Performed:

1. Go to staging.new.expensify.com.
2. Go to workspace chat > + > Split bill > Scan.
3. Create a split scan request.
4. Click on the spit preview.
5. Click Split.

Expected Result:

No console error shows up.

Actual Result:

Console error shows up.

Workaround:

Unknown

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

Bug6308982_1702302781697.bandicam_2023-12-11_20-01-06-062__1_.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~014fa32ba034ba77de
• Upwork Job ID: 1734211176724713472
• Last Price Increase: 2023-12-11
• Automatic offers:
• shubham1206agra | Reviewer | 28035520
• bernhardoj | Contributor | 28035524

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[mountiny]

Most likely related to the refactor #28618

[abzokhattab]

Proposal

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

Err occur when clicking on empty split action menu

What is the root cause of that problem?

we are not safely accessing the trasnactionID:

    const {transactionID} = updatedTransaction;

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

use lodash get

lodashGet(updatedTransaction,"transactionID")

[tgolen]

I'll take this and look into it this morning.

[mountiny]

@abzokhattab that solution is not enough as there are other properties required which cause console error

[mountiny]

Thanks Tim, I have to head out earlier tonight

[shubham1206agra]

Maybe this will help

const onConfirm = useCallback(
        () => 
        {
            if (!props.draftTransaction) {
                Navigation.dismissModal(reportID);
                return;
            }
            IOU.completeSplitBill(reportID, reportAction, props.draftTransaction, props.session.accountID, props.session.email)
        },
        [reportID, reportAction, props.draftTransaction, props.session.accountID, props.session.email],
    );

[shubham1206agra]

@mountiny Please remove Help wanted label here

[mountiny]

We should show error on the form if the Amount and merchant have not been updated

[shubham1206agra]

I think disabling the button will be more correct

[mountiny]

ah yeah, whatever our Forms.md dictate

[shubham1206agra]

Sorry, we should show error according to Forms.md

[bernhardoj]

Sure, writing it now

[bernhardoj]

Proposal

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

A console error is shown when pressing the Split button on a split bill details page with a receipt and empty required fields.

What is the root cause of that problem?

When we press the split button, we call confirm and have several checks before calling the onConfirm props.

App/src/components/MoneyRequestConfirmationList.js

Lines 461 to 477 in a16df0d

	} else {
	// validate the amount for distance requests
	const decimals = CurrencyUtils.getCurrencyDecimals(props.iouCurrencyCode);
	if (props.isDistanceRequest && !isDistanceRequestWithoutRoute && !MoneyRequestUtils.validateAmount(String(props.iouAmount), decimals)) {
	setFormError('common.error.invalidAmount');
	return;
	}
	if (props.isEditingSplitBill && TransactionUtils.areRequiredFieldsEmpty(transaction)) {
	setDidConfirmSplit(true);
	setFormError('iou.error.genericSmartscanFailureMessage');
	return;
	}
	setDidConfirm(true);
	onConfirm(selectedParticipants);
	}

If the split transaction data is empty, it should show a required fields error (iou.error.genericSmartscanFailureMessage), but TransactionUtils.areRequiredFieldsEmpty always returns false, thus the onConfirm is called and then completeSplitBill is called with an undefined draftTransaction.

App/src/pages/iou/SplitBillDetailsPage.js

Lines 107 to 110 in a16df0d

	const onConfirm = useCallback(
	() => IOU.completeSplitBill(reportID, reportAction, props.draftTransaction, props.session.accountID, props.session.email),
	[reportID, reportAction, props.draftTransaction, props.session.accountID, props.session.email],
	);

In completeSplitBill, we try to get the transactionID from the undefined transaction, so an error is thrown.

App/src/libs/actions/IOU.js

Line 1690 in a16df0d

const {transactionID} = updatedTransaction;

TransactionUtils.areRequiredFieldsEmpty is always false because we pass an empty transaction object to it, but it expects the object to have a property like amount.

App/src/libs/TransactionUtils.ts

Lines 153 to 155 in a16df0d

	function areRequiredFieldsEmpty(transaction: Transaction): boolean {
	return isMerchantMissing(transaction) || isAmountMissing(transaction) || isCreatedMissing(transaction);
	}

App/src/libs/TransactionUtils.ts

Lines 145 to 147 in a16df0d

	function isAmountMissing(transaction: Transaction) {
	return transaction.amount === 0 && (!transaction.modifiedAmount || transaction.modifiedAmount === 0);
	}

The transaction object comes from SplitBillDetailsPage where we pass a draftTransaction to it.

App/src/pages/iou/SplitBillDetailsPage.js

Line 146 in a16df0d

transaction={isEditingSplitBill ? props.draftTransaction : props.transaction}

But draftTransaction is undefined if we never edit the transaction and transaction props in SplitBillDetailPage have a default props of an empty object ({}).

Before, we fall back to transaction.

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

Fall back to transaction if draftTransaction doesn't exist.

In SplitBillDetailsPage

App/src/pages/iou/SplitBillDetailsPage.js

Line 146 in a16df0d

transaction={isEditingSplitBill ? props.draftTransaction : props.transaction}

transaction={isEditingSplitBill ? props.draftTransaction || props.transaction : props.transaction}

[tgolen]

Thanks, that looks great. Assigning to you now.

[melvin-bot]

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

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

[bernhardoj]

PR is ready

[melvin-bot]

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

[melvin-bot]

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

• Fix error when pressing the split button on the split bill details page with a receipt #32821

If no regressions arise, payment will be issued on 2023-12-20. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

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

• @bernhardoj requires payment automatic offer (Contributor)
• @shubham1206agra requires payment automatic offer (Reviewer)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@shubham1206agra] The PR that introduced the bug has been identified. Link to the PR:
• [@shubham1206agra] 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:
• [@shubham1206agra] A discussion in #expensify-bugs 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:
• [@shubham1206agra] Determine if we should create a regression test for this bug.
• [@shubham1206agra] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@puneetlath] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[puneetlath]

@shubham1206agra friendly reminder on the checklist so that we can pay this out tomorrow.

[shubham1206agra]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@shubham1206agra] The PR that introduced the bug has been identified. Link to the PR: Refactor the money request creation flow #28618
• [@shubham1206agra] 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: https://github.com/Expensify/App/pull/28618/files#r1432819995
• [@shubham1206agra] A discussion in #expensify-bugs 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: Not required
• [@shubham1206agra] Determine if we should create a regression test for this bug. No as we have already caught this in deploy testing
• [@shubham1206agra] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@puneetlath] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[puneetlath]

Great. @shubham1206agra can you also accept the upwork offer linked above?

[shubham1206agra]

Done

[puneetlath]

All paid. Thanks everyone!