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

Migrate withReportAndPrivateNotesOrNotFound from withOnyx to useOnyx #49238

Merged
merged 13 commits into from
Oct 22, 2024
Merged

Migrate withReportAndPrivateNotesOrNotFound from withOnyx to useOnyx #49238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
4348206
Migrate withReportAndPrivateNotesOrNotFound from withOnyx to useOnyx
nkdengineer Sep 16, 2024
051ae85
fix: add new props HOC
nkdengineer Sep 16, 2024
f56fecc
fix lint
nkdengineer Sep 16, 2024
f22c5e8
migrate: withReportAndPrivateNotesOrNotFound and withReportOrNotFound
nkdengineer Sep 26, 2024
ea550f7
Merge branch 'main' into fix/49106
nkdengineer Sep 26, 2024
e11b7ac
remigrate withReportAndPrivateNotesOrNotFound
nkdengineer Sep 26, 2024
c29f3c8
fix lint
nkdengineer Sep 26, 2024
8850155
fix lint
nkdengineer Sep 26, 2024
1a8e8e6
Merge branch 'main' into fix/49106
nkdengineer Sep 30, 2024
80fe616
merge main
nkdengineer Oct 9, 2024
760ad29
Merge branch 'main' into fix/49106
nkdengineer Oct 21, 2024
14b7e7c
fix: using accountID
nkdengineer Oct 21, 2024
021c725
fix lint
nkdengineer Oct 21, 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
6 changes: 3 additions & 3 deletions src/libs/ReportUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4112,11 +4112,11 @@ function navigateBackAfterDeleteTransaction(backRoute: Route | undefined, isFrom
/**
* Go back to the previous page from the edit private page of a given report
*/
function goBackFromPrivateNotes(report: OnyxEntry<Report>, session: OnyxEntry<Session>, backTo?: string) {
if (isEmpty(report) || isEmpty(session) || !session.accountID) {
function goBackFromPrivateNotes(report: OnyxEntry<Report>, accountID?: number, backTo?: string) {
if (isEmpty(report) || !accountID) {
return;
}
const currentUserPrivateNote = report.privateNotes?.[session.accountID]?.note ?? '';
const currentUserPrivateNote = report.privateNotes?.[accountID]?.note ?? '';
if (isEmpty(currentUserPrivateNote)) {
const participantAccountIDs = getParticipantsAccountIDsForDisplay(report);

Expand Down
24 changes: 6 additions & 18 deletions src/pages/PrivateNotes/PrivateNotesEditPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ import {Str} from 'expensify-common';
import lodashDebounce from 'lodash/debounce';
import React, {useCallback, useMemo, useRef, useState} from 'react';
import {Keyboard} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import {useOnyx} from 'react-native-onyx';
import FormProvider from '@components/Form/FormProvider';
import InputWrapper from '@components/Form/InputWrapper';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
Expand All @@ -31,25 +30,20 @@ import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type SCREENS from '@src/SCREENS';
import INPUT_IDS from '@src/types/form/PrivateNotesForm';
import type {PersonalDetailsList, Report} from '@src/types/onyx';
import type {Report} from '@src/types/onyx';
import type {Note} from '@src/types/onyx/Report';

type PrivateNotesEditPageOnyxProps = {
/** All of the personal details for everyone */
personalDetailsList: OnyxEntry<PersonalDetailsList>;
};

type PrivateNotesEditPageProps = WithReportAndPrivateNotesOrNotFoundProps &
PrivateNotesEditPageOnyxProps &
StackScreenProps<PrivateNotesNavigatorParamList, typeof SCREENS.PRIVATE_NOTES.EDIT> & {
/** The report currently being looked at */
report: Report;
};

function PrivateNotesEditPage({route, personalDetailsList, report, session}: PrivateNotesEditPageProps) {
function PrivateNotesEditPage({route, report, accountID}: PrivateNotesEditPageProps) {
const backTo = route.params.backTo;
const styles = useThemeStyles();
const {translate} = useLocalize();
const [personalDetailsList] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST);

// We need to edit the note in markdown format, but display it in HTML format
const [privateNote, setPrivateNote] = useState(
Expand Down Expand Up @@ -117,7 +111,7 @@ function PrivateNotesEditPage({route, personalDetailsList, report, session}: Pri
>
<HeaderWithBackButton
title={translate('privateNotes.title')}
onBackButtonPress={() => ReportUtils.goBackFromPrivateNotes(report, session, backTo)}
onBackButtonPress={() => ReportUtils.goBackFromPrivateNotes(report, accountID, backTo)}
shouldShowBackButton
onCloseButtonPress={() => Navigation.dismissModal()}
/>
Expand Down Expand Up @@ -178,10 +172,4 @@ function PrivateNotesEditPage({route, personalDetailsList, report, session}: Pri

PrivateNotesEditPage.displayName = 'PrivateNotesEditPage';

export default withReportAndPrivateNotesOrNotFound('privateNotes.title')(
withOnyx<PrivateNotesEditPageProps, PrivateNotesEditPageOnyxProps>({
personalDetailsList: {
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
},
})(PrivateNotesEditPage),
);
export default withReportAndPrivateNotesOrNotFound('privateNotes.title')(PrivateNotesEditPage);
8 changes: 4 additions & 4 deletions src/pages/PrivateNotes/PrivateNotesListPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type NoteListItem = {
accountID: string;
};

function PrivateNotesListPage({report, session}: PrivateNotesListPageProps) {
function PrivateNotesListPage({report, accountID: sessionAccountID}: PrivateNotesListPageProps) {
const route = useRoute<RouteProp<PrivateNotesNavigatorParamList, typeof SCREENS.PRIVATE_NOTES.LIST>>();
const backTo = route.params.backTo;
const [personalDetailsList] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST);
Expand Down Expand Up @@ -76,14 +76,14 @@ function PrivateNotesListPage({report, session}: PrivateNotesListPageProps) {
return {
reportID: report.reportID,
accountID,
title: Number(session?.accountID) === Number(accountID) ? translate('privateNotes.myNote') : personalDetailsList?.[accountID]?.login ?? '',
title: Number(sessionAccountID) === Number(accountID) ? translate('privateNotes.myNote') : personalDetailsList?.[accountID]?.login ?? '',
action: () => Navigation.navigate(ROUTES.PRIVATE_NOTES_EDIT.getRoute(report.reportID, accountID, backTo)),
brickRoadIndicator: privateNoteBrickRoadIndicator(Number(accountID)),
note: privateNote?.note ?? '',
disabled: Number(session?.accountID) !== Number(accountID),
disabled: Number(sessionAccountID) !== Number(accountID),
};
});
}, [report, personalDetailsList, session, translate, backTo]);
}, [report, personalDetailsList, sessionAccountID, translate, backTo]);

return (
<ScreenWrapper
Expand Down
33 changes: 13 additions & 20 deletions src/pages/home/report/withReportAndPrivateNotesOrNotFound.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React, {useEffect, useMemo} from 'react';
import type {ComponentType, ForwardedRef, RefAttributes} from 'react';
import type {OnyxEntry} from 'react-native-onyx';
import {useOnyx, withOnyx} from 'react-native-onyx';
import {useOnyx} from 'react-native-onyx';
import useLocalize from '@hooks/useLocalize';
import useNetwork from '@hooks/useNetwork';
import usePrevious from '@hooks/usePrevious';
Expand All @@ -12,31 +11,30 @@ import NotFoundPage from '@pages/ErrorPage/NotFoundPage';
import LoadingPage from '@pages/LoadingPage';
import type {TranslationPaths} from '@src/languages/types';
import ONYXKEYS from '@src/ONYXKEYS';
import type * as OnyxTypes from '@src/types/onyx';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import type {WithReportOrNotFoundProps} from './withReportOrNotFound';
import withReportOrNotFound from './withReportOrNotFound';

type WithReportAndPrivateNotesOrNotFoundOnyxProps = {
/** Session of currently logged in user */
session: OnyxEntry<OnyxTypes.Session>;
/** ID of the current user */
accountID?: number;
};

type WithReportAndPrivateNotesOrNotFoundProps = WithReportAndPrivateNotesOrNotFoundOnyxProps & WithReportOrNotFoundProps;
type WithReportAndPrivateNotesOrNotFoundProps = WithReportOrNotFoundProps & WithReportAndPrivateNotesOrNotFoundOnyxProps;

export default function (pageTitle: TranslationPaths) {
// eslint-disable-next-line rulesdir/no-negated-variables
return <TProps extends WithReportAndPrivateNotesOrNotFoundProps, TRef>(
WrappedComponent: ComponentType<TProps & RefAttributes<TRef>>,
): React.ComponentType<Omit<TProps, keyof WithReportAndPrivateNotesOrNotFoundOnyxProps> & RefAttributes<TRef>> => {
// eslint-disable-next-line rulesdir/no-negated-variables
function WithReportAndPrivateNotesOrNotFound(props: TProps, ref: ForwardedRef<TRef>) {
function WithReportAndPrivateNotesOrNotFound(props: Omit<TProps, keyof WithReportAndPrivateNotesOrNotFoundOnyxProps>, ref: ForwardedRef<TRef>) {
const {translate} = useLocalize();
const {isOffline} = useNetwork();
const {route, report, session} = props;
const [session] = useOnyx(ONYXKEYS.SESSION);
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like only accountID is used from the session, so let's narrow it down with a selector such that we only listen to and pass down the accountID.

However, when doing this, it looks like it requires changes in PrivateNotesEditPage and PrivateNotesListPage, so we'd need to migrate those to useOnyx as well. When I tried to do that locally, I ran into type errors, I think coming from withReportOrNotFound. So I think maybe we'll want to HOLD this one on the withReportOrNotFound migration

Copy link
Contributor

Choose a reason for hiding this comment

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

@roryabraham This will create a holding deadlock cycle.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shubham1206agra why we need to hold on this PR in withReportOrNotFound migration PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, let me explain in some more detail what led me to say that...

Basically I started with this diff locally:

diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts
index 78ebdd92751..511123053aa 100644
--- a/src/libs/ReportUtils.ts
+++ b/src/libs/ReportUtils.ts
@@ -4045,11 +4045,11 @@ function navigateBackAfterDeleteTransaction(backRoute: Route | undefined, isFrom
 /**
  * Go back to the previous page from the edit private page of a given report
  */
-function goBackFromPrivateNotes(report: OnyxEntry<Report>, session: OnyxEntry<Session>, backTo?: string) {
-    if (isEmpty(report) || isEmpty(session) || !session.accountID) {
+function goBackFromPrivateNotes(report: OnyxEntry<Report>, accountID: number, backTo?: string) {
+    if (isEmpty(report) || !accountID) {
         return;
     }
-    const currentUserPrivateNote = report.privateNotes?.[session.accountID]?.note ?? '';
+    const currentUserPrivateNote = report.privateNotes?.[accountID]?.note ?? '';
     if (isEmpty(currentUserPrivateNote)) {
         const participantAccountIDs = getParticipantsAccountIDsForDisplay(report);
 
diff --git a/src/pages/PrivateNotes/PrivateNotesEditPage.tsx b/src/pages/PrivateNotes/PrivateNotesEditPage.tsx
index ac8eb7f862b..3be994d1cbc 100644
--- a/src/pages/PrivateNotes/PrivateNotesEditPage.tsx
+++ b/src/pages/PrivateNotes/PrivateNotesEditPage.tsx
@@ -46,7 +46,7 @@ type PrivateNotesEditPageProps = WithReportAndPrivateNotesOrNotFoundProps &
         report: Report;
     };
 
-function PrivateNotesEditPage({route, personalDetailsList, report, session}: PrivateNotesEditPageProps) {
+function PrivateNotesEditPage({route, personalDetailsList, report, accountID}: PrivateNotesEditPageProps) {
     const backTo = route.params.backTo;
     const styles = useThemeStyles();
     const {translate} = useLocalize();
@@ -117,7 +117,7 @@ function PrivateNotesEditPage({route, personalDetailsList, report, session}: Pri
         >
             <HeaderWithBackButton
                 title={translate('privateNotes.title')}
-                onBackButtonPress={() => ReportUtils.goBackFromPrivateNotes(report, session, backTo)}
+                onBackButtonPress={() => ReportUtils.goBackFromPrivateNotes(report, accountID, backTo)}
                 shouldShowBackButton
                 onCloseButtonPress={() => Navigation.dismissModal()}
             />
diff --git a/src/pages/PrivateNotes/PrivateNotesListPage.tsx b/src/pages/PrivateNotes/PrivateNotesListPage.tsx
index cc7ee9f54da..71548bb81f9 100644
--- a/src/pages/PrivateNotes/PrivateNotesListPage.tsx
+++ b/src/pages/PrivateNotes/PrivateNotesListPage.tsx
@@ -43,7 +43,7 @@ type NoteListItem = {
     accountID: string;
 };
 
-function PrivateNotesListPage({report, personalDetailsList, session}: PrivateNotesListPageProps) {
+function PrivateNotesListPage({report, personalDetailsList, accountID: currentUserAccountID}: PrivateNotesListPageProps) {
     const route = useRoute<RouteProp<PrivateNotesNavigatorParamList, typeof SCREENS.PRIVATE_NOTES.LIST>>();
     const backTo = route.params.backTo;
     const styles = useThemeStyles();
@@ -82,14 +82,14 @@ function PrivateNotesListPage({report, personalDetailsList, session}: PrivateNot
             return {
                 reportID: report.reportID,
                 accountID,
-                title: Number(session?.accountID) === Number(accountID) ? translate('privateNotes.myNote') : personalDetailsList?.[accountID]?.login ?? '',
+                title: currentUserAccountID === Number(accountID) ? translate('privateNotes.myNote') : personalDetailsList?.[accountID]?.login ?? '',
                 action: () => Navigation.navigate(ROUTES.PRIVATE_NOTES_EDIT.getRoute(report.reportID, accountID, backTo)),
                 brickRoadIndicator: privateNoteBrickRoadIndicator(Number(accountID)),
                 note: privateNote?.note ?? '',
-                disabled: Number(session?.accountID) !== Number(accountID),
+                disabled: currentUserAccountID !== Number(accountID),
             };
         });
-    }, [report, personalDetailsList, session, translate, backTo]);
+    }, [currentUserAccountID, report, personalDetailsList, translate, backTo]);
 
     return (
         <ScreenWrapper
diff --git a/src/pages/home/report/withReportAndPrivateNotesOrNotFound.tsx b/src/pages/home/report/withReportAndPrivateNotesOrNotFound.tsx
index 410af482a36..3ca623e0bc3 100644
--- a/src/pages/home/report/withReportAndPrivateNotesOrNotFound.tsx
+++ b/src/pages/home/report/withReportAndPrivateNotesOrNotFound.tsx
@@ -12,14 +12,13 @@ import NotFoundPage from '@pages/ErrorPage/NotFoundPage';
 import LoadingPage from '@pages/LoadingPage';
 import type {TranslationPaths} from '@src/languages/types';
 import ONYXKEYS from '@src/ONYXKEYS';
-import type {Session} from '@src/types/onyx';
 import {isEmptyObject} from '@src/types/utils/EmptyObject';
 import type {WithReportOrNotFoundOnyxProps, WithReportOrNotFoundProps} from './withReportOrNotFound';
 import withReportOrNotFound from './withReportOrNotFound';
 
 type WithReportAndPrivateNotesOrNotFoundOnyxProps = {
-    /** Session of currently logged in user */
-    session: OnyxEntry<Session>;
+    /** accountID of currently logged in user */
+    accountID: number;
 };
 
 type WithReportAndPrivateNotesOrNotFoundProps = WithReportOrNotFoundProps & WithReportAndPrivateNotesOrNotFoundOnyxProps;
@@ -32,16 +31,16 @@ export default function (pageTitle: TranslationPaths) {
         function WithReportAndPrivateNotesOrNotFound(props: Omit<TProps, keyof WithReportAndPrivateNotesOrNotFoundOnyxProps>, ref: ForwardedRef<TRef>) {
             const {translate} = useLocalize();
             const {isOffline} = useNetwork();
-            const [session] = useOnyx(ONYXKEYS.SESSION);
+            const [currentUserAccountID] = useOnyx(ONYXKEYS.SESSION, {selector: (val) => val?.accountID});
             const {route, report} = props;
             const [reportNameValuePairs] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report?.reportID ?? -1}`);
-            const accountID = ('accountID' in route.params && route.params.accountID) || '';
+            const accountIDFromRoute = Number(('accountID' in route.params && route.params.accountID) || '');
             const isPrivateNotesFetchTriggered = report?.isLoadingPrivateNotes !== undefined;
             const prevIsOffline = usePrevious(isOffline);
             const isReconnecting = prevIsOffline && !isOffline;
-            const isOtherUserNote = !!accountID && Number(session?.accountID) !== Number(accountID);
+            const isOtherUserNote = !!accountIDFromRoute && currentUserAccountID !== accountIDFromRoute;
             const isPrivateNotesFetchFinished = isPrivateNotesFetchTriggered && !report.isLoadingPrivateNotes;
-            const isPrivateNotesUndefined = accountID ? report?.privateNotes?.[Number(accountID)]?.note === undefined : isEmptyObject(report?.privateNotes);
+            const isPrivateNotesUndefined = accountIDFromRoute ? report?.privateNotes?.[accountIDFromRoute]?.note === undefined : isEmptyObject(report?.privateNotes);
 
             useEffect(() => {
                 // Do not fetch private notes if isLoadingPrivateNotes is already defined, or if network is offline.
@@ -75,7 +74,7 @@ export default function (pageTitle: TranslationPaths) {
                 return <LoadingPage title={translate(pageTitle)} />;
             }
 
-            if (shouldShowNotFoundPage) {
+            if (shouldShowNotFoundPage || !currentUserAccountID) {
                 return <NotFoundPage />;
             }
 
@@ -84,7 +83,7 @@ export default function (pageTitle: TranslationPaths) {
                     // eslint-disable-next-line react/jsx-props-no-spreading
                     {...(props as TProps)}
                     ref={ref}
-                    session={session}
+                    accountID={currentUserAccountID}
                 />
             );
         }

So far so good... but then we have to migrate PrivateNotesEditPage and PrivateNotesListPage to useOnyx. So on top of the previous diff, I added these changes:

diff --git a/src/pages/PrivateNotes/PrivateNotesEditPage.tsx b/src/pages/PrivateNotes/PrivateNotesEditPage.tsx
index 3be994d1cbc..071c76f8a9d 100644
--- a/src/pages/PrivateNotes/PrivateNotesEditPage.tsx
+++ b/src/pages/PrivateNotes/PrivateNotesEditPage.tsx
@@ -4,8 +4,7 @@ import {Str} from 'expensify-common';
 import lodashDebounce from 'lodash/debounce';
 import React, {useCallback, useMemo, useRef, useState} from 'react';
 import {Keyboard} from 'react-native';
-import type {OnyxEntry} from 'react-native-onyx';
-import {withOnyx} from 'react-native-onyx';
+import {useOnyx} from 'react-native-onyx';
 import FormProvider from '@components/Form/FormProvider';
 import InputWrapper from '@components/Form/InputWrapper';
 import HeaderWithBackButton from '@components/HeaderWithBackButton';
@@ -31,26 +30,22 @@ import ONYXKEYS from '@src/ONYXKEYS';
 import ROUTES from '@src/ROUTES';
 import type SCREENS from '@src/SCREENS';
 import INPUT_IDS from '@src/types/form/PrivateNotesForm';
-import type {PersonalDetailsList, Report} from '@src/types/onyx';
+import type {Report} from '@src/types/onyx';
 import type {Note} from '@src/types/onyx/Report';
 
-type PrivateNotesEditPageOnyxProps = {
-    /** All of the personal details for everyone */
-    personalDetailsList: OnyxEntry<PersonalDetailsList>;
-};
-
 type PrivateNotesEditPageProps = WithReportAndPrivateNotesOrNotFoundProps &
-    PrivateNotesEditPageOnyxProps &
     StackScreenProps<PrivateNotesNavigatorParamList, typeof SCREENS.PRIVATE_NOTES.EDIT> & {
         /** The report currently being looked at */
         report: Report;
     };
 
-function PrivateNotesEditPage({route, personalDetailsList, report, accountID}: PrivateNotesEditPageProps) {
+function PrivateNotesEditPage({route, report, accountID}: PrivateNotesEditPageProps) {
     const backTo = route.params.backTo;
     const styles = useThemeStyles();
     const {translate} = useLocalize();
 
+    const [personalDetailsList] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST);
+
     // We need to edit the note in markdown format, but display it in HTML format
     const [privateNote, setPrivateNote] = useState(
         () => ReportActions.getDraftPrivateNote(report.reportID).trim() || Parser.htmlToMarkdown(report?.privateNotes?.[Number(route.params.accountID)]?.note ?? '').trim(),
@@ -178,10 +173,4 @@ function PrivateNotesEditPage({route, personalDetailsList, report, accountID}: P
 
 PrivateNotesEditPage.displayName = 'PrivateNotesEditPage';
 
-export default withReportAndPrivateNotesOrNotFound('privateNotes.title')(
-    withOnyx<PrivateNotesEditPageProps, PrivateNotesEditPageOnyxProps>({
-        personalDetailsList: {
-            key: ONYXKEYS.PERSONAL_DETAILS_LIST,
-        },
-    })(PrivateNotesEditPage),
-);
+export default withReportAndPrivateNotesOrNotFound('privateNotes.title')(PrivateNotesEditPage);
diff --git a/src/pages/PrivateNotes/PrivateNotesListPage.tsx b/src/pages/PrivateNotes/PrivateNotesListPage.tsx
index 71548bb81f9..cb5e9a6f51e 100644
--- a/src/pages/PrivateNotes/PrivateNotesListPage.tsx
+++ b/src/pages/PrivateNotes/PrivateNotesListPage.tsx
@@ -1,8 +1,7 @@
 import type {RouteProp} from '@react-navigation/native';
 import {useRoute} from '@react-navigation/native';
 import React, {useCallback, useMemo} from 'react';
-import type {OnyxEntry} from 'react-native-onyx';
-import {withOnyx} from 'react-native-onyx';
+import {useOnyx} from 'react-native-onyx';
 import type {ValueOf} from 'type-fest';
 import {AttachmentContext} from '@components/AttachmentContext';
 import HeaderWithBackButton from '@components/HeaderWithBackButton';
@@ -20,19 +19,13 @@ import CONST from '@src/CONST';
 import ONYXKEYS from '@src/ONYXKEYS';
 import ROUTES from '@src/ROUTES';
 import type SCREENS from '@src/SCREENS';
-import type {PersonalDetailsList, Report} from '@src/types/onyx';
+import type {Report} from '@src/types/onyx';
 
-type PrivateNotesListPageOnyxProps = {
-    /** All of the personal details for everyone */
-    personalDetailsList: OnyxEntry<PersonalDetailsList>;
+type PrivateNotesListPageProps = WithReportAndPrivateNotesOrNotFoundProps & {
+    /** The report currently being looked at */
+    report: Report;
 };
 
-type PrivateNotesListPageProps = WithReportAndPrivateNotesOrNotFoundProps &
-    PrivateNotesListPageOnyxProps & {
-        /** The report currently being looked at */
-        report: Report;
-    };
-
 type NoteListItem = {
     title: string;
     action: () => void;
@@ -43,13 +36,15 @@ type NoteListItem = {
     accountID: string;
 };
 
-function PrivateNotesListPage({report, personalDetailsList, accountID: currentUserAccountID}: PrivateNotesListPageProps) {
+function PrivateNotesListPage({report, accountID: currentUserAccountID}: PrivateNotesListPageProps) {
     const route = useRoute<RouteProp<PrivateNotesNavigatorParamList, typeof SCREENS.PRIVATE_NOTES.LIST>>();
     const backTo = route.params.backTo;
     const styles = useThemeStyles();
     const {translate} = useLocalize();
     const getAttachmentValue = useCallback((item: NoteListItem) => ({reportID: item.reportID, accountID: Number(item.accountID), type: CONST.ATTACHMENT_TYPE.NOTE}), []);
 
+    const [personalDetailsList] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST);
+
     /**
      * Gets the menu item for each workspace
      */
@@ -115,10 +110,4 @@ function PrivateNotesListPage({report, personalDetailsList, accountID: currentUs
 
 PrivateNotesListPage.displayName = 'PrivateNotesListPage';
 
-export default withReportAndPrivateNotesOrNotFound('privateNotes.title')(
-    withOnyx<PrivateNotesListPageProps, PrivateNotesListPageOnyxProps>({
-        personalDetailsList: {
-            key: ONYXKEYS.PERSONAL_DETAILS_LIST,
-        },
-    })(PrivateNotesListPage),
-);
+export default withReportAndPrivateNotesOrNotFound('privateNotes.title')(PrivateNotesListPage);

But now I get a type error on the last line of PrivateNotesEditPage, which ultimately is Property navigation is missing in type.... In other words, the type problem is that withReportOrNotFound adds the route prop but not the navigation prop. So that problem needs to be fixed in withReportOrNotFound.

Copy link
Contributor

Choose a reason for hiding this comment

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

Now that the other PR is merged, @nkdengineer can we address this comment from Rory?

It looks like only accountID is used from the session, so let's narrow it down with a selector such that we only listen to and pass down the accountID.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@akinwale @youssef-lr i updated, please check again

const {route, report} = props;
const [reportNameValuePairs] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report?.reportID ?? -1}`);
const accountID = ('accountID' in route.params && route.params.accountID) || '';
const isPrivateNotesFetchTriggered = report.isLoadingPrivateNotes !== undefined;
const isPrivateNotesFetchTriggered = report?.isLoadingPrivateNotes !== undefined;
const prevIsOffline = usePrevious(isOffline);
const isReconnecting = prevIsOffline && !isOffline;
const isOtherUserNote = !!accountID && Number(session?.accountID) !== Number(accountID);
Expand All @@ -49,9 +47,9 @@ export default function (pageTitle: TranslationPaths) {
return;
}

Report.getReportPrivateNote(report.reportID);
Report.getReportPrivateNote(report?.reportID);
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps -- do not add report.isLoadingPrivateNotes to dependencies
}, [report.reportID, isOffline, isPrivateNotesFetchTriggered, isReconnecting]);
}, [report?.reportID, isOffline, isPrivateNotesFetchTriggered, isReconnecting]);

const shouldShowFullScreenLoadingIndicator = !isPrivateNotesFetchFinished;

Expand Down Expand Up @@ -82,21 +80,16 @@ export default function (pageTitle: TranslationPaths) {
return (
<WrappedComponent
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
{...(props as TProps)}
ref={ref}
accountID={session?.accountID}
/>
);
}

WithReportAndPrivateNotesOrNotFound.displayName = `withReportAndPrivateNotesOrNotFound(${getComponentDisplayName(WrappedComponent)})`;

return withReportOrNotFound()(
withOnyx<TProps & RefAttributes<TRef>, WithReportAndPrivateNotesOrNotFoundOnyxProps>({
session: {
key: ONYXKEYS.SESSION,
},
})(WithReportAndPrivateNotesOrNotFound),
);
return withReportOrNotFound()(WithReportAndPrivateNotesOrNotFound);
};
}

Expand Down
Loading