Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Error message only changes after clicking Split expense button for the second time #42450

Merged
merged 6 commits into from
Jun 4, 2024
Merged

fix: Error message only changes after clicking Split expense button for the second time #42450

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
da3c468
fix: Error message only changes after clicking Split expense button f…
nkdengineer May 22, 2024
5560668
fix logic show money request form error
nkdengineer May 22, 2024
9f8b638
Merge branch 'main' into fix/41336
nkdengineer May 30, 2024
11a896d
Update src/components/MoneyRequestConfirmationList.tsx
nkdengineer May 30, 2024
05932cc
Merge branch 'fix/41336' of https://github.com/nkdengineer/App into f…
nkdengineer May 30, 2024
4eaa6c3
fix lint
nkdengineer May 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 18 additions & 19 deletions src/components/MoneyRequestConfirmationList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,14 +346,15 @@ function MoneyRequestConfirmationList({
const isCategoryRequired = !!policy?.requiresCategory;

useEffect(() => {
if (shouldDisplayFieldError && hasSmartScanFailed) {
setFormError('iou.receiptScanningFailed');
return;
}
if (shouldDisplayFieldError && didConfirmSplit) {
setFormError('iou.error.genericSmartscanFailureMessage');
return;
}

if (shouldDisplayFieldError && hasSmartScanFailed) {
setFormError('iou.receiptScanningFailed');
return;
}
// reset the form error whenever the screen gains or loses focus
setFormError('');

Expand Down Expand Up @@ -705,21 +706,7 @@ function MoneyRequestConfirmationList({
setFormError('iou.error.invalidCategoryLength');
return;
}

if (formError) {
return;
}

if (iouType === CONST.IOU.TYPE.PAY) {
if (!paymentMethod) {
return;
}

setDidConfirm(true);

Log.info(`[IOU] Sending money via: ${paymentMethod}`);
onSendMoney?.(paymentMethod);
} else {
if (iouType !== CONST.IOU.TYPE.PAY) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to confirm that we are not doing this by mistake. It is intentional that we don't check formErrors when iouType !== CONST.IOU.TYPE.PAY, right?

We don't need

            if (formError) {
                return;
            }

inside this if. Correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to confirm that we are not doing this by mistake. It is intentional that we don't check formErrors when iouType !== CONST.IOU.TYPE.PAY, right?

@aldo-expensify The block after check formErrors will do nothing if iouType !== CONST.IOU.TYPE.PAY so we don't need to check this for the case iouType !== CONST.IOU.TYPE.PAY.

if (formError) {
    return;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The block after check formErrors will do nothing

Which block is that? I'm missing what you are referring to. I see that we do stuff in iouType !== CONST.IOU.TYPE.PAY even if there are formErrors, is that fine? This code will run:

image

right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which block is that

@aldo-expensify That is here. With the change of this PR we only do this after the check formError

if (iouType === CONST.IOU.TYPE.PAY) {
    if (!paymentMethod) {
        return;
    }

    setDidConfirm(true);

    Log.info(`[IOU] Sending money via: ${paymentMethod}`);
    onSendMoney?.(paymentMethod);
}

I see that we do stuff in iouType !== CONST.IOU.TYPE.PAY even if there are formErrors, is that fine

That is the aim of this PR to fix the issue, we should move the logic to setting the error before the check formError

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, I see what you are saying now, thanks for explaining!

// validate the amount for distance expenses
const decimals = CurrencyUtils.getCurrencyDecimals(iouCurrencyCode);
if (isDistanceRequest && !isDistanceRequestWithPendingRoute && !MoneyRequestUtils.validateAmount(String(iouAmount), decimals)) {
Expand All @@ -736,6 +723,18 @@ function MoneyRequestConfirmationList({
playSound(SOUNDS.DONE);
setDidConfirm(true);
onConfirm?.(selectedParticipants);
} else {
if (formError) {
return;
}
if (!paymentMethod) {
return;
}

setDidConfirm(true);

Log.info(`[IOU] Sending money via: ${paymentMethod}`);
onSendMoney?.(paymentMethod);
}
},
[
Expand Down
Loading