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

Jump to bottom

fix: Migrate withReportAndReportActionOrNotFound from withOnyx to useOnyx #49460

Merged
merged 26 commits into from
Oct 7, 2024
Merged

fix: Migrate withReportAndReportActionOrNotFound from withOnyx to useOnyx #49460

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
07b78ce
Use useOnyx for report
etCoderDysto Sep 17, 2024
b882c6d
fetch parentReport with useOnyx
etCoderDysto Sep 17, 2024
8398950
fetch reportMetadata and use it in WithReportOrNotFound
etCoderDysto Sep 17, 2024
7e3931c
fetch isLoadingReportData
etCoderDysto Sep 17, 2024
a65f34f
fetch betas
etCoderDysto Sep 17, 2024
242103d
fetch policies using useOnyx
etCoderDysto Sep 17, 2024
ca1e4e5
fetch reportAction
etCoderDysto Sep 17, 2024
77cbc79
fetch parentReportAction
etCoderDysto Sep 17, 2024
1649082
Change props.report with report
etCoderDysto Sep 17, 2024
dc936ba
remove withOnyx usage
etCoderDysto Sep 18, 2024
cc065e8
fix lint and typecheck
etCoderDysto Sep 19, 2024
84472ba
Merge branch 'main' into useOnyx-migration-notfound
etCoderDysto Sep 19, 2024
ddb8c9e
pass only props used by other components
etCoderDysto Sep 21, 2024
aa98328
Remove OnyxProps, and add type for props passed to FlagComment
etCoderDysto Sep 21, 2024
0329631
change reportAction with linkedReportAction and pass it to wrapped co…
etCoderDysto Sep 21, 2024
5a63090
fix type issue for withReport
etCoderDysto Sep 21, 2024
5ca151b
use reportAction in FlagCommentPage
etCoderDysto Sep 21, 2024
f70dfaf
apply suggested changes and remove useOnyx
etCoderDysto Sep 23, 2024
26f1d07
simplify selector for parentReportActions
etCoderDysto Sep 23, 2024
0398b3f
Don't pass reportActions to wrapped component
etCoderDysto Sep 27, 2024
ae366bb
fix conflict with main
etCoderDysto Sep 27, 2024
2a2c70a
Merge branch 'main' into useOnyx-migration-notfound
etCoderDysto Sep 27, 2024
3702c9a
fix type error caused from conflict
etCoderDysto Sep 27, 2024
93dd702
Use logical or to get IOUTransactionID safely
etCoderDysto Oct 3, 2024
b656710
fix typo on COLLECTION.SPLIT_TRANSACTION_DRAFT
etCoderDysto Oct 3, 2024
94f75b5
use ternay operator instead of or
etCoderDysto Oct 4, 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
31 changes: 4 additions & 27 deletions src/pages/FlagCommentPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import type {StackScreenProps} from '@react-navigation/stack';
import React, {useCallback} from 'react';
import React from 'react';
import {View} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import type {SvgProps} from 'react-native-svg';
import type {ValueOf} from 'type-fest';
import FullPageNotFoundView from '@components/BlockingViews/FullPageNotFoundView';
Expand All @@ -20,18 +19,12 @@ import * as Report from '@userActions/Report';
import * as Session from '@userActions/Session';
import CONST from '@src/CONST';
import type SCREENS from '@src/SCREENS';
import type * as OnyxTypes from '@src/types/onyx';
import withReportAndReportActionOrNotFound from './home/report/withReportAndReportActionOrNotFound';
import type {WithReportAndReportActionOrNotFoundProps} from './home/report/withReportAndReportActionOrNotFound';

type FlagCommentPageWithOnyxProps = {
/** The report action from the parent report */
parentReportAction: OnyxEntry<OnyxTypes.ReportAction>;
};

type FlagCommentPageNavigationProps = StackScreenProps<FlagCommentNavigatorParamList, typeof SCREENS.FLAG_COMMENT_ROOT>;

type FlagCommentPageProps = WithReportAndReportActionOrNotFoundProps & FlagCommentPageNavigationProps & FlagCommentPageWithOnyxProps;
type FlagCommentPageProps = WithReportAndReportActionOrNotFoundProps & FlagCommentPageNavigationProps;

type Severity = ValueOf<typeof CONST.MODERATION>;

Expand All @@ -53,7 +46,7 @@ function getReportID(route: FlagCommentPageNavigationProps['route']) {
return route.params.reportID.toString();
}

function FlagCommentPage({parentReportAction, route, report, parentReport, reportActions}: FlagCommentPageProps) {
function FlagCommentPage({parentReportAction, route, report, parentReport, reportAction}: FlagCommentPageProps) {
const styles = useThemeStyles();
const {translate} = useLocalize();

Expand Down Expand Up @@ -108,24 +101,8 @@ function FlagCommentPage({parentReportAction, route, report, parentReport, repor
},
];

const getActionToFlag = useCallback((): OnyxTypes.ReportAction | null => {
let reportAction = reportActions?.[`${route.params.reportActionID.toString()}`];

// Handle threads if needed
if (reportAction?.reportActionID === undefined && parentReportAction) {
reportAction = parentReportAction;
}

if (!reportAction) {
return null;
}

return reportAction;
}, [reportActions, route.params.reportActionID, parentReportAction]);

const flagComment = (severity: Severity) => {
let reportID: string | undefined = getReportID(route);
const reportAction = getActionToFlag();

// Handle threads if needed
if (ReportUtils.isChatThread(report) && reportAction?.reportActionID === parentReportAction?.reportActionID) {
Expand Down Expand Up @@ -158,7 +135,7 @@ function FlagCommentPage({parentReportAction, route, report, parentReport, repor
testID={FlagCommentPage.displayName}
>
{({safeAreaPaddingBottomStyle}) => (
<FullPageNotFoundView shouldShow={!ReportUtils.shouldShowFlagComment(getActionToFlag(), report)}>
<FullPageNotFoundView shouldShow={!ReportUtils.shouldShowFlagComment(reportAction, report)}>
<HeaderWithBackButton
title={translate('reportActionContextMenu.flagAsOffensive')}
onBackButtonPress={() => Navigation.goBack(route.params.backTo)}
Expand Down
115 changes: 44 additions & 71 deletions src/pages/home/report/withReportAndReportActionOrNotFound.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
/* eslint-disable rulesdir/no-negated-variables */
import type {StackScreenProps} from '@react-navigation/stack';
import type {ComponentType, ForwardedRef, RefAttributes} from 'react';
import React, {useCallback, useEffect} from 'react';
import type {OnyxCollection, OnyxEntry, WithOnyxState} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import React, {useEffect, useMemo} from 'react';
import type {OnyxEntry} from 'react-native-onyx';
import {useOnyx} from 'react-native-onyx';
import FullscreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
import useResponsiveLayout from '@hooks/useResponsiveLayout';
import getComponentDisplayName from '@libs/getComponentDisplayName';
Expand All @@ -16,68 +16,71 @@ import type SCREENS from '@src/SCREENS';
import type * as OnyxTypes from '@src/types/onyx';
import {isEmptyObject} from '@src/types/utils/EmptyObject';

type OnyxProps = {
type WithReportAndReportActionOrNotFoundProps = StackScreenProps<
FlagCommentNavigatorParamList & SplitDetailsNavigatorParamList,
typeof SCREENS.FLAG_COMMENT_ROOT | typeof SCREENS.SPLIT_DETAILS.ROOT
> & {
/** The report currently being looked at */
report: OnyxEntry<OnyxTypes.Report>;
report: OnyxTypes.Report;

/** The reportAction from the current route */
reportAction: OnyxTypes.ReportAction;

/** The parent report if the current report is a thread and it has a parent */
parentReport: OnyxEntry<OnyxTypes.Report>;

/** The report metadata */
reportMetadata: OnyxEntry<OnyxTypes.ReportMetadata>;

/** Array of report actions for this report */
reportActions: OnyxEntry<OnyxTypes.ReportActions>;

/** The report's parentReportAction */
parentReportAction: NonNullable<OnyxEntry<OnyxTypes.ReportAction>> | null;

/** The policies which the user has access to */
policies: OnyxCollection<OnyxTypes.Policy>;

/** Beta features list */
betas: OnyxEntry<OnyxTypes.Beta[]>;

/** Indicated whether the report data is loading */
isLoadingReportData: OnyxEntry<boolean>;
};

type WithReportAndReportActionOrNotFoundProps = OnyxProps &
StackScreenProps<FlagCommentNavigatorParamList & SplitDetailsNavigatorParamList, typeof SCREENS.FLAG_COMMENT_ROOT | typeof SCREENS.SPLIT_DETAILS.ROOT>;

export default function <TProps extends WithReportAndReportActionOrNotFoundProps, TRef>(
WrappedComponent: ComponentType<TProps & RefAttributes<TRef>>,
): ComponentType<Omit<TProps & RefAttributes<TRef>, keyof OnyxProps>> {
): ComponentType<TProps & RefAttributes<TRef>> {
function WithReportOrNotFound(props: TProps, ref: ForwardedRef<TRef>) {
const getReportAction = useCallback(() => {
let reportAction: OnyxEntry<OnyxTypes.ReportAction> = props.reportActions?.[`${props.route.params.reportActionID}`];
const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${props.route.params.reportID}`);
const [parentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${report ? report.parentReportID : '-1'}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Wrong condition is set here, If parentReportID is undefined it is still concatenating to onyx key, resulting in report_undefined key.

#50786

const [reportMetadata] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${props.route.params.reportID}`);
const [isLoadingReportData] = useOnyx(ONYXKEYS.IS_LOADING_REPORT_DATA);
const [betas] = useOnyx(ONYXKEYS.BETAS);
const [policies] = useOnyx(ONYXKEYS.COLLECTION.POLICY);
const [reportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${props.route.params.reportID}`, {canEvict: false});
const [parentReportAction] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report ? report.parentReportID : '-1'}`, {
selector: (parentReportActions) => {
const parentReportActionID = report?.parentReportActionID;
if (!parentReportActionID) {
return null;
}
return parentReportActions?.[parentReportActionID] ?? null;
},
canEvict: false,
});
const linkedReportAction = useMemo(() => {
let reportAction: OnyxEntry<OnyxTypes.ReportAction> = reportActions?.[`${props.route.params.reportActionID}`];

// Handle threads if needed
if (!reportAction?.reportActionID) {
reportAction = props?.parentReportAction ?? undefined;
reportAction = parentReportAction ?? undefined;
}

return reportAction;
}, [props.reportActions, props.route.params.reportActionID, props.parentReportAction]);

const reportAction = getReportAction();
}, [reportActions, props.route.params.reportActionID, parentReportAction]);

const {shouldUseNarrowLayout} = useResponsiveLayout();

// For small screen, we don't call openReport API when we go to a sub report page by deeplink
// So we need to call openReport here for small screen
useEffect(() => {
if (!shouldUseNarrowLayout || (!isEmptyObject(props.report) && !isEmptyObject(reportAction))) {
if (!shouldUseNarrowLayout || (!isEmptyObject(report) && !isEmptyObject(linkedReportAction))) {
return;
}
Report.openReport(props.route.params.reportID);
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
}, [shouldUseNarrowLayout, props.route.params.reportID]);

// Perform all the loading checks
const isLoadingReport = props.isLoadingReportData && !props.report?.reportID;
const isLoadingReportAction = isEmptyObject(props.reportActions) || (props.reportMetadata?.isLoadingInitialReportActions && isEmptyObject(getReportAction()));
const shouldHideReport = !isLoadingReport && (!props.report?.reportID || !ReportUtils.canAccessReport(props.report, props.policies, props.betas));
const isLoadingReport = isLoadingReportData && !report?.reportID;
const isLoadingReportAction = isEmptyObject(reportActions) || (reportMetadata?.isLoadingInitialReportActions && isEmptyObject(linkedReportAction));
const shouldHideReport = !isLoadingReport && (!report?.reportID || !ReportUtils.canAccessReport(report, policies, betas));

// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
if ((isLoadingReport || isLoadingReportAction) && !shouldHideReport) {
Expand All @@ -86,57 +89,27 @@ export default function <TProps extends WithReportAndReportActionOrNotFoundProps

// Perform the access/not found checks
// Be sure to avoid showing the not-found page while the parent report actions are still being read from Onyx. The parentReportAction will be undefined while it's being read from Onyx
// and then reportAction will either be a valid parentReportAction or an empty object. In the case of an empty object, then it's OK to show the not-found page.
if (shouldHideReport || (props?.parentReportAction !== undefined && isEmptyObject(reportAction))) {
// and then linkedReportAction will either be a valid parentReportAction or an empty object. In the case of an empty object, then it's OK to show the not-found page.
if (shouldHideReport || (parentReportAction !== undefined && isEmptyObject(linkedReportAction))) {
return <NotFoundPage />;
}

return (
<WrappedComponent
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
report={report}
parentReport={parentReport}
reportAction={linkedReportAction}
parentReportAction={parentReportAction}
ref={ref}
/>
);
}

WithReportOrNotFound.displayName = `withReportOrNotFound(${getComponentDisplayName(WrappedComponent)})`;

return withOnyx<TProps & RefAttributes<TRef>, OnyxProps>({
report: {
key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${route.params.reportID}`,
},
parentReport: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT}${report ? report.parentReportID : '-1'}`,
},
reportMetadata: {
key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_METADATA}${route.params.reportID}`,
},
isLoadingReportData: {
key: ONYXKEYS.IS_LOADING_REPORT_DATA,
},
betas: {
key: ONYXKEYS.BETAS,
},
policies: {
key: ONYXKEYS.COLLECTION.POLICY,
},
reportActions: {
key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${route.params.reportID}`,
canEvict: false,
},
parentReportAction: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report ? report.parentReportID : 0}`,
selector: (parentReportActions: OnyxEntry<OnyxTypes.ReportActions>, props?: WithOnyxState<OnyxProps>): NonNullable<OnyxEntry<OnyxTypes.ReportAction>> | null => {
const parentReportActionID = props?.report?.parentReportActionID;
if (!parentReportActionID) {
return null;
}
return parentReportActions?.[parentReportActionID] ?? null;
},
canEvict: false,
},
})(React.forwardRef(WithReportOrNotFound));
return React.forwardRef(WithReportOrNotFound);
}

export type {WithReportAndReportActionOrNotFoundProps};
86 changes: 15 additions & 71 deletions src/pages/iou/SplitBillDetailsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import type {StackScreenProps} from '@react-navigation/stack';
import type {ComponentType} from 'react';
import React, {useCallback, useMemo, useState} from 'react';
import React, {useCallback, useState} from 'react';
import {View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import type {OnyxEntry} from 'react-native-onyx';
import {useOnyx} from 'react-native-onyx';
import FullPageNotFoundView from '@components/BlockingViews/FullPageNotFoundView';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import Icon from '@components/Icon';
Expand All @@ -27,46 +25,25 @@ import * as IOU from '@userActions/IOU';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type SCREENS from '@src/SCREENS';
import type {PersonalDetailsList, Report, ReportAction, Session, Transaction} from '@src/types/onyx';
import type {Participant} from '@src/types/onyx/IOU';
import type {ReportActions} from '@src/types/onyx/ReportAction';
import {isEmptyObject} from '@src/types/utils/EmptyObject';

type SplitBillDetailsPageTransactionOnyxProps = {
/** The current transaction */
transaction: OnyxEntry<Transaction>;
type SplitBillDetailsPageProps = WithReportAndReportActionOrNotFoundProps & StackScreenProps<SplitDetailsNavigatorParamList, typeof SCREENS.SPLIT_DETAILS.ROOT>;

/** The draft transaction that holds data to be persisited on the current transaction */
draftTransaction: OnyxEntry<Transaction>;
};

type SplitBillDetailsPageOnyxPropsWithoutTransaction = {
/** The personal details of the person who is logged in */
personalDetails: OnyxEntry<PersonalDetailsList>;

/** The active report */
report: OnyxEntry<Report>;

/** Array of report actions for this report */
reportActions: OnyxEntry<ReportActions>;

/** Session info for the currently logged in user. */
session: OnyxEntry<Session>;
};

type SplitBillDetailsPageOnyxProps = SplitBillDetailsPageTransactionOnyxProps & SplitBillDetailsPageOnyxPropsWithoutTransaction;

type SplitBillDetailsPageProps = WithReportAndReportActionOrNotFoundProps &
SplitBillDetailsPageOnyxProps &
StackScreenProps<SplitDetailsNavigatorParamList, typeof SCREENS.SPLIT_DETAILS.ROOT>;

function SplitBillDetailsPage({personalDetails, report, route, reportActions, transaction, draftTransaction, session}: SplitBillDetailsPageProps) {
function SplitBillDetailsPage({route, report, reportAction}: SplitBillDetailsPageProps) {
const styles = useThemeStyles();
const reportID = report?.reportID ?? '-1';
const {translate} = useLocalize();
const theme = useTheme();
const reportAction = useMemo(() => reportActions?.[route.params.reportActionID] ?? ({} as ReportAction), [reportActions, route.params.reportActionID]);
const participantAccountIDs = ReportActionsUtils.isMoneyRequestAction(reportAction) ? ReportActionsUtils.getOriginalMessage(reportAction)?.participantAccountIDs ?? [] : [];

const reportID = report?.reportID ?? '-1';
const originalMessage = reportAction && ReportActionsUtils.isMoneyRequestAction(reportAction) ? ReportActionsUtils.getOriginalMessage(reportAction) : undefined;
const IOUTransactionID = originalMessage?.IOUTransactionID ? originalMessage.IOUTransactionID : '-1';
const participantAccountIDs = originalMessage?.participantAccountIDs ?? [];

const [transaction] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${IOUTransactionID}`);
const [draftTransaction] = useOnyx(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${IOUTransactionID}`);
const [personalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST);
const [session] = useOnyx(ONYXKEYS.SESSION);

// In case this is workspace split expense, we manually add the workspace as the second participant of the split expense
// because we don't save any accountID in the report action's originalMessage other than the payee's accountID
Expand Down Expand Up @@ -166,37 +143,4 @@ function SplitBillDetailsPage({personalDetails, report, route, reportActions, tr

SplitBillDetailsPage.displayName = 'SplitBillDetailsPage';

const WrappedComponent = withOnyx<SplitBillDetailsPageProps, SplitBillDetailsPageTransactionOnyxProps>({
transaction: {
key: ({route, reportActions}) => {
const reportAction = reportActions?.[route.params.reportActionID];
const originalMessage = reportAction && ReportActionsUtils.isMoneyRequestAction(reportAction) ? ReportActionsUtils.getOriginalMessage(reportAction) : undefined;
const IOUTransactionID = reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU && originalMessage?.IOUTransactionID ? originalMessage.IOUTransactionID : 0;
return `${ONYXKEYS.COLLECTION.TRANSACTION}${IOUTransactionID}`;
},
},
draftTransaction: {
key: ({route, reportActions}) => {
const reportAction = reportActions?.[route.params.reportActionID];
const originalMessage = reportAction && ReportActionsUtils.isMoneyRequestAction(reportAction) ? ReportActionsUtils.getOriginalMessage(reportAction) : undefined;
const IOUTransactionID = reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU && originalMessage?.IOUTransactionID ? originalMessage.IOUTransactionID : 0;
return `${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${IOUTransactionID}`;
},
},
})(withReportAndReportActionOrNotFound(SplitBillDetailsPage) as ComponentType<SplitBillDetailsPageProps>);

export default withOnyx<Omit<SplitBillDetailsPageProps, keyof SplitBillDetailsPageTransactionOnyxProps>, SplitBillDetailsPageOnyxPropsWithoutTransaction>({
report: {
key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${route.params.reportID}`,
},
reportActions: {
key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${route.params.reportID}`,
canEvict: false,
},
personalDetails: {
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
},
session: {
key: ONYXKEYS.SESSION,
},
})(WrappedComponent);
export default withReportAndReportActionOrNotFound(SplitBillDetailsPage);
Loading