yuuza/outline
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

chore: Improvements to document move behavior (#4689)

Browse Source 
* chore: Improvements to document move behavior

* test
This commit is contained in:
Tom Moor
2023-01-12 21:48:09 -05:00
committed by GitHub
parent 17a8dbb3f0
commit e347404502
7 changed files with 112 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files

49
server/commands/documentMover.test.ts
View File

@@ -1,10 +1,6 @@
import { sequelize } from "@server/database/sequelize";
import Pin from "@server/models/Pin";
import {
buildDocument,
buildCollection,
buildUser,
} from "@server/test/factories";
import { buildDocument, buildCollection } from "@server/test/factories";
import { setupTestDatabase, seed } from "@server/test/support";
import documentMover from "./documentMover";
@@ -25,28 +21,31 @@ describe("documentMover", () => {
expect(response.documents.length).toEqual(1);
});
it("should error when not in source collection documentStructure", async () => {
const user = await buildUser();
const collection = await buildCollection({
teamId: user.teamId,
});
const document = await buildDocument({
it("should succeed when not in source collection documentStructure", async () => {
const { document, user, collection } = await seed();
const newDocument = await buildDocument({
parentDocumentId: document.id,
collectionId: collection.id,
teamId: collection.teamId,
userId: collection.createdById,
title: "Child document",
text: "content",
});
await document.archive(user.id);
let error;
try {
await documentMover({
user,
document,
collectionId: collection.id,
ip,
});
} catch (err) {
error = err;
}
expect(error).toBeTruthy();
const response = await documentMover({
user,
document,
collectionId: collection.id,
parentDocumentId: undefined,
index: 0,
ip,
});
expect(response.collections[0].documentStructure![0].children[0].id).toBe(
newDocument.id
);
expect(response.collections.length).toEqual(1);
expect(response.documents.length).toEqual(1);
expect(response.documents[0].collection?.id).toEqual(collection.id);
expect(response.documents[0].updatedBy.id).toEqual(user.id);
});
it("should move with children", async () => {

5
server/commands/documentMover.ts
View File

@@ -1,6 +1,5 @@
import invariant from "invariant";
import { Transaction } from "sequelize";
import { ValidationError } from "@server/errors";
import { traceFunction } from "@server/logging/tracing";
import { User, Document, Collection, Pin, Event } from "@server/models";
import pinDestroyer from "./pinDestroyer";
@@ -74,11 +73,11 @@ async function documentMover({
save: collectionChanged,
});
const documentJson = response?.[0];
let documentJson = response?.[0];
const fromIndex = response?.[1] || 0;
if (!documentJson) {
throw ValidationError("The document was not found in the collection");
documentJson = await document.toNavigationNode({ transaction });
}
// if we're reordering from within the same parent

47
server/models/Collection.test.ts
View File

@@ -33,7 +33,10 @@ describe("getDocumentParents", () => {
const document = await buildDocument();
const collection = await buildCollection({
documentStructure: [
{ ...parent.toJSON(), children: [document.toJSON()] },
{
...(await parent.toNavigationNode()),
children: [await document.toNavigationNode()],
},
],
});
const result = collection.getDocumentParents(document.id);
@@ -46,7 +49,10 @@ describe("getDocumentParents", () => {
const document = await buildDocument();
const collection = await buildCollection({
documentStructure: [
{ ...parent.toJSON(), children: [document.toJSON()] },
{
...(await parent.toNavigationNode()),
children: [await document.toNavigationNode()],
},
],
});
const result = collection.getDocumentParents(parent.id);
@@ -66,9 +72,11 @@ describe("getDocumentTree", () => {
test("should return document tree", async () => {
const document = await buildDocument();
const collection = await buildCollection({
documentStructure: [document.toJSON()],
documentStructure: [await document.toNavigationNode()],
});
expect(collection.getDocumentTree(document.id)).toEqual(document.toJSON());
expect(collection.getDocumentTree(document.id)).toEqual(
await document.toNavigationNode()
);
});
test("should return nested documents in tree", async () => {
@@ -76,15 +84,20 @@ describe("getDocumentTree", () => {
const document = await buildDocument();
const collection = await buildCollection({
documentStructure: [
{ ...parent.toJSON(), children: [document.toJSON()] },
{
...(await parent.toNavigationNode()),
children: [await document.toNavigationNode()],
},
],
});
expect(collection.getDocumentTree(parent.id)).toEqual({
...parent.toJSON(),
children: [document.toJSON()],
...(await parent.toNavigationNode()),
children: [await document.toNavigationNode()],
});
expect(collection.getDocumentTree(document.id)).toEqual(document.toJSON());
expect(collection.getDocumentTree(document.id)).toEqual(
await document.toNavigationNode()
);
});
});
@@ -92,10 +105,11 @@ describe("#addDocumentToStructure", () => {
test("should add as last element without index", async () => {
const { collection } = await seed();
const id = uuidv4();
const newDocument = new Document({
const newDocument = await buildDocument({
id,
title: "New end node",
parentDocumentId: null,
teamId: collection.teamId,
});
await collection.addDocumentToStructure(newDocument);
expect(collection.documentStructure!.length).toBe(2);
@@ -105,10 +119,11 @@ describe("#addDocumentToStructure", () => {
test("should add with an index", async () => {
const { collection } = await seed();
const id = uuidv4();
const newDocument = new Document({
const newDocument = await buildDocument({
id,
title: "New end node",
parentDocumentId: null,
teamId: collection.teamId,
});
await collection.addDocumentToStructure(newDocument, 1);
expect(collection.documentStructure!.length).toBe(2);
@@ -118,10 +133,11 @@ describe("#addDocumentToStructure", () => {
test("should add as a child if with parent", async () => {
const { collection, document } = await seed();
const id = uuidv4();
const newDocument = new Document({
const newDocument = await buildDocument({
id,
title: "New end node",
parentDocumentId: document.id,
teamId: collection.teamId,
});
await collection.addDocumentToStructure(newDocument, 1);
expect(collection.documentStructure!.length).toBe(1);
@@ -132,16 +148,18 @@ describe("#addDocumentToStructure", () => {
test("should add as a child if with parent with index", async () => {
const { collection, document } = await seed();
const newDocument = new Document({
const newDocument = await buildDocument({
id: uuidv4(),
title: "node",
parentDocumentId: document.id,
teamId: collection.teamId,
});
const id = uuidv4();
const secondDocument = new Document({
const secondDocument = await buildDocument({
id,
title: "New start node",
parentDocumentId: document.id,
teamId: collection.teamId,
});
await collection.addDocumentToStructure(newDocument);
await collection.addDocumentToStructure(secondDocument, 0);
@@ -154,10 +172,11 @@ describe("#addDocumentToStructure", () => {
test("should append supplied json over document's own", async () => {
const { collection } = await seed();
const id = uuidv4();
const newDocument = new Document({
const newDocument = await buildDocument({
id: uuidv4(),
title: "New end node",
parentDocumentId: null,
teamId: collection.teamId,
});
await collection.addDocumentToStructure(newDocument, undefined, {
documentJson: {

33
server/models/Collection.ts
View File

@@ -554,7 +554,7 @@ class Collection extends ParanoidModel {
*/
updateDocument = async function (
updatedDocument: Document,
options?: { transaction?: Transaction | null }
options?: { transaction?: Transaction | null | undefined }
) {
if (!this.documentStructure) {
return;
@@ -563,21 +563,23 @@ class Collection extends ParanoidModel {
const { id } = updatedDocument;
const updateChildren = (documents: NavigationNode[]) => {
return documents.map((document) => {
if (document.id === id) {
document = {
...(updatedDocument.toJSON() as NavigationNode),
children: document.children,
};
} else {
document.children = updateChildren(document.children);
}
return Promise.all(
documents.map(async (document) => {
if (document.id === id) {
document = {
...(await updatedDocument.toNavigationNode(options)),
children: document.children,
};
} else {
document.children = await updateChildren(document.children);
}
return document;
});
return document;
})
);
};
this.documentStructure = updateChildren(this.documentStructure);
this.documentStructure = await updateChildren(this.documentStructure);
// Sequelize doesn't seem to set the value with splice on JSONB field
// https://github.com/sequelize/sequelize/blob/e1446837196c07b8ff0c23359b958d68af40fd6d/src/model.js#L3937
this.changed("documentStructure", true);
@@ -602,7 +604,10 @@ class Collection extends ParanoidModel {
}
// If moving existing document with children, use existing structure
const documentJson = { ...document.toJSON(), ...options.documentJson };
const documentJson = {
...(await document.toNavigationNode(options)),
...options.documentJson,
};
if (!document.parentDocumentId) {
// Note: Index is supported on DB level but it's being ignored

36
server/models/Document.ts
View File

@@ -36,6 +36,7 @@ import parseTitle from "@shared/utils/parseTitle";
import { SLUG_URL_REGEX } from "@shared/utils/urlHelpers";
import { DocumentValidation } from "@shared/validations";
import slugify from "@server/utils/slugify";
import type { NavigationNode } from "~/types";
import Backlink from "./Backlink";
import Collection from "./Collection";
import FileOperation from "./FileOperation";
@@ -750,14 +751,41 @@ class Document extends ParanoidModel {
return notEmpty ? lines[1] : "";
};
toJSON = () => {
// Warning: only use for new documents as order of children is
// handled in the collection's documentStructure
/**
* Returns a JSON representation of the document suitable for use in the
* collection documentStructure.
*
* @param options Optional transaction to use for the query
* @returns Promise resolving to a NavigationNode
*/
toNavigationNode = async (options?: {
transaction?: Transaction | null | undefined;
}): Promise<NavigationNode> => {
const childDocuments = await (this.constructor as typeof Document)
.unscoped()
.findAll({
where: {
teamId: this.teamId,
parentDocumentId: this.id,
archivedAt: {
[Op.is]: null,
},
publishedAt: {
[Op.ne]: null,
},
},
transaction: options?.transaction,
});
const children = await Promise.all(
childDocuments.map((child) => child.toNavigationNode(options))
);
return {
id: this.id,
title: this.title,
url: this.url,
children: [],
children,
};
};
}

4
server/routes/api/documents/documents.test.ts
View File

@@ -231,7 +231,7 @@ describe("#documents.info", () => {
expect(body.data.document.id).toEqual(document.id);
expect(body.data.document.createdBy).toEqual(undefined);
expect(body.data.document.updatedBy).toEqual(undefined);
expect(body.data.sharedTree).toEqual(document.toJSON());
expect(body.data.sharedTree).toEqual(await document.toNavigationNode());
});
it("should not return sharedTree if child documents not shared", async () => {
const { document, user } = await seed();
@@ -2622,7 +2622,7 @@ describe("#documents.update", () => {
title: "Another doc",
children: [],
},
{ ...document.toJSON(), children: [] },
{ ...(await document.toNavigationNode()), children: [] },
],
},
];