fix: Models are not all removed from local store upon access change (#3729)

* fix: Clean data from stores correctly on 401/403 response

* Convert DataLoader from class component, remove observables and caching

* types
This commit is contained in:
Tom Moor
2022-07-03 22:48:50 +02:00
committed by GitHub
parent 9cd26168e1
commit 5d498632c6
7 changed files with 165 additions and 212 deletions

View File

@@ -6,6 +6,7 @@ import * as React from "react";
import io from "socket.io-client";
import RootStore from "~/stores/RootStore";
import withStores from "~/components/withStores";
import { AuthorizationError, NotFoundError } from "~/utils/errors";
import { getVisibilityListener, getPageVisible } from "~/utils/pageVisibility";
type SocketWithAuthentication = {
@@ -154,7 +155,10 @@ class SocketProvider extends React.Component<Props> {
force: true,
});
} catch (err) {
if (err.statusCode === 404 || err.statusCode === 403) {
if (
err instanceof AuthorizationError ||
err instanceof NotFoundError
) {
documents.remove(documentId);
return;
}
@@ -216,7 +220,10 @@ class SocketProvider extends React.Component<Props> {
force: true,
});
} catch (err) {
if (err.statusCode === 404 || err.statusCode === 403) {
if (
err instanceof AuthorizationError ||
err instanceof NotFoundError
) {
documents.removeCollectionDocuments(collectionId);
memberships.removeCollectionMemberships(collectionId);
collections.remove(collectionId);
@@ -245,7 +252,10 @@ class SocketProvider extends React.Component<Props> {
force: true,
});
} catch (err) {
if (err.statusCode === 404 || err.statusCode === 403) {
if (
err instanceof AuthorizationError ||
err instanceof NotFoundError
) {
groups.remove(groupId);
}
}

View File

@@ -1,14 +1,12 @@
import invariant from "invariant";
import { observable } from "mobx";
import { observer } from "mobx-react";
import * as React from "react";
import { RouteComponentProps, StaticContext } from "react-router";
import RootStore from "~/stores/RootStore";
import { useLocation, RouteComponentProps, StaticContext } from "react-router";
import Document from "~/models/Document";
import Revision from "~/models/Revision";
import Error404 from "~/scenes/Error404";
import ErrorOffline from "~/scenes/ErrorOffline";
import withStores from "~/components/withStores";
import usePolicy from "~/hooks/usePolicy";
import useStores from "~/hooks/useStores";
import { NavigationNode } from "~/types";
import { NotFoundError, OfflineError } from "~/utils/errors";
import history from "~/utils/history";
@@ -16,146 +14,99 @@ import { matchDocumentEdit } from "~/utils/routeHelpers";
import HideSidebar from "./HideSidebar";
import Loading from "./Loading";
type Props = RootStore &
RouteComponentProps<
{
documentSlug: string;
revisionId?: string;
shareId?: string;
title?: string;
},
StaticContext,
{
title?: string;
}
> & {
children: (arg0: any) => React.ReactNode;
};
type Params = {
documentSlug: string;
revisionId?: string;
shareId?: string;
};
@observer
class DataLoader extends React.Component<Props> {
sharedTree: NavigationNode | null | undefined;
type LocationState = {
title?: string;
restore?: boolean;
revisionId?: string;
};
@observable
document: Document | null | undefined;
type Children = (options: {
document: Document;
revision: Revision | undefined;
abilities: Record<string, boolean>;
isEditing: boolean;
readOnly: boolean;
onCreateLink: (title: string) => Promise<string>;
sharedTree: NavigationNode | undefined;
}) => React.ReactNode;
@observable
revision: Revision | null | undefined;
type Props = RouteComponentProps<Params, StaticContext, LocationState> & {
children: Children;
};
@observable
shapshot: Blob | null | undefined;
function DataLoader({ match, children }: Props) {
const { ui, shares, documents, auth, revisions } = useStores();
const { team } = auth;
const [error, setError] = React.useState<Error | null>(null);
const { revisionId, shareId, documentSlug } = match.params;
const document = documents.getByUrl(match.params.documentSlug);
const revision = revisionId ? revisions.get(revisionId) : undefined;
const sharedTree = document
? documents.getSharedTree(document.id)
: undefined;
const isEditRoute = match.path === matchDocumentEdit;
const isEditing = isEditRoute || !!auth.team?.collaborativeEditing;
const can = usePolicy(document ? document.id : "");
const location = useLocation<LocationState>();
@observable
error: Error | null | undefined;
componentDidMount() {
const { documents, match } = this.props;
this.document = documents.getByUrl(match.params.documentSlug);
this.sharedTree = this.document
? documents.getSharedTree(this.document.id)
: undefined;
this.loadDocument();
}
componentDidUpdate(prevProps: Props) {
// If we have the document in the store, but not it's policy then we need to
// reload from the server otherwise the UI will not know which authorizations
// the user has
if (this.document) {
const document = this.document;
const policy = this.props.policies.get(document.id);
if (
!policy &&
!this.error &&
this.props.auth.user &&
this.props.auth.user.id
) {
this.loadDocument();
}
}
// Also need to load the revision if it changes
const { revisionId } = this.props.match.params;
if (
prevProps.match.params.revisionId !== revisionId &&
revisionId &&
revisionId !== "latest"
) {
this.loadRevision();
}
}
get isEditRoute() {
return this.props.match.path === matchDocumentEdit;
}
get isEditing() {
return this.isEditRoute || this.props.auth?.team?.collaborativeEditing;
}
onCreateLink = async (title: string) => {
const document = this.document;
invariant(document, "document must be loaded to create link");
const newDocument = await this.props.documents.create({
collectionId: document.collectionId,
parentDocumentId: document.parentDocumentId,
title,
text: "",
});
return newDocument.url;
};
loadRevision = async () => {
const { revisionId } = this.props.match.params;
if (revisionId) {
this.revision = await this.props.revisions.fetch(revisionId);
}
};
loadDocument = async () => {
const { shareId, documentSlug, revisionId } = this.props.match.params;
// sets the document as active in the sidebar if we already have it loaded
if (this.document) {
this.props.ui.setActiveDocument(this.document);
}
try {
const response = await this.props.documents.fetchWithSharedTree(
documentSlug,
{
React.useEffect(() => {
async function fetchDocument() {
try {
await documents.fetchWithSharedTree(documentSlug, {
shareId,
}
);
this.sharedTree = response.sharedTree;
this.document = response.document;
if (revisionId && revisionId !== "latest") {
await this.loadRevision();
} else {
this.revision = undefined;
});
} catch (err) {
setError(err);
}
} catch (err) {
this.error = err;
return;
}
fetchDocument();
}, [ui, documents, document, shareId, documentSlug]);
const document = this.document;
React.useEffect(() => {
async function fetchRevision() {
if (revisionId && revisionId !== "latest") {
try {
await revisions.fetch(revisionId);
} catch (err) {
setError(err);
}
}
}
fetchRevision();
}, [revisions, revisionId]);
const onCreateLink = React.useCallback(
async (title: string) => {
if (!document) {
throw new Error("Document not loaded yet");
}
const newDocument = await documents.create({
collectionId: document.collectionId,
parentDocumentId: document.parentDocumentId,
title,
text: "",
});
return newDocument.url;
},
[document, documents]
);
React.useEffect(() => {
if (document) {
const can = this.props.policies.abilities(document.id);
// sets the document as active in the sidebar, ideally in the future this
// will be route driven.
this.props.ui.setActiveDocument(document);
// sets the current document as active in the sidebar
ui.setActiveDocument(document);
// If we're attempting to update an archived, deleted, or otherwise
// uneditable document then forward to the canonical read url.
if (!can.update && this.isEditRoute) {
if (!can.update && isEditRoute) {
history.push(document.url);
return;
}
@@ -163,72 +114,51 @@ class DataLoader extends React.Component<Props> {
// Prevents unauthorized request to load share information for the document
// when viewing a public share link
if (can.read) {
this.props.shares.fetch(document.id).catch((err) => {
shares.fetch(document.id).catch((err) => {
if (!(err instanceof NotFoundError)) {
throw err;
}
});
}
}
};
}, [can.read, can.update, document, isEditRoute, shares, ui]);
render() {
const { location, policies, auth, match, ui } = this.props;
const { revisionId } = match.params;
if (this.error) {
return this.error instanceof OfflineError ? (
<ErrorOffline />
) : (
<Error404 />
);
}
const team = auth.team;
const document = this.document;
const revision = this.revision;
if (!document || !team || (revisionId && !revision)) {
return (
<>
<Loading location={location} />
{this.isEditing && !team?.collaborativeEditing && (
<HideSidebar ui={ui} />
)}
</>
);
}
const abilities = policies.abilities(document.id);
// We do not want to remount the document when changing from view->edit
// on the multiplayer flag as the doc is guaranteed to be upto date.
const key = team.collaborativeEditing
? ""
: this.isEditing
? "editing"
: "read-only";
if (error) {
return error instanceof OfflineError ? <ErrorOffline /> : <Error404 />;
}
if (!document || !team || (revisionId && !revision)) {
return (
<React.Fragment key={key}>
{this.isEditing && !team.collaborativeEditing && (
<HideSidebar ui={ui} />
)}
{this.props.children({
document,
revision,
abilities,
isEditing: this.isEditing,
readOnly:
!this.isEditing ||
!abilities.update ||
document.isArchived ||
!!revisionId,
onCreateLink: this.onCreateLink,
sharedTree: this.sharedTree,
})}
</React.Fragment>
<>
<Loading location={location} />
{isEditing && !team?.collaborativeEditing && <HideSidebar ui={ui} />}
</>
);
}
// We do not want to remount the document when changing from view->edit
// on the multiplayer flag as the doc is guaranteed to be upto date.
const key = team.collaborativeEditing
? ""
: isEditing
? "editing"
: "read-only";
return (
<React.Fragment key={key}>
{isEditing && !team.collaborativeEditing && <HideSidebar ui={ui} />}
{children({
document,
revision,
abilities: can,
isEditing,
readOnly:
!isEditing || !can.update || document.isArchived || !!revisionId,
onCreateLink,
sharedTree,
})}
</React.Fragment>
);
}
export default withStores(DataLoader);
export default observer(DataLoader);

View File

@@ -55,13 +55,21 @@ import References from "./References";
const AUTOSAVE_DELAY = 3000;
type Params = {
documentSlug: string;
revisionId?: string;
shareId?: string;
};
type LocationState = {
title?: string;
restore?: boolean;
revisionId?: string;
};
type Props = WithTranslation &
RootStore &
RouteComponentProps<
Record<string, string>,
StaticContext,
{ restore?: boolean; revisionId?: string }
> & {
RouteComponentProps<Params, StaticContext, LocationState> & {
sharedTree?: NavigationNode;
abilities: Record<string, any>;
document: Document;

View File

@@ -7,13 +7,21 @@ import DataLoader from "./components/DataLoader";
import Document from "./components/Document";
import SocketPresence from "./components/SocketPresence";
export default function DocumentScene(
props: RouteComponentProps<
{ documentSlug: string; revisionId: string },
StaticContext,
{ title?: string }
>
) {
type Params = {
documentSlug: string;
revisionId?: string;
shareId?: string;
};
type LocationState = {
title?: string;
restore?: boolean;
revisionId?: string;
};
type Props = RouteComponentProps<Params, StaticContext, LocationState>;
export default function DocumentScene(props: Props) {
const { ui } = useStores();
const team = useCurrentTeam();
const { documentSlug, revisionId } = props.match.params;
@@ -47,12 +55,12 @@ export default function DocumentScene(
if (isActive && !isMultiplayer) {
return (
<SocketPresence documentId={document.id} isEditing={isEditing}>
<Document document={document} match={props.match} {...rest} />
<Document document={document} {...rest} />
</SocketPresence>
);
}
return <Document document={document} match={props.match} {...rest} />;
return <Document document={document} {...rest} />;
}}
</DataLoader>
);

View File

@@ -7,6 +7,7 @@ import BaseModel from "~/models/BaseModel";
import Policy from "~/models/Policy";
import { PaginationParams } from "~/types";
import { client } from "~/utils/ApiClient";
import { AuthorizationError, NotFoundError } from "~/utils/errors";
type PartialWithId<T> = Partial<T> & { id: string };
@@ -209,7 +210,7 @@ export default abstract class BaseStore<T extends BaseModel> {
this.addPolicies(res.policies);
return this.add(res.data);
} catch (err) {
if (err.statusCode === 403) {
if (err instanceof AuthorizationError || err instanceof NotFoundError) {
this.remove(id);
}

View File

@@ -4,6 +4,7 @@ import { computed, action } from "mobx";
import Collection from "~/models/Collection";
import { NavigationNode } from "~/types";
import { client } from "~/utils/ApiClient";
import { AuthorizationError, NotFoundError } from "~/utils/errors";
import BaseStore from "./BaseStore";
import RootStore from "./RootStore";
@@ -158,7 +159,7 @@ export default class CollectionsStore extends BaseStore<Collection> {
this.addPolicies(res.policies);
return this.add(res.data);
} catch (err) {
if (err.statusCode === 403) {
if (err instanceof AuthorizationError || err instanceof NotFoundError) {
this.remove(id);
}

View File

@@ -141,16 +141,11 @@ class ApiClient {
// Handle failed responses
const error: {
statusCode?: number;
response?: Response;
message?: string;
error?: string;
data?: Record<string, any>;
} = {};
error.statusCode = response.status;
error.response = response;
try {
const parsed = await response.json();
error.message = parsed.message || "";
@@ -186,7 +181,7 @@ class ApiClient {
throw new ServiceUnavailableError(error.message);
}
throw new RequestError(`Error ${error.statusCode}: ${error.message}`);
throw new RequestError(`Error ${response.status}: ${error.message}`);
};
get = (