Skip to content
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

fix: app crash when opening split view #51244

Merged
merged 3 commits into from
Nov 5, 2024
+65 −122
Merged

fix: app crash when opening split view #51244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
28940c2
fix: app crash when opening split view
daledah Oct 22, 2024
e5a552c
Merge branch 'main' into fix/51235
daledah Oct 25, 2024
2f2165b
fix: migrate withFullTransactionOrNotFound
daledah Oct 25, 2024
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
147 changes: 46 additions & 101 deletions src/components/MoneyRequestConfirmationList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import {useFocusEffect, useIsFocused} from '@react-navigation/native';
import lodashIsEqual from 'lodash/isEqual';
import React, {memo, useCallback, useEffect, useMemo, useRef, useState} from 'react';
import {InteractionManager, View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import type {OnyxEntry} from 'react-native-onyx';
import {useOnyx} from 'react-native-onyx';
import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails';
import useDebouncedState from '@hooks/useDebouncedState';
import useLocalize from '@hooks/useLocalize';
Expand All @@ -14,7 +14,6 @@ import useThemeStyles from '@hooks/useThemeStyles';
import blurActiveElement from '@libs/Accessibility/blurActiveElement';
import * as CurrencyUtils from '@libs/CurrencyUtils';
import DistanceRequestUtils from '@libs/DistanceRequestUtils';
import type {MileageRate} from '@libs/DistanceRequestUtils';
import * as IOUUtils from '@libs/IOUUtils';
import Log from '@libs/Log';
import * as MoneyRequestUtils from '@libs/MoneyRequestUtils';
Expand Down Expand Up @@ -49,33 +48,7 @@ import UserListItem from './SelectionList/UserListItem';
import SettlementButton from './SettlementButton';
import Text from './Text';

type MoneyRequestConfirmationListOnyxProps = {
/** Collection of categories attached to a policy */
policyCategories: OnyxEntry<OnyxTypes.PolicyCategories>;

/** Collection of draft categories attached to a policy */
policyCategoriesDraft: OnyxEntry<OnyxTypes.PolicyCategories>;

/** Collection of tags attached to a policy */
policyTags: OnyxEntry<OnyxTypes.PolicyTagLists>;

/** The policy of the report */
policy: OnyxEntry<OnyxTypes.Policy>;

/** The draft policy of the report */
policyDraft: OnyxEntry<OnyxTypes.Policy>;

/** Mileage rate default for the policy */
defaultMileageRate: OnyxEntry<MileageRate>;

/** Last selected distance rates */
lastSelectedDistanceRates: OnyxEntry<Record<string, string>>;

/** List of currencies */
currencyList: OnyxEntry<OnyxTypes.CurrencyList>;
};

type MoneyRequestConfirmationListProps = MoneyRequestConfirmationListOnyxProps & {
type MoneyRequestConfirmationListProps = {
/** Callback to inform parent modal of success */
onConfirm?: (selectedParticipants: Participant[]) => void;

Expand Down Expand Up @@ -178,23 +151,18 @@ function MoneyRequestConfirmationList({
onConfirm,
iouType = CONST.IOU.TYPE.SUBMIT,
iouAmount,
policyCategories: policyCategoriesReal,
policyCategoriesDraft,
isDistanceRequest = false,
policy: policyReal,
policyDraft,
isPolicyExpenseChat = false,
iouCategory = '',
shouldShowSmartScanFields = true,
isEditingSplitBill,
policyTags,
iouCurrencyCode,
iouMerchant,
selectedParticipants: selectedParticipantsProp,
payeePersonalDetails: payeePersonalDetailsProp,
isReadOnly = false,
bankAccountRoute = '',
policyID = '',
policyID,
reportID = '',
receiptPath = '',
iouAttendees,
Expand All @@ -205,14 +173,22 @@ function MoneyRequestConfirmationList({
onToggleBillable,
hasSmartScanFailed,
reportActionID,
defaultMileageRate,
lastSelectedDistanceRates,
action = CONST.IOU.ACTION.CREATE,
currencyList,
shouldDisplayReceipt = false,
shouldPlaySound = true,
isConfirmed,
}: MoneyRequestConfirmationListProps) {
const [policyCategoriesReal] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policyID ?? '-1'}`);
const [policyTags] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID ?? '-1'}`);
const [policyReal] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${policyID ?? '-1'}`);
const [policyDraft] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_DRAFTS}${policyID ?? '-1'}`);
const [defaultMileageRate] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_DRAFTS}${policyID ?? '-1'}`, {
selector: (selectedPolicy) => DistanceRequestUtils.getDefaultMileageRate(selectedPolicy),
});
const [policyCategoriesDraft] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES_DRAFT}${policyID ?? '-1'}`);
const [lastSelectedDistanceRates] = useOnyx(ONYXKEYS.NVP_LAST_SELECTED_DISTANCE_RATES);
const [currencyList] = useOnyx(ONYXKEYS.CURRENCY_LIST);

const policy = policyReal ?? policyDraft;
const policyCategories = policyCategoriesReal ?? policyCategoriesDraft;

Expand Down Expand Up @@ -972,67 +948,36 @@ function MoneyRequestConfirmationList({

MoneyRequestConfirmationList.displayName = 'MoneyRequestConfirmationList';

export default withOnyx<MoneyRequestConfirmationListProps, MoneyRequestConfirmationListOnyxProps>({
policyCategories: {
key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policyID}`,
},
policyCategoriesDraft: {
key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY_CATEGORIES_DRAFT}${policyID}`,
},
policyTags: {
key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`,
},
defaultMileageRate: {
key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
selector: DistanceRequestUtils.getDefaultMileageRate,
},
policy: {
key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
},
policyDraft: {
key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY_DRAFTS}${policyID}`,
},
lastSelectedDistanceRates: {
key: ONYXKEYS.NVP_LAST_SELECTED_DISTANCE_RATES,
},
currencyList: {
key: ONYXKEYS.CURRENCY_LIST,
},
})(
memo(
MoneyRequestConfirmationList,
(prevProps, nextProps) =>
lodashIsEqual(prevProps.transaction, nextProps.transaction) &&
prevProps.onSendMoney === nextProps.onSendMoney &&
prevProps.onConfirm === nextProps.onConfirm &&
prevProps.iouType === nextProps.iouType &&
prevProps.iouAmount === nextProps.iouAmount &&
prevProps.isDistanceRequest === nextProps.isDistanceRequest &&
prevProps.isPolicyExpenseChat === nextProps.isPolicyExpenseChat &&
prevProps.iouCategory === nextProps.iouCategory &&
prevProps.shouldShowSmartScanFields === nextProps.shouldShowSmartScanFields &&
prevProps.isEditingSplitBill === nextProps.isEditingSplitBill &&
prevProps.iouCurrencyCode === nextProps.iouCurrencyCode &&
prevProps.iouMerchant === nextProps.iouMerchant &&
lodashIsEqual(prevProps.selectedParticipants, nextProps.selectedParticipants) &&
lodashIsEqual(prevProps.payeePersonalDetails, nextProps.payeePersonalDetails) &&
prevProps.isReadOnly === nextProps.isReadOnly &&
prevProps.bankAccountRoute === nextProps.bankAccountRoute &&
prevProps.policyID === nextProps.policyID &&
prevProps.reportID === nextProps.reportID &&
prevProps.receiptPath === nextProps.receiptPath &&
prevProps.iouAttendees === nextProps.iouAttendees &&
prevProps.iouComment === nextProps.iouComment &&
prevProps.receiptFilename === nextProps.receiptFilename &&
prevProps.iouCreated === nextProps.iouCreated &&
prevProps.iouIsBillable === nextProps.iouIsBillable &&
prevProps.onToggleBillable === nextProps.onToggleBillable &&
prevProps.hasSmartScanFailed === nextProps.hasSmartScanFailed &&
prevProps.reportActionID === nextProps.reportActionID &&
lodashIsEqual(prevProps.defaultMileageRate, nextProps.defaultMileageRate) &&
lodashIsEqual(prevProps.lastSelectedDistanceRates, nextProps.lastSelectedDistanceRates) &&
lodashIsEqual(prevProps.action, nextProps.action) &&
lodashIsEqual(prevProps.currencyList, nextProps.currencyList) &&
prevProps.shouldDisplayReceipt === nextProps.shouldDisplayReceipt,
),
export default memo(
MoneyRequestConfirmationList,
(prevProps, nextProps) =>
lodashIsEqual(prevProps.transaction, nextProps.transaction) &&
prevProps.onSendMoney === nextProps.onSendMoney &&
prevProps.onConfirm === nextProps.onConfirm &&
prevProps.iouType === nextProps.iouType &&
prevProps.iouAmount === nextProps.iouAmount &&
prevProps.isDistanceRequest === nextProps.isDistanceRequest &&
prevProps.isPolicyExpenseChat === nextProps.isPolicyExpenseChat &&
prevProps.iouCategory === nextProps.iouCategory &&
prevProps.shouldShowSmartScanFields === nextProps.shouldShowSmartScanFields &&
prevProps.isEditingSplitBill === nextProps.isEditingSplitBill &&
prevProps.iouCurrencyCode === nextProps.iouCurrencyCode &&
prevProps.iouMerchant === nextProps.iouMerchant &&
lodashIsEqual(prevProps.selectedParticipants, nextProps.selectedParticipants) &&
lodashIsEqual(prevProps.payeePersonalDetails, nextProps.payeePersonalDetails) &&
prevProps.isReadOnly === nextProps.isReadOnly &&
prevProps.bankAccountRoute === nextProps.bankAccountRoute &&
prevProps.policyID === nextProps.policyID &&
prevProps.reportID === nextProps.reportID &&
prevProps.receiptPath === nextProps.receiptPath &&
prevProps.iouAttendees === nextProps.iouAttendees &&
prevProps.iouComment === nextProps.iouComment &&
prevProps.receiptFilename === nextProps.receiptFilename &&
prevProps.iouCreated === nextProps.iouCreated &&
prevProps.iouIsBillable === nextProps.iouIsBillable &&
prevProps.onToggleBillable === nextProps.onToggleBillable &&
prevProps.hasSmartScanFailed === nextProps.hasSmartScanFailed &&
prevProps.reportActionID === nextProps.reportActionID &&
lodashIsEqual(prevProps.action, nextProps.action) &&
prevProps.shouldDisplayReceipt === nextProps.shouldDisplayReceipt,
);
40 changes: 19 additions & 21 deletions src/pages/iou/request/step/withFullTransactionOrNotFound.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {useIsFocused} from '@react-navigation/native';
import type {ComponentType, ForwardedRef, RefAttributes} from 'react';
import React, {forwardRef} from 'react';
import type {OnyxEntry} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import {useOnyx} from 'react-native-onyx';
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi team, coming from this issue #52137, this migration to useOnyx hook caused a regression bug on expense flow where it didn't auto selecting tag. More details: #52137 (comment)

import FullPageNotFoundView from '@components/BlockingViews/FullPageNotFoundView';
import getComponentDisplayName from '@libs/getComponentDisplayName';
import * as IOUUtils from '@libs/IOUUtils';
Expand Down Expand Up @@ -38,14 +38,24 @@ type MoneyRequestRouteName =
| typeof SCREENS.MONEY_REQUEST.STEP_SEND_FROM
| typeof SCREENS.MONEY_REQUEST.STEP_COMPANY_INFO;

type Route<T extends MoneyRequestRouteName> = RouteProp<MoneyRequestNavigatorParamList, T>;
type Route<TRouteName extends MoneyRequestRouteName> = RouteProp<MoneyRequestNavigatorParamList, TRouteName>;

type WithFullTransactionOrNotFoundProps<T extends MoneyRequestRouteName> = WithFullTransactionOrNotFoundOnyxProps & {route: Route<T>};
type WithFullTransactionOrNotFoundProps<TRouteName extends MoneyRequestRouteName> = WithFullTransactionOrNotFoundOnyxProps & {
route: Route<TRouteName>;
};

export default function <TProps extends WithFullTransactionOrNotFoundProps<MoneyRequestRouteName>, TRef>(WrappedComponent: ComponentType<TProps & RefAttributes<TRef>>) {
export default function <TProps extends WithFullTransactionOrNotFoundProps<MoneyRequestRouteName>, TRef>(
WrappedComponent: ComponentType<TProps & RefAttributes<TRef>>,
): React.ComponentType<Omit<TProps, keyof WithFullTransactionOrNotFoundOnyxProps> & RefAttributes<TRef>> {
// eslint-disable-next-line rulesdir/no-negated-variables
function WithFullTransactionOrNotFound(props: TProps, ref: ForwardedRef<TRef>) {
const transactionID = props.transaction?.transactionID;
function WithFullTransactionOrNotFound(props: Omit<TProps, keyof WithFullTransactionOrNotFoundOnyxProps>, ref: ForwardedRef<TRef>) {
const {route} = props;
const transactionID = route.params.transactionID ?? -1;
const userAction = 'action' in route.params && route.params.action ? route.params.action : CONST.IOU.ACTION.CREATE;

const shouldUseTransactionDraft = IOUUtils.shouldUseTransactionDraft(userAction);
const [transaction] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`);
const [transactionDraft] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`);

const isFocused = useIsFocused();

Expand All @@ -55,31 +65,19 @@ export default function <TProps extends WithFullTransactionOrNotFoundProps<Money
if (!transactionID) {
return <FullPageNotFoundView shouldShow={isFocused} />;
}

return (
<WrappedComponent
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
{...(props as TProps)}
transaction={shouldUseTransactionDraft ? transactionDraft : transaction}
ref={ref}
/>
);
}

WithFullTransactionOrNotFound.displayName = `withFullTransactionOrNotFound(${getComponentDisplayName(WrappedComponent)})`;
// eslint-disable-next-line deprecation/deprecation
return withOnyx<TProps & RefAttributes<TRef>, WithFullTransactionOrNotFoundOnyxProps>({
transaction: {
key: ({route}) => {
const transactionID = route.params.transactionID ?? -1;
const userAction = 'action' in route.params && route.params.action ? route.params.action : CONST.IOU.ACTION.CREATE;

if (IOUUtils.shouldUseTransactionDraft(userAction)) {
return `${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}` as `${typeof ONYXKEYS.COLLECTION.TRANSACTION}${string}`;
}
return `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`;
},
},
})(forwardRef(WithFullTransactionOrNotFound));
return forwardRef(WithFullTransactionOrNotFound);
}

export type {WithFullTransactionOrNotFoundProps};
Loading