[$250] [HOLD for payment 2024-05-06] Group split scan - Error appears for Merchant when clicking Split expense for the second time

[kavimuru]

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.64-0
Reproducible in staging?: y
Reproducible in production?: n
If this was caught during regression testing, add the test name, ID and link from TestRail: n/a
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 FAB > Split expense > Scan.
3. Create split scan expense with a few users.
4. In group chat, click on the split preview.
5. Click Split expense.
6. Click Split expense again.

Expected Result:

There will be no error under Merchant field because Merchant is not a requirement for non-workspace expense.

Actual Result:

Error shows up under Merchant field when clicking Split expense for the second time.

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

Bug6458020_1713821712347.20240423_053140.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~013158f9aa7567f244
• Upwork Job ID: 1790113061570207744
• Last Price Increase: 2024-05-13

Issue Owner

Current Issue Owner: @

[melvin-bot]

Triggered auto assignment to @robertjchen (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[melvin-bot]

Triggered auto assignment to @johncschuster (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.

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

[kavimuru]

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

[kavimuru]

We think this bug might be related to #vip-split

[allgandalf]

Proposal

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

Error appears for merchant even when it is not required for non workspace expenses

What is the root cause of that problem?

This is a regression from #38543, Here we have added a new condition to display merchant error:

App/src/components/MoneyRequestConfirmationList.tsx

Lines 580 to 582 in 64695c8

	if ((isMerchantRequired && isMerchantEmpty) || (shouldDisplayFieldError && TransactionUtils.isMerchantMissing(transaction ?? null))) {
	setMerchantError(true);
	return;

The second part of this condition would return true when we the merchant is empty and shouldDisplayFieldError would be set to true the first time we click on submit button because the amount field is empty.

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

We need to add a extra && check of isMerchantRequired, so that we only set the error to true if the merchant is required:

diff --git a/src/components/MoneyRequestConfirmationList.tsx b/src/components/MoneyRequestConfirmationList.tsx
index 99901dd261..87d605a7a5 100755
@@ -577,7 +577,7 @@ function MoneyRequestConfirmationList({
             if (selectedParticipants.length === 0) {
                 return;
             }
-            if ((isMerchantRequired && isMerchantEmpty) || (shouldDisplayFieldError && TransactionUtils.isMerchantMissing(transaction ?? null))) {
+            if ((isMerchantRequired && isMerchantEmpty) || (isMerchantRequired && shouldDisplayFieldError && TransactionUtils.isMerchantMissing(transaction ?? null))) {
                 setMerchantError(true);
                 return;
             }

What alternative solutions did you explore? (Optional)

[saifelance]

@GandalfGwaihir
We should modify the condition to ensure that the error is set to true only if the merchant is required. So, an additional && check for isMerchantRequired needs to be added. This adjustment ensures that the error is set only when the merchant is both required and empty:

`diff --git a/src/components/MoneyRequestConfirmationList.tsx b/src/components/MoneyRequestConfirmationList.tsx
index 99901dd261..87d605a7a5 100755
@@ -577,7 +577,7 @@ function MoneyRequestConfirmationList({
if (selectedParticipants.length === 0) {
return;
}

•        if ((isMerchantRequired && isMerchantEmpty) || (shouldDisplayFieldError && TransactionUtils.isMerchantMissing(transaction ?? null))) {
  

•        if ((isMerchantRequired && isMerchantEmpty) || (isMerchantRequired && shouldDisplayFieldError && TransactionUtils.isMerchantMissing(transaction ?? null))) {
             setMerchantError(true);
             return;
         }
  

`

[mountiny]

This is minor UX issue, demoting @koko57 seems like its coming from your PR

[koko57]

@mountiny As the follow-up PR wasn't merged yet I can fix it there

[koko57]

@mountiny PR opened: #40868

[mountiny]

LEts just go through upwork until ND is ready for you as it would create a backlog

[melvin-bot]

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

[melvin-bot]

Current assignee @Ollyws is eligible for the External assigner, not assigning anyone new.

[johncschuster]

Applying the External label to create the job. @Ollyws, Upwork isn't returning you when I search for you because... Upwork 🙄 . Can you apply to the job and ping me? I'll get payment issued ASAP!

[johncschuster]

@Ollyws Aha! I found you. Your offer is here. Can you accept that?

Edit: This offer has been withdrawn so you can request via ND.

[johncschuster]

@Ollyws, I see you've just been added to the list of contributors who can accept payment via NewDot. I'll go ahead and post a payment summary instead so you can be paid via that method.

[johncschuster]

Payment Summary

Contributor+: @Ollyws - $250 via Manual Request

[johncschuster]

@robertjchen can you complete the BZ checklist above?

[Ollyws]

Requested in ND, thanks!

[JmillsExpensify]

$250 approved for @Ollyws

[robertjchen]

Working through checklist

[johncschuster]

Bumping this for Melvin. How's the checklist going, Robert?

[robertjchen]

@johncschuster Made my assessment, and left the last item for you 👍

[johncschuster]

Thanks @robertjchen! Looks like we can close this up!