From f5040bfccb2ea4f808a94fc721b6116affd98401 Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Thu, 7 Mar 2024 14:49:42 -0500 Subject: [PATCH 1/9] clear isOptimisticReport on success --- src/libs/actions/IOU.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index cb3aa20ab6a7..6d8638bf4403 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -592,6 +592,7 @@ function buildOnyxDataForMoneyRequest( value: { pendingFields: null, errorFields: null, + isOptimisticReport: false, }, }); } From 1245b7963674bb46a19b47ddc28a4be7ae457d02 Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Thu, 7 Mar 2024 15:07:15 -0500 Subject: [PATCH 2/9] unify pre-fetch report checks and fix issues with fetching optimistic reports --- src/libs/shouldFetchReport.ts | 5 +++++ src/pages/home/ReportScreen.js | 5 +++++ src/pages/home/report/ReportActionsView.js | 17 ++++++++++++----- 3 files changed, 22 insertions(+), 5 deletions(-) create mode 100644 src/libs/shouldFetchReport.ts diff --git a/src/libs/shouldFetchReport.ts b/src/libs/shouldFetchReport.ts new file mode 100644 index 000000000000..f84058de4b99 --- /dev/null +++ b/src/libs/shouldFetchReport.ts @@ -0,0 +1,5 @@ +import {Report} from '@src/types/onyx'; + +export default function shouldFetchReport(report: Report) { + return !report?.isOptimisticReport && !report?.errorFields?.createChat; +} diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index da5a8e4aae27..2cc15c9d0356 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -30,6 +30,7 @@ import Performance from '@libs/Performance'; import * as PersonalDetailsUtils from '@libs/PersonalDetailsUtils'; import * as ReportActionsUtils from '@libs/ReportActionsUtils'; import * as ReportUtils from '@libs/ReportUtils'; +import shouldFetchReport from '@libs/shouldFetchReport'; import reportMetadataPropTypes from '@pages/reportMetadataPropTypes'; import reportPropTypes from '@pages/reportPropTypes'; import * as ComposerActions from '@userActions/Composer'; @@ -359,6 +360,10 @@ function ReportScreen({ return; } + if (!shouldFetchReport(report)) { + return; + } + Report.openReport(reportIDFromPath); }, [report.reportID, route, isLoadingInitialReportActions]); diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index ca3ee7d2ab6a..2d9b18b46b59 100755 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -17,6 +17,7 @@ import Performance from '@libs/Performance'; import * as ReportActionsUtils from '@libs/ReportActionsUtils'; import {isUserCreatedPolicyRoom} from '@libs/ReportUtils'; import {didUserLogInDuringSession} from '@libs/SessionUtils'; +import shouldFetchReport from '@libs/shouldFetchReport'; import {ReactionListContext} from '@pages/home/ReportScreenContext'; import reportPropTypes from '@pages/reportPropTypes'; import * as Report from '@userActions/Report'; @@ -107,15 +108,21 @@ function ReportActionsView(props) { const isReportFullyVisible = useMemo(() => getIsReportFullyVisible(isFocused), [isFocused]); const openReportIfNecessary = () => { - const createChatError = _.get(props.report, ['errorFields', 'createChat']); - // If the report is optimistic (AKA not yet created) we don't need to call openReport again - if (props.report.isOptimisticReport || !_.isEmpty(createChatError)) { + if (!shouldFetchReport(props.report)) { return; } Report.openReport(reportID); }; + const reconnectReportIfNecessary = () => { + if (!shouldFetchReport(props.report)) { + return; + } + + Report.reconnect(reportID); + }; + useEffect(() => { openReportIfNecessary(); // eslint-disable-next-line react-hooks/exhaustive-deps @@ -131,7 +138,7 @@ function ReportActionsView(props) { if (isReportFullyVisible) { openReportIfNecessary(); } else { - Report.reconnect(reportID); + reconnectReportIfNecessary(); } } // update ref with current network state @@ -145,7 +152,7 @@ function ReportActionsView(props) { if (isReportFullyVisible) { openReportIfNecessary(); } else { - Report.reconnect(reportID); + reconnectReportIfNecessary(); } } // eslint-disable-next-line react-hooks/exhaustive-deps From b0d6dca3348a6992e630f3990de8599fbcf7e1e5 Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Thu, 7 Mar 2024 15:08:55 -0500 Subject: [PATCH 3/9] add comment --- src/libs/shouldFetchReport.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libs/shouldFetchReport.ts b/src/libs/shouldFetchReport.ts index f84058de4b99..9e47641e3b29 100644 --- a/src/libs/shouldFetchReport.ts +++ b/src/libs/shouldFetchReport.ts @@ -1,5 +1,7 @@ import {Report} from '@src/types/onyx'; export default function shouldFetchReport(report: Report) { + // If the report is optimistic, there's no need to fetch it. The original action should create it. + // If there is an error for creating the chat, there's no need to fetch it since it doesn't exist return !report?.isOptimisticReport && !report?.errorFields?.createChat; } From 03dba7ffa00dbb7dd66c30fab037e500b9428cf2 Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Thu, 7 Mar 2024 15:26:18 -0500 Subject: [PATCH 4/9] fix type import --- src/libs/shouldFetchReport.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/shouldFetchReport.ts b/src/libs/shouldFetchReport.ts index 9e47641e3b29..5259e88ca6d8 100644 --- a/src/libs/shouldFetchReport.ts +++ b/src/libs/shouldFetchReport.ts @@ -1,4 +1,4 @@ -import {Report} from '@src/types/onyx'; +import type Report from '@src/types/onyx/Report'; export default function shouldFetchReport(report: Report) { // If the report is optimistic, there's no need to fetch it. The original action should create it. From 6c74c15dc0bd413a970598ae4a56c365412d6652 Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Thu, 7 Mar 2024 15:26:52 -0500 Subject: [PATCH 5/9] update dependencies --- src/pages/home/ReportScreen.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 2cc15c9d0356..dc77c9d6ac7a 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -365,7 +365,7 @@ function ReportScreen({ } Report.openReport(reportIDFromPath); - }, [report.reportID, route, isLoadingInitialReportActions]); + }, [report, route, isLoadingInitialReportActions]); const dismissBanner = useCallback(() => { setIsBannerVisible(false); From 03bd7dbc1a692fcd650e6f918a992e635ac1eead Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Thu, 14 Mar 2024 16:27:04 -0400 Subject: [PATCH 6/9] check for notFound errors --- src/libs/ReportUtils.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 97b7b576b303..870a82c75201 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -4075,6 +4075,10 @@ function canAccessReport(report: OnyxEntry, policies: OnyxCollection Date: Thu, 14 Mar 2024 16:29:10 -0400 Subject: [PATCH 7/9] clear notFound on success --- src/libs/actions/Report.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/libs/actions/Report.ts b/src/libs/actions/Report.ts index b677b5369b68..ffaf6836288b 100644 --- a/src/libs/actions/Report.ts +++ b/src/libs/actions/Report.ts @@ -609,6 +609,15 @@ function openReport( ]; const successData: OnyxUpdate[] = [ + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, + value: { + errorFields: { + notFound: null, + }, + }, + }, { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`, From 31bd599d0d56885d72003f69c950c0b067835a3f Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Thu, 14 Mar 2024 16:29:40 -0400 Subject: [PATCH 8/9] prevent blocking the screen for non-404 errors --- src/pages/home/ReportScreen.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 94dcdfb27028..9d27ef8815b8 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -232,7 +232,10 @@ function ReportScreen({ // There are no reportActions at all to display and we are still in the process of loading the next set of actions. const isLoadingInitialReportActions = reportActions.length === 0 && !!reportMetadata?.isLoadingInitialReportActions; const isOptimisticDelete = report.statusNum === CONST.REPORT.STATUS_NUM.CLOSED; - const shouldHideReport = !ReportUtils.canAccessReport(report, policies, betas); + + // If there's a non-404 error for the report we should show it instead of blocking the screen + const hasHelpfulErrors = Object.keys(report?.errorFields ?? {}).some(key => key !== 'notFound'); + const shouldHideReport = !hasHelpfulErrors && !ReportUtils.canAccessReport(report, policies, betas) ; const isLoading = !reportID || !isSidebarLoaded || PersonalDetailsUtils.isPersonalDetailsEmpty(); const lastReportAction: OnyxEntry = useMemo( From 4a4e1a2d70e13ebe9148a4e48d8b745bc18b9008 Mon Sep 17 00:00:00 2001 From: Andrew Rosiclair Date: Fri, 15 Mar 2024 13:59:57 -0400 Subject: [PATCH 9/9] prettier --- src/pages/home/ReportScreen.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pages/home/ReportScreen.tsx b/src/pages/home/ReportScreen.tsx index 9d27ef8815b8..99f0e51df9c5 100644 --- a/src/pages/home/ReportScreen.tsx +++ b/src/pages/home/ReportScreen.tsx @@ -232,10 +232,10 @@ function ReportScreen({ // There are no reportActions at all to display and we are still in the process of loading the next set of actions. const isLoadingInitialReportActions = reportActions.length === 0 && !!reportMetadata?.isLoadingInitialReportActions; const isOptimisticDelete = report.statusNum === CONST.REPORT.STATUS_NUM.CLOSED; - + // If there's a non-404 error for the report we should show it instead of blocking the screen - const hasHelpfulErrors = Object.keys(report?.errorFields ?? {}).some(key => key !== 'notFound'); - const shouldHideReport = !hasHelpfulErrors && !ReportUtils.canAccessReport(report, policies, betas) ; + const hasHelpfulErrors = Object.keys(report?.errorFields ?? {}).some((key) => key !== 'notFound'); + const shouldHideReport = !hasHelpfulErrors && !ReportUtils.canAccessReport(report, policies, betas); const isLoading = !reportID || !isSidebarLoaded || PersonalDetailsUtils.isPersonalDetailsEmpty(); const lastReportAction: OnyxEntry = useMemo(