From c229369efd440f2565ae984d0a399c236e34afb8 Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Sun, 4 Jun 2017 14:40:27 -0700 Subject: [PATCH] Changed Collection documents to documentStructure and other WIP stuff --- server/api/documents.js | 81 ++++--- server/api/middlewares/validation.js | 11 +- ...collection-documentStructure-migration.js} | 6 +- server/models/Collection.js | 213 ++++++------------ server/models/Document.js | 8 + server/presenters.js | 4 +- 6 files changed, 131 insertions(+), 192 deletions(-) rename server/migrations/{20170603185012-add-collection-documents-migration.js => 20170603185012-add-collection-documentStructure-migration.js} (61%) diff --git a/server/api/documents.js b/server/api/documents.js index a8fa7982b..adcfaf153 100644 --- a/server/api/documents.js +++ b/server/api/documents.js @@ -1,12 +1,11 @@ +// @flow import Router from 'koa-router'; import httpErrors from 'http-errors'; -import { lock } from '../redis'; import isUUID from 'validator/lib/isUUID'; const URL_REGEX = /^[a-zA-Z0-9-]*-([a-zA-Z0-9]{10,15})$/; import auth from './middlewares/authentication'; -// import pagination from './middlewares/pagination'; import { presentDocument } from '../presenters'; import { Document, Collection } from '../models'; @@ -96,10 +95,14 @@ router.post('documents.search', auth(), async ctx => { }); router.post('documents.create', auth(), async ctx => { - const { collection, title, text, parentDocument } = ctx.body; + const { collection, title, text, parentDocument, index } = ctx.body; ctx.assertPresent(collection, 'collection is required'); + ctx.assertUuid(collection, 'collection must be an uuid'); ctx.assertPresent(title, 'title is required'); ctx.assertPresent(text, 'text is required'); + if (parentDocument) + ctx.assertUuid(parentDocument, 'parentDocument must be an uuid'); + if (index) ctx.assertPositiveInteger(index, 'index must be an integer (>=0)'); const user = ctx.state.user; const ownerCollection = await Collection.findOne({ @@ -111,45 +114,40 @@ router.post('documents.create', auth(), async ctx => { if (!ownerCollection) throw httpErrors.BadRequest(); - const document = await (() => { - return new Promise(resolve => { - lock(ownerCollection.id, 10000, async done => { - // FIXME: should we validate the existance of parentDocument? - let parentDocumentObj = {}; - if (parentDocument && ownerCollection.type === 'atlas') { - parentDocumentObj = await Document.findOne({ - where: { - id: parentDocument, - atlasId: ownerCollection.id, - }, - }); - } - - const newDocument = await Document.create({ - parentDocumentId: parentDocumentObj.id, - atlasId: ownerCollection.id, - teamId: user.teamId, - userId: user.id, - lastModifiedById: user.id, - createdById: user.id, - title, - text, - }); - - // TODO: Move to afterSave hook if possible with imports - if (parentDocument && ownerCollection.type === 'atlas') { - await ownerCollection.reload(); - ownerCollection.addNodeToNavigationTree(newDocument); - await ownerCollection.save(); - } - - done(resolve(newDocument)); - }); + // FIXME: should we validate the existance of parentDocument? + let parentDocumentObj = {}; + if (parentDocument && ownerCollection.type === 'atlas') { + parentDocumentObj = await Document.findOne({ + where: { + id: parentDocument, + atlasId: ownerCollection.id, + }, }); - })(); + } + + const newDocument = await Document.create({ + parentDocumentId: parentDocumentObj.id, + atlasId: ownerCollection.id, + teamId: user.teamId, + userId: user.id, + lastModifiedById: user.id, + createdById: user.id, + title, + text, + }); + + // TODO: Move to afterSave hook if possible with imports + if (parentDocument && ownerCollection.type === 'atlas') { + ownerCollection.addDocument( + newDocument, + newDocument.parentDocumentId, + index || -1 + ); + await ownerCollection.save(); + } ctx.body = { - data: await presentDocument(ctx, document, { + data: await presentDocument(ctx, newDocument, { includeCollection: true, includeCollaborators: true, }), @@ -174,11 +172,9 @@ router.post('documents.update', auth(), async ctx => { document.lastModifiedById = user.id; await document.save(); - // Update - // TODO: Add locking const collection = await Collection.findById(document.atlasId); if (collection.type === 'atlas') { - await collection.updateNavigationTree(); + await collection.updateDocument(document); } ctx.body = { @@ -200,7 +196,6 @@ router.post('documents.delete', auth(), async ctx => { if (!document || document.teamId !== user.teamId) throw httpErrors.BadRequest(); - // TODO: Add locking if (collection.type === 'atlas') { // Don't allow deletion of root docs if (!document.parentDocumentId) { diff --git a/server/api/middlewares/validation.js b/server/api/middlewares/validation.js index a99d6a597..4078453f1 100644 --- a/server/api/middlewares/validation.js +++ b/server/api/middlewares/validation.js @@ -1,3 +1,4 @@ +// @flow import apiError from '../../errors'; import validator from 'validator'; @@ -9,18 +10,24 @@ export default function validation() { } }; - ctx.assertEmail = function assertEmail(value, message) { + ctx.assertEmail = (value, message) => { if (!validator.isEmail(value)) { throw apiError(400, 'validation_error', message); } }; - ctx.assertUuid = function assertUuid(value, message) { + ctx.assertUuid = (value, message) => { if (!validator.isUUID(value)) { throw apiError(400, 'validation_error', message); } }; + ctx.assertPositiveInteger = (value, message) => { + if (!validator.isInt(value, { min: 0 })) { + throw apiError(400, 'validation_error', message); + } + }; + return next(); }; } diff --git a/server/migrations/20170603185012-add-collection-documents-migration.js b/server/migrations/20170603185012-add-collection-documentStructure-migration.js similarity index 61% rename from server/migrations/20170603185012-add-collection-documents-migration.js rename to server/migrations/20170603185012-add-collection-documentStructure-migration.js index 6529706f4..aa92416d2 100644 --- a/server/migrations/20170603185012-add-collection-documents-migration.js +++ b/server/migrations/20170603185012-add-collection-documentStructure-migration.js @@ -1,14 +1,14 @@ module.exports = { up: (queryInterface, Sequelize) => { queryInterface.renameTable('atlases', 'collections'); - queryInterface.addColumn('collections', 'documents', { - type: Sequelize.ARRAY(Sequelize.JSONB), + queryInterface.addColumn('collections', 'documentStructure', { + type: Sequelize.JSONB, allowNull: true, }); }, down: (queryInterface, _Sequelize) => { queryInterface.renameTable('collections', 'atlases'); - queryInterface.removeColumn('atlases', 'documents'); + queryInterface.removeColumn('atlases', 'documentStructure'); }, }; diff --git a/server/models/Collection.js b/server/models/Collection.js index b750b1fbe..bd19d14b7 100644 --- a/server/models/Collection.js +++ b/server/models/Collection.js @@ -28,7 +28,7 @@ const Collection = sequelize.define( /* type: atlas */ navigationTree: DataTypes.JSONB, // legacy - documents: DataTypes.ARRAY(DataTypes.JSONB), + documentStructure: DataTypes.JSONB, }, { tableName: 'collections', @@ -40,7 +40,7 @@ const Collection = sequelize.define( afterCreate: async collection => { if (collection.type !== 'atlas') return; - await Document.create({ + const document = await Document.create({ parentDocumentId: null, atlasId: collection.id, teamId: collection.teamId, @@ -50,7 +50,12 @@ const Collection = sequelize.define( title: 'Introduction', text: '# Introduction\n\nLets get started...', }); - await collection.buildStructure(); + collection.documentStructure = [ + { + ...document.toJSON(), + children: [], + }, + ]; await collection.save(); }, }, @@ -60,157 +65,79 @@ const Collection = sequelize.define( // return `/${slugifiedName}-c${this.urlId}`; return `/collections/${this.id}`; }, - async buildStructure() { - if (this.navigationTree) return this.navigationTree; - const getNodeForDocument = async document => { - const children = await Document.findAll({ - where: { - parentDocumentId: document.id, - atlasId: this.id, - }, + async getDocumentsStructure() { + // Lazy fill this.documentStructure + if (!this.documentStructure) { + this.documentStructure = this.navigationTree.children; + + // Remove parent references from all root documents + await this.navigationTree.children.forEach(async ({ id }) => { + const document = await Document.findById(id); + document.parentDocumentId = null; + await document.save(); }); - const childNodes = []; - await Promise.all( - children.map(async child => { - return childNodes.push(await getNodeForDocument(child)); - }) - ); + // Remove root document + const rootDocument = await Document.findById(this.navigationTree.id); + await rootDocument.destroy(); - return { - title: document.title, - id: document.id, - url: document.getUrl(), - children: childNodes, - }; - }; - - const rootDocument = await Document.findOne({ - where: { - parentDocumentId: null, - atlasId: this.id, - }, - }); - - this.navigationTree = await getNodeForDocument(rootDocument); - return this.navigationTree; - }, - async updateNavigationTree(tree = this.navigationTree) { - const nodeIds = []; - nodeIds.push(tree.id); - - const rootDocument = await Document.findOne({ - where: { - id: tree.id, - atlasId: this.id, - }, - }); - if (!rootDocument) throw new Error(); - - const newTree = { - id: tree.id, - title: rootDocument.title, - url: rootDocument.getUrl(), - children: [], - }; - - const getIdsForChildren = async children => { - const childNodes = []; - for (const child of children) { - const childDocument = await Document.findOne({ - where: { - id: child.id, - atlasId: this.id, - }, - }); - if (childDocument) { - childNodes.push({ - id: childDocument.id, - title: childDocument.title, - url: childDocument.getUrl(), - children: await getIdsForChildren(child.children), - }); - nodeIds.push(child.id); - } - } - return childNodes; - }; - newTree.children = await getIdsForChildren(tree.children); - - const documents = await Document.findAll({ - attributes: ['id'], - where: { - atlasId: this.id, - }, - }); - const documentIds = documents.map(doc => doc.id); - - if (!_.isEqual(nodeIds.sort(), documentIds.sort())) { - throw new Error('Invalid navigation tree'); + await this.save(); } - this.navigationTree = newTree; - await this.save(); - - return newTree; + return this.documentStructure; }, - async addNodeToNavigationTree(document) { - const newNode = { - id: document.id, - title: document.title, - url: document.getUrl(), - children: [], - }; - const insertNode = node => { - if (document.parentDocumentId === node.id) { - node.children.push(newNode); - } else { - node.children = node.children.map(childNode => { - return insertNode(childNode); - }); - } - - return node; - }; - - this.navigationTree = insertNode(this.navigationTree); - return this.navigationTree; - }, - async deleteDocument(document) { - const deleteNodeAndDocument = async ( - node, - documentId, - shouldDelete = false - ) => { - // Delete node if id matches - if (document.id === node.id) shouldDelete = true; - - const newChildren = []; - node.children.forEach(async childNode => { - const child = await deleteNodeAndDocument( - childNode, - documentId, - shouldDelete - ); - if (child) newChildren.push(child); + async addDocument(document, parentDocumentId, index) { + if (!parentDocumentId) { + this.documentStructure.splice(index, 0, document.toJSON()); + } else { + this.documentStructure = this.documentStructure.forEach(doc => { + if (parentDocumentId === document) { + return doc.children.splice(index, 0, document.toJSON()); + } }); - node.children = newChildren; + } - if (shouldDelete) { - const doc = await Document.findById(node.id); - await doc.destroy(); - } - - return shouldDelete ? null : node; - }; - - this.navigationTree = await deleteNodeAndDocument( - this.navigationTree, - document.id - ); + return this.documentStructure; }, + + async updateDocument(document) { + // Update document info in this.documents + }, + // async deleteDocument(document) { + // const deleteNodeAndDocument = async ( + // node, + // documentId, + // shouldDelete = false + // ) => { + // // Delete node if id matches + // if (document.id === node.id) shouldDelete = true; + + // const newChildren = []; + // node.children.forEach(async childNode => { + // const child = await deleteNodeAndDocument( + // childNode, + // documentId, + // shouldDelete + // ); + // if (child) newChildren.push(child); + // }); + // node.children = newChildren; + + // if (shouldDelete) { + // const doc = await Document.findById(node.id); + // await doc.destroy(); + // } + + // return shouldDelete ? null : node; + // }; + + // this.navigationTree = await deleteNodeAndDocument( + // this.navigationTree, + // document.id + // ); + // }, }, } ); diff --git a/server/models/Document.js b/server/models/Document.js index 4e6e165e7..c041eaa82 100644 --- a/server/models/Document.js +++ b/server/models/Document.js @@ -1,3 +1,4 @@ +// @flow import slug from 'slug'; import _ from 'lodash'; import randomstring from 'randomstring'; @@ -98,6 +99,13 @@ const Document = sequelize.define( const slugifiedTitle = slugify(this.title); return `/d/${slugifiedTitle}-${this.urlId}`; }, + toJSON() { + return { + id: this.id, + title: this.title, + url: this.getUrl(), + }; + }, }, } ); diff --git a/server/presenters.js b/server/presenters.js index 54846cf2f..d70e5fb99 100644 --- a/server/presenters.js +++ b/server/presenters.js @@ -92,8 +92,10 @@ export async function presentCollection( updatedAt: collection.updatedAt, }; - if (collection.type === 'atlas') + if (collection.type === 'atlas') { data.navigationTree = collection.navigationTree; + data.documents = await collection.getDocumentsStructure(); + } if (includeRecentDocuments) { const documents = await Document.findAll({