[HOLD for payment 2024-03-14] [$500] Split scan - Scan split request can be created without Merchant

[m-natarajan]

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.47-0
Reproducible in staging?: yes
Reproducible in production?: no
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.
3. Go to + > Split scan.
4. Create a split scan request.
5. Click on the split scan preview.
6. Enter amount and save it.
7. Click Split.

Expected Result:

App will prevent the bill from splitting because merchant is not entered as merchant is a requirement in workspace expense.

Actual Result:

Scan split request can be created without Merchant.

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 - test build not yet available

Screenshots/Videos

Add any screenshot/video evidence

Bug6402232_1709602219456.bandicam_2024-03-05_07-02-15-673__1_.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01718d2bb982e868c3
• Upwork Job ID: 1764959862414594048
• Last Price Increase: 2024-03-05

[melvin-bot]

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

[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 @techievivek (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[m-natarajan]

We think that this bug might be related to #wave7-collect-submitters
cc @RachCHopkins

[m-natarajan]

@techievivek 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.

[dukenv0307]

Proposal

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

Scan split request can be created without Merchant.

What is the root cause of that problem?

We don't display an error if we're editing the split bill

App/src/components/MoneyRequestConfirmationList.js

Line 285 in a79c119

const shouldDisplayMerchantError = props.isPolicyExpenseChat && shouldDisplayFieldError && isMerchantEmpty;

App/src/components/MoneyRequestConfirmationList.js

Lines 277 to 279 in a79c119

	if (!props.isEditingSplitBill) {
	return false;
	}

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

We can remove this check to allow display error while editing split bill

App/src/components/MoneyRequestConfirmationList.js

Lines 277 to 279 in a79c119

	if (!props.isEditingSplitBill) {
	return false;
	}

What alternative solutions did you explore? (Optional)

NA

[bernhardoj]

Proposal

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

We can complete a split in policy expense chat without filling in the merchant-required field.

What is the root cause of that problem?

When we press the split button, we will check first whether the required fields are empty or not.

App/src/components/MoneyRequestConfirmationList.js

Lines 497 to 500 in a79c119

	if (props.isEditingSplitBill && TransactionUtils.areRequiredFieldsEmpty(transaction)) {
	setDidConfirmSplit(true);
	return;
	}

App/src/libs/TransactionUtils.ts

Lines 167 to 171 in a79c119

	function areRequiredFieldsEmpty(transaction: Transaction): boolean {
	const parentReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${transaction?.reportID}`] ?? null;
	const isFromExpenseReport = parentReport?.type === CONST.REPORT.TYPE.EXPENSE;
	return (isFromExpenseReport && isMerchantMissing(transaction)) || isAmountMissing(transaction) || isCreatedMissing(transaction);
	}

The merchant is only required if it's an expense request and it should be true for our case. It checks the IOU report from the transaction reportID, but in the split case, the report ID is a constant of -2 because split bill doesn't have an IOU report.

App/src/libs/actions/IOU.js

Lines 1418 to 1421 in 8f58898

	const splitTransaction = TransactionUtils.buildOptimisticTransaction(
	amount,
	currency,
	CONST.REPORT.SPLIT_REPORTID,

So, the expense request check is always false, and updating the amount is enough to bypass the merchant field.

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

Instead of relying on the transaction report ID, we can pass the value from the params.

areRequiredFieldsEmpty(transaction, isFromExpenseReport) // or isPolicyExpenseChat

and we can pass props.isPolicyExpenseChat as the value.

But to prevent big changes, we can keep the IOU parent report type check and the new param, so we will rely on 2 values, with the new param as the fallback value.

else, we can pass anything optional like report ID that can be used to detect whether it's an expense report or not

What alternative solutions did you explore? (Optional)

Currently, the transaction only contains a reportID, that is the IOU report ID. To make it easier, I think it might be good if we add a chatReportID. But this requires BE changes too.

Or

We can check the transaction splits participants and check if there is a policy expense chat. This way, we don't need to add a new param.

const isPolicyExpenseChat = transaction.comment.splits?.some(participant => allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${participant.chatReportID}`]?.isOwnPolicyExpenseChat)

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

❌ There was an error making the offer to @mkhutornyi for the Contributor role. The BZ member will need to manually hire the contributor.

[mkhutornyi]

@bernhardoj what is the offending PR?

[mkhutornyi]

This was caused by #37393

[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.48-0 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix able to confirm split a scan request in policy expense chat even though the merchant is empty #37740

If no regressions arise, payment will be issued on 2024-03-14. 🎊

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

• @allroundexperts requires payment through NewDot Manual Requests
• @bernhardoj requires payment (Needs manual offer from BZ)

[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:

• [@allroundexperts] The PR that introduced the bug has been identified. Link to the PR:
• [@allroundexperts] 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:
• [@allroundexperts] 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:
• [@allroundexperts] Determine if we should create a regression test for this bug.
• [@allroundexperts] 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.
• [@laurenreidexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

Payment Summary

Upwork Job

• Reviewer: @allroundexperts owed $500 via NewDot
• ROLE: @bernhardoj paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@laurenreidexpensify)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1764959862414594048/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[laurenreidexpensify]

Payment Summary
Upwork Job

Reviewer: @allroundexperts owed $500 via NewDot
Contributor: waiting for @bernhardoj to accept Upwork job $500 for payment

[techievivek]

Not overdue, finishing up the payment ^

[laurenreidexpensify]

Still waiting on @bernhardoj to accept offer in upwork 👍

[bernhardoj]

Applied to the job

[laurenreidexpensify]

@bernhardoj the offer is still showing as Pending in Upwork - can you confirm you've accepted - Upwork Job

[bernhardoj]

It shows I already submitted the proposal.

[laurenreidexpensify]

Payment Summary
Upwork Job

Reviewer: @allroundexperts owed $500 via NewDot
Contributor: @bernhardoj paid via Upwork $500

[laurenreidexpensify]

Final Steps: @allroundexperts pls complete checklist thanks

[allroundexperts]

Checklist

1. I was not able to pinpoint the exact PR which caused this. As discussed above, TransactionUtils.areRequiredFieldsEmpty seemed to be borked from the beginning.
2. N/A
3. N/A
4. A regression test would be helpful here.

Regression test steps

1. Go to staging.new.expensify.com
2. Go to workspace chat.
3. Go to + > Split scan.
4. Create a split scan request.
5. Click on the split scan preview and enter amount and save it.
6. Click Split and verify that the App will prevent the bill from splitting because merchant is not entered.

Do we 👍 or 👎 ?

[JmillsExpensify]

$500 approved for @allroundexperts.