diff --git a/app/scenes/DocumentMove.tsx b/app/scenes/DocumentMove.tsx index e333bec13..6ce2ec232 100644 --- a/app/scenes/DocumentMove.tsx +++ b/app/scenes/DocumentMove.tsx @@ -63,13 +63,6 @@ function DocumentMove({ document, onRequestClose }: Props) { if (onlyShowCollections) { results = results.filter((result) => result.type === "collection"); } else { - // Exclude root from search results if document is already at the root - if (!document.parentDocumentId) { - results = results.filter( - (result) => result.id !== document.collectionId - ); - } - // Exclude document if on the path to result, or the same result results = results.filter( (result) => diff --git a/server/commands/documentMover.test.ts b/server/commands/documentMover.test.ts index 5c80613a2..8627aaded 100644 --- a/server/commands/documentMover.test.ts +++ b/server/commands/documentMover.test.ts @@ -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 () => { diff --git a/server/commands/documentMover.ts b/server/commands/documentMover.ts index 6f8ed56e6..dc6502d33 100644 --- a/server/commands/documentMover.ts +++ b/server/commands/documentMover.ts @@ -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 diff --git a/server/models/Collection.test.ts b/server/models/Collection.test.ts index 38a159137..68d27159c 100644 --- a/server/models/Collection.test.ts +++ b/server/models/Collection.test.ts @@ -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: { diff --git a/server/models/Collection.ts b/server/models/Collection.ts index 1660defa1..287f34428 100644 --- a/server/models/Collection.ts +++ b/server/models/Collection.ts @@ -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 diff --git a/server/models/Document.ts b/server/models/Document.ts index bb98a5314..563053d2c 100644 --- a/server/models/Document.ts +++ b/server/models/Document.ts @@ -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 => { + 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, }; }; } diff --git a/server/routes/api/documents/documents.test.ts b/server/routes/api/documents/documents.test.ts index 568aac9c2..796ade083 100644 --- a/server/routes/api/documents/documents.test.ts +++ b/server/routes/api/documents/documents.test.ts @@ -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: [] }, ], }, ];