-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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.
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: Migrate withReportAndReportActionOrNotFound from withOnyx to useOnyx #49460
fix: Migrate withReportAndReportActionOrNotFound from withOnyx to useOnyx #49460
Conversation
@hoangzinh Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also think we should take a closer look at the props we're drilling down and only pass those that are actually used by wrapped components. That does require changing some other consuming files, but overall it's a big simplification.
- pass down only
report
,reportAction
(i.e: the reportAction linked in the route),parentReport
,parentReportAction
- Remove Onyx props
- Remove duplicate props from withOnyx in consuming components
Full diff
diff --git a/src/pages/FlagCommentPage.tsx b/src/pages/FlagCommentPage.tsx
index c54edae479d..3e16bc76ba4 100644
--- a/src/pages/FlagCommentPage.tsx
+++ b/src/pages/FlagCommentPage.tsx
@@ -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';
@@ -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>;
@@ -53,7 +46,7 @@ function getReportID(route: FlagCommentPageNavigationProps['route']) {
return route.params.reportID.toString();
}
-function FlagCommentPage({parentReportAction, route, report, parentReport, reportActions}: FlagCommentPageProps) {
+function FlagCommentPage({route, report, reportAction, parentReport, parentReportAction}: FlagCommentPageProps) {
const styles = useThemeStyles();
const {translate} = useLocalize();
@@ -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) {
@@ -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')} />
<ScrollView
contentContainerStyle={safeAreaPaddingBottomStyle}
diff --git a/src/pages/home/report/withReportAndReportActionOrNotFound.tsx b/src/pages/home/report/withReportAndReportActionOrNotFound.tsx
index fcf7af4b42b..db6a37764b8 100644
--- a/src/pages/home/report/withReportAndReportActionOrNotFound.tsx
+++ b/src/pages/home/report/withReportAndReportActionOrNotFound.tsx
@@ -1,8 +1,8 @@
/* 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} 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';
@@ -16,35 +16,23 @@ 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<TProps & RefAttributes<TRef>> {
@@ -66,7 +54,8 @@ export default function <TProps extends WithReportAndReportActionOrNotFoundProps
},
canEvict: false,
});
- const getReportAction = useCallback(() => {
+
+ const linkedReportAction = useMemo(() => {
let reportAction: OnyxEntry<OnyxTypes.ReportAction> = reportActions?.[`${props.route.params.reportActionID}`];
// Handle threads if needed
@@ -75,16 +64,14 @@ export default function <TProps extends WithReportAndReportActionOrNotFoundProps
}
return reportAction;
- }, [reportActions, props.route.params.reportActionID, parentReportAction]);
-
- const reportAction = getReportAction();
+ }, [parentReportAction, props.route.params.reportActionID, reportActions]);
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(report) && !isEmptyObject(reportAction))) {
+ if (!shouldUseNarrowLayout || (!isEmptyObject(report) && !isEmptyObject(linkedReportAction))) {
return;
}
Report.openReport(props.route.params.reportID);
@@ -93,7 +80,7 @@ export default function <TProps extends WithReportAndReportActionOrNotFoundProps
// Perform all the loading checks
const isLoadingReport = isLoadingReportData && !report?.reportID;
- const isLoadingReportAction = isEmptyObject(reportActions) || (reportMetadata?.isLoadingInitialReportActions && isEmptyObject(getReportAction()));
+ 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
@@ -104,14 +91,18 @@ 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 || (parentReportAction !== undefined && isEmptyObject(reportAction))) {
+ if (shouldHideReport || (parentReportAction !== undefined && isEmptyObject(linkedReportAction))) {
return <NotFoundPage />;
}
return (
<WrappedComponent
// eslint-disable-next-line react/jsx-props-no-spreading
- {...{...props, report, parentReport, reportMetadata, isLoadingReportData, betas, policies, reportActions, parentReportAction}}
+ {...props}
+ report={report}
+ reportAction={linkedReportAction}
+ parentReport={parentReport}
+ parentReportAction={parentReportAction}
ref={ref}
/>
);
diff --git a/src/pages/iou/SplitBillDetailsPage.tsx b/src/pages/iou/SplitBillDetailsPage.tsx
index efefc8ccc9e..95a539f8cd4 100644
--- a/src/pages/iou/SplitBillDetailsPage.tsx
+++ b/src/pages/iou/SplitBillDetailsPage.tsx
@@ -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';
@@ -26,46 +24,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({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 ?? '-1';
+ const participantAccountIDs = originalMessage?.participantAccountIDs ?? [];
+
+ const [transaction] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${IOUTransactionID}`);
+ const [draftTransaction] = useOnyx(`${ONYXKEYS.COLLECTION.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
@@ -162,37 +139,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);
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 : 0}`, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const [parentReportAction] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report ? report.parentReportID : 0}`, { | |
const [parentReportAction] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report ? report.parentReportID : '-1'}`, { |
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 : 0}`, { | ||
selector: (parentReportActions: OnyxEntry<OnyxTypes.ReportActions>): NonNullable<OnyxEntry<OnyxTypes.ReportAction>> | null => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can simplify the selector types by relying on the better type inference we get from useOnyx
:
diff --git a/src/pages/home/report/withReportAndReportActionOrNotFound.tsx b/src/pages/home/report/withReportAndReportActionOrNotFound.tsx
index fcf7af4b42b..52a4c389337 100644
--- a/src/pages/home/report/withReportAndReportActionOrNotFound.tsx
+++ b/src/pages/home/report/withReportAndReportActionOrNotFound.tsx
@@ -56,8 +56,8 @@ export default function <TProps extends WithReportAndReportActionOrNotFoundProps
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 : 0}`, {
- selector: (parentReportActions: OnyxEntry<OnyxTypes.ReportActions>): NonNullable<OnyxEntry<OnyxTypes.ReportAction>> | null => {
+ const [parentReportAction] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report ? report.parentReportID : '-1'}`, {
+ selector: (parentReportActions) => {
const parentReportActionID = report?.parentReportActionID;
if (!parentReportActionID) {
return null;
return <NotFoundPage />; | ||
} | ||
|
||
return ( | ||
<WrappedComponent | ||
// eslint-disable-next-line react/jsx-props-no-spreading | ||
{...props} | ||
{...{...props, report, parentReport, reportMetadata, isLoadingReportData, betas, policies, reportActions, parentReportAction}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's simplify this and only use the minimum amount of spread props:
diff --git a/src/pages/home/report/withReportAndReportActionOrNotFound.tsx b/src/pages/home/report/withReportAndReportActionOrNotFound.tsx
index fcf7af4b42b..2bb555ed550 100644
--- a/src/pages/home/report/withReportAndReportActionOrNotFound.tsx
+++ b/src/pages/home/report/withReportAndReportActionOrNotFound.tsx
@@ -111,7 +111,15 @@ export default function <TProps extends WithReportAndReportActionOrNotFoundProps
return (
<WrappedComponent
// eslint-disable-next-line react/jsx-props-no-spreading
- {...{...props, report, parentReport, reportMetadata, isLoadingReportData, betas, policies, reportActions, parentReportAction}}
+ {...props}
+ report={report}
+ parentReport={parentReport}
+ reportMetadata={reportMetadata}
+ isLoadingReportData={isLoadingReportData}
+ betas={betas}
+ policies={policies}
+ reportActions={reportActions}
+ parentReportAction={parentReportAction}
ref={ref}
/>
);
@etCoderDysto, do you have any updates on @roryabraham's feedback above? |
Yes, @hoangzinh. I am almost done. It will be ready for further review today. |
@hoangzinh @roryabraham I have applied the suggested chagnes. |
Thanks for the updates. I will review today |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍🏼
|
||
const reportID = report?.reportID ?? '-1'; | ||
const originalMessage = reportAction && ReportActionsUtils.isMoneyRequestAction(reportAction) ? ReportActionsUtils.getOriginalMessage(reportAction) : undefined; | ||
const IOUTransactionID = originalMessage?.IOUTransactionID ?? '-1'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that what we have here is correct. We still check for the reportAction being of type IOU on the previous line when we get the originalMessage
|
||
const reportID = report?.reportID ?? '-1'; | ||
const originalMessage = reportAction && ReportActionsUtils.isMoneyRequestAction(reportAction) ? ReportActionsUtils.getOriginalMessage(reportAction) : undefined; | ||
const IOUTransactionID = originalMessage?.IOUTransactionID ?? '-1'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though @hoangzinh could be right that ||
would be safer than ??
here to account for the possibility of IOUTransactionID
being an empty string. But in practice that should never be the case
@etCoderDysto do you have a ETA for the next review? Thank you. |
@hoangzinh Pr is ready for further review. Sorry for my late reply. |
LGTM, pulling in @thienlnam to take over further review in my absence |
@etCoderDysto overall, code looks good. Can you add another test steps for "Not found" case? Thank you. |
Reviewer Checklist
Screenshots/VideosAndroid: NativeScreen.Recording.2024-10-05.at.14.31.56.movAndroid: mWeb ChromeScreen.Recording.2024-10-05.at.14.00.48.android.chrome.moviOS: NativeScreen.Recording.2024-10-05.at.14.23.39.ios.moviOS: mWeb SafariScreen.Recording.2024-10-05.at.14.30.29.movMacOS: Chrome / SafariScreen.Recording.2024-10-05.at.13.47.16.web.movMacOS: DesktopScreen.Recording.2024-10-05.at.13.50.01.desktop.mov |
all yours @thienlnam |
Great! I will add not found case today |
I have added test steps for not found case, and uploaded videos except desktop. I was not able to open deeplink in desktop. I will try adding desktop when this is pushed to staging. |
We can use the Chrome dev tool to open in Desktop. But it's okay, no worry. Thanks for update @etCoderDysto |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, let's do it!
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/thienlnam in version: 9.0.46-0 🚀
|
🚀 Deployed to production by https://github.com/thienlnam in version: 9.0.46-5 🚀
|
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'}`); |
There was a problem hiding this comment.
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.
Details
Fixed Issues
$ #49107
PROPOSAL: #49107 (comment)
Tests
Test 1
Test 2
4. After completing step 3, copy the URL while split expense details page is opened
5. Add few number at the end of the URL
6. Copy the edited URL
7. Send it to any message
8. Open the URL
Test 3
Offline tests
Same as test stpes
QA Steps
Same as test steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Android.mp4
Not found page test
Android.mp4
Android: mWeb Chrome
moblie.chrome.mp4
Not found page test
https://github.com/user-attachments/assets/2d6c3eaa-abaa-4f0a-92df-1176aea33780
iOS: Native
ios.mp4
Not found page test
IOS.mp4
iOS: mWeb Safari
safari.mp4
Not found page test
Screen.Recording.2024-10-07.at.12.19.58.in.the.afternoon.mp4
MacOS: Chrome / Safari
Web.mp4
Not found page test
Desktop.web.mp4
MacOS: Desktop
Desktop.mp4