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.

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

[No QA] Fix optimistic reports getting cleared by unnecessary OpenReport calls #37930

Merged
merged 12 commits into from
Mar 21, 2024
4 changes: 4 additions & 0 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4124,6 +4124,10 @@ function canAccessReport(report: OnyxEntry<Report>, policies: OnyxCollection<Pol
return false;
}

if (report?.errorFields?.notFound) {
return false;
}

return true;
}

Expand Down
1 change: 1 addition & 0 deletions src/libs/actions/IOU.ts
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,7 @@ function buildOnyxDataForMoneyRequest(
value: {
pendingFields: null,
errorFields: null,
isOptimisticReport: false,
},
});
}
Expand Down
9 changes: 9 additions & 0 deletions src/libs/actions/Report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,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}`,
Expand Down
7 changes: 7 additions & 0 deletions src/libs/shouldFetchReport.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
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.
// 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;
Copy link
Contributor

Choose a reason for hiding this comment

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

From #54512
we need to avoid the complications caused by passing and monitoring a complex object.

}
12 changes: 10 additions & 2 deletions src/pages/home/ReportScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,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 type {CentralPaneNavigatorParamList} from '@navigation/types';
import * as ComposerActions from '@userActions/Composer';
import * as Report from '@userActions/Report';
Expand Down Expand Up @@ -245,7 +246,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<OnyxTypes.ReportAction> = useMemo(
Expand Down Expand Up @@ -334,8 +338,12 @@ function ReportScreen({
return;
}

if (!shouldFetchReport(report)) {
return;
}

Report.openReport(reportIDFromPath);
}, [report.reportID, route, isLoadingInitialReportActions]);
}, [report, route, isLoadingInitialReportActions]);

const dismissBanner = useCallback(() => {
setIsBannerVisible(false);
Expand Down
18 changes: 12 additions & 6 deletions src/pages/home/report/ReportActionsView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ 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 * as Report from '@userActions/Report';
import Timing from '@userActions/Timing';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type * as OnyxTypes from '@src/types/onyx';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import PopoverReactionList from './ReactionList/PopoverReactionList';
import ReportActionsList from './ReportActionsList';
import type {LoadNewerChats} from './ReportActionsList';
Expand Down Expand Up @@ -81,15 +81,21 @@ function ReportActionsView({
const isReportFullyVisible = useMemo((): boolean => getIsReportFullyVisible(isFocused), [isFocused]);

const openReportIfNecessary = () => {
const createChatError = report.errorFields?.createChat;
// If the report is optimistic (AKA not yet created) we don't need to call openReport again
if (!!report.isOptimisticReport || !isEmptyObject(createChatError)) {
if (!shouldFetchReport(report)) {
return;
}

Report.openReport(reportID);
};

const reconnectReportIfNecessary = () => {
if (!shouldFetchReport(report)) {
return;
}

Report.reconnect(reportID);
};

useEffect(() => {
const interactionTask = InteractionManager.runAfterInteractions(() => {
openReportIfNecessary();
Expand All @@ -113,7 +119,7 @@ function ReportActionsView({
if (isReportFullyVisible) {
openReportIfNecessary();
} else {
Report.reconnect(reportID);
reconnectReportIfNecessary();
}
}
// update ref with current network state
Expand All @@ -127,7 +133,7 @@ function ReportActionsView({
if (isReportFullyVisible) {
openReportIfNecessary();
} else {
Report.reconnect(reportID);
reconnectReportIfNecessary();
}
}
// eslint-disable-next-line react-hooks/exhaustive-deps
Expand Down
Loading