fix: Documents in deleted collection should appear in trash (#1362)

* fix: Documents from deleted collection should show in trash

* improve messaging

* test: Add documents.move tests
feat: Add ability to restore trashed documents from deleted collection

* update store

* fix

* ui

* lint

* fix: Improve breadcrumb
This commit is contained in:
Tom Moor
2020-09-07 11:51:09 -07:00
committed by GitHub
parent c5de2da115
commit 4de3f69474
12 changed files with 258 additions and 50 deletions

View File

@ -1,11 +1,13 @@
// @flow
import { observer, inject } from "mobx-react";
import {
PadlockIcon,
ArchiveIcon,
EditIcon,
GoToIcon,
MoreIcon,
PadlockIcon,
ShapesIcon,
EditIcon,
TrashIcon,
} from "outline-icons";
import * as React from "react";
import { Link } from "react-router-dom";
@ -25,11 +27,73 @@ type Props = {
onlyText: boolean,
};
const Breadcrumb = observer(({ document, collections, onlyText }: Props) => {
const collection = collections.get(document.collectionId);
if (!collection) return <div />;
function Icon({ document }) {
if (document.isDeleted) {
return (
<>
<CollectionName to="/trash">
<TrashIcon color="currentColor" />
&nbsp;
<span>Trash</span>
</CollectionName>
<Slash />
</>
);
}
if (document.isArchived) {
return (
<>
<CollectionName to="/archive">
<ArchiveIcon color="currentColor" />
&nbsp;
<span>Archive</span>
</CollectionName>
<Slash />
</>
);
}
if (document.isDraft) {
return (
<>
<CollectionName to="/drafts">
<EditIcon color="currentColor" />
&nbsp;
<span>Drafts</span>
</CollectionName>
<Slash />
</>
);
}
if (document.isTemplate) {
return (
<>
<CollectionName to="/templates">
<ShapesIcon color="currentColor" />
&nbsp;
<span>Templates</span>
</CollectionName>
<Slash />
</>
);
}
return null;
}
const path = collection.pathToDocument(document).slice(0, -1);
const Breadcrumb = observer(({ document, collections, onlyText }: Props) => {
let collection = collections.get(document.collectionId);
if (!collection) {
if (!document.deletedAt) return <div />;
collection = {
id: document.collectionId,
name: "Deleted Collection",
color: "currentColor",
};
}
const path = collection.pathToDocument
? collection.pathToDocument(document).slice(0, -1)
: [];
if (onlyText === true) {
return (
@ -50,34 +114,13 @@ const Breadcrumb = observer(({ document, collections, onlyText }: Props) => {
);
}
const isTemplate = document.isTemplate;
const isDraft = !document.publishedAt && !isTemplate;
const isNestedDocument = path.length > 1;
const lastPath = path.length ? path[path.length - 1] : undefined;
const menuPath = isNestedDocument ? path.slice(0, -1) : [];
return (
<Wrapper justify="flex-start" align="center">
{isTemplate && (
<>
<CollectionName to="/templates">
<ShapesIcon color="currentColor" />
&nbsp;
<span>Templates</span>
</CollectionName>
<Slash />
</>
)}
{isDraft && (
<>
<CollectionName to="/drafts">
<EditIcon color="currentColor" />
&nbsp;
<span>Drafts</span>
</CollectionName>
<Slash />
</>
)}
<Icon document={document} />
<CollectionName to={collectionUrl(collection.id)}>
<CollectionIcon collection={collection} expanded />
&nbsp;

View File

@ -18,7 +18,7 @@ function ResolvedCollectionIcon({ collection, expanded, size, ui }: Props) {
// If the chosen icon color is very dark then we invert it in dark mode
// otherwise it will be impossible to see against the dark background.
const color =
ui.resolvedTheme === "dark"
ui.resolvedTheme === "dark" && collection.color !== "currentColor"
? getLuminance(collection.color) > 0.12
? collection.color
: "currentColor"

View File

@ -11,7 +11,12 @@ import Document from "models/Document";
import DocumentDelete from "scenes/DocumentDelete";
import DocumentShare from "scenes/DocumentShare";
import DocumentTemplatize from "scenes/DocumentTemplatize";
import { DropdownMenu, DropdownMenuItem } from "components/DropdownMenu";
import CollectionIcon from "components/CollectionIcon";
import {
DropdownMenu,
DropdownMenuItem,
Header,
} from "components/DropdownMenu";
import Modal from "components/Modal";
import {
documentHistoryUrl,
@ -100,8 +105,11 @@ class DocumentMenu extends React.Component<Props> {
this.props.ui.showToast("Document archived");
};
handleRestore = async (ev: SyntheticEvent<>) => {
await this.props.document.restore();
handleRestore = async (
ev: SyntheticEvent<>,
options?: { collectionId: string }
) => {
await this.props.document.restore(options);
this.props.ui.showToast("Document restored");
};
@ -154,6 +162,7 @@ class DocumentMenu extends React.Component<Props> {
showPrint,
showPin,
auth,
collections,
onOpen,
onClose,
} = this.props;
@ -161,6 +170,7 @@ class DocumentMenu extends React.Component<Props> {
const can = policies.abilities(document.id);
const canShareDocuments = can.share && auth.team && auth.team.sharing;
const canViewHistory = can.read && !can.restore;
const collection = collections.get(document.collectionId);
return (
<>
@ -170,11 +180,45 @@ class DocumentMenu extends React.Component<Props> {
onOpen={onOpen}
onClose={onClose}
>
{(can.unarchive || can.restore) && (
{can.unarchive && (
<DropdownMenuItem onClick={this.handleRestore}>
Restore
</DropdownMenuItem>
)}
{can.restore &&
(collection ? (
<DropdownMenuItem onClick={this.handleRestore}>
Restore
</DropdownMenuItem>
) : (
<DropdownMenu
label={<DropdownMenuItem>Restore</DropdownMenuItem>}
style={{
left: -170,
position: "relative",
top: -40,
}}
hover
>
<Header>Choose a collection</Header>
{collections.orderedData.map((collection) => {
const can = policies.abilities(collection.id);
return (
<DropdownMenuItem
key={collection.id}
onClick={(ev) =>
this.handleRestore(ev, { collectionId: collection.id })
}
disabled={!can.update}
>
<CollectionIcon collection={collection} />
&nbsp;{collection.name}
</DropdownMenuItem>
);
})}
</DropdownMenu>
))}
{showPin &&
(document.pinned
? can.unpin && (

View File

@ -24,7 +24,7 @@ type Props = {
class RevisionMenu extends React.Component<Props> {
handleRestore = async (ev: SyntheticEvent<>) => {
ev.preventDefault();
await this.props.document.restore(this.props.revision);
await this.props.document.restore({ revisionId: this.props.revision.id });
this.props.ui.showToast("Document restored");
this.props.history.push(this.props.document.url);
};

View File

@ -6,7 +6,6 @@ import parseTitle from "shared/utils/parseTitle";
import unescape from "shared/utils/unescape";
import DocumentsStore from "stores/DocumentsStore";
import BaseModel from "models/BaseModel";
import Revision from "models/Revision";
import User from "models/User";
type SaveOptions = {
@ -141,8 +140,8 @@ export default class Document extends BaseModel {
return this.store.archive(this);
};
restore = (revision: Revision) => {
return this.store.restore(this, revision);
restore = (options) => {
return this.store.restore(this, options);
};
unpublish = () => {

View File

@ -46,8 +46,9 @@ class CollectionDelete extends React.Component<Props> {
<form onSubmit={this.handleSubmit}>
<HelpText>
Are you sure about that? Deleting the{" "}
<strong>{collection.name}</strong> collection is permanent and will
also delete all of the documents within it, so be extra careful.
<strong>{collection.name}</strong> collection is permanent and
cannot be restored, however documents within will be moved to the
trash.
</HelpText>
<Button type="submit" disabled={this.isDeleting} autoFocus danger>
{this.isDeleting ? "Deleting…" : "Im sure  Delete"}

View File

@ -12,11 +12,9 @@ import {
} from "lodash";
import { observable, action, computed, runInAction } from "mobx";
import naturalSort from "shared/utils/naturalSort";
import BaseStore from "stores/BaseStore";
import RootStore from "stores/RootStore";
import Document from "models/Document";
import Revision from "models/Revision";
import type { FetchOptions, PaginationParams, SearchResult } from "types";
import { client } from "utils/ApiClient";
@ -435,6 +433,7 @@ export default class DocumentsStore extends BaseStore<Document> {
res.data.documents.forEach(this.add);
res.data.collections.forEach(this.rootStore.collections.add);
this.addPolicies(res.policies);
};
@action
@ -527,10 +526,10 @@ export default class DocumentsStore extends BaseStore<Document> {
};
@action
restore = async (document: Document, revision?: Revision) => {
restore = async (document: Document, options = {}) => {
const res = await client.post("/documents.restore", {
id: document.id,
revisionId: revision ? revision.id : undefined,
...options,
});
runInAction("Document#restore", () => {
invariant(res && res.data, "Data should be available");

View File

@ -210,7 +210,7 @@ router.post("documents.deleted", auth(), pagination(), async (ctx) => {
if (direction !== "ASC") direction = "DESC";
const user = ctx.state.user;
const collectionIds = await user.collectionIds();
const collectionIds = await user.collectionIds({ paranoid: false });
const collectionScope = { method: ["withCollection", user.id] };
const documents = await Document.scope(collectionScope).findAll({
@ -444,7 +444,7 @@ router.post("documents.export", auth({ required: false }), async (ctx) => {
});
router.post("documents.restore", auth(), async (ctx) => {
const { id, revisionId } = ctx.body;
const { id, collectionId, revisionId } = ctx.body;
ctx.assertPresent(id, "id is required");
const user = ctx.state.user;
@ -453,6 +453,16 @@ router.post("documents.restore", auth(), async (ctx) => {
paranoid: false,
});
if (collectionId) {
ctx.assertUuid(collectionId, "collectionId must be a uuid");
authorize(user, "restore", document);
const collection = await Collection.findByPk(collectionId);
authorize(user, "update", collection);
document.collectionId = collectionId;
}
if (document.deletedAt) {
authorize(user, "restore", document);
@ -938,6 +948,9 @@ router.post("documents.move", auth(), async (ctx) => {
const document = await Document.findByPk(id, { userId: user.id });
authorize(user, "move", document);
const collection = await Collection.findByPk(collectionId);
authorize(user, "update", collection);
if (parentDocumentId) {
const parent = await Document.findByPk(parentDocumentId, {
userId: user.id,

View File

@ -1137,7 +1137,109 @@ describe("#documents.pin", () => {
});
});
describe("#documents.move", () => {
it("should move the document", async () => {
const { user, document } = await seed();
const collection = await buildCollection({ teamId: user.teamId });
const res = await server.post("/api/documents.move", {
body: {
token: user.getJwtToken(),
id: document.id,
collectionId: collection.id,
},
});
const body = await res.json();
expect(res.status).toEqual(200);
expect(body.data.documents[0].collectionId).toEqual(collection.id);
});
it("should not allow moving the document to a collection the user cannot access", async () => {
const { user, document } = await seed();
const collection = await buildCollection();
const res = await server.post("/api/documents.move", {
body: {
token: user.getJwtToken(),
id: document.id,
collectionId: collection.id,
},
});
expect(res.status).toEqual(403);
});
it("should require authentication", async () => {
const res = await server.post("/api/documents.move");
expect(res.status).toEqual(401);
});
it("should require authorization", async () => {
const { document, collection } = await seed();
const user = await buildUser();
const res = await server.post("/api/documents.move", {
body: {
token: user.getJwtToken(),
id: document.id,
collectionId: collection.id,
},
});
expect(res.status).toEqual(403);
});
});
describe("#documents.restore", () => {
it("should allow restore of trashed documents", async () => {
const { user, document } = await seed();
await document.destroy(user.id);
const res = await server.post("/api/documents.restore", {
body: { token: user.getJwtToken(), id: document.id },
});
const body = await res.json();
expect(res.status).toEqual(200);
expect(body.data.deletedAt).toEqual(null);
});
it("should allow restore of trashed documents with collectionId", async () => {
const { user, document } = await seed();
const collection = await buildCollection({
userId: user.id,
teamId: user.teamId,
});
await document.destroy(user.id);
const res = await server.post("/api/documents.restore", {
body: {
token: user.getJwtToken(),
id: document.id,
collectionId: collection.id,
},
});
const body = await res.json();
expect(res.status).toEqual(200);
expect(body.data.deletedAt).toEqual(null);
expect(body.data.collectionId).toEqual(collection.id);
});
it("should now allow restore of trashed documents to collection user cannot access", async () => {
const { user, document } = await seed();
const collection = await buildCollection();
await document.destroy(user.id);
const res = await server.post("/api/documents.restore", {
body: {
token: user.getJwtToken(),
id: document.id,
collectionId: collection.id,
},
});
expect(res.status).toEqual(403);
});
it("should allow restore of archived documents", async () => {
const { user, document } = await seed();
await document.archive(user.id);
@ -1146,6 +1248,8 @@ describe("#documents.restore", () => {
body: { token: user.getJwtToken(), id: document.id },
});
const body = await res.json();
expect(res.status).toEqual(200);
expect(body.data.archivedAt).toEqual(null);
});
@ -1164,6 +1268,8 @@ describe("#documents.restore", () => {
body: { token: user.getJwtToken(), id: childDocument.id },
});
const body = await res.json();
expect(res.status).toEqual(200);
expect(body.data.parentDocumentId).toEqual(undefined);
expect(body.data.archivedAt).toEqual(null);
});
@ -1184,6 +1290,8 @@ describe("#documents.restore", () => {
body: { token: user.getJwtToken(), id: document.id, revisionId },
});
const body = await res.json();
expect(res.status).toEqual(200);
expect(body.data.text).toEqual(previousText);
});

View File

@ -16,8 +16,7 @@ router.post("events.list", auth(), pagination(), async (ctx) => {
if (direction !== "ASC") direction = "DESC";
const user = ctx.state.user;
const paranoid = false;
const collectionIds = await user.collectionIds(paranoid);
const collectionIds = await user.collectionIds({ paranoid: false });
let where = {
name: Event.ACTIVITY_EVENTS,

View File

@ -14,7 +14,7 @@ export default async function documentMover({
user: Context,
document: Document,
collectionId: string,
parentDocumentId: string,
parentDocumentId?: string,
index?: number,
ip: string,
}) {
@ -39,6 +39,7 @@ export default async function documentMover({
// remove from original collection
const collection = await Collection.findByPk(document.collectionId, {
transaction,
paranoid: false,
});
const documentJson = await collection.removeDocumentInStructure(
document,

View File

@ -71,13 +71,14 @@ User.associate = (models) => {
};
// Instance methods
User.prototype.collectionIds = async function (paranoid: boolean = true) {
User.prototype.collectionIds = async function (options = {}) {
const collectionStubs = await Collection.scope({
method: ["withMembership", this.id],
}).findAll({
attributes: ["id", "private"],
where: { teamId: this.teamId },
paranoid,
paranoid: true,
...options,
});
return collectionStubs