Skip to content
Merged
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
8 changes: 6 additions & 2 deletions src/components/ParticipantPicker/types.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import type {IOUAction, IOUType} from '@src/CONST';
import type CONST from '@src/CONST';
import type {Policy} from '@src/types/onyx';
import type {Participant} from '@src/types/onyx/IOU';

import type {OnyxEntry} from 'react-native-onyx';

type ParticipantPickerProps = {
/** Selected participants */
participants?: Participant[] | typeof CONST.EMPTY_ARRAY;
Expand All @@ -24,8 +27,9 @@ type ParticipantPickerProps = {
/** Whether to exclude P2P recipients (and the invite-by-email option) from the list. Used for negative amounts, which P2P chats don't support. */
shouldExcludeP2P?: boolean;

/** Callback fired when participants are updated */
onParticipantsAdded: (value: Participant[]) => void;
/** Callback fired when participants are updated. selectedPolicy is the chosen workspace's policy, used by the
* confirmation step to reset the rate and category. */
onParticipantsAdded: (value: Participant[], selectedPolicy?: OnyxEntry<Policy>) => void;

/** Callback fired when participant selection is completed */
onFinish?: (value?: string, participants?: Participant[]) => void;
Expand Down
6 changes: 4 additions & 2 deletions src/pages/iou/request/MoneyRequestParticipantsSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ import getPlatform from '@libs/getPlatform';

import type {IOUAction, IOUType} from '@src/CONST';
import CONST from '@src/CONST';
import type {Policy} from '@src/types/onyx';
import type {Participant} from '@src/types/onyx/IOU';

import type {Ref} from 'react';
import type {OnyxEntry} from 'react-native-onyx';

import {useIsFocused} from '@react-navigation/native';
import {Activity, useImperativeHandle, useRef, useState} from 'react';
Expand All @@ -21,8 +23,8 @@ type MoneyRequestParticipantsSelectorProps = {
/** Callback to request parent modal to go to next step, which should be split */
onFinish?: (value?: string, participants?: Participant[]) => void;

/** Callback to add participants in MoneyRequestModal */
onParticipantsAdded: (value: Participant[]) => void;
/** Callback to add participants in MoneyRequestModal. selectedPolicy is forwarded to the confirmation step. */
onParticipantsAdded: (value: Participant[], selectedPolicy?: OnyxEntry<Policy>) => void;

/** Selected participants from MoneyRequestModal with login */
participants?: Participant[] | typeof CONST.EMPTY_ARRAY;
Expand Down
11 changes: 8 additions & 3 deletions src/pages/iou/request/ParticipantSearchResults.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,13 @@ import type {IOUAction, IOUType} from '@src/CONST';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type {Policy} from '@src/types/onyx';
import type {Participant} from '@src/types/onyx/IOU';
import {isEmptyObject} from '@src/types/utils/EmptyObject';

import type {Ref} from 'react';
import type {GestureResponderEvent} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';

import lodashPick from 'lodash/pick';
import React, {useContext, useEffect} from 'react';
Expand Down Expand Up @@ -104,8 +106,8 @@ type ParticipantSearchResultsProps = {
/** Setter to toggle textInputAutoFocus from the contact permission flow */
setTextInputAutoFocus: (value: boolean) => void;

/** Callback to propagate selected participants to the parent flow */
onParticipantsAdded: (value: Participant[]) => void;
/** Callback to propagate selected participants to the parent flow. selectedPolicy is the chosen workspace's policy. */
onParticipantsAdded: (value: Participant[], selectedPolicy?: OnyxEntry<Policy>) => void;

/** Callback to advance the parent flow */
onFinish: (value?: string, participants?: Participant[]) => void;
Expand Down Expand Up @@ -233,7 +235,10 @@ function ParticipantSearchResults({
});
}

onParticipantsAdded(newParticipants);
// Resolve the chosen workspace's policy and pass it up so the confirmation step can reset the rate and category
// without subscribing to every policy. allPolicies is already loaded on this screen, so this adds no new subscription.
const selectedPolicy = option.policyID ? allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${option.policyID}`] : undefined;
Comment on lines +238 to +240

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Mount the policy subscriber only while the picker is visible

On web and Android, this does not actually limit the policy subscription to an open picker: ParticipantPicker/index.tsx intentionally keeps BaseParticipantPicker mounted when isVisible is false, so ParticipantSearchResults and its useOnyx(ONYXKEYS.COLLECTION.POLICY) subscription remain active for the confirmation page's entire lifetime. Consequently high-traffic accounts still load and react to the full policy collection on the confirmation screen; the picker content or its policy-dependent subtree needs to be gated by visibility while preserving the modal shell required for its exit animation.

Useful? React with 👍 / 👎.

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.

The purpose of this change appears to be removing a redundant subscription in IOURequestStepConfirmation

onParticipantsAdded(newParticipants, selectedPolicy);

if (!option.isSelfDM) {
onFinish(undefined, newParticipants);
Expand Down
19 changes: 2 additions & 17 deletions src/pages/iou/request/step/IOURequestStepConfirmation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import useFetchRoute from '@hooks/useFetchRoute';
import useFilesValidation from '@hooks/useFilesValidation';
import {useMemoizedLazyExpensifyIcons} from '@hooks/useLazyAsset';
import useLocalize from '@hooks/useLocalize';
import useMappedPolicies from '@hooks/useMappedPolicies';
import useNetwork from '@hooks/useNetwork';
import useOdometerReceiptStitcher from '@hooks/useOdometerReceiptStitcher';
import useOnyx from '@hooks/useOnyx';
Expand Down Expand Up @@ -119,18 +118,6 @@ import useExpenseSubmission from './confirmation/useExpenseSubmission';
import withFullTransactionOrNotFound from './withFullTransactionOrNotFound';
import withWritableReportOrNotFound from './withWritableReportOrNotFound';

const policyMapper = (policy: OnyxEntry<Policy>): OnyxEntry<Policy> =>
policy && {
id: policy.id,
name: policy.name,
type: policy.type,
role: policy.role,
owner: policy.owner,
outputCurrency: policy.outputCurrency,
isPolicyExpenseChatEnabled: policy.isPolicyExpenseChatEnabled,
customUnits: policy.customUnits,
};

type IOURequestStepConfirmationIncomingRouteName = typeof SCREENS.MONEY_REQUEST.STEP_CONFIRMATION | typeof SCREENS.MONEY_REQUEST.CREATE;

type StepConfirmationParams = MoneyRequestNavigatorParamList[typeof SCREENS.MONEY_REQUEST.STEP_CONFIRMATION];
Expand Down Expand Up @@ -310,7 +297,6 @@ function IOURequestStepConfirmation({
}, [transactionReport, currentUserPersonalDetails.accountID, transaction?.transactionID, iouType]);

const participantsPolicies = useParticipantsPolicies(transaction?.participants ?? []);
const [mappedPolicies] = useMappedPolicies(policyMapper);

const participants = useMemo(
() =>
Expand Down Expand Up @@ -395,7 +381,7 @@ function IOURequestStepConfirmation({
}, [activeTransactionID]);

const handleParticipantsAdded = useCallback(
(participantsList: Participant[]) => {
(participantsList: Participant[], selectedPolicy?: OnyxEntry<Policy>) => {
if (!activeTransactionID) {
return;
}
Expand Down Expand Up @@ -456,7 +442,7 @@ function IOURequestStepConfirmation({
setMoneyRequestCategory(activeTransactionID, '', undefined, getCurrencyDecimals);
setMoneyRequestTag(activeTransactionID, '');
} else {
const workspacePolicy = firstParticipant.policyID ? mappedPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${firstParticipant.policyID}`] : undefined;
const workspacePolicy = selectedPolicy;
if (isDistanceRequest) {
const currentRateID = transaction?.comment?.customUnit?.customUnitRateID;
const isCurrentRateFromWorkspace = !!currentRateID && !!DistanceRequestUtils.getMileageRates(workspacePolicy)[currentRateID];
Expand Down Expand Up @@ -501,7 +487,6 @@ function IOURequestStepConfirmation({
transaction,
personalPolicy?.outputCurrency,
blockDistanceRequestIfNeeded,
mappedPolicies,
getCurrencyDecimals,
policyID,
],
Expand Down
24 changes: 22 additions & 2 deletions tests/ui/components/IOURequestStepConfirmationPageTest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ jest.mock('@components/ProductTrainingContext', () => ({
// Stands in for the participant picker so a test can hand the page a selection without driving the real selector.
// The picker is only rendered under the new manual expense flow beta, so this is inert for every other test here.
let mockSelectedParticipants: Participant[] = [];
let mockSelectedPolicy: OnyxEntry<Policy>;
jest.mock('@components/ParticipantPicker', () => {
const ReactModule = jest.requireActual<typeof React>('react');
const {Text, TouchableOpacity} = jest.requireActual<{
Expand All @@ -106,10 +107,10 @@ jest.mock('@components/ParticipantPicker', () => {
}>('react-native');
return {
__esModule: true,
default: ({onParticipantsAdded}: {onParticipantsAdded: (participants: Participant[]) => void}) =>
default: ({onParticipantsAdded}: {onParticipantsAdded: (participants: Participant[], selectedPolicy?: OnyxEntry<Policy>) => void}) =>
ReactModule.createElement(
TouchableOpacity,
{testID: 'MockParticipantPicker', onPress: () => onParticipantsAdded(mockSelectedParticipants)},
{testID: 'MockParticipantPicker', onPress: () => onParticipantsAdded(mockSelectedParticipants, mockSelectedPolicy)},
ReactModule.createElement(Text, null, 'Select participant'),
),
};
Expand Down Expand Up @@ -1515,6 +1516,7 @@ describe('IOURequestStepConfirmationPageTest', () => {

beforeEach(async () => {
mockSelectedParticipants = [];
mockSelectedPolicy = undefined;
await signInWithTestUser(ACCOUNT_ID, ACCOUNT_LOGIN);
await act(async () => {
await Onyx.set(ONYXKEYS.BETAS, [CONST.BETAS.NEW_MANUAL_EXPENSE_FLOW]);
Expand Down Expand Up @@ -1587,8 +1589,26 @@ describe('IOURequestStepConfirmationPageTest', () => {
await waitForBatchedUpdatesWithAct();
}

function getPolicyByID(policyID?: string) {
return new Promise<OnyxEntry<Policy>>((resolve) => {
if (!policyID) {
resolve(undefined);
return;
}
const connection = Onyx.connect({
key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
callback: (value) => {
resolve(value);
Onyx.disconnect(connection);
},
});
});
}

async function selectParticipants(participants: Participant[]) {
mockSelectedParticipants = participants;
// Mirror the real picker: resolve the chosen workspace's policy and pass it to onParticipantsAdded.
mockSelectedPolicy = await getPolicyByID(participants.at(0)?.policyID);
fireEvent.press(await screen.findByTestId('MockParticipantPicker'));
await waitForBatchedUpdatesWithAct();
}
Expand Down
Loading