Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/libs/actions/IOU/Split.ts
Original file line number Diff line number Diff line change
Expand Up @@ -948,6 +948,7 @@ function completeSplitBill({
receipt: {
state: CONST.IOU.RECEIPT_STATE.OPEN,
},
iouRequestType: CONST.IOU.REQUEST_TYPE.MANUAL,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

src/libs/actions/IOU/Split.ts:951

🟢 Could we add a short comment here explaining why ? It's not obvious from the code alone why completing a split flips the type. Without context, someone could later "clean up" this line and quietly bring back #102491.

Something like:

                receipt: {
                    state: CONST.IOU.RECEIPT_STATE.OPEN,
                },
                // The user filled the fields in by hand, so the receipt is no longer scanned. Mirror what the server returns
                // so the split details page renders the manual layout while offline instead of waiting for the response.
                iouRequestType: CONST.IOU.REQUEST_TYPE.MANUAL,

},
},
{
Expand Down
79 changes: 79 additions & 0 deletions tests/actions/IOUTest/SplitTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1256,6 +1256,85 @@ describe('split expense', () => {
expect(splitTransaction?.comment?.comment).toBe('<h1>test</h1>');
});

it('should stop treating the split as a scan request once it is completed while offline', async () => {
// Given a scan split bill started offline, so it is marked as a scan request
const reportID = '1';
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {
reportID,
type: CONST.REPORT.TYPE.CHAT,
chatType: CONST.REPORT.CHAT_TYPE.GROUP,
participants: {
[RORY_ACCOUNT_ID]: {notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS},
[CARLOS_ACCOUNT_ID]: {notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS},
},
});

const participants: IOUParticipant[] = [{accountID: CARLOS_ACCOUNT_ID, login: CARLOS_EMAIL}];
const participantsPolicyTags = await getParticipantsPolicyTags(participants);

mockFetch?.pause?.();

startSplitBill({
isFirstSplitInBatch: true,
getCurrencyDecimals: getCurrencyDecimalsLocal,
participants,
currentUserLogin: RORY_EMAIL,
currentUserAccountID: RORY_ACCOUNT_ID,
comment: '',
currency: CONST.CURRENCY.USD,
existingSplitChatReportID: reportID,
receipt: {source: 'file://receipt.jpg', filename: 'receipt.jpg', state: CONST.IOU.RECEIPT_STATE.SCAN_READY},
category: undefined,
tag: undefined,
taxCode: '',
taxAmount: 0,
quickAction: undefined,
policyRecentlyUsedCurrencies: [],
policyRecentlyUsedTags: undefined,
participantsPolicyTags,
delegateAccountID: undefined,
formatPhoneNumber,
});

await waitForBatchedUpdates();

let splitTransaction = await getScanSplitTransaction();
const splitTransactionID = splitTransaction?.transactionID;
expect(splitTransaction?.iouRequestType).toBe(CONST.IOU.REQUEST_TYPE.SCAN);

const reportActions = await getOnyxValue(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`);
const iouAction = Object.values(reportActions ?? {}).find((action) => isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.IOU));

// When the user fills the fields in by hand and completes the split while still offline
completeSplitBill({
isVendorMatchingBetaEnabled: false,
getCurrencyDecimals: getCurrencyDecimalsLocal,
chatReportID: reportID,
reportAction: iouAction,
updatedTransaction: splitTransaction ? {...splitTransaction, amount: 100, modifiedAmount: 100, modifiedCurrency: CONST.CURRENCY.USD} : undefined,
sessionAccountID: RORY_ACCOUNT_ID,
isASAPSubmitBetaEnabled: false,
quickAction: undefined,
transactionViolations: {},
personalDetails: mockPersonalDetails,
delegateAccountID: undefined,
isTrackIntentUser: false,
sessionEmail: RORY_EMAIL,
formatPhoneNumber,
rules: undefined,
});

await waitForBatchedUpdates();

// Then the optimistic transaction is no longer a scan request, so the split details page renders the
// manual layout right away instead of waiting for the server response to arrive
splitTransaction = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION}${splitTransactionID}`);
expect(splitTransaction?.iouRequestType).toBe(CONST.IOU.REQUEST_TYPE.MANUAL);

mockFetch?.resume?.();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

tests/actions/IOUTest/SplitTest.ts:1334

🟢 The fix depends on failureData spreading ...unmodifiedTransaction to put iouRequestType: SCAN back if CompleteSplitBill fails. Right now nothing tests that path.

Can we fail the request here and check that it rolls back ? Then a future change to failureData (for example, narrowing the spread to a few fields) can't leave a failed split stuck in the manual layout with an RBR:

        mockFetch?.fail?.();
        await mockFetch?.resume?.();
        await waitForBatchedUpdates();

        // Then the split goes back to being a scan request so the user can retry from the original state
        splitTransaction = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION}${splitTransactionID}`);
        expect(splitTransaction?.iouRequestType).toBe(CONST.IOU.REQUEST_TYPE.SCAN);
        expect(splitTransaction?.errors).toBeTruthy();

(This also awaits resume() the way the other tests in this file do.)

await waitForBatchedUpdates();
});

it('should calculate proportional convertedAmount for split transactions with foreign currency', async () => {
jest.setTimeout(10 * 1000);

Expand Down
Loading