diff --git a/.eslintrc.changed.js b/.eslintrc.changed.js index 9e37069c1d18..b2c9a46df078 100644 --- a/.eslintrc.changed.js +++ b/.eslintrc.changed.js @@ -24,7 +24,6 @@ module.exports = { files: [ 'src/libs/actions/IOU.ts', 'src/libs/actions/Report.ts', - 'src/pages/home/ReportScreen.tsx', 'src/pages/workspace/WorkspaceInitialPage.tsx', 'src/pages/home/report/PureReportActionItem.tsx', 'src/libs/SidebarUtils.ts', diff --git a/src/ROUTES.ts b/src/ROUTES.ts index fa40bfad4e63..2be9368a10b3 100644 --- a/src/ROUTES.ts +++ b/src/ROUTES.ts @@ -297,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; @@ -363,7 +366,12 @@ const ROUTES = { }, REPORT_WITH_ID_DETAILS: { route: 'r/:reportID/details', - getRoute: (reportID: string | undefined, 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', @@ -399,7 +407,12 @@ const ROUTES = { }, REPORT_DESCRIPTION: { route: 'r/:reportID/description', - getRoute: (reportID: string | undefined, 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', @@ -896,7 +909,12 @@ const ROUTES = { }, WORKSPACE_PROFILE_DESCRIPTION: { route: 'settings/workspaces/:policyID/profile/description', - getRoute: (policyID: string | undefined) => `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/hooks/usePaginatedReportActions.ts b/src/hooks/usePaginatedReportActions.ts index 6e02248ca00a..c152f7e43547 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'; @@ -8,18 +9,16 @@ 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}`); +function usePaginatedReportActions(reportID: string | undefined, reportActionID?: string) { + const nonEmptyStringReportID = getNonEmptyStringOnyxID(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, @@ -33,7 +32,7 @@ function usePaginatedReportActions(reportID?: string, reportActionID?: string) { }, [reportActionID, reportActionPages, sortedAllReportActions]); const linkedAction = useMemo( - () => sortedAllReportActions?.find((reportAction) => 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 4baf1abee139..5a917e21ea0b 100644 --- a/src/libs/Notification/LocalNotification/index.desktop.ts +++ b/src/libs/Notification/LocalNotification/index.desktop.ts @@ -14,7 +14,10 @@ function showModifiedExpenseNotification(report: Report, reportAction: ReportAct BrowserNotifications.pushModifiedExpenseNotification(report, reportAction, onClick); } -function clearReportNotifications(reportID: string) { +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 2141000d807d..6416bc8a872b 100644 --- a/src/libs/Notification/LocalNotification/index.ts +++ b/src/libs/Notification/LocalNotification/index.ts @@ -14,7 +14,10 @@ function showModifiedExpenseNotification(report: Report, reportAction: ReportAct BrowserNotifications.pushModifiedExpenseNotification(report, reportAction, onClick, true); } -function clearReportNotifications(reportID: string) { +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 ad7d80a5b832..e33f3e9bf335 100644 --- a/src/libs/Notification/clearReportNotifications/index.native.ts +++ b/src/libs/Notification/clearReportNotifications/index.native.ts @@ -13,9 +13,13 @@ const parseNotificationAndReportIDs = (pushPayload: PushPayload) => { }; }; -const clearReportNotifications: ClearReportNotifications = (reportID: string) => { +const clearReportNotifications: ClearReportNotifications = (reportID: string | undefined) => { Log.info('[PushNotification] clearing report notifications', false, {reportID}); + if (!reportID) { + return; + } + Airship.push .getActiveNotifications() .then((pushPayloads) => { 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/ReportUtils.ts b/src/libs/ReportUtils.ts index 7b26e6f6f2a9..8864367b253a 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -7487,8 +7487,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', 'undefined', '0', '-1'].includes(reportIDFromPath); } /** diff --git a/src/libs/actions/Report.ts b/src/libs/actions/Report.ts index 783a2c80d10e..709d160a6f08 100644 --- a/src/libs/actions/Report.ts +++ b/src/libs/actions/Report.ts @@ -454,7 +454,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/libs/getNonEmptyStringOnyxID.ts b/src/libs/getNonEmptyStringOnyxID.ts new file mode 100644 index 000000000000..b9e1e52d9346 --- /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 ebfab1acaace..770a40fd7887 100644 --- a/src/pages/home/HeaderView.tsx +++ b/src/pages/home/HeaderView.tsx @@ -24,6 +24,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 {getPersonalDetailsForAccountIDs} from '@libs/OptionsListUtils'; import Parser from '@libs/Parser'; @@ -76,7 +77,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; @@ -94,11 +95,9 @@ function HeaderView({report, parentReportAction, onNavigationMenuButtonClicked, 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 : CONST.DEFAULT_NUMBER_ID}`, - ); - // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing - const [parentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${report?.parentReportID || report?.reportID || CONST.DEFAULT_NUMBER_ID}`); + 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); diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index b02ccec1a56b..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'; @@ -68,13 +69,6 @@ const defaultReportMetadata = { isOptimisticReport: false, }; -/** Get the currently viewed report ID as number */ -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); -} - /** * Check is the report is deleted. * We currently use useMemo to memorize every properties of the report @@ -93,14 +87,14 @@ 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); @@ -189,7 +180,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, @@ -254,7 +245,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], ); @@ -271,12 +262,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); @@ -319,7 +310,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)); @@ -381,8 +372,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; @@ -500,7 +490,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 @@ -510,7 +500,7 @@ function ReportScreen({route, navigation}: ReportScreenProps) { return; } - clearReportNotifications(reportID ?? ''); + clearReportNotifications(reportID); }, [reportID, isTopMostReportId]); useEffect(clearNotifications, [clearNotifications]); @@ -526,7 +516,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. @@ -559,7 +549,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. @@ -723,7 +713,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; @@ -758,7 +748,7 @@ function ReportScreen({route, navigation}: ReportScreenProps) { navigation={navigation} style={screenWrapperStyle} shouldEnableKeyboardAvoidingView={(isTopMostReportId || isInNarrowPaneModal) && (!isComposerFocus || showSoftInputOnFocus)} - testID={`report-screen-${reportID ?? ''}`} + testID={`report-screen-${reportID}`} > { 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);