From 961f7245f7bd308471698f99e80b81983eae9345 Mon Sep 17 00:00:00 2001 From: VickyStash Date: Fri, 10 Jan 2025 12:01:19 +0100 Subject: [PATCH 01/12] Remove ReportScreen from the ignored list --- .eslintrc.changed.js | 1 - src/pages/home/ReportScreen.tsx | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.eslintrc.changed.js b/.eslintrc.changed.js index 55472b10ea86..bdff4055fe30 100644 --- a/.eslintrc.changed.js +++ b/.eslintrc.changed.js @@ -16,7 +16,6 @@ module.exports = { 'src/libs/actions/Task.ts', 'src/libs/OptionsListUtils.ts', 'src/libs/TransactionUtils/index.ts', - 'src/pages/home/ReportScreen.tsx', 'src/pages/workspace/WorkspaceInitialPage.tsx', 'src/pages/home/report/PureReportActionItem.tsx', ], diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index ef3137a8c7d2..9662b0e7bfbf 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -724,7 +724,7 @@ function ReportScreen({route, navigation}: ReportScreenProps) { return; } // After creating the task report then navigating to task detail we don't have any report actions and the last read time is empty so We need to update the initial last read time when opening the task report detail. - Report.readNewestAction(report?.reportID ?? ''); + Report.readNewestAction(report?.reportID); }, [report]); const mostRecentReportAction = reportActions.at(0); const isMostRecentReportIOU = mostRecentReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU; From 1f904253a8fb9ffbeb2cbc2fa17ffcc64c0204b9 Mon Sep 17 00:00:00 2001 From: VickyStash Date: Fri, 10 Jan 2025 14:53:25 +0100 Subject: [PATCH 02/12] Align default ids usages --- src/ROUTES.ts | 6 +++- .../LocalNotification/index.desktop.ts | 2 +- .../Notification/LocalNotification/index.ts | 2 +- .../clearReportNotifications/index.native.ts | 2 +- .../clearReportNotifications/types.ts | 2 +- src/libs/actions/Report.ts | 2 +- src/pages/home/ReportScreen.tsx | 33 +++++++++---------- 7 files changed, 25 insertions(+), 24 deletions(-) diff --git a/src/ROUTES.ts b/src/ROUTES.ts index 062da712cd7f..caa3674c7601 100644 --- a/src/ROUTES.ts +++ b/src/ROUTES.ts @@ -3,6 +3,7 @@ import type {SearchQueryString} from './components/Search/types'; import type CONST from './CONST'; import type {IOUAction, IOUType} from './CONST'; import type {IOURequestType} from './libs/actions/IOU'; +import Log from './libs/Log'; import type {ExitReason} from './types/form/ExitSurveyReasonForm'; import type {ConnectionName, SageIntacctMappingName} from './types/onyx/Policy'; import type AssertTypesNotEqual from './types/utils/AssertTypesNotEqual'; @@ -296,7 +297,10 @@ const ROUTES = { REPORT: 'r', REPORT_WITH_ID: { route: 'r/:reportID?/:reportActionID?', - getRoute: (reportID: string, reportActionID?: string, referrer?: string) => { + getRoute: (reportID: string | undefined, reportActionID?: string, referrer?: string) => { + if (!reportID) { + Log.warn('Invalid reportID is used to build the REPORT_WITH_ID route'); + } const baseRoute = reportActionID ? (`r/${reportID}/${reportActionID}` as const) : (`r/${reportID}` as const); const referrerParam = referrer ? `?referrer=${encodeURIComponent(referrer)}` : ''; return `${baseRoute}${referrerParam}` as const; diff --git a/src/libs/Notification/LocalNotification/index.desktop.ts b/src/libs/Notification/LocalNotification/index.desktop.ts index 4baf1abee139..e0ecd11fd6f7 100644 --- a/src/libs/Notification/LocalNotification/index.desktop.ts +++ b/src/libs/Notification/LocalNotification/index.desktop.ts @@ -14,7 +14,7 @@ function showModifiedExpenseNotification(report: Report, reportAction: ReportAct BrowserNotifications.pushModifiedExpenseNotification(report, reportAction, onClick); } -function clearReportNotifications(reportID: string) { +function clearReportNotifications(reportID: string | undefined) { BrowserNotifications.clearNotifications((notificationData) => notificationData.reportID === reportID); } diff --git a/src/libs/Notification/LocalNotification/index.ts b/src/libs/Notification/LocalNotification/index.ts index 2141000d807d..997d796ba0ad 100644 --- a/src/libs/Notification/LocalNotification/index.ts +++ b/src/libs/Notification/LocalNotification/index.ts @@ -14,7 +14,7 @@ function showModifiedExpenseNotification(report: Report, reportAction: ReportAct BrowserNotifications.pushModifiedExpenseNotification(report, reportAction, onClick, true); } -function clearReportNotifications(reportID: string) { +function clearReportNotifications(reportID: string | undefined) { BrowserNotifications.clearNotifications((notificationData) => notificationData.reportID === reportID); } diff --git a/src/libs/Notification/clearReportNotifications/index.native.ts b/src/libs/Notification/clearReportNotifications/index.native.ts index ad7d80a5b832..bddb2dcb42a5 100644 --- a/src/libs/Notification/clearReportNotifications/index.native.ts +++ b/src/libs/Notification/clearReportNotifications/index.native.ts @@ -13,7 +13,7 @@ const parseNotificationAndReportIDs = (pushPayload: PushPayload) => { }; }; -const clearReportNotifications: ClearReportNotifications = (reportID: string) => { +const clearReportNotifications: ClearReportNotifications = (reportID: string | undefined) => { Log.info('[PushNotification] clearing report notifications', false, {reportID}); Airship.push diff --git a/src/libs/Notification/clearReportNotifications/types.ts b/src/libs/Notification/clearReportNotifications/types.ts index ec01dc0aaeb3..86e36d10b75c 100644 --- a/src/libs/Notification/clearReportNotifications/types.ts +++ b/src/libs/Notification/clearReportNotifications/types.ts @@ -1,3 +1,3 @@ -type ClearReportNotifications = (reportID: string) => void; +type ClearReportNotifications = (reportID: string | undefined) => void; export default ClearReportNotifications; diff --git a/src/libs/actions/Report.ts b/src/libs/actions/Report.ts index 1e157b983483..e345a092de3f 100644 --- a/src/libs/actions/Report.ts +++ b/src/libs/actions/Report.ts @@ -800,7 +800,7 @@ function clearAvatarErrors(reportID: string) { * @param participantAccountIDList The list of accountIDs that are included in a new chat, not including the user creating it */ function openReport( - reportID: string, + reportID: string | undefined, reportActionID?: string, participantLoginList: string[] = [], newReportObject?: ReportUtils.OptimisticChatReport, diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 9662b0e7bfbf..8fd83f755853 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -73,7 +73,7 @@ const defaultReportMetadata = { function getReportID(route: ReportScreenNavigationProps['route']): string { // The report ID is used in an onyx key. If it's an empty string, onyx will return // a collection instead of an individual report. - return String(route.params?.reportID || 0); + return String(route.params?.reportID || undefined); } /** @@ -94,7 +94,7 @@ function getParentReportAction(parentReportActions: OnyxEntry getParentReportAction(parentReportActions, reportOnyx?.parentReportActionID ?? ''), + selector: (parentReportActions) => getParentReportAction(parentReportActions, reportOnyx?.parentReportActionID), }); const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP); const wasLoadingApp = usePrevious(isLoadingApp); @@ -190,7 +187,7 @@ function ReportScreen({route, navigation}: ReportScreenProps) { () => reportOnyx && { lastReadTime: reportOnyx.lastReadTime, - reportID: reportOnyx.reportID ?? '', + reportID: reportOnyx.reportID, policyID: reportOnyx.policyID, lastVisibleActionCreated: reportOnyx.lastVisibleActionCreated, statusNum: reportOnyx.statusNum, @@ -272,12 +269,12 @@ function ReportScreen({route, navigation}: ReportScreenProps) { const hasHelpfulErrors = Object.keys(report?.errorFields ?? {}).some((key) => key !== 'notFound'); const shouldHideReport = !hasHelpfulErrors && !ReportUtils.canAccessReport(report, policies, betas); - const transactionThreadReportID = ReportActionsUtils.getOneTransactionThreadReportID(reportID ?? '', reportActions ?? [], isOffline); + const transactionThreadReportID = ReportActionsUtils.getOneTransactionThreadReportID(reportID, reportActions ?? [], isOffline); const [transactionThreadReportActions = {}] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transactionThreadReportID}`); const combinedReportActions = ReportActionsUtils.getCombinedReportActions(reportActions, transactionThreadReportID ?? null, Object.values(transactionThreadReportActions)); const lastReportAction = [...combinedReportActions, parentReportAction].find((action) => ReportUtils.canEditReportAction(action) && !ReportActionsUtils.isMoneyRequestAction(action)); const isSingleTransactionView = ReportUtils.isMoneyRequest(report) || ReportUtils.isTrackExpenseReport(report); - const policy = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID ?? '-1'}`]; + const policy = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`]; const isTopMostReportId = currentReportIDValue?.currentReportID === reportIDFromRoute; const didSubscribeToReportLeavingEvents = useRef(false); const [showSoftInputOnFocus, setShowSoftInputOnFocus] = useState(false); @@ -320,7 +317,7 @@ function ReportScreen({route, navigation}: ReportScreenProps) { } useEffect(() => { - if (!transactionThreadReportID || !route?.params?.reportActionID || !ReportUtils.isOneTransactionThread(linkedAction?.childReportID ?? '-1', reportID ?? '', linkedAction)) { + if (!transactionThreadReportID || !route?.params?.reportActionID || !ReportUtils.isOneTransactionThread(linkedAction?.childReportID, reportID, linkedAction)) { return; } Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(route?.params?.reportID)); @@ -501,7 +498,7 @@ function ReportScreen({route, navigation}: ReportScreenProps) { }, []); const chatWithAccountManager = useCallback(() => { - Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(accountManagerReportID ?? '')); + Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(accountManagerReportID)); }, [accountManagerReportID]); // Clear notifications for the current report when it's opened and re-focused @@ -511,7 +508,7 @@ function ReportScreen({route, navigation}: ReportScreenProps) { return; } - clearReportNotifications(reportID ?? ''); + clearReportNotifications(reportID); }, [reportID, isTopMostReportId]); useEffect(clearNotifications, [clearNotifications]); @@ -527,7 +524,7 @@ function ReportScreen({route, navigation}: ReportScreenProps) { return; } - Report.unsubscribeFromLeavingRoomReportChannel(reportID ?? ''); + Report.unsubscribeFromLeavingRoomReportChannel(reportID); }; // I'm disabling the warning, as it expects to use exhaustive deps, even though we want this useEffect to run only on the first render. @@ -560,7 +557,7 @@ function ReportScreen({route, navigation}: ReportScreenProps) { if (!shouldUseNarrowLayout || !isFocused || prevIsFocused || !ReportUtils.isChatThread(report) || !ReportUtils.isHiddenForCurrentUser(report) || isSingleTransactionView) { return; } - Report.openReport(reportID ?? ''); + Report.openReport(reportID); // We don't want to run this useEffect every time `report` is changed // Excluding shouldUseNarrowLayout from the dependency list to prevent re-triggering on screen resize events. @@ -759,7 +756,7 @@ function ReportScreen({route, navigation}: ReportScreenProps) { navigation={navigation} style={screenWrapperStyle} shouldEnableKeyboardAvoidingView={(isTopMostReportId || isInNarrowPaneModal) && (!isComposerFocus || showSoftInputOnFocus)} - testID={`report-screen-${reportID ?? ''}`} + testID={`report-screen-${reportID}`} > Date: Fri, 10 Jan 2025 15:37:03 +0100 Subject: [PATCH 03/12] Update the test --- src/pages/home/ReportScreen.tsx | 2 +- tests/ui/PaginationTest.tsx | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 8fd83f755853..e1236cb336c1 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -101,7 +101,7 @@ function ReportScreen({route, navigation}: ReportScreenProps) { const styles = useThemeStyles(); const {translate} = useLocalize(); const reportIDFromRoute = getReportID(route); - const reportActionIDFromRoute = route?.params?.reportActionID ?? ''; + const reportActionIDFromRoute = route?.params?.reportActionID; const isFocused = useIsFocused(); const prevIsFocused = usePrevious(isFocused); const firstRenderRef = useRef(true); diff --git a/tests/ui/PaginationTest.tsx b/tests/ui/PaginationTest.tsx index 8fcd6dbac1d6..adef58aa55f2 100644 --- a/tests/ui/PaginationTest.tsx +++ b/tests/ui/PaginationTest.tsx @@ -281,7 +281,7 @@ describe('Pagination', () => { expect(getReportActions()).toHaveLength(5); TestHelper.expectAPICommandToHaveBeenCalled('OpenReport', 1); - TestHelper.expectAPICommandToHaveBeenCalledWith('OpenReport', 0, {reportID: REPORT_ID, reportActionID: ''}); + TestHelper.expectAPICommandToHaveBeenCalledWith('OpenReport', 0, {reportID: REPORT_ID}); TestHelper.expectAPICommandToHaveBeenCalled('GetOlderActions', 0); TestHelper.expectAPICommandToHaveBeenCalled('GetNewerActions', 0); @@ -305,7 +305,7 @@ describe('Pagination', () => { expect(getReportActions()).toHaveLength(CONST.REPORT.MIN_INITIAL_REPORT_ACTION_COUNT); TestHelper.expectAPICommandToHaveBeenCalled('OpenReport', 1); - TestHelper.expectAPICommandToHaveBeenCalledWith('OpenReport', 0, {reportID: REPORT_ID, reportActionID: ''}); + TestHelper.expectAPICommandToHaveBeenCalledWith('OpenReport', 0, {reportID: REPORT_ID}); TestHelper.expectAPICommandToHaveBeenCalled('GetOlderActions', 0); TestHelper.expectAPICommandToHaveBeenCalled('GetNewerActions', 0); From 880cb7efa69fecde8deefe19b729d0099a3cc033 Mon Sep 17 00:00:00 2001 From: VickyStash Date: Fri, 10 Jan 2025 17:11:47 +0100 Subject: [PATCH 04/12] Code improvements --- src/ROUTES.ts | 14 ++++++++++++-- src/libs/ReportUtils.ts | 4 ++-- src/libs/actions/Report.ts | 2 +- src/pages/home/HeaderView.tsx | 2 +- src/pages/home/ReportScreen.tsx | 15 +++++++-------- 5 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/ROUTES.ts b/src/ROUTES.ts index caa3674c7601..e181e08ec2b5 100644 --- a/src/ROUTES.ts +++ b/src/ROUTES.ts @@ -366,7 +366,12 @@ const ROUTES = { }, REPORT_WITH_ID_DETAILS: { route: 'r/:reportID/details', - getRoute: (reportID: string, backTo?: string) => getUrlWithBackToParam(`r/${reportID}/details`, backTo), + getRoute: (reportID: string | undefined, backTo?: string) => { + if (!reportID) { + Log.warn('Invalid reportID is used to build the REPORT_WITH_ID_DETAILS route'); + } + return getUrlWithBackToParam(`r/${reportID}/details`, backTo); + }, }, REPORT_WITH_ID_DETAILS_EXPORT: { route: 'r/:reportID/details/export/:connectionName', @@ -402,7 +407,12 @@ const ROUTES = { }, REPORT_DESCRIPTION: { route: 'r/:reportID/description', - getRoute: (reportID: string, backTo?: string) => getUrlWithBackToParam(`r/${reportID}/description` as const, backTo), + getRoute: (reportID: string | undefined, backTo?: string) => { + if (!reportID) { + Log.warn('Invalid reportID is used to build the REPORT_DESCRIPTION route'); + } + return getUrlWithBackToParam(`r/${reportID}/description` as const, backTo); + }, }, TASK_ASSIGNEE: { route: 'r/:reportID/assignee', diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 7494901cf786..08d940ee96b6 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -7360,8 +7360,8 @@ function isReportDataReady(): boolean { /** * Return true if reportID from path is valid */ -function isValidReportIDFromPath(reportIDFromPath: string): boolean { - return !['', 'null', '0', '-1'].includes(reportIDFromPath); +function isValidReportIDFromPath(reportIDFromPath: string | undefined): boolean { + return !!reportIDFromPath && !['', 'null', '0', '-1'].includes(reportIDFromPath); } /** diff --git a/src/libs/actions/Report.ts b/src/libs/actions/Report.ts index e345a092de3f..39743b1d348a 100644 --- a/src/libs/actions/Report.ts +++ b/src/libs/actions/Report.ts @@ -413,7 +413,7 @@ function subscribeToReportTypingEvents(reportID: string) { } /** Initialize our pusher subscriptions to listen for someone leaving a room. */ -function subscribeToReportLeavingEvents(reportID: string) { +function subscribeToReportLeavingEvents(reportID: string | undefined) { if (!reportID) { return; } diff --git a/src/pages/home/HeaderView.tsx b/src/pages/home/HeaderView.tsx index cf7e2adfccd0..9e7ce8bb2b00 100644 --- a/src/pages/home/HeaderView.tsx +++ b/src/pages/home/HeaderView.tsx @@ -52,7 +52,7 @@ type HeaderViewProps = { parentReportAction: OnyxEntry | null; /** The reportID of the current report */ - reportID: string; + reportID: string | undefined; /** Whether we should display the header as in narrow layout */ shouldUseNarrowLayout?: boolean; diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index e1236cb336c1..a03d3786458c 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -69,11 +69,11 @@ const defaultReportMetadata = { isOptimisticReport: false, }; -/** Get the currently viewed report ID as number */ -function getReportID(route: ReportScreenNavigationProps['route']): string { +/** Make sure the report id won't be an empty string as it can break an onyx key */ +function getNonEmptyStringReportID(reportID: string | undefined): string | undefined { // The report ID is used in an onyx key. If it's an empty string, onyx will return // a collection instead of an individual report. - return String(route.params?.reportID || undefined); + return reportID !== '' ? reportID : undefined; } /** @@ -100,7 +100,7 @@ function getParentReportAction(parentReportActions: OnyxEntry getParentReportAction(parentReportActions, reportOnyx?.parentReportActionID), }); @@ -379,8 +379,7 @@ function ReportScreen({route, navigation}: ReportScreenProps) { isEmptyObject(reportOnyx) || isLoadingReportOnyx || !isCurrentReportLoadedFromOnyx || - // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing - (deleteTransactionNavigateBackUrl && ReportUtils.getReportIDFromLink(deleteTransactionNavigateBackUrl) === report?.reportID) || + (!!deleteTransactionNavigateBackUrl && ReportUtils.getReportIDFromLink(deleteTransactionNavigateBackUrl) === report?.reportID) || (!reportMetadata.isOptimisticReport && isLoading); const isLinkedActionBecomesDeleted = prevIsLinkedActionDeleted !== undefined && !prevIsLinkedActionDeleted && isLinkedActionDeleted; From 0ca71c29f9078b4ff6723e4e5c6b51bdec5a049c Mon Sep 17 00:00:00 2001 From: VickyStash Date: Fri, 10 Jan 2025 17:23:57 +0100 Subject: [PATCH 05/12] Fix lint errors --- src/ROUTES.ts | 7 ++++++- src/libs/ReportUtils.ts | 8 ++++++++ src/pages/home/HeaderView.tsx | 11 ++++++----- src/pages/home/ReportScreen.tsx | 13 +++---------- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/ROUTES.ts b/src/ROUTES.ts index e181e08ec2b5..66c7ddc7219d 100644 --- a/src/ROUTES.ts +++ b/src/ROUTES.ts @@ -909,7 +909,12 @@ const ROUTES = { }, WORKSPACE_PROFILE_DESCRIPTION: { route: 'settings/workspaces/:policyID/profile/description', - getRoute: (policyID: string) => `settings/workspaces/${policyID}/profile/description` as const, + getRoute: (policyID: string | undefined) => { + if (!policyID) { + Log.warn('Invalid policyID is used to build the WORKSPACE_PROFILE_DESCRIPTION route'); + } + return `settings/workspaces/${policyID}/profile/description` as const; + }, }, WORKSPACE_PROFILE_SHARE: { route: 'settings/workspaces/:policyID/profile/share', diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 08d940ee96b6..8944fa357d57 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -8688,6 +8688,13 @@ function getReportMetadata(reportID: string | undefined) { return reportID ? allReportMetadataKeyValue[reportID] : undefined; } +/** Make sure the report id is not an empty string as it can break an onyx key */ +function getNonEmptyStringReportID(reportID: string | undefined): string | undefined { + // The report ID is used in an onyx key. If it's an empty string, onyx will return + // a collection instead of an individual report. + return reportID !== '' ? reportID : undefined; +} + export { addDomainToShortMention, completeShortMention, @@ -9013,6 +9020,7 @@ export { shouldUnmaskChat, getReportMetadata, buildOptimisticSelfDMReport, + getNonEmptyStringReportID, isHiddenForCurrentUser, }; diff --git a/src/pages/home/HeaderView.tsx b/src/pages/home/HeaderView.tsx index 9e7ce8bb2b00..a61ced1cbc8b 100644 --- a/src/pages/home/HeaderView.tsx +++ b/src/pages/home/HeaderView.tsx @@ -70,9 +70,10 @@ function HeaderView({report, parentReportAction, reportID, onNavigationMenuButto const {isSmallScreenWidth} = useResponsiveLayout(); const route = useRoute(); const [isDeleteTaskConfirmModalVisible, setIsDeleteTaskConfirmModalVisible] = React.useState(false); - const [invoiceReceiverPolicy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${report?.invoiceReceiver && 'policyID' in report.invoiceReceiver ? report.invoiceReceiver.policyID : -1}`); - // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing - const [parentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${report?.parentReportID || report?.reportID || '-1'}`); + const [invoiceReceiverPolicy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${report?.invoiceReceiver && 'policyID' in report.invoiceReceiver ? report.invoiceReceiver.policyID : undefined}`); + const [parentReport] = useOnyx( + `${ONYXKEYS.COLLECTION.REPORT}${ReportUtils.getNonEmptyStringReportID(report?.parentReportID) ?? ReportUtils.getNonEmptyStringReportID(report?.reportID)}`, + ); const policy = usePolicy(report?.policyID); const [personalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST); @@ -100,7 +101,7 @@ function HeaderView({report, parentReportAction, reportID, onNavigationMenuButto const reportDescription = Parser.htmlToText(ReportUtils.getReportDescription(report)); const policyName = ReportUtils.getPolicyName(report, true); const policyDescription = ReportUtils.getPolicyDescriptionText(policy); - const isPersonalExpenseChat = isPolicyExpenseChat && ReportUtils.isCurrentUserSubmitter(report?.reportID ?? ''); + const isPersonalExpenseChat = isPolicyExpenseChat && ReportUtils.isCurrentUserSubmitter(report?.reportID); const shouldShowSubtitle = () => { if (!subtitle) { return false; @@ -258,7 +259,7 @@ function HeaderView({report, parentReportAction, reportID, onNavigationMenuButto { if (ReportUtils.canEditPolicyDescription(policy)) { - Navigation.navigate(ROUTES.WORKSPACE_PROFILE_DESCRIPTION.getRoute(report.policyID ?? '-1')); + Navigation.navigate(ROUTES.WORKSPACE_PROFILE_DESCRIPTION.getRoute(report.policyID)); return; } Navigation.navigate(ROUTES.REPORT_WITH_ID_DETAILS.getRoute(reportID, Navigation.getReportRHPActiveRoute())); diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index a03d3786458c..f1f37871295f 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -69,13 +69,6 @@ const defaultReportMetadata = { isOptimisticReport: false, }; -/** Make sure the report id won't be an empty string as it can break an onyx key */ -function getNonEmptyStringReportID(reportID: string | undefined): string | undefined { - // The report ID is used in an onyx key. If it's an empty string, onyx will return - // a collection instead of an individual report. - return reportID !== '' ? reportID : undefined; -} - /** * Check is the report is deleted. * We currently use useMemo to memorize every properties of the report @@ -100,7 +93,7 @@ function getParentReportAction(parentReportActions: OnyxEntry getParentReportAction(parentReportActions, reportOnyx?.parentReportActionID), }); From a5ed2e098e981f9024d5b5de11d791bebc85520b Mon Sep 17 00:00:00 2001 From: VickyStash Date: Mon, 13 Jan 2025 12:43:16 +0100 Subject: [PATCH 06/12] Minor improvement --- src/libs/ReportUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index f7129c5bc5c2..8778e23f21da 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -7361,7 +7361,7 @@ function isReportDataReady(): boolean { * Return true if reportID from path is valid */ function isValidReportIDFromPath(reportIDFromPath: string | undefined): boolean { - return !!reportIDFromPath && !['', 'null', '0', '-1'].includes(reportIDFromPath); + return !!reportIDFromPath && !['', 'null', 'undefined', '0', '-1'].includes(reportIDFromPath); } /** From d8c95eb7137618ead65dd68777b401a08d0fc01d Mon Sep 17 00:00:00 2001 From: VickyStash Date: Mon, 13 Jan 2025 17:33:34 +0100 Subject: [PATCH 07/12] Fixes --- src/hooks/usePaginatedReportActions.ts | 2 +- src/pages/home/ReportScreen.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hooks/usePaginatedReportActions.ts b/src/hooks/usePaginatedReportActions.ts index 6e02248ca00a..636304cde520 100644 --- a/src/hooks/usePaginatedReportActions.ts +++ b/src/hooks/usePaginatedReportActions.ts @@ -33,7 +33,7 @@ function usePaginatedReportActions(reportID?: string, reportActionID?: string) { }, [reportActionID, reportActionPages, sortedAllReportActions]); const linkedAction = useMemo( - () => sortedAllReportActions?.find((reportAction) => String(reportAction.reportActionID) === String(reportActionID)), + () => sortedAllReportActions?.find((reportAction) => reportActionID && String(reportAction.reportActionID) === String(reportActionID)), [reportActionID, sortedAllReportActions], ); diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 6f5a69d3905f..ff1a939877a3 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -244,7 +244,7 @@ function ReportScreen({route, navigation}: ReportScreenProps) { const screenWrapperStyle: ViewStyle[] = [styles.appContent, styles.flex1, {marginTop: viewportOffsetTop}]; const isOptimisticDelete = report?.statusNum === CONST.REPORT.STATUS_NUM.CLOSED; const indexOfLinkedMessage = useMemo( - (): number => reportActions.findIndex((obj) => String(obj.reportActionID) === String(reportActionIDFromRoute)), + (): number => reportActions.findIndex((obj) => reportActionIDFromRoute && String(obj.reportActionID) === String(reportActionIDFromRoute)), [reportActions, reportActionIDFromRoute], ); From a09caa52bacdb1049dda8b96dff5695f8cc92a22 Mon Sep 17 00:00:00 2001 From: VickyStash Date: Mon, 13 Jan 2025 17:36:47 +0100 Subject: [PATCH 08/12] Lint fixes --- src/hooks/usePaginatedReportActions.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/hooks/usePaginatedReportActions.ts b/src/hooks/usePaginatedReportActions.ts index 636304cde520..b0f082ea1e5a 100644 --- a/src/hooks/usePaginatedReportActions.ts +++ b/src/hooks/usePaginatedReportActions.ts @@ -9,17 +9,15 @@ import ONYXKEYS from '@src/ONYXKEYS'; * Get the longest continuous chunk of reportActions including the linked reportAction. If not linking to a specific action, returns the continuous chunk of newest reportActions. */ function usePaginatedReportActions(reportID?: string, reportActionID?: string) { - // Use `||` instead of `??` to handle empty string. - // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing - const reportIDWithDefault = reportID || '-1'; - const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportIDWithDefault}`); + const nonEmptyStringReportID = ReportUtils.getNonEmptyStringReportID(reportID); + const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${nonEmptyStringReportID}`); const canUserPerformWriteAction = ReportUtils.canUserPerformWriteAction(report); - const [sortedAllReportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportIDWithDefault}`, { + const [sortedAllReportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${nonEmptyStringReportID}`, { canEvict: false, selector: (allReportActions) => ReportActionsUtils.getSortedReportActionsForDisplay(allReportActions, canUserPerformWriteAction, true), }); - const [reportActionPages] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_PAGES}${reportIDWithDefault}`); + const [reportActionPages] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_PAGES}${nonEmptyStringReportID}`); const { data: reportActions, From ee611e6bca445fba25184951a9bd5dc58311211e Mon Sep 17 00:00:00 2001 From: VickyStash Date: Tue, 14 Jan 2025 10:27:28 +0100 Subject: [PATCH 09/12] Use getNonEmptyStringOnyxID instead of getNonEmptyStringReportID --- src/hooks/usePaginatedReportActions.ts | 3 ++- src/libs/ReportUtils.ts | 8 -------- src/libs/getNonEmptyStringOnyxID.ts | 6 ++++++ src/pages/home/HeaderView.tsx | 5 ++--- src/pages/home/ReportScreen.tsx | 7 ++++--- 5 files changed, 14 insertions(+), 15 deletions(-) create mode 100644 src/libs/getNonEmptyStringOnyxID.ts diff --git a/src/hooks/usePaginatedReportActions.ts b/src/hooks/usePaginatedReportActions.ts index b0f082ea1e5a..433296bc84d7 100644 --- a/src/hooks/usePaginatedReportActions.ts +++ b/src/hooks/usePaginatedReportActions.ts @@ -1,5 +1,6 @@ import {useMemo} from 'react'; import {useOnyx} from 'react-native-onyx'; +import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID'; import PaginationUtils from '@libs/PaginationUtils'; import * as ReportActionsUtils from '@libs/ReportActionsUtils'; import * as ReportUtils from '@libs/ReportUtils'; @@ -9,7 +10,7 @@ import ONYXKEYS from '@src/ONYXKEYS'; * Get the longest continuous chunk of reportActions including the linked reportAction. If not linking to a specific action, returns the continuous chunk of newest reportActions. */ function usePaginatedReportActions(reportID?: string, reportActionID?: string) { - const nonEmptyStringReportID = ReportUtils.getNonEmptyStringReportID(reportID); + const nonEmptyStringReportID = getNonEmptyStringOnyxID(reportID); const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${nonEmptyStringReportID}`); const canUserPerformWriteAction = ReportUtils.canUserPerformWriteAction(report); diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 5437382756b0..060bc7c0b971 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -8804,13 +8804,6 @@ function getReportMetadata(reportID: string | undefined) { return reportID ? allReportMetadataKeyValue[reportID] : undefined; } -/** Make sure the report id is not an empty string as it can break an onyx key */ -function getNonEmptyStringReportID(reportID: string | undefined): string | undefined { - // The report ID is used in an onyx key. If it's an empty string, onyx will return - // a collection instead of an individual report. - return reportID !== '' ? reportID : undefined; -} - export { addDomainToShortMention, completeShortMention, @@ -9139,7 +9132,6 @@ export { shouldUnmaskChat, getReportMetadata, buildOptimisticSelfDMReport, - getNonEmptyStringReportID, isHiddenForCurrentUser, }; diff --git a/src/libs/getNonEmptyStringOnyxID.ts b/src/libs/getNonEmptyStringOnyxID.ts new file mode 100644 index 000000000000..eebf605a8983 --- /dev/null +++ b/src/libs/getNonEmptyStringOnyxID.ts @@ -0,0 +1,6 @@ +/** Make sure the id is not an empty string as it can break an onyx key */ +export default function getNonEmptyStringOnyxID(onyxID: string | undefined): string | undefined { + // The onyx ID is used inside the onyx key. If it's an empty string, onyx will return + // a collection instead of an individual item, which is not an expected behaviour. + return onyxID !== '' ? onyxID : undefined; +} diff --git a/src/pages/home/HeaderView.tsx b/src/pages/home/HeaderView.tsx index 049d5864d6d3..ce63f3297152 100644 --- a/src/pages/home/HeaderView.tsx +++ b/src/pages/home/HeaderView.tsx @@ -25,6 +25,7 @@ import usePolicy from '@hooks/usePolicy'; import useResponsiveLayout from '@hooks/useResponsiveLayout'; import useTheme from '@hooks/useTheme'; import useThemeStyles from '@hooks/useThemeStyles'; +import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID'; import Navigation from '@libs/Navigation/Navigation'; import * as OptionsListUtils from '@libs/OptionsListUtils'; import Parser from '@libs/Parser'; @@ -71,9 +72,7 @@ function HeaderView({report, parentReportAction, reportID, onNavigationMenuButto const route = useRoute(); const [isDeleteTaskConfirmModalVisible, setIsDeleteTaskConfirmModalVisible] = React.useState(false); const [invoiceReceiverPolicy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${report?.invoiceReceiver && 'policyID' in report.invoiceReceiver ? report.invoiceReceiver.policyID : undefined}`); - const [parentReport] = useOnyx( - `${ONYXKEYS.COLLECTION.REPORT}${ReportUtils.getNonEmptyStringReportID(report?.parentReportID) ?? ReportUtils.getNonEmptyStringReportID(report?.reportID)}`, - ); + const [parentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(report?.parentReportID) ?? getNonEmptyStringOnyxID(report?.reportID)}`); const policy = usePolicy(report?.policyID); const [personalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST); diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index ff1a939877a3..0afc4b3fc122 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -28,6 +28,7 @@ import usePrevious from '@hooks/usePrevious'; import useResponsiveLayout from '@hooks/useResponsiveLayout'; import useThemeStyles from '@hooks/useThemeStyles'; import useViewportOffsetTop from '@hooks/useViewportOffsetTop'; +import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID'; import Log from '@libs/Log'; import Navigation from '@libs/Navigation/Navigation'; import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types'; @@ -92,7 +93,7 @@ function getParentReportAction(parentReportActions: OnyxEntry getParentReportAction(parentReportActions, reportOnyx?.parentReportActionID), }); From c6f17e402403080a8bcf5258919b6d3e9af3b03d Mon Sep 17 00:00:00 2001 From: VickyStash Date: Tue, 14 Jan 2025 10:31:11 +0100 Subject: [PATCH 10/12] Prettier fix --- src/libs/getNonEmptyStringOnyxID.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libs/getNonEmptyStringOnyxID.ts b/src/libs/getNonEmptyStringOnyxID.ts index eebf605a8983..b9e1e52d9346 100644 --- a/src/libs/getNonEmptyStringOnyxID.ts +++ b/src/libs/getNonEmptyStringOnyxID.ts @@ -1,6 +1,6 @@ /** Make sure the id is not an empty string as it can break an onyx key */ export default function getNonEmptyStringOnyxID(onyxID: string | undefined): string | undefined { - // The onyx ID is used inside the onyx key. If it's an empty string, onyx will return - // a collection instead of an individual item, which is not an expected behaviour. - return onyxID !== '' ? onyxID : undefined; + // The onyx ID is used inside the onyx key. If it's an empty string, onyx will return + // a collection instead of an individual item, which is not an expected behaviour. + return onyxID !== '' ? onyxID : undefined; } From e515c2308ca6979f2b520e75909e7079de59cb2b Mon Sep 17 00:00:00 2001 From: VickyStash Date: Tue, 14 Jan 2025 10:42:48 +0100 Subject: [PATCH 11/12] Minor improvement --- src/pages/home/HeaderView.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pages/home/HeaderView.tsx b/src/pages/home/HeaderView.tsx index ce63f3297152..055ef05fb958 100644 --- a/src/pages/home/HeaderView.tsx +++ b/src/pages/home/HeaderView.tsx @@ -71,7 +71,8 @@ function HeaderView({report, parentReportAction, reportID, onNavigationMenuButto const {isSmallScreenWidth} = useResponsiveLayout(); const route = useRoute(); const [isDeleteTaskConfirmModalVisible, setIsDeleteTaskConfirmModalVisible] = React.useState(false); - const [invoiceReceiverPolicy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${report?.invoiceReceiver && 'policyID' in report.invoiceReceiver ? report.invoiceReceiver.policyID : undefined}`); + const invoiceReceiverPolicyID = report?.invoiceReceiver && 'policyID' in report.invoiceReceiver ? report.invoiceReceiver.policyID : undefined; + const [invoiceReceiverPolicy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${invoiceReceiverPolicyID}`); const [parentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(report?.parentReportID) ?? getNonEmptyStringOnyxID(report?.reportID)}`); const policy = usePolicy(report?.policyID); const [personalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST); From f56d380c3fe5f3a4a5184b378a0ceb4954c7e04a Mon Sep 17 00:00:00 2001 From: VickyStash Date: Wed, 15 Jan 2025 11:41:40 +0100 Subject: [PATCH 12/12] Apply reviewer feedback --- src/hooks/usePaginatedReportActions.ts | 4 ++-- src/libs/Notification/LocalNotification/index.desktop.ts | 3 +++ src/libs/Notification/LocalNotification/index.ts | 3 +++ .../Notification/clearReportNotifications/index.native.ts | 4 ++++ 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/hooks/usePaginatedReportActions.ts b/src/hooks/usePaginatedReportActions.ts index 433296bc84d7..c152f7e43547 100644 --- a/src/hooks/usePaginatedReportActions.ts +++ b/src/hooks/usePaginatedReportActions.ts @@ -9,7 +9,7 @@ import ONYXKEYS from '@src/ONYXKEYS'; /** * Get the longest continuous chunk of reportActions including the linked reportAction. If not linking to a specific action, returns the continuous chunk of newest reportActions. */ -function usePaginatedReportActions(reportID?: string, reportActionID?: string) { +function usePaginatedReportActions(reportID: string | undefined, reportActionID?: string) { const nonEmptyStringReportID = getNonEmptyStringOnyxID(reportID); const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${nonEmptyStringReportID}`); const canUserPerformWriteAction = ReportUtils.canUserPerformWriteAction(report); @@ -32,7 +32,7 @@ function usePaginatedReportActions(reportID?: string, reportActionID?: string) { }, [reportActionID, reportActionPages, sortedAllReportActions]); const linkedAction = useMemo( - () => sortedAllReportActions?.find((reportAction) => reportActionID && String(reportAction.reportActionID) === String(reportActionID)), + () => (reportActionID ? sortedAllReportActions?.find((reportAction) => String(reportAction.reportActionID) === String(reportActionID)) : undefined), [reportActionID, sortedAllReportActions], ); diff --git a/src/libs/Notification/LocalNotification/index.desktop.ts b/src/libs/Notification/LocalNotification/index.desktop.ts index e0ecd11fd6f7..5a917e21ea0b 100644 --- a/src/libs/Notification/LocalNotification/index.desktop.ts +++ b/src/libs/Notification/LocalNotification/index.desktop.ts @@ -15,6 +15,9 @@ function showModifiedExpenseNotification(report: Report, reportAction: ReportAct } function clearReportNotifications(reportID: string | undefined) { + if (!reportID) { + return; + } BrowserNotifications.clearNotifications((notificationData) => notificationData.reportID === reportID); } diff --git a/src/libs/Notification/LocalNotification/index.ts b/src/libs/Notification/LocalNotification/index.ts index 997d796ba0ad..6416bc8a872b 100644 --- a/src/libs/Notification/LocalNotification/index.ts +++ b/src/libs/Notification/LocalNotification/index.ts @@ -15,6 +15,9 @@ function showModifiedExpenseNotification(report: Report, reportAction: ReportAct } function clearReportNotifications(reportID: string | undefined) { + if (!reportID) { + return; + } BrowserNotifications.clearNotifications((notificationData) => notificationData.reportID === reportID); } diff --git a/src/libs/Notification/clearReportNotifications/index.native.ts b/src/libs/Notification/clearReportNotifications/index.native.ts index bddb2dcb42a5..e33f3e9bf335 100644 --- a/src/libs/Notification/clearReportNotifications/index.native.ts +++ b/src/libs/Notification/clearReportNotifications/index.native.ts @@ -16,6 +16,10 @@ const parseNotificationAndReportIDs = (pushPayload: PushPayload) => { const clearReportNotifications: ClearReportNotifications = (reportID: string | undefined) => { Log.info('[PushNotification] clearing report notifications', false, {reportID}); + if (!reportID) { + return; + } + Airship.push .getActiveNotifications() .then((pushPayloads) => {