Skip to content

Commit

Permalink
Merge pull request #31257 from hungvu193/fix-30246
Browse files Browse the repository at this point in the history 
fix showing NotFound page when offline
  • Loading branch information
@puneetlath
puneetlath authored Jan 9, 2024
2 parents e9f27ab + 8036a5a commit 8925a89
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 69 deletions.
36 changes: 36 additions & 0 deletions src/pages/LoadingPage.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import PropTypes from 'prop-types';
import React from 'react';
import FullScreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import ScreenWrapper from '@components/ScreenWrapper';
import useThemeStyles from '@hooks/useThemeStyles';

const propTypes = {
/** Method to trigger when pressing back button of the header */
onBackButtonPress: PropTypes.func,
title: PropTypes.string.isRequired,
};

const defaultProps = {
onBackButtonPress: undefined,
};

function LoadingPage(props) {
const styles = useThemeStyles();
return (
<ScreenWrapper testID={LoadingPage.displayName}>
<HeaderWithBackButton
onBackButtonPress={props.onBackButtonPress}
shouldShowBackButton
title={props.title}
/>
<FullScreenLoadingIndicator style={[styles.flex1, styles.pRelative]} />
</ScreenWrapper>
);
}

LoadingPage.displayName = 'LoadingPage';
LoadingPage.propTypes = propTypes;
LoadingPage.defaultProps = defaultProps;

export default LoadingPage;
2 changes: 1 addition & 1 deletion src/pages/PrivateNotes/PrivateNotesEditPage.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ PrivateNotesEditPage.defaultProps = defaultProps;

export default compose(
withLocalize,
withReportAndPrivateNotesOrNotFound,
withReportAndPrivateNotesOrNotFound('privateNotes.title'),
withOnyx({
personalDetailsList: {
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
Expand Down
2 changes: 1 addition & 1 deletion src/pages/PrivateNotes/PrivateNotesListPage.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ PrivateNotesListPage.displayName = 'PrivateNotesListPage';

export default compose(
withLocalize,
withReportAndPrivateNotesOrNotFound,
withReportAndPrivateNotesOrNotFound('privateNotes.title'),
withOnyx({
personalDetailsList: {
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
Expand Down
2 changes: 1 addition & 1 deletion src/pages/PrivateNotes/PrivateNotesViewPage.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ PrivateNotesViewPage.defaultProps = defaultProps;

export default compose(
withLocalize,
withReportAndPrivateNotesOrNotFound,
withReportAndPrivateNotesOrNotFound('privateNotes.title'),
withOnyx({
personalDetailsList: {
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
Expand Down
141 changes: 75 additions & 66 deletions src/pages/home/report/withReportAndPrivateNotesOrNotFound.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,16 @@ import PropTypes from 'prop-types';
import React, {useEffect, useMemo} from 'react';
import {withOnyx} from 'react-native-onyx';
import _ from 'underscore';
import FullScreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
import networkPropTypes from '@components/networkPropTypes';
import {withNetwork} from '@components/OnyxProvider';
import useLocalize from '@hooks/useLocalize';
import usePrevious from '@hooks/usePrevious';
import * as Report from '@libs/actions/Report';
import compose from '@libs/compose';
import getComponentDisplayName from '@libs/getComponentDisplayName';
import * as ReportUtils from '@libs/ReportUtils';
import NotFoundPage from '@pages/ErrorPage/NotFoundPage';
import LoadingPage from '@pages/LoadingPage';
import reportPropTypes from '@pages/reportPropTypes';
import ONYXKEYS from '@src/ONYXKEYS';
import withReportOrNotFound from './withReportOrNotFound';
Expand Down Expand Up @@ -50,81 +52,88 @@ const defaultProps = {
},
};

export default function (WrappedComponent) {
export default function (pageTitle) {
// eslint-disable-next-line rulesdir/no-negated-variables
function WithReportAndPrivateNotesOrNotFound({forwardedRef, ...props}) {
const {route, report, network, session} = props;
const accountID = route.params.accountID;
const isPrivateNotesFetchTriggered = !_.isUndefined(report.isLoadingPrivateNotes);

useEffect(() => {
// Do not fetch private notes if isLoadingPrivateNotes is already defined, or if network is offline.
if (isPrivateNotesFetchTriggered || network.isOffline) {
return;
}

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

const isPrivateNotesEmpty = accountID ? _.isEmpty(lodashGet(report, ['privateNotes', accountID, 'note'], '')) : _.isEmpty(report.privateNotes);
const shouldShowFullScreenLoadingIndicator = !isPrivateNotesFetchTriggered || (isPrivateNotesEmpty && report.isLoadingPrivateNotes);

return (WrappedComponent) => {
// eslint-disable-next-line rulesdir/no-negated-variables
const shouldShowNotFoundPage = useMemo(() => {
// Show not found view if the report is archived, or if the note is not of current user.
if (ReportUtils.isArchivedRoom(report) || (accountID && Number(session.accountID) !== Number(accountID))) {
return true;
function WithReportAndPrivateNotesOrNotFound({forwardedRef, ...props}) {
const {translate} = useLocalize();
const {route, report, network, session} = props;
const accountID = route.params.accountID;
const isPrivateNotesFetchTriggered = !_.isUndefined(report.isLoadingPrivateNotes);
const prevIsOffline = usePrevious(network.isOffline);
const isReconnecting = prevIsOffline && !network.isOffline;
const isOtherUserNote = accountID && Number(session.accountID) !== Number(accountID);
const isPrivateNotesEmpty = accountID ? _.has(lodashGet(report, ['privateNotes', accountID, 'note'], '')) : _.isEmpty(report.privateNotes);

useEffect(() => {
// Do not fetch private notes if isLoadingPrivateNotes is already defined, or if network is offline.
if ((isPrivateNotesFetchTriggered && !isReconnecting) || network.isOffline) {
return;
}

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

const shouldShowFullScreenLoadingIndicator = !isPrivateNotesFetchTriggered || (isPrivateNotesEmpty && (report.isLoadingPrivateNotes || !isOtherUserNote));

// eslint-disable-next-line rulesdir/no-negated-variables
const shouldShowNotFoundPage = useMemo(() => {
// Show not found view if the report is archived, or if the note is not of current user.
if (ReportUtils.isArchivedRoom(report) || (accountID && Number(session.accountID) !== Number(accountID))) {
return true;
}

// Don't show not found view if the notes are still loading, or if the notes are non-empty.
if (shouldShowFullScreenLoadingIndicator || !isPrivateNotesEmpty || isReconnecting) {
return false;
}

// As notes being empty and not loading is a valid case, show not found view only in offline mode.
return network.isOffline;
}, [report, network.isOffline, accountID, session.accountID, isPrivateNotesEmpty, shouldShowFullScreenLoadingIndicator, isReconnecting]);

if (shouldShowFullScreenLoadingIndicator) {
return <LoadingPage title={translate(pageTitle)} />;
}

// Don't show not found view if the notes are still loading, or if the notes are non-empty.
if (shouldShowFullScreenLoadingIndicator || !isPrivateNotesEmpty) {
return false;
if (shouldShowNotFoundPage) {
return <NotFoundPage />;
}

// As notes being empty and not loading is a valid case, show not found view only in offline mode.
return network.isOffline;
}, [report, network.isOffline, accountID, session.accountID, isPrivateNotesEmpty, shouldShowFullScreenLoadingIndicator]);

if (shouldShowFullScreenLoadingIndicator) {
return <FullScreenLoadingIndicator />;
return (
<WrappedComponent
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
ref={forwardedRef}
/>
);
}

if (shouldShowNotFoundPage) {
return <NotFoundPage />;
}
WithReportAndPrivateNotesOrNotFound.propTypes = propTypes;
WithReportAndPrivateNotesOrNotFound.defaultProps = defaultProps;
WithReportAndPrivateNotesOrNotFound.displayName = `withReportAndPrivateNotesOrNotFound(${getComponentDisplayName(WrappedComponent)})`;

return (
<WrappedComponent
// eslint-disable-next-line rulesdir/no-negated-variables
const WithReportAndPrivateNotesOrNotFoundWithRef = React.forwardRef((props, ref) => (
<WithReportAndPrivateNotesOrNotFound
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
ref={forwardedRef}
forwardedRef={ref}
/>
);
}

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

// eslint-disable-next-line rulesdir/no-negated-variables
const WithReportAndPrivateNotesOrNotFoundWithRef = React.forwardRef((props, ref) => (
<WithReportAndPrivateNotesOrNotFound
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
forwardedRef={ref}
/>
));

WithReportAndPrivateNotesOrNotFoundWithRef.displayName = 'WithReportAndPrivateNotesOrNotFoundWithRef';

return compose(
withReportOrNotFound(),
withOnyx({
session: {
key: ONYXKEYS.SESSION,
},
}),
withNetwork(),
)(WithReportAndPrivateNotesOrNotFoundWithRef);
));

WithReportAndPrivateNotesOrNotFoundWithRef.displayName = 'WithReportAndPrivateNotesOrNotFoundWithRef';

return compose(
withReportOrNotFound(),
withOnyx({
session: {
key: ONYXKEYS.SESSION,
},
}),
withNetwork(),
)(WithReportAndPrivateNotesOrNotFoundWithRef);
};
}

0 comments on commit 8925a89

Please sign in to comment.