Skip to content

refactor: move per diem expense navigation into the UI #99525

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

Merged
Valforte merged 6 commits into from
Aug 28, 2026
+181 −56
Merged

refactor: move per diem expense navigation into the UI #99525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
00e3504
refactor: move per diem expense navigation into the UI
TaduJR Aug 24, 2026
5f3a448
Merge branch 'main' of https://github.com/TaduJR/App into refactor-co…
TaduJR Aug 26, 2026
40b8a79
chore: Split per diem predicate comment into two sentences
TaduJR Aug 26, 2026
ac78106
Merge branch 'main' of https://github.com/TaduJR/App into refactor-co…
TaduJR Aug 27, 2026
1dfb8a0
chore: Log an alert when a per diem submit is skipped by the UI guard
TaduJR Aug 27, 2026
9c9ef7a
Merge branch 'main' of https://github.com/TaduJR/App into refactor-co…
TaduJR Aug 28, 2026
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
48 changes: 20 additions & 28 deletions src/libs/actions/IOU/PerDiem.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ import {deferOrExecuteWrite} from '@libs/deferredLayoutWrite';
import {getMicroSecondOnyxErrorWithTranslationKey} from '@libs/ErrorUtils';
import {updateIOUOwnerAndTotal} from '@libs/IOUUtils';
import {validateAmount} from '@libs/MoneyRequestUtils';
import Navigation from '@libs/Navigation/Navigation';
import TransitionTracker from '@libs/Navigation/TransitionTracker';
import {buildOptimisticNextStep} from '@libs/NextStepUtils';
import * as NumberUtils from '@libs/NumberUtils';
import {addSMSDomainIfPhoneNumber} from '@libs/PhoneNumber';
Expand Down Expand Up @@ -43,7 +41,6 @@ import {buildOptimisticTransaction} from '@libs/TransactionUtils';

import {buildOptimisticPolicyRecentlyUsedTags} from '@userActions/Policy/Tag';
import {notifyNewAction} from '@userActions/Report';
import {removeDraftTransaction} from '@userActions/TransactionEdit';

import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
Expand Down Expand Up @@ -202,6 +199,20 @@ function isValidPerDiemExpenseAmount(customUnit: TransactionCustomUnit, decimals
return validateAmount(perDiemAmountString, decimals, undefined, true);
}

type CompletePerDiemCustomUnit = TransactionCustomUnit & Required<Pick<TransactionCustomUnit, 'customUnitID' | 'customUnitRateID' | 'attributes'>>;

/** Shared so the UI can gate cleanup/nav on the same check the actions bail on. The type-predicate form narrows `customUnit` for the builders. */
Comment thread
TaduJR marked this conversation as resolved.
function hasCompletePerDiemCustomUnit(customUnit: TransactionCustomUnit | undefined): customUnit is CompletePerDiemCustomUnit {
Comment thread
TaduJR marked this conversation as resolved.
return (
!!customUnit &&
!isEmptyObject(customUnit) &&
!!customUnit.customUnitID &&
!!customUnit.customUnitRateID &&
(customUnit.subRates ?? []).length > 0 &&
!isEmptyObject(customUnit.attributes)
);
}

function computeDefaultPerDiemExpenseComment(customUnit: TransactionCustomUnit, currency: string) {
const subRates = customUnit.subRates ?? [];
const subRateComments = subRates.map((subRate) => {
Expand Down Expand Up @@ -247,6 +258,7 @@ type PerDiemExpenseInformation = {
shouldDeferAutoSubmit?: boolean;
optimisticChatReportID?: string;
optimisticTransactionID?: string;
notifyReportID?: string;
formatPhoneNumber: LocaleContextProps['formatPhoneNumber'];
delegateAccountID: number | undefined;
isTrackIntentUser: boolean | undefined;
Expand Down Expand Up @@ -1006,6 +1018,7 @@ function submitPerDiemExpense(submitPerDiemExpenseInformation: PerDiemExpenseInf
shouldDeferAutoSubmit,
optimisticChatReportID,
optimisticTransactionID,
notifyReportID,
formatPhoneNumber,
delegateAccountID,
isTrackIntentUser,
Expand All @@ -1014,14 +1027,7 @@ function submitPerDiemExpense(submitPerDiemExpenseInformation: PerDiemExpenseInf
} = submitPerDiemExpenseInformation;
const {currency, comment = '', category, tag, created, customUnit, attendees, isFromGlobalCreate} = transactionParams;

if (
isEmptyObject(policyParams.policy) ||
isEmptyObject(customUnit) ||
!customUnit.customUnitID ||
!customUnit.customUnitRateID ||
(customUnit.subRates ?? []).length === 0 ||
isEmptyObject(customUnit.attributes)
) {
if (isEmptyObject(policyParams.policy) || !hasCompletePerDiemCustomUnit(customUnit)) {
return;
}

Expand Down Expand Up @@ -1070,8 +1076,6 @@ function submitPerDiemExpense(submitPerDiemExpenseInformation: PerDiemExpenseInf
getCurrencyDecimals,
});

const activeReportID = isMoneyRequestReport && Navigation.getTopmostReportId() === report?.reportID ? report?.reportID : chatReport.reportID;

const customUnitRate = getPerDiemRateCustomUnitRate(policyParams.policy, customUnit.customUnitRateID);

const customUnitRateParam = {
Expand Down Expand Up @@ -1122,13 +1126,9 @@ function submitPerDiemExpense(submitPerDiemExpenseInformation: PerDiemExpenseInf
onDeferred: () => addOptimization(CONST.TELEMETRY.SUBMIT_OPTIMIZATION.DEFERRED_WRITE),
});

TransitionTracker.runAfterTransitions({callback: () => removeDraftTransaction(CONST.IOU.OPTIMISTIC_TRANSACTION_ID), waitForUpcomingTransition: true});

highlightTransactionOnSearchRouteIfNeeded(isFromGlobalCreate, transaction.transactionID, CONST.SEARCH.DATA_TYPES.EXPENSE);

if (activeReportID) {
notifyNewAction(activeReportID, undefined, participantParams.payeeAccountID === currentUserAccountIDParam);
}
notifyNewAction(notifyReportID ?? chatReport.reportID, undefined, participantParams.payeeAccountID === currentUserAccountIDParam);

return {iouReport, transactionID: transaction.transactionID};
}
Expand All @@ -1152,14 +1152,7 @@ function submitPerDiemExpenseForSelfDM(submitPerDiemExpenseInformation: PerDiemE
} = submitPerDiemExpenseInformation;
const {currency, comment = '', category, tag, created, customUnit, attendees, billable, reimbursable} = transactionParams;

if (
isEmptyObject(policy) ||
isEmptyObject(customUnit) ||
!customUnit.customUnitID ||
!customUnit.customUnitRateID ||
(customUnit.subRates ?? []).length === 0 ||
isEmptyObject(customUnit.attributes)
) {
if (isEmptyObject(policy) || !hasCompletePerDiemCustomUnit(customUnit)) {
return;
}

Expand Down Expand Up @@ -1220,8 +1213,6 @@ function submitPerDiemExpenseForSelfDM(submitPerDiemExpenseInformation: PerDiemE
onDeferred: () => addOptimization(CONST.TELEMETRY.SUBMIT_OPTIMIZATION.DEFERRED_WRITE),
});

TransitionTracker.runAfterTransitions({callback: () => removeDraftTransaction(CONST.IOU.OPTIMISTIC_TRANSACTION_ID), waitForUpcomingTransition: true});

notifyNewAction(chatReport.reportID, undefined, true);
}

Expand All @@ -1232,6 +1223,7 @@ export {
addSubrate,
computePerDiemExpenseAmount,
isValidPerDiemExpenseAmount,
hasCompletePerDiemCustomUnit,
getPerDiemExpensePolicyID,
getPerDiemExpenseInformation,
submitPerDiemExpense,
Expand Down
70 changes: 42 additions & 28 deletions src/pages/iou/request/step/confirmation/useExpenseSubmission.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import cleanupAndNavigateAfterExpenseCreate from '@libs/Navigation/helpers/clean
import dismissModalAndOpenReportInInboxTab from '@libs/Navigation/helpers/dismissModalAndOpenReportInInboxTab';
import isSearchTopmostFullScreenRoute from '@libs/Navigation/helpers/isSearchTopmostFullScreenRoute';
import navigateAfterExpenseCreate from '@libs/Navigation/helpers/navigateAfterExpenseCreate';
import Navigation from '@libs/Navigation/Navigation';
import {rand64, roundToTwoDecimalPlaces} from '@libs/NumberUtils';
import {isTrackOnboardingChoice} from '@libs/OnboardingUtils';
import {getNewAccountIDsAndLogins} from '@libs/PersonalDetailsUtils';
Expand Down Expand Up @@ -61,7 +62,7 @@ import {
import {resolveChatTargetForSubmitCleanup} from '@pages/iou/request/step/resolveChatTarget';

import {isOneToTwoTransactionTransition} from '@userActions/IOU/PendingNewTransactions';
import {getPerDiemExpensePolicyID, submitPerDiemExpenseForSelfDM, submitPerDiemExpense as submitPerDiemExpenseIOUActions} from '@userActions/IOU/PerDiem';
import {getPerDiemExpensePolicyID, hasCompletePerDiemCustomUnit, submitPerDiemExpenseForSelfDM, submitPerDiemExpense as submitPerDiemExpenseIOUActions} from '@userActions/IOU/PerDiem';
import {getReceiverType, sendInvoice} from '@userActions/IOU/SendInvoice';
import {sendMoneyElsewhere, sendMoneyWithWallet} from '@userActions/IOU/SendMoney';
import {createDistanceRequest as createDistanceRequestIOUActions, resolveOptimisticSplitChatReportID, splitBill, splitBillAndOpenReport, startSplitBill} from '@userActions/IOU/Split';
Expand Down Expand Up @@ -606,33 +607,41 @@ function useExpenseSubmission(params: UseExpenseSubmissionParams) {
return;
}
if (isTrackExpense) {
const optimisticChatReportID = selfDMReport?.reportID ?? generateReportID();
submitPerDiemExpenseForSelfDM({
dateFnsLocale,
getCurrencyDecimals,
selfDMReport,
policy,
transactionParams: {
currency: transaction.currency,
created: transaction.created,
comment: trimmedComment,
category: transaction.category,
tag: transaction.tag,
customUnit: transaction.comment?.customUnit,
billable: transaction.billable,
reimbursable: transaction.reimbursable,
attendees: transaction.comment?.attendees,
isFromGlobalCreate: getIsFromGlobalCreate(transaction),
},
currentUserAccountIDParam: currentUserPersonalDetails.accountID,
currentUserEmailParam: currentUserPersonalDetails.login ?? '',
quickAction,
optimisticChatReportID,
delegateAccountID,
isTrackIntentUser,
});
if (shouldHandleNavigation) {
dismissModalAndOpenReportInInboxTab(optimisticChatReportID, false, false);
// Mirror the action's bail: a submit it would no-op must not clean up or dismiss.
if (!isEmptyObject(policy) && hasCompletePerDiemCustomUnit(transaction.comment?.customUnit)) {
Comment thread
TaduJR marked this conversation as resolved.
const optimisticChatReportID = selfDMReport?.reportID ?? generateReportID();
submitPerDiemExpenseForSelfDM({
dateFnsLocale,
getCurrencyDecimals,
selfDMReport,
policy,
transactionParams: {
currency: transaction.currency,
created: transaction.created,
comment: trimmedComment,
category: transaction.category,
tag: transaction.tag,
customUnit: transaction.comment?.customUnit,
billable: transaction.billable,
reimbursable: transaction.reimbursable,
attendees: transaction.comment?.attendees,
isFromGlobalCreate: getIsFromGlobalCreate(transaction),
},
currentUserAccountIDParam: currentUserPersonalDetails.accountID,
currentUserEmailParam: currentUserPersonalDetails.login ?? '',
quickAction,
optimisticChatReportID,
delegateAccountID,
isTrackIntentUser,
});
if (shouldHandleNavigation) {
cleanupAfterExpenseCreate({draftTransactionIDs: [CONST.IOU.OPTIMISTIC_TRANSACTION_ID], shouldWaitForUpcomingTransition: true});
dismissModalAndOpenReportInInboxTab(optimisticChatReportID, false, false);
} else {
cleanupAfterExpenseCreate({draftTransactionIDs: [CONST.IOU.OPTIMISTIC_TRANSACTION_ID]});
}
} else {
Log.alert('[useExpenseSubmission] Skipped per diem self-DM submit: missing policy or incomplete custom unit');

@Valforte Valforte Aug 27, 2026

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.

We should do something on the UI otherwise the user is stuck in the same place with no idea that something is happening. Can we surface that some error has happened?

@TaduJR TaduJR Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If a per diem is missing its subrate (or the amount isn't valid), the confirm screen already blocks the submit and shows the error right there, either "There must be at least one subrate" or "Please enter a valid quantity". I checked it browser. With the subrate removed, tapping Create expense doesn't go through and the error shows. So the user gets feedback, not a silent dead end.

The guard we added only kicks in for a deeper broken state, where the destination or date info is missing even though the earlier steps always fill it in. You can't reach that by clicking through the flow. I could only trigger it by manually corrupting the saved draft. And even then it behaved well. It blocked the submit and kept the draft intact, nothing lost. It just logs instead of showing a second error.

So the reachable cases already show a message, and the guard is a safety net for a state a user can't actually create. If you'd still like extra coverage, we could add a simple "Something went wrong, please try again" whenever a per diem submit gets skipped. Otherwise I'd lean on the check that's already there.

WDYT?

@Valforte Valforte Aug 28, 2026

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.

that's fine then, we can leave it as is

}
} else {
const isExpenseReport = isMoneyRequestReportReportUtils(report);
Expand All @@ -647,6 +656,7 @@ function useExpenseSubmission(params: UseExpenseSubmissionParams) {
existingChatReport,
);
const activeReportID = isExpenseReport ? report?.reportID : chatReportID;
const notifyReportID = isExpenseReport && Navigation.getTopmostReportId() === report?.reportID ? report?.reportID : chatReportID;

const perDiemParticipantParams = {
payeeEmail: currentUserPersonalDetails.login,
Expand Down Expand Up @@ -690,6 +700,7 @@ function useExpenseSubmission(params: UseExpenseSubmissionParams) {
betas,
personalDetails,
optimisticChatReportID,
notifyReportID,
formatPhoneNumber,
delegateAccountID,
isTrackIntentUser,
Expand All @@ -698,6 +709,9 @@ function useExpenseSubmission(params: UseExpenseSubmissionParams) {
// When backToReport exists we are creating the expense from chat, not the expense report, so no pending transaction registration needed.
const isOneToTwoTransition = !backToReport && isOneToTwoTransactionTransition(isMoneyRequestReport, reportTransactions);

if (result) {
cleanupAfterExpenseCreate({draftTransactionIDs: [CONST.IOU.OPTIMISTIC_TRANSACTION_ID], shouldWaitForUpcomingTransition: shouldHandleNavigation});
}
if (result && targetReportID) {
navigateAfterExpenseCreate({
activeReportID: targetReportID,
Expand Down
34 changes: 34 additions & 0 deletions tests/actions/IOU/PerDiemTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
computePerDiemExpenseAmount,
getPerDiemExpenseInformation,
getPerDiemExpensePolicyID,
hasCompletePerDiemCustomUnit,
removeSubrate,
submitPerDiemExpense,
updateSubrate,
Expand Down Expand Up @@ -1167,4 +1168,37 @@ describe('PerDiem', () => {
expect(perDiemTransactions.length).toBeGreaterThan(0);
});
});

describe('hasCompletePerDiemCustomUnit', () => {
const completeCustomUnit: TransactionCustomUnit = {
customUnitID: 'unit-1',
customUnitRateID: 'rate-1',
subRates: [{id: 'sub-1', name: 'Meals', quantity: 1, rate: 2500}],
attributes: {dates: {start: '2026-04-24', end: '2026-04-24'}},
};

it('is true when every required field is present', () => {
expect(hasCompletePerDiemCustomUnit(completeCustomUnit)).toBe(true);
});

it('is false when the custom unit is undefined', () => {
expect(hasCompletePerDiemCustomUnit(undefined)).toBe(false);
});

it('is false when the customUnitID is missing', () => {
expect(hasCompletePerDiemCustomUnit({...completeCustomUnit, customUnitID: undefined})).toBe(false);
});

it('is false when the customUnitRateID is missing', () => {
expect(hasCompletePerDiemCustomUnit({...completeCustomUnit, customUnitRateID: undefined})).toBe(false);
});

it('is false when there are no sub-rates', () => {
expect(hasCompletePerDiemCustomUnit({...completeCustomUnit, subRates: []})).toBe(false);
});

it('is false when the attributes are missing', () => {
expect(hasCompletePerDiemCustomUnit({...completeCustomUnit, attributes: undefined})).toBe(false);
});
});
});
Loading
Loading