[HOLD for payment 2024-11-14] [$250] QAB - Error shows up when submitting split distance expense via split manual shortcut in QAB

[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: 9.0.48-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to group chat (more than 2).
3. Click + > Split expense > Manual.
4. Split a manual expense in the group chat.
5. Click FAB.
6. Click Split manual under Quick action.
7. Click Distance.
8. Submit a split distance expense.

Expected Result:

No error will show up when submitting split distance expense via split manual shortcut in QAB.

Actual Result:

Error shows up when submitting split distance expense via split manual shortcut in QAB.

Workaround:

Unknown

Platforms:

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6633315_1728815023833.20241013_182042.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021845889937723990662
• Upwork Job ID: 1845889937723990662
• Last Price Increase: 2024-10-21
• Automatic offers:
• Krishna2323 | Contributor | 104666501

Issue Owner

Current Issue Owner: @slafortune

[melvin-bot]

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

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

[Krishna2323]

Edited by proposal-police: This proposal was edited at 2024-10-23 09:58:53 UTC.

Proposal


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

QAB - Error shows up when submitting split distance expense via split manual shortcut in QAB

What is the root cause of that problem?

• The confirmation step is skipped when the request is of any type but the distance request calculations happens on the confirmation.
  

  App/src/pages/iou/request/step/IOURequestStepDistance.tsx

  Lines 248 to 325 in e4969b2

  	if (shouldSkipConfirmation) {
  	if (iouType === CONST.IOU.TYPE.SPLIT) {
  	IOU.splitBill({
  	participants,
  	currentUserLogin: currentUserPersonalDetails.login ?? '',
  	currentUserAccountID: currentUserPersonalDetails.accountID,
  	amount: 0,
  	comment: '',
  	currency: transaction?.currency ?? 'USD',
  	merchant: translate('iou.fieldPending'),
  	created: transaction?.created ?? '',
  	category: '',
  	tag: '',
  	billable: false,
  	iouRequestType,
  	existingSplitChatReportID: report?.reportID,
  	});
  	return;
  	}
  	IOU.setMoneyRequestPendingFields(transactionID, {waypoints: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD});
  	IOU.setMoneyRequestMerchant(transactionID, translate('iou.fieldPending'), false);
  	const participant = participants.at(0);
  	if (iouType === CONST.IOU.TYPE.TRACK && participant) {
  	IOU.trackExpense(
  	report,
  	0,
  	transaction?.currency ?? 'USD',
  	transaction?.created ?? '',
  	translate('iou.fieldPending'),
  	currentUserPersonalDetails.login,
  	currentUserPersonalDetails.accountID,
  	participant,
  	'',
  	{},
  	'',
  	'',
  	'',
  	0,
  	false,
  	policy,
  	undefined,
  	undefined,
  	undefined,
  	TransactionUtils.getValidWaypoints(waypoints, true),
  	undefined,
  	undefined,
  	undefined,
  	undefined,
  	customUnitRateID,
  	);
  	return;
  	}
  	IOU.createDistanceRequest(
  	report,
  	participants,
  	'',
  	transaction?.created ?? '',
  	'',
  	'',
  	'',
  	0,
  	0,
  	transaction?.currency ?? 'USD',
  	translate('iou.fieldPending'),
  	!!policy?.defaultBillable,
  	TransactionUtils.getValidWaypoints(waypoints, true),
  	undefined,
  	undefined,
  	undefined,
  	DistanceRequestUtils.getCustomUnitRateID(report.reportID),
  	currentUserPersonalDetails.login ?? '',
  	currentUserPersonalDetails.accountID,
  	transaction?.splitShares,
  	iouType,
  	);
  	return;
  	}

• The calculations happening on the confirmation page:
  

  App/src/components/MoneyRequestConfirmationList.tsx

  Lines 340 to 365 in f251dbc

  	useEffect(() => {
  	if (isFirstUpdatedDistanceAmount.current) {
  	return;
  	}
  	if (!isDistanceRequest) {
  	return;
  	}
  	const amount = DistanceRequestUtils.getDistanceRequestAmount(distance, unit ?? CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES, rate ?? 0);
  	IOU.setMoneyRequestAmount(transactionID, amount, currency ?? '');
  	isFirstUpdatedDistanceAmount.current = true;
  	}, [distance, rate, unit, transactionID, currency, isDistanceRequest]);
  	useEffect(() => {
  	if (!shouldCalculateDistanceAmount) {
  	return;
  	}
  	const amount = distanceRequestAmount;
  	IOU.setMoneyRequestAmount(transactionID, amount, currency ?? '');
  	// If it's a split request among individuals, set the split shares
  	const participantAccountIDs: number[] = selectedParticipantsProp.map((participant) => participant.accountID ?? -1);
  	if (isTypeSplit && !isPolicyExpenseChat && amount && transaction?.currency) {
  	IOU.setSplitShares(transaction, amount, currency, participantAccountIDs);
  	}
  	}, [shouldCalculateDistanceAmount, distanceRequestAmount, transactionID, currency, isTypeSplit, isPolicyExpenseChat, selectedParticipantsProp, transaction]);

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


• We should not skip the confirmation when iouType === CONST.IOU.TYPE.SPLIT is true. if (shouldSkipConfirmation && iouType !== CONST.IOU.TYPE.SPLIT) {.
• We should also remove this part.
• OPTIONAL: We can add the split request check here.

What alternative solutions did you explore? (Optional)

Result

[twilight2294]

Edited by proposal-police: This proposal was edited at 2024-10-21 17:06:08 UTC.

Proposal

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

For distance request via split expense for quick actions, we directly create split request instead of distance request and the BE throws error

What is the root cause of that problem?

When we create a manual split request, and then from quick actions create a distance request due to shouldSkipConfirmation being true, we create a split request for distance instead of a distance request:

App/src/pages/iou/request/step/IOURequestStepDistance.tsx

Lines 247 to 251 in e4969b2

	});
	if (shouldSkipConfirmation) {
	if (iouType === CONST.IOU.TYPE.SPLIT) {
	IOU.splitBill({
	participants,

Which in turn gives us API error:

This is because shouldSkipConfirmation is true For quick button actions, we'll skip the confirmation page unless the report is archived or this is a workspace:

App/src/pages/iou/request/step/IOURequestStepDistance.tsx

Lines 109 to 110 in e4969b2

	// For quick button actions, we'll skip the confirmation page unless the report is archived or this is a workspace
	// request and the workspace requires a category or a tag

So we never considered the case for group chats, and hence we skip the confirmation step and call the wrong api and get error.

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

Current shouldSkipConfirmation doesn't consider the case for group chats, so we should add a check that we should not skip the confirmation step when we are on group chat:

App/src/pages/iou/request/step/IOURequestStepDistance.tsx

Lines 111 to 119 in e4969b2

	const shouldSkipConfirmation: boolean = useMemo(() => {
	if (!skipConfirmation || !report?.reportID) {
	return false;
	}
	return (
	!ReportUtils.isArchivedRoom(report, reportNameValuePairs) && !(ReportUtils.isPolicyExpenseChat(report) && ((policy?.requiresCategory ?? false) || (policy?.requiresTag ?? false)))
	);
	}, [report, skipConfirmation, policy, reportNameValuePairs]);

We should update it to:

--- a/src/pages/iou/request/step/IOURequestStepDistance.tsx
+++ b/src/pages/iou/request/step/IOURequestStepDistance.tsx
@@ -114,9 +114,10 @@ function IOURequestStepDistance({
    const shouldSkipConfirmation: boolean = useMemo(() => {
        if (!skipConfirmation || !report?.reportID) {
            return false;
        }
 
         return (
-            !ReportUtils.isArchivedRoom(report, reportNameValuePairs) && !(ReportUtils.isPolicyExpenseChat(report) && ((policy?.requiresCategory ?? false) || (policy?.requiresTag ?? false)))
+            !ReportUtils.isGroupChat(report) && !ReportUtils.isArchivedRoom(report, reportNameValuePairs) && !(ReportUtils.isPolicyExpenseChat(report) && ((policy?.requiresCategory ?? false) || (policy?.requiresTag ?? false)))
         );
     }, [report, skipConfirmation, policy, reportNameValuePairs]);

Result Video

Screen.Recording.2024-10-14.at.11.51.02.AM.mov

Similar to #47021. The correct solution is to avoid shouldSkipConfirmation=true when moving to a distance request.

What alternative solutions did you explore? (Optional)

We should not skip the confirmation through QAB for split expenses, so we should update the following code and set shouldSkipConfirmation to false:

App/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.tsx

Lines 259 to 261 in e4969b2

	case CONST.QUICK_ACTIONS.SPLIT_MANUAL:
	selectOption(() => IOU.startMoneyRequest(CONST.IOU.TYPE.SPLIT, quickActionReportID, CONST.IOU.REQUEST_TYPE.MANUAL, true), true);
	return;

App/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.tsx

Lines 262 to 264 in e4969b2

	case CONST.QUICK_ACTIONS.SPLIT_SCAN:
	selectOption(() => IOU.startMoneyRequest(CONST.IOU.TYPE.SPLIT, quickActionReportID, CONST.IOU.REQUEST_TYPE.SCAN, true), true);
	return;

            case CONST.QUICK_ACTIONS.SPLIT_MANUAL:
                selectOption(() => IOU.startMoneyRequest(CONST.IOU.TYPE.SPLIT, quickActionReportID, CONST.IOU.REQUEST_TYPE.MANUAL, false), true);
                return;
             case CONST.QUICK_ACTIONS.SPLIT_SCAN:
                selectOption(() => IOU.startMoneyRequest(CONST.IOU.TYPE.SPLIT, quickActionReportID, CONST.IOU.REQUEST_TYPE.SCAN, false), true);
                return;

Alternative solution 2:

Like we do on QAB split manual request page, there too shouldSkipConfirmation is true still we do not skip confirmation page because we have the following check that only calls IOU.splitBill when it is a scan request and defaults to confirmation page for manual request:

App/src/pages/iou/request/step/IOURequestStepAmount.tsx

Lines 195 to 199 in e4969b2

	if (shouldSkipConfirmation) {
	// Only skip confirmation when the split is not configurable, for now Smartscanned splits cannot be configured
	if (iouType === CONST.IOU.TYPE.SPLIT && transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.SCAN) {
	IOU.splitBill({
	participants,

We will do the same for distance page as well, update the following:

App/src/pages/iou/request/step/IOURequestStepDistance.tsx

Lines 248 to 250 in e4969b2

	if (shouldSkipConfirmation) {
	if (iouType === CONST.IOU.TYPE.SPLIT) {
	IOU.splitBill({

To:

            if (shouldSkipConfirmation) {
                // Only skip confirmation when the split is not configurable, for now Smartscanned splits cannot be configured
                if (iouType === CONST.IOU.TYPE.SPLIT && transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.SCAN) {
                    IOU.splitBill({

OR

            if (shouldSkipConfirmation && transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.SCAN) {
                // Only skip confirmation when the split is not configurable, for now Smartscanned splits cannot be configured
                if (iouType === CONST.IOU.TYPE.SPLIT) {
                    IOU.splitBill({

[twilight2294]

@slafortune , you need to be on P2P distance beta to be able to reproduce this bug, let me know if you are not able to reproduce

[slafortune]

Was able to recreate 👍

[melvin-bot]

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

[melvin-bot]

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

[neil-marcellini]

@Krishna2323 and @twilight294, would you please update your root cause and suggested solutions to be a bit more specific?

For @Krishna2323's proposal:
What distance request calculations happen on the confirmation page that shouldn't be skipped? Why were we skipping the confirmation before and why should we no longer skip it?

For @twilight294:

When we create a manual split request, and then from quick actions create a distance request due to shouldSkipConfirmation being true, we create a split request for distance instead of a distance request:

Ok, that makes sense that it breaks when calling SplitBill. Shouldn't we change the type when distance is used instead of leaving it as manual? Why is the solution to not skip the confirmation for group chats? What exact problem does that solve?

Can we not skip confirmation specifically for distance expenses, or can we make it work while skipping confirmation. It would definitely be good to figure out why we are skipping the confirmation page in the first place.

[twilight2294]

. Shouldn't we change the type when distance is used instead of leaving it as manual?

We are still making a split expense itself in distance request , if you create it via split expense then you can see that we call distance request itself. it is only in QAB that we make split request wrongly,

Why is the solution to not skip the confirmation for group chats?

That is because even when it is QAB, we still need confirmation screen to see the exact split for each person and adjust accordingly

It would definitely be good to figure out why we are skipping the confirmation page in the first place.

We are doing it wrong for QAB, it works fine for normal distance split requests

If you check for manual split, there also we do not skip confirmation so we missed the case when we have distance request

[twilight2294]

@neil-marcellini do let me know if you have more doubts, my solution solves this bug correctly by calling distance request api here

Even for the first distance request via split, we call the distance api and not split:

Screen.Recording.2024-10-17.at.1.39.26.AM.mov

[twilight2294]

bump for review @mollfpr @neil-marcellini

[mollfpr]

Sorry for the late response. I'm reviewing it now.

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[mollfpr]

Can we not skip confirmation specifically for distance expenses, or can we make it work while skipping confirmation. It would definitely be good to figure out why we are skipping the confirmation page in the first place.

We don't skip confirmation on QAB split distance, because the shouldSkipConfirmation is retained false.

Screen.Recording.2024-10-21.at.20.36.28.mp4

The issue is when on QAB split expense, the shouldSkipConfirmation is true, similar to #47021. I think the correct solution is to avoid shouldSkipConfirmation=true when moving to a distance request.

[twilight2294]

Can you check please, in my 2nd solution, for QAB split manual request we have the check in place:

App/src/pages/iou/request/step/IOURequestStepAmount.tsx

Lines 195 to 199 in e4969b2

	if (shouldSkipConfirmation) {
	// Only skip confirmation when the split is not configurable, for now Smartscanned splits cannot be configured
	if (iouType === CONST.IOU.TYPE.SPLIT && transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.SCAN) {
	IOU.splitBill({
	participants,

In QAB manual we have set that shouldSkipConfirmation to true:

App/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.tsx

Lines 259 to 261 in e4969b2

	case CONST.QUICK_ACTIONS.SPLIT_MANUAL:
	selectOption(() => IOU.startMoneyRequest(CONST.IOU.TYPE.SPLIT, quickActionReportID, CONST.IOU.REQUEST_TYPE.MANUAL, true), true);
	return;

Still we do not skip confirmation because we have the check iouType === CONST.IOU.TYPE.SPLIT && transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.SCAN in place, similarly we should do it for distance page as well.

[mollfpr]

transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.SCAN

What's the check for? So there's a possibility that the IOU.splitBill is still working, could you show us the step?

[melvin-bot]

@slafortune @mollfpr @neil-marcellini this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[mollfpr]

Friendly bump @neil-marcellini for final thoughts #50694 (comment)

[neil-marcellini]

I agree with what @mollfpr said and I'd like to go with @Krishna2323's proposal. Thanks for your effort as well @twilight2294.

[melvin-bot]

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

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

• fix: QAB - Error shows up when submitting split distance expense via split manual shortcut in QAB. #51931

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

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

• @mollfpr requires payment through NewDot Manual Requests
• @Krishna2323 requires payment automatic offer (Contributor)

[melvin-bot]

@mollfpr @slafortune The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]