diff --git a/src/components/MoneyRequestConfirmationList/DistanceRequestController.tsx b/src/components/MoneyRequestConfirmationList/DistanceRequestController.tsx index cf10c4d85431..f02830b2317b 100644 --- a/src/components/MoneyRequestConfirmationList/DistanceRequestController.tsx +++ b/src/components/MoneyRequestConfirmationList/DistanceRequestController.tsx @@ -23,6 +23,7 @@ import ONYXKEYS from '@src/ONYXKEYS'; import type {Policy, Transaction} from '@src/types/onyx'; import type {Participant} from '@src/types/onyx/IOU'; import type {Unit} from '@src/types/onyx/Policy'; +import {isEmptyObject} from '@src/types/utils/EmptyObject'; import type {OnyxEntry} from 'react-native-onyx'; @@ -100,11 +101,24 @@ function DistanceRequestController({ // We want this effect to run when the transaction is moving from Self DM to an expense chat, or when the policy changes const isPolicyChanged = prevPolicy?.id !== policy?.id; const didSwitchPolicy = !!prevPolicy?.id && prevPolicy.id !== policy?.id; - if (!transactionID || !isDistanceRequest || !isPolicyExpenseChat || (!isMovingTransactionFromTrackExpense && !isPolicyChanged)) { + const errorKey = 'iou.error.invalidRate'; + + if (!transactionID || !isDistanceRequest) { + return; + } + + // Moving the expense back to the self DM (or to a P2P recipient) leaves no workspace to validate against, so a + // rate error raised for the workspace it just left no longer applies. It has to be cleared here: every branch + // below is workspace-specific, so nothing else would ever take the message off the screen. + if (!isPolicyExpenseChat) { + clearFormErrors([errorKey]); + return; + } + + if (!isMovingTransactionFromTrackExpense && !isPolicyChanged) { return; } - const errorKey = 'iou.error.invalidRate'; const policyRates = DistanceRequestUtils.getMileageRates(policy); if (didSwitchPolicy && transaction?.comment?.customUnit?.rateAutoUpdated) { @@ -125,6 +139,13 @@ function DistanceRequestController({ return; } + // The workspace's custom units can still be loading at this point: selecting a participant resolves the new + // policy before Onyx has its rates, so validating now would flash an error that clears itself a moment later. + // With no rates to compare against we cannot tell whether the selected rate is valid - wait for the next run. + if (isEmptyObject(policyRates)) { + return; + } + // If none of the above conditions are met, display the rate error setFormError(errorKey); }, [ diff --git a/src/hooks/usePolicyForTransaction.ts b/src/hooks/usePolicyForTransaction.ts index 335ae1790ab1..15ef22041b06 100644 --- a/src/hooks/usePolicyForTransaction.ts +++ b/src/hooks/usePolicyForTransaction.ts @@ -1,6 +1,8 @@ +import {getSelectedWorkspacePolicyID} from '@libs/IOUUtils'; import {getPolicyByCustomUnitID} from '@libs/PolicyUtils'; import {isExpenseUnreported} from '@libs/TransactionUtils'; +import type {IOUAction} from '@src/CONST'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type {Policy, Transaction} from '@src/types/onyx'; @@ -18,7 +20,7 @@ type UsePolicyForTransactionParams = { reportPolicyID: string | undefined; /** The current action being performed */ - action: string; + action: IOUAction; /** The type of IOU (split, track, submit, etc.) */ iouType: string; @@ -46,11 +48,17 @@ function usePolicyForTransaction({ const [customUnitPolicy] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {selector: (policies: OnyxCollection) => getPolicyByCustomUnitID(transaction, policies)}); - const [reportPolicy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${reportPolicyID}`); + // The route report can lag behind the workspace the user actually picked. The in-place "To" picker on the + // confirmation page rewrites the transaction participants but leaves the route on the report the flow started + // from - the self-DM, whose policyID is the '_FAKE_' placeholder. Resolving the picked workspace here means + // every step page reached from the confirmation reads the same policy, instead of each one re-deriving it. + const resolvedPolicyID = getSelectedWorkspacePolicyID(transaction, action) ?? reportPolicyID; + + const [reportPolicy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${resolvedPolicyID}`); // Fall back to the draft policy from Onyx so callers that don't explicitly pass one still resolve a // freshly created draft workspace (e.g. "Submit to my employer" with no existing workspace). Real // policies always take precedence below, so this only kicks in while the workspace is still a draft. - const [policyDraftFromOnyx] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_DRAFTS}${reportPolicyID}`); + const [policyDraftFromOnyx] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_DRAFTS}${resolvedPolicyID}`); const policyDraft = policyDraftProp ?? policyDraftFromOnyx; const isUnreportedExpense = isExpenseUnreported(transaction); diff --git a/src/pages/iou/request/step/IOURequestStepConfirmation.tsx b/src/pages/iou/request/step/IOURequestStepConfirmation.tsx index c46ca02e2a5a..9b2d17093178 100644 --- a/src/pages/iou/request/step/IOURequestStepConfirmation.tsx +++ b/src/pages/iou/request/step/IOURequestStepConfirmation.tsx @@ -27,6 +27,7 @@ import useOnyx from '@hooks/useOnyx'; import useOptimisticDraftTransactions from '@hooks/useOptimisticDraftTransactions'; import useParticipantsPolicies from '@hooks/useParticipantsPolicies'; import usePersonalPolicy from '@hooks/usePersonalPolicy'; +import usePolicyForMovingExpenses from '@hooks/usePolicyForMovingExpenses'; import usePolicyForTransaction from '@hooks/usePolicyForTransaction'; import usePreMountDestination from '@hooks/usePreMountDestination'; import usePrivateIsArchivedMap from '@hooks/usePrivateIsArchivedMap'; @@ -254,6 +255,7 @@ function IOURequestStepConfirmationContent({ const isTimeRequest = requestType === CONST.IOU.REQUEST_TYPE.TIME; const [lastLocationPermissionPrompt] = useOnyx(ONYXKEYS.NVP_LAST_LOCATION_PERMISSION_PROMPT); const [lastSelectedDistanceRates] = useOnyx(ONYXKEYS.NVP_LAST_SELECTED_DISTANCE_RATES); + const {policyForMovingExpenses} = usePolicyForMovingExpenses(); const [introSelected] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED); const isLookingAroundUser = isLookingAroundSearchRoutingActive(introSelected?.choice === CONST.ONBOARDING_CHOICES.LOOKING_AROUND, isOffline); const privateIsArchivedMap = usePrivateIsArchivedMap(); @@ -440,6 +442,29 @@ function IOURequestStepConfirmationContent({ if (shouldKeepOnSelfDM) { setMoneyRequestParticipantsFromReport(activeTransactionID, selfDMReport, currentUserPersonalDetails.accountID); setTransactionReport(activeTransactionID, {reportID: CONST.REPORT.UNREPORTED_REPORT_ID}, true); + + // The rate the expense picked up from a workspace does not exist outside it, so leaving it in place + // makes the Rate field read "Pending..." and the amount go blank once the expense is back on the self + // DM. Re-resolve the rate the self DM itself uses, the same way starting a track distance expense does. + if (isDistanceRequest) { + const selfDMRateID = DistanceRequestUtils.getCustomUnitRateID({ + reportID: selfDMReport?.reportID, + isPolicyExpenseChat: false, + isTrackDistanceExpense: true, + policy: policyForMovingExpenses, + lastSelectedDistanceRates, + expenseDate: transaction?.created, + }); + setCustomUnitRateID( + activeTransactionID, + selfDMRateID, + transaction, + policyForMovingExpenses, + false, + policyForMovingExpenses?.outputCurrency ?? personalPolicy?.outputCurrency, + ); + } + if (iouType !== CONST.IOU.TYPE.TRACK) { navigation.setParams({iouType: CONST.IOU.TYPE.TRACK}); } @@ -527,6 +552,7 @@ function IOURequestStepConfirmationContent({ blockDistanceRequestIfNeeded, getCurrencyDecimals, policyID, + policyForMovingExpenses, ], ); diff --git a/tests/unit/components/MoneyRequestConfirmationList/DistanceRequestController.test.tsx b/tests/unit/components/MoneyRequestConfirmationList/DistanceRequestController.test.tsx index 0db2b1a58cf0..c4f497cd5e9e 100644 --- a/tests/unit/components/MoneyRequestConfirmationList/DistanceRequestController.test.tsx +++ b/tests/unit/components/MoneyRequestConfirmationList/DistanceRequestController.test.tsx @@ -5,7 +5,7 @@ import DistanceRequestController from '@components/MoneyRequestConfirmationList/ import DistanceRequestUtils from '@libs/DistanceRequestUtils'; import CONST from '@src/CONST'; -import type {Transaction} from '@src/types/onyx'; +import type {Policy, Transaction} from '@src/types/onyx'; import React from 'react'; @@ -116,4 +116,89 @@ describe('DistanceRequestController', () => { }), ); }); + + describe('rate validation when the selected workspace changes', () => { + const RATE_ERROR = 'iou.error.invalidRate'; + + /** A workspace whose distance rates have not arrived from Onyx yet. */ + const policyWithoutRates = createMock({id: 'workspaceB', customUnits: {}}); + + /** A loaded workspace whose only rate matches neither the selected rate ID nor its value/unit (the unit comes from the custom unit attributes). */ + const policyWithUnrelatedRate = createMock({ + id: 'workspaceB', + customUnits: { + unitID: { + customUnitID: 'unitID', + name: CONST.CUSTOM_UNITS.NAME_DISTANCE, + attributes: {unit: CONST.CUSTOM_UNITS.DISTANCE_UNIT_KILOMETERS}, + enabled: true, + rates: { + rateB: {customUnitRateID: 'rateB', rate: 999, currency: CONST.CURRENCY.USD, enabled: true, name: 'Other rate'}, + }, + }, + }, + }); + + const renderController = ({ + policy, + isPolicyExpenseChat, + setFormError, + clearFormErrors, + }: { + policy: Policy | undefined; + isPolicyExpenseChat: boolean; + setFormError: jest.Mock; + clearFormErrors: jest.Mock; + }) => + render( + , + ); + + it('does not flag the rate while the newly selected workspace still has no rates loaded', () => { + const setFormError = jest.fn(); + renderController({policy: policyWithoutRates, isPolicyExpenseChat: true, setFormError, clearFormErrors: jest.fn()}); + + expect(setFormError).not.toHaveBeenCalled(); + }); + + it('still flags the rate once the workspace rates are loaded and none of them match', () => { + const setFormError = jest.fn(); + renderController({policy: policyWithUnrelatedRate, isPolicyExpenseChat: true, setFormError, clearFormErrors: jest.fn()}); + + expect(setFormError).toHaveBeenCalledWith(RATE_ERROR); + }); + + it('clears a workspace rate error once the expense is no longer on a workspace chat', () => { + const clearFormErrors = jest.fn(); + renderController({policy: undefined, isPolicyExpenseChat: false, setFormError: jest.fn(), clearFormErrors}); + + expect(clearFormErrors).toHaveBeenCalledWith([RATE_ERROR]); + }); + }); }); diff --git a/tests/unit/hooks/usePolicyForTransactionTest.tsx b/tests/unit/hooks/usePolicyForTransactionTest.tsx new file mode 100644 index 000000000000..00b3a3e75996 --- /dev/null +++ b/tests/unit/hooks/usePolicyForTransactionTest.tsx @@ -0,0 +1,172 @@ +import {renderHook, waitFor} from '@testing-library/react-native'; + +import OnyxListItemProvider from '@components/OnyxListItemProvider'; + +import usePolicyForTransaction from '@hooks/usePolicyForTransaction'; + +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {Policy, Transaction} from '@src/types/onyx'; + +import React from 'react'; +import Onyx from 'react-native-onyx'; + +import createRandomPolicy from '../../utils/collections/policies'; +import createRandomTransaction from '../../utils/collections/transaction'; +import waitForBatchedUpdates from '../../utils/waitForBatchedUpdates'; + +const WORKSPACE_POLICY_ID = 'workspace-with-distance-rates'; +const OTHER_POLICY_ID = 'some-other-workspace'; +const WORKSPACE_CHAT_REPORT_ID = 'workspace-chat-report'; + +/** createRandomPolicy randomizes type/role/pendingAction, which usePolicyForMovingExpenses filters on - pin them. */ +function createGroupPolicy(index: number, id: string): Policy { + return { + ...createRandomPolicy(index, CONST.POLICY.TYPE.TEAM), + id, + role: CONST.POLICY.ROLE.ADMIN, + pendingAction: null, + }; +} + +const workspacePolicy: Policy = { + ...createGroupPolicy(1, WORKSPACE_POLICY_ID), + customUnits: { + unitID: { + attributes: {unit: 'mi'}, + customUnitID: 'unitID', + enabled: true, + name: CONST.CUSTOM_UNITS.NAME_DISTANCE, + rates: { + rateID: {currency: 'USD', customUnitRateID: 'rateID', enabled: true, name: 'Default Rate', rate: 65.5}, + }, + }, + }, +}; + +const otherPolicy: Policy = createGroupPolicy(2, OTHER_POLICY_ID); + +/** + * The state the Rate page is rendered with after "Track distance > Manual" and picking a workspace chat in the + * in-place "To" picker: participants point at the workspace, the route report is still the self DM. + */ +const transactionOnWorkspaceChat: Transaction = { + ...createRandomTransaction(1), + reportID: WORKSPACE_CHAT_REPORT_ID, + participants: [{accountID: 0, selected: true, isPolicyExpenseChat: true, policyID: WORKSPACE_POLICY_ID, reportID: WORKSPACE_CHAT_REPORT_ID}], +}; + +const wrapper = ({children}: {children: React.ReactNode}) => {children}; + +describe('usePolicyForTransaction', () => { + beforeAll(() => { + Onyx.init({keys: ONYXKEYS}); + }); + + beforeEach(async () => { + await Onyx.clear(); + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${WORKSPACE_POLICY_ID}`, workspacePolicy); + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${OTHER_POLICY_ID}`, otherPolicy); + await waitForBatchedUpdates(); + }); + + it('resolves the workspace picked in the "To" field when the route report still carries the fake self-DM policy', async () => { + const {result} = renderHook( + () => + usePolicyForTransaction({ + transaction: transactionOnWorkspaceChat, + reportPolicyID: CONST.POLICY.ID_FAKE, + action: CONST.IOU.ACTION.CREATE, + iouType: CONST.IOU.TYPE.CREATE, + }), + {wrapper}, + ); + + await waitFor(() => { + expect(result.current.policy?.id).toBe(WORKSPACE_POLICY_ID); + }); + }); + + it('resolves the workspace picked in the "To" field when the route report has no policy at all', async () => { + const {result} = renderHook( + () => + usePolicyForTransaction({ + transaction: transactionOnWorkspaceChat, + reportPolicyID: undefined, + action: CONST.IOU.ACTION.CREATE, + iouType: CONST.IOU.TYPE.CREATE, + }), + {wrapper}, + ); + + await waitFor(() => { + expect(result.current.policy?.id).toBe(WORKSPACE_POLICY_ID); + }); + }); + + it('keeps the route report authoritative while editing an existing expense', async () => { + const {result} = renderHook( + () => + usePolicyForTransaction({ + transaction: transactionOnWorkspaceChat, + reportPolicyID: OTHER_POLICY_ID, + action: CONST.IOU.ACTION.EDIT, + iouType: CONST.IOU.TYPE.SUBMIT, + }), + {wrapper}, + ); + + await waitFor(() => { + expect(result.current.policy?.id).toBe(OTHER_POLICY_ID); + }); + }); + + it('keeps the route report authoritative for a P2P participant', async () => { + const p2pTransaction: Transaction = { + ...createRandomTransaction(2), + reportID: WORKSPACE_CHAT_REPORT_ID, + participants: [{accountID: 1, selected: true}], + }; + + const {result} = renderHook( + () => + usePolicyForTransaction({ + transaction: p2pTransaction, + reportPolicyID: OTHER_POLICY_ID, + action: CONST.IOU.ACTION.CREATE, + iouType: CONST.IOU.TYPE.SUBMIT, + }), + {wrapper}, + ); + + await waitFor(() => { + expect(result.current.policy?.id).toBe(OTHER_POLICY_ID); + }); + }); + + it('still hands a self DM track expense to the moving-expenses policy, not to the participants', async () => { + await Onyx.set(ONYXKEYS.NVP_ACTIVE_POLICY_ID, OTHER_POLICY_ID); + await waitForBatchedUpdates(); + + const selfDMTransaction: Transaction = { + ...createRandomTransaction(3), + reportID: CONST.REPORT.UNREPORTED_REPORT_ID, + participants: [{accountID: 0, selected: true, isPolicyExpenseChat: true, policyID: WORKSPACE_POLICY_ID}], + }; + + const {result} = renderHook( + () => + usePolicyForTransaction({ + transaction: selfDMTransaction, + reportPolicyID: CONST.POLICY.ID_FAKE, + action: CONST.IOU.ACTION.CREATE, + iouType: CONST.IOU.TYPE.TRACK, + }), + {wrapper}, + ); + + await waitFor(() => { + expect(result.current.policy?.id).toBe(OTHER_POLICY_ID); + }); + }); +});