diff --git a/server/api/documents.js b/server/api/documents.js index 9d989255..b21b7da8 100644 --- a/server/api/documents.js +++ b/server/api/documents.js @@ -235,17 +235,15 @@ router.post('documents.update', auth(), async ctx => { if (text) document.text = text; document.lastModifiedById = user.id; - const [updatedDocument, updatedCollection] = await Promise.all([ - document.save(), - collection.type === 'atlas' - ? await collection.updateDocument(document) - : Promise.resolve(), - ]); + await document.save(); + if (collection.type === 'atlas') { + await collection.updateDocument(document); + } - updatedDocument.collection = updatedCollection; + document.collection = collection; ctx.body = { - data: await presentDocument(ctx, updatedDocument), + data: await presentDocument(ctx, document), }; }); @@ -258,6 +256,10 @@ router.post('documents.move', auth(), async ctx => { const user = ctx.state.user; const document = await Document.findById(id); + const collection = await Collection.findById(document.atlasId); + + if (collection.type !== 'atlas') + throw httpErrors.BadRequest("This document can't be moved"); if (!document || document.teamId !== user.teamId) throw httpErrors.NotFound(); @@ -277,16 +279,10 @@ router.post('documents.move', auth(), async ctx => { document.parentDocumentId = parentDocument; await document.save(); - const collection = await Collection.findById(document.atlasId); - if (collection.type === 'atlas') { - await collection.deleteDocument(document); - await collection.addDocumentToStructure(document, index); - } + await collection.moveDocument(document, index); // Update collection document.collection = collection; - document.collection = collection; - ctx.body = { data: await presentDocument(ctx, document), }; @@ -311,9 +307,9 @@ router.post('documents.delete', auth(), async ctx => { ); } - // Delete all children + // Delete document and all of its children try { - await collection.deleteDocument(document); + await collection.removeDocument(document); } catch (e) { throw httpErrors.BadRequest('Error while deleting'); } diff --git a/server/models/Collection.js b/server/models/Collection.js index 7e2d29c2..bc272403 100644 --- a/server/models/Collection.js +++ b/server/models/Collection.js @@ -94,13 +94,11 @@ Collection.associate = models => { // Instance methods Collection.prototype.getUrl = function() { - // const slugifiedName = slug(this.name); - // return `/${slugifiedName}-c${this.urlId}`; return `/collections/${this.id}`; }; Collection.prototype.getDocumentsStructure = async function() { - // Lazy fill this.documentStructure + // Lazy fill this.documentStructure - TMP for internal release if (!this.documentStructure) { this.documentStructure = this.navigationTree.children; @@ -121,26 +119,35 @@ Collection.prototype.getDocumentsStructure = async function() { return this.documentStructure; }; -Collection.prototype.addDocumentToStructure = async function(document, index) { +Collection.prototype.addDocumentToStructure = async function( + document, + index, + options = {} +) { if (!this.documentStructure) return; + // If moving existing document with children, use existing structure to + // keep everything in shape and not loose documents + const documentJson = { + ...document.toJSON(), + ...options.documentJson, + }; + if (!document.parentDocumentId) { this.documentStructure.splice( - index || this.documentStructure.length, + index !== undefined ? index : this.documentStructure.length, 0, - document.toJSON() + documentJson ); - // Sequelize doesn't seem to set the value with splice on JSONB field - this.documentStructure = this.documentStructure; } else { // Recursively place document const placeDocument = documentList => { return documentList.map(childDocument => { if (document.parentDocumentId === childDocument.id) { childDocument.children.splice( - index || childDocument.children.length, + index !== undefined ? index : childDocument.children.length, 0, - document.toJSON() + documentJson ); } else { childDocument.children = placeDocument(childDocument.children); @@ -152,10 +159,16 @@ Collection.prototype.addDocumentToStructure = async function(document, index) { this.documentStructure = placeDocument(this.documentStructure); } + // Sequelize doesn't seem to set the value with splice on JSONB field + this.documentStructure = this.documentStructure; await this.save(); + return this; }; +/** + * Update document's title and url in the documentStructure + */ Collection.prototype.updateDocument = async function(updatedDocument) { if (!this.documentStructure) return; const { id } = updatedDocument; @@ -179,30 +192,83 @@ Collection.prototype.updateDocument = async function(updatedDocument) { return this; }; -Collection.prototype.deleteDocument = async function(document) { +/** + * moveDocument is combination of removing the document from the structure + * and placing it back the the new location with the existing children. + */ +Collection.prototype.moveDocument = async function(document, index) { if (!this.documentStructure) return; - const deleteFromChildren = (children, id) => { - if (_.find(children, { id })) { - _.remove(children, { id }); - } else { - children = children.map(childDocument => { + const documentJson = await this.removeDocument(document, { + deleteDocument: false, + }); + await this.addDocumentToStructure(document, index, { documentJson }); + + return this; +}; + +type DeleteDocumentOptions = { + deleteDocument: boolean, +}; + +/** + * removeDocument is used for both deleting documents (deleteDocument: true) + * and removing them temporarily from the structure while they are being moved + * (deleteDocument: false). + */ +Collection.prototype.removeDocument = async function( + document, + options: DeleteDocumentOptions = { deleteDocument: true } +) { + if (!this.documentStructure) return; + let returnValue; + + // Helper to destroy all child documents for a document + const deleteChildren = async documentId => { + const childDocuments = await Document.findAll({ + where: { parentDocumentId: documentId }, + }); + childDocuments.forEach(async child => { + await deleteChildren(child.id); + await child.destroy(); + }); + }; + + // Prune, and destroy if needed, from the document structure + const deleteFromChildren = async (children, id) => { + children = await Promise.all( + children.map(async childDocument => { return { ...childDocument, - children: deleteFromChildren(childDocument.children, id), + children: await deleteFromChildren(childDocument.children, id), }; - }); + }) + ); + + const match = _.find(children, { id }); + if (match) { + if (!options.deleteDocument && !returnValue) returnValue = match; + _.remove(children, { id }); + + if (options.deleteDocument) { + const childDocument = await Document.findById(id); + // Delete the actual document + await childDocument.destroy(); + // Delete all child documents + await deleteChildren(id); + } } + return children; }; - this.documentStructure = deleteFromChildren( + this.documentStructure = await deleteFromChildren( this.documentStructure, document.id ); - await this.save(); - return this; + if (options.deleteDocument) await this.save(); + return returnValue; }; export default Collection; diff --git a/server/models/Collection.test.js b/server/models/Collection.test.js new file mode 100644 index 00000000..1327b0ae --- /dev/null +++ b/server/models/Collection.test.js @@ -0,0 +1,275 @@ +/* eslint-disable flowtype/require-valid-file-annotation */ +import { flushdb, seed } from '../test/support'; +import Collection from '../models/Collection'; +import Document from '../models/Document'; + +beforeEach(flushdb); +beforeEach(jest.resetAllMocks); + +describe('#getUrl', () => { + test('should return correct url for the collection', () => { + const collection = new Collection({ id: '1234' }); + expect(collection.getUrl()).toBe('/collections/1234'); + }); +}); + +describe('#addDocumentToStructure', async () => { + test('should add as last element without index', async () => { + const { collection } = await seed(); + const newDocument = new Document({ + id: '5', + title: 'New end node', + parentDocumentId: null, + }); + + await collection.addDocumentToStructure(newDocument); + expect(collection.documentStructure.length).toBe(3); + expect(collection.documentStructure[2].id).toBe('5'); + }); + + test('should add with an index', async () => { + const { collection } = await seed(); + const newDocument = new Document({ + id: '5', + title: 'New end node', + parentDocumentId: null, + }); + + await collection.addDocumentToStructure(newDocument, 1); + expect(collection.documentStructure.length).toBe(3); + expect(collection.documentStructure[1].id).toBe('5'); + }); + + test('should add as a child if with parent', async () => { + const { collection, document } = await seed(); + const newDocument = new Document({ + id: '5', + title: 'New end node', + parentDocumentId: document.id, + }); + + await collection.addDocumentToStructure(newDocument, 1); + expect(collection.documentStructure.length).toBe(2); + expect(collection.documentStructure[1].id).toBe(document.id); + expect(collection.documentStructure[1].children.length).toBe(1); + expect(collection.documentStructure[1].children[0].id).toBe('5'); + }); + + test('should add as a child if with parent with index', async () => { + const { collection, document } = await seed(); + const newDocument = new Document({ + id: '5', + title: 'node', + parentDocumentId: document.id, + }); + const secondDocument = new Document({ + id: '6', + title: 'New start node', + parentDocumentId: document.id, + }); + + await collection.addDocumentToStructure(newDocument); + await collection.addDocumentToStructure(secondDocument, 0); + expect(collection.documentStructure.length).toBe(2); + expect(collection.documentStructure[1].id).toBe(document.id); + expect(collection.documentStructure[1].children.length).toBe(2); + expect(collection.documentStructure[1].children[0].id).toBe('6'); + }); + + describe('options: documentJson', async () => { + test("should append supplied json over document's own", async () => { + const { collection } = await seed(); + const newDocument = new Document({ + id: '5', + title: 'New end node', + parentDocumentId: null, + }); + + await collection.addDocumentToStructure(newDocument, undefined, { + documentJson: { + children: [ + { + id: '7', + title: 'Totally fake', + children: [], + }, + ], + }, + }); + expect(collection.documentStructure[2].children.length).toBe(1); + expect(collection.documentStructure[2].children[0].id).toBe('7'); + }); + }); +}); + +describe('#updateDocument', () => { + test("should update root document's data", async () => { + const { collection, document } = await seed(); + + document.title = 'Updated title'; + await document.save(); + + await collection.updateDocument(document); + + expect(collection.documentStructure[1].title).toBe('Updated title'); + }); + + test("should update child document's data", async () => { + const { collection, document } = await seed(); + // Add a child for testing + const newDocument = await Document.create({ + parentDocumentId: document.id, + atlasId: collection.id, + teamId: collection.teamId, + userId: collection.creatorId, + lastModifiedById: collection.creatorId, + createdById: collection.creatorId, + title: 'Child document', + text: 'content', + }); + await collection.addDocumentToStructure(newDocument); + + newDocument.title = 'Updated title'; + await newDocument.save(); + + await collection.updateDocument(newDocument); + + expect(collection.documentStructure[1].children[0].title).toBe( + 'Updated title' + ); + }); +}); + +describe('#moveDocument', () => { + test('should move a document without children', async () => { + const { collection, document } = await seed(); + + expect(collection.documentStructure[1].id).toBe(document.id); + await collection.moveDocument(document, 0); + expect(collection.documentStructure[0].id).toBe(document.id); + }); + + test('should move a document with children', async () => { + const { collection, document } = await seed(); + + // Add a child for testing + const newDocument = await Document.create({ + parentDocumentId: document.id, + atlasId: collection.id, + teamId: collection.teamId, + userId: collection.creatorId, + lastModifiedById: collection.creatorId, + createdById: collection.creatorId, + title: 'Child document', + text: 'content', + }); + await collection.addDocumentToStructure(newDocument); + + await collection.moveDocument(document, 0); + expect(collection.documentStructure[0].children[0].id).toBe(newDocument.id); + }); +}); + +describe('#removeDocument', () => { + const destroyMock = jest.fn(); + + test('should save if removing', async () => { + const { collection, document } = await seed(); + jest.spyOn(collection, 'save'); + + await collection.removeDocument(document); + expect(collection.save).toBeCalled(); + }); + + test('should remove documents from root', async () => { + const { collection, document } = await seed(); + + await collection.removeDocument(document); + expect(collection.documentStructure.length).toBe(1); + + // Verify that the document was removed + const collectionDocuments = await Document.findAndCountAll({ + where: { + atlasId: collection.id, + }, + }); + expect(collectionDocuments.count).toBe(1); + }); + + test('should remove a document with child documents', async () => { + const { collection, document } = await seed(); + + // Add a child for testing + const newDocument = await Document.create({ + parentDocumentId: document.id, + atlasId: collection.id, + teamId: collection.teamId, + userId: collection.creatorId, + lastModifiedById: collection.creatorId, + createdById: collection.creatorId, + title: 'Child document', + text: 'content', + }); + await collection.addDocumentToStructure(newDocument); + expect(collection.documentStructure[1].children.length).toBe(1); + + // Remove the document + await collection.removeDocument(document); + expect(collection.documentStructure.length).toBe(1); + const collectionDocuments = await Document.findAndCountAll({ + where: { + atlasId: collection.id, + }, + }); + expect(collectionDocuments.count).toBe(1); + }); + + test('should remove a child document', async () => { + const { collection, document } = await seed(); + + // Add a child for testing + const newDocument = await Document.create({ + parentDocumentId: document.id, + atlasId: collection.id, + teamId: collection.teamId, + userId: collection.creatorId, + lastModifiedById: collection.creatorId, + createdById: collection.creatorId, + title: 'Child document', + text: 'content', + }); + await collection.addDocumentToStructure(newDocument); + expect(collection.documentStructure.length).toBe(2); + expect(collection.documentStructure[1].children.length).toBe(1); + + // Remove the document + await collection.removeDocument(newDocument); + + expect(collection.documentStructure.length).toBe(2); + expect(collection.documentStructure[0].children.length).toBe(0); + expect(collection.documentStructure[1].children.length).toBe(0); + + const collectionDocuments = await Document.findAndCountAll({ + where: { + atlasId: collection.id, + }, + }); + expect(collectionDocuments.count).toBe(2); + }); + + describe('options: deleteDocument = false', () => { + test('should remove documents from the structure but not destroy them from the DB', async () => { + const { collection, document } = await seed(); + jest.spyOn(collection, 'save'); + + const removedNode = await collection.removeDocument(document, { + deleteDocument: false, + }); + expect(collection.documentStructure.length).toBe(1); + expect(destroyMock).not.toBeCalled(); + expect(collection.save).not.toBeCalled(); + expect(removedNode.id).toBe(document.id); + expect(removedNode.children).toEqual([]); + }); + }); +});