From 524bfba44332819a46c5079ef3b2e6e992529049 Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Sat, 3 Jun 2017 12:01:29 -0700 Subject: [PATCH 01/11] Easier to remember sql migration commands --- package.json | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index a108a8c8f..817a0373f 100644 --- a/package.json +++ b/package.json @@ -13,8 +13,9 @@ "lint:js": "eslint frontend", "lint:flow": "flow check", "deploy": "git push heroku master", - "heroku-postbuild": "npm run build && npm run sequelize db:migrate", - "sequelize": "./node_modules/.bin/sequelize", + "heroku-postbuild": "npm run build && npm run sequelize:migrate", + "sequelize:create-migration": "sequelize migration:create", + "sequelize:migrate": "sequelize db:migrate", "test": "npm run test:frontend && npm run test:server", "test:frontend": "jest", "test:server": "jest --config=server/.jest-config --runInBand", From 9631e58e6592b4278f24264ef025c3e5a84ab5db Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Sat, 3 Jun 2017 12:04:13 -0700 Subject: [PATCH 02/11] Renamed collections table and added a new column for documents --- ...603185012-add-collection-documents-migration.js | 14 ++++++++++++++ server/models/Collection.js | 8 +++++--- 2 files changed, 19 insertions(+), 3 deletions(-) create mode 100644 server/migrations/20170603185012-add-collection-documents-migration.js diff --git a/server/migrations/20170603185012-add-collection-documents-migration.js b/server/migrations/20170603185012-add-collection-documents-migration.js new file mode 100644 index 000000000..6529706f4 --- /dev/null +++ b/server/migrations/20170603185012-add-collection-documents-migration.js @@ -0,0 +1,14 @@ +module.exports = { + up: (queryInterface, Sequelize) => { + queryInterface.renameTable('atlases', 'collections'); + queryInterface.addColumn('collections', 'documents', { + type: Sequelize.ARRAY(Sequelize.JSONB), + allowNull: true, + }); + }, + + down: (queryInterface, _Sequelize) => { + queryInterface.renameTable('collections', 'atlases'); + queryInterface.removeColumn('atlases', 'documents'); + }, +}; diff --git a/server/models/Collection.js b/server/models/Collection.js index 01f8cf662..b750b1fbe 100644 --- a/server/models/Collection.js +++ b/server/models/Collection.js @@ -1,3 +1,4 @@ +// @flow import slug from 'slug'; import randomstring from 'randomstring'; import { DataTypes, sequelize } from '../sequelize'; @@ -9,7 +10,7 @@ slug.defaults.mode = 'rfc3986'; const allowedCollectionTypes = [['atlas', 'journal']]; const Collection = sequelize.define( - 'atlas', + 'collection', { id: { type: DataTypes.UUID, @@ -26,10 +27,11 @@ const Collection = sequelize.define( creatorId: DataTypes.UUID, /* type: atlas */ - navigationTree: DataTypes.JSONB, + navigationTree: DataTypes.JSONB, // legacy + documents: DataTypes.ARRAY(DataTypes.JSONB), }, { - tableName: 'atlases', + tableName: 'collections', paranoid: true, hooks: { beforeValidate: collection => { From c229369efd440f2565ae984d0a399c236e34afb8 Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Sun, 4 Jun 2017 14:40:27 -0700 Subject: [PATCH 03/11] 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({ From a4dca58ae7ef4d9d35624d0cae1e741608026b16 Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Sun, 4 Jun 2017 22:12:36 -0700 Subject: [PATCH 04/11] Fixed Collection#updateDocument --- server/api/documents.js | 11 ++-- server/models/Collection.js | 109 ++++++++++++++++++++++-------------- server/models/Document.js | 3 + server/presenters.js | 19 ++++--- 4 files changed, 85 insertions(+), 57 deletions(-) diff --git a/server/api/documents.js b/server/api/documents.js index adcfaf153..41540d1ad 100644 --- a/server/api/documents.js +++ b/server/api/documents.js @@ -157,18 +157,16 @@ router.post('documents.create', auth(), async ctx => { router.post('documents.update', auth(), async ctx => { const { id, title, text } = ctx.body; ctx.assertPresent(id, 'id is required'); - ctx.assertPresent(title, 'title is required'); - ctx.assertPresent(text, 'text is required'); + ctx.assertPresent(title || text, 'title or text is required'); const user = ctx.state.user; const document = await getDocumentForId(id); - if (!document || document.teamId !== user.teamId) - throw httpErrors.BadRequest(); + if (!document || document.teamId !== user.teamId) throw httpErrors.NotFound(); // Update document - document.title = title; - document.text = text; + if (title) document.title = title; + if (text) document.text = text; document.lastModifiedById = user.id; await document.save(); @@ -181,6 +179,7 @@ router.post('documents.update', auth(), async ctx => { data: await presentDocument(ctx, document, { includeCollection: true, includeCollaborators: true, + collection: collection, }), }; }); diff --git a/server/models/Collection.js b/server/models/Collection.js index bd19d14b7..8d69f88c3 100644 --- a/server/models/Collection.js +++ b/server/models/Collection.js @@ -50,12 +50,7 @@ const Collection = sequelize.define( title: 'Introduction', text: '# Introduction\n\nLets get started...', }); - collection.documentStructure = [ - { - ...document.toJSON(), - children: [], - }, - ]; + collection.documentStructure = [document.toJSON()]; await collection.save(); }, }, @@ -88,56 +83,86 @@ const Collection = sequelize.define( return this.documentStructure; }, - async addDocument(document, parentDocumentId, index) { + async addDocument(document, parentDocumentId, index = -1) { + if (!this.documentStructure) return; + 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()); + this.documentStructure = this.documentStructure.map(childDocument => { + if (parentDocumentId === childDocument.id) { + childDocument.children = childDocument.children.splice( + index, + 0, + document.toJSON() + ); } + return childDocument; }); } - return this.documentStructure; + return this; }, async updateDocument(document) { - // Update document info in this.documents + // if (!this.documentStructure) return; + + const updateChildren = (children, document) => { + const id = document.id; + console.log(id); + if (_.find(children, { id })) { + console.log(1); + children = children.map(childDocument => { + console.log( + childDocument.id, + childDocument.title, + childDocument.id === id + ); + if (childDocument.id === id) { + childDocument = { + ...document.toJSON(), + children: childDocument.children, + }; + } + return childDocument; + }); + } else { + console.log(2); + children = children.map(childDocument => { + return updateChildren(childDocument.children, id); + }); + } + return children; + }; + + this.documentStructure = updateChildren( + this.documentStructure, + document + ); + this.save(); + return this; }, - // 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; + async deleteDocument(document) { + if (!this.documentStructure) return; - // if (shouldDelete) { - // const doc = await Document.findById(node.id); - // await doc.destroy(); - // } + const deleteFromChildren = (children, id) => { + if (_.find(children, { id })) { + _.remove(children, { id }); + } else { + children = children.map(childDocument => { + return deleteFromChildren(childDocument.children, id); + }); + } + return children; + }; - // return shouldDelete ? null : node; - // }; - - // this.navigationTree = await deleteNodeAndDocument( - // this.navigationTree, - // document.id - // ); - // }, + this.documentStructure = deleteFromChildren( + this.documentStructure, + document.id + ); + return this; + }, }, } ); diff --git a/server/models/Document.js b/server/models/Document.js index c041eaa82..9e70cef35 100644 --- a/server/models/Document.js +++ b/server/models/Document.js @@ -100,10 +100,13 @@ const Document = sequelize.define( return `/d/${slugifiedTitle}-${this.urlId}`; }, toJSON() { + // Warning: only use for new documents as order of children is + // handled in the collection's documentStructure return { id: this.id, title: this.title, url: this.getUrl(), + children: [], }; }, }, diff --git a/server/presenters.js b/server/presenters.js index d70e5fb99..3e95840e5 100644 --- a/server/presenters.js +++ b/server/presenters.js @@ -39,14 +39,16 @@ export async function presentDocument(ctx, document, options) { }; if (options.includeCollection) { - data.collection = await ctx.cache.get(document.atlasId, async () => { - const collection = await Collection.findOne({ - where: { - id: document.atlasId, - }, - }); - return await presentCollection(ctx, collection); - }); + data.collection = + options.collection || + (await ctx.cache.get(document.atlasId, async () => { + const collection = await Collection.findOne({ + where: { + id: document.atlasId, + }, + }); + return await presentCollection(ctx, collection); + })); } if (options.includeCollaborators) { @@ -93,7 +95,6 @@ export async function presentCollection( }; if (collection.type === 'atlas') { - data.navigationTree = collection.navigationTree; data.documents = await collection.getDocumentsStructure(); } From 1be0ff81052300e1929a7e415e2cb613fcd47469 Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Mon, 5 Jun 2017 00:36:50 -0700 Subject: [PATCH 05/11] Added deletion of documents --- server/api/documents.js | 18 +++++++----------- server/models/Collection.js | 28 ++++++++++++++++++++-------- server/presenters.js | 14 +++++++------- 3 files changed, 34 insertions(+), 26 deletions(-) diff --git a/server/api/documents.js b/server/api/documents.js index 41540d1ad..e1308595e 100644 --- a/server/api/documents.js +++ b/server/api/documents.js @@ -136,20 +136,15 @@ router.post('documents.create', auth(), async ctx => { 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(); + if (ownerCollection.type === 'atlas') { + await ownerCollection.addDocumentToStructure(newDocument, index); } ctx.body = { data: await presentDocument(ctx, newDocument, { includeCollection: true, includeCollaborators: true, + collection: ownerCollection, }), }; }); @@ -197,14 +192,15 @@ router.post('documents.delete', auth(), async ctx => { if (collection.type === 'atlas') { // Don't allow deletion of root docs - if (!document.parentDocumentId) { - throw httpErrors.BadRequest("Unable to delete atlas's root document"); + if (collection.documentStructure.length === 1) { + throw httpErrors.BadRequest( + "Unable to delete collection's only document" + ); } // Delete all chilren try { await collection.deleteDocument(document); - await collection.save(); } catch (e) { throw httpErrors.BadRequest('Error while deleting'); } diff --git a/server/models/Collection.js b/server/models/Collection.js index 8d69f88c3..c582cda37 100644 --- a/server/models/Collection.js +++ b/server/models/Collection.js @@ -83,16 +83,22 @@ const Collection = sequelize.define( return this.documentStructure; }, - async addDocument(document, parentDocumentId, index = -1) { + async addDocumentToStructure(document, index) { if (!this.documentStructure) return; - if (!parentDocumentId) { - this.documentStructure.splice(index, 0, document.toJSON()); + if (!document.parentDocumentId) { + this.documentStructure.splice( + index || this.documentStructure.length, + 0, + document.toJSON() + ); + // Sequelize doesn't seem to set the value with splice on JSONB field + this.documentStructure = this.documentStructure; } else { this.documentStructure = this.documentStructure.map(childDocument => { - if (parentDocumentId === childDocument.id) { - childDocument.children = childDocument.children.splice( - index, + if (document.parentDocumentId === childDocument.id) { + childDocument.children.splice( + index || childDocument.children.length, 0, document.toJSON() ); @@ -101,11 +107,12 @@ const Collection = sequelize.define( }); } + await this.save(); return this; }, async updateDocument(document) { - // if (!this.documentStructure) return; + if (!this.documentStructure) return; const updateChildren = (children, document) => { const id = document.id; @@ -151,7 +158,10 @@ const Collection = sequelize.define( _.remove(children, { id }); } else { children = children.map(childDocument => { - return deleteFromChildren(childDocument.children, id); + return { + ...childDocument, + children: deleteFromChildren(childDocument.children, id), + }; }); } return children; @@ -161,6 +171,8 @@ const Collection = sequelize.define( this.documentStructure, document.id ); + + await this.save(); return this; }, }, diff --git a/server/presenters.js b/server/presenters.js index 3e95840e5..10dc20807 100644 --- a/server/presenters.js +++ b/server/presenters.js @@ -39,16 +39,16 @@ export async function presentDocument(ctx, document, options) { }; if (options.includeCollection) { - data.collection = - options.collection || - (await ctx.cache.get(document.atlasId, async () => { - const collection = await Collection.findOne({ + data.collection = await ctx.cache.get(document.atlasId, async () => { + const collection = + options.collection || + (await Collection.findOne({ where: { id: document.atlasId, }, - }); - return await presentCollection(ctx, collection); - })); + })); + return await presentCollection(ctx, collection); + }); } if (options.includeCollaborators) { From 3528b2d0ef19a14bc697f5fa0178434e308992f8 Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Mon, 5 Jun 2017 01:00:29 -0700 Subject: [PATCH 06/11] Further cleanup --- server/api/collections.js | 21 --------------------- server/models/Collection.js | 12 +++--------- 2 files changed, 3 insertions(+), 30 deletions(-) diff --git a/server/api/collections.js b/server/api/collections.js index 6cc5f6ff3..b129c0852 100644 --- a/server/api/collections.js +++ b/server/api/collections.js @@ -74,25 +74,4 @@ router.post('collections.list', auth(), pagination(), async ctx => { }; }); -router.post('collections.updateNavigationTree', auth(), async ctx => { - const { id, tree } = ctx.body; - ctx.assertPresent(id, 'id is required'); - - const user = ctx.state.user; - const atlas = await Collection.findOne({ - where: { - id, - teamId: user.teamId, - }, - }); - - if (!atlas) throw httpErrors.NotFound(); - - await atlas.updateNavigationTree(tree); - - ctx.body = { - data: await presentCollection(ctx, atlas, true), - }; -}); - export default router; diff --git a/server/models/Collection.js b/server/models/Collection.js index c582cda37..187c70364 100644 --- a/server/models/Collection.js +++ b/server/models/Collection.js @@ -116,15 +116,9 @@ const Collection = sequelize.define( const updateChildren = (children, document) => { const id = document.id; - console.log(id); + if (_.find(children, { id })) { - console.log(1); children = children.map(childDocument => { - console.log( - childDocument.id, - childDocument.title, - childDocument.id === id - ); if (childDocument.id === id) { childDocument = { ...document.toJSON(), @@ -134,7 +128,6 @@ const Collection = sequelize.define( return childDocument; }); } else { - console.log(2); children = children.map(childDocument => { return updateChildren(childDocument.children, id); }); @@ -146,7 +139,8 @@ const Collection = sequelize.define( this.documentStructure, document ); - this.save(); + + await this.save(); return this; }, From 9027b55930d5e61e5db492ea6c3170e332712f4d Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Mon, 5 Jun 2017 23:12:47 -0700 Subject: [PATCH 07/11] Removed mentions of navigationTree --- frontend/models/Collection.js | 2 +- frontend/scenes/Collection/Collection.js | 2 +- frontend/scenes/Document/DocumentStore.js | 76 ++++++++------------- frontend/scenes/Document/components/Menu.js | 41 ++++++----- frontend/types/index.js | 1 - 5 files changed, 54 insertions(+), 68 deletions(-) diff --git a/frontend/models/Collection.js b/frontend/models/Collection.js index 47c4e9323..4b89f27c3 100644 --- a/frontend/models/Collection.js +++ b/frontend/models/Collection.js @@ -14,7 +14,7 @@ class Collection { id: string; name: string; type: 'atlas' | 'journal'; - navigationTree: NavigationNode; + documents: Array; updatedAt: string; url: string; diff --git a/frontend/scenes/Collection/Collection.js b/frontend/scenes/Collection/Collection.js index 72f2f039b..96210a58c 100644 --- a/frontend/scenes/Collection/Collection.js +++ b/frontend/scenes/Collection/Collection.js @@ -40,7 +40,7 @@ type State = { throw new Error('TODO code up non-atlas collections'); this.setState({ - redirectUrl: collection.navigationTree.url, + redirectUrl: collection.documents[0].url, }); }) .catch(() => { diff --git a/frontend/scenes/Document/DocumentStore.js b/frontend/scenes/Document/DocumentStore.js index 9a5aa2da2..e4d3dbe36 100644 --- a/frontend/scenes/Document/DocumentStore.js +++ b/frontend/scenes/Document/DocumentStore.js @@ -1,5 +1,5 @@ // @flow -import { observable, action, computed, toJS } from 'mobx'; +import { observable, action, computed } from 'mobx'; import get from 'lodash/get'; import invariant from 'invariant'; import { client } from 'utils/ApiClient'; @@ -49,42 +49,22 @@ class DocumentStore { return !!this.document && this.document.collection.type === 'atlas'; } - @computed get collectionTree(): ?Object { - if ( - this.document && - this.document.collection && - this.document.collection.type === 'atlas' - ) { - const tree = this.document.collection.navigationTree; - const collapseNodes = node => { - node.collapsed = this.collapsedNodes.includes(node.id); - node.children = node.children.map(childNode => { - return collapseNodes(childNode); - }); - - return node; - }; - - return collapseNodes(toJS(tree)); - } - } - @computed get pathToDocument(): Array { let path; - const traveler = (node, previousPath) => { - if (this.document && node.id === this.document.id) { - path = previousPath; - return; - } else { - node.children.forEach(childNode => { - const newPath = [...previousPath, node]; + const traveler = (nodes, previousPath) => { + nodes.forEach(childNode => { + const newPath = [...previousPath, childNode]; + if (childNode.id === this.document.id) { + path = previousPath; + return; + } else { return traveler(childNode, newPath); - }); - } + } + }); }; - if (this.document && this.collectionTree) { - traveler(this.collectionTree, []); + if (this.document && this.document.collection.documents) { + traveler(this.document.collection.documents, []); invariant(path, 'Path is not available for collection, abort'); return path.splice(1); } @@ -97,23 +77,23 @@ class DocumentStore { @action fetchDocument = async () => { this.isFetching = true; - try { - const res = await client.get( - '/documents.info', - { - id: this.documentId, - }, - { cache: true } - ); - invariant(res && res.data, 'Data should be available'); - if (this.newChildDocument) { - this.parentDocument = res.data; - } else { - this.document = res.data; - } - } catch (e) { - console.error('Something went wrong'); + // try { + const res = await client.get( + '/documents.info', + { + id: this.documentId, + }, + { cache: true } + ); + invariant(res && res.data, 'Data should be available'); + if (this.newChildDocument) { + this.parentDocument = res.data; + } else { + this.document = res.data; } + // } catch (e) { + // console.error('Something went wrong'); + // } this.isFetching = false; }; diff --git a/frontend/scenes/Document/components/Menu.js b/frontend/scenes/Document/components/Menu.js index 38e493d8c..6d050aa49 100644 --- a/frontend/scenes/Document/components/Menu.js +++ b/frontend/scenes/Document/components/Menu.js @@ -55,24 +55,31 @@ type Props = { render() { const document = get(this.props, 'document'); - const collection = get(document, 'collection.type') === 'atlas'; - const allowDelete = - collection && - document.id !== get(document, 'collection.navigationTree.id'); + if (document) { + const collection = document.collection; + debugger; + const allowDelete = + collection && + collection.type === 'atlas' && + collection.documents && + collection.documents.length > 1; - return ( - }> - {collection && -
- - New document - - New child -
} - Export - {allowDelete && Delete} - - ); + return ( + }> + {collection && +
+ + New document + + New child +
} + Export + {allowDelete && Delete} + + ); + } else { + return
; + } } } diff --git a/frontend/types/index.js b/frontend/types/index.js index 9b0ed127e..1e2d2d0e1 100644 --- a/frontend/types/index.js +++ b/frontend/types/index.js @@ -15,7 +15,6 @@ export type NavigationNode = { id: string, title: string, url: string, - collapsed: boolean, children: Array, }; From 3d02c49b055b680efaa61ae8ca7f4ffcd8ce3ce7 Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Mon, 5 Jun 2017 23:50:32 -0700 Subject: [PATCH 08/11] Added endpoint for moving documents --- server/api/documents.js | 43 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/server/api/documents.js b/server/api/documents.js index e1308595e..02b934d66 100644 --- a/server/api/documents.js +++ b/server/api/documents.js @@ -179,6 +179,49 @@ router.post('documents.update', auth(), async ctx => { }; }); +router.post('documents.move', auth(), async ctx => { + const { id, parentDocument, index } = ctx.body; + ctx.assertPresent(id, 'id 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 document = await getDocumentForId(id); + + if (!document || document.teamId !== user.teamId) throw httpErrors.NotFound(); + + // Set parent document + if (parentDocument) { + const parent = await getDocumentForId(parentDocument); + if (parent.atlasId !== document.atlasId) + throw httpErrors.BadRequest( + 'Invalid parentDocument (must be same collection)' + ); + } + + if (parentDocument === id) + throw httpErrors.BadRequest('Infinite loop detected and prevented!'); + + // If no parent document is provided, set it as null (move to root level) + 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); + } + + ctx.body = { + data: await presentDocument(ctx, document, { + includeCollection: true, + includeCollaborators: true, + collection: collection, + }), + }; +}); + router.post('documents.delete', auth(), async ctx => { const { id } = ctx.body; ctx.assertPresent(id, 'id is required'); From 1b25408272906ceb6d3b5c1347a1e2c3fab8a48b Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Mon, 5 Jun 2017 23:54:26 -0700 Subject: [PATCH 09/11] Added error handling back --- frontend/scenes/Document/DocumentStore.js | 32 +++++++++++------------ 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/frontend/scenes/Document/DocumentStore.js b/frontend/scenes/Document/DocumentStore.js index e4d3dbe36..e8835ba6c 100644 --- a/frontend/scenes/Document/DocumentStore.js +++ b/frontend/scenes/Document/DocumentStore.js @@ -77,23 +77,23 @@ class DocumentStore { @action fetchDocument = async () => { this.isFetching = true; - // try { - const res = await client.get( - '/documents.info', - { - id: this.documentId, - }, - { cache: true } - ); - invariant(res && res.data, 'Data should be available'); - if (this.newChildDocument) { - this.parentDocument = res.data; - } else { - this.document = res.data; + try { + const res = await client.get( + '/documents.info', + { + id: this.documentId, + }, + { cache: true } + ); + invariant(res && res.data, 'Data should be available'); + if (this.newChildDocument) { + this.parentDocument = res.data; + } else { + this.document = res.data; + } + } catch (e) { + console.error('Something went wrong'); } - // } catch (e) { - // console.error('Something went wrong'); - // } this.isFetching = false; }; From a80f58b0e2b32644d337265068f12223bbb6ed6c Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Tue, 6 Jun 2017 00:16:09 -0700 Subject: [PATCH 10/11] this should be validated --- server/api/documents.js | 1 - 1 file changed, 1 deletion(-) diff --git a/server/api/documents.js b/server/api/documents.js index 02b934d66..13857d8b1 100644 --- a/server/api/documents.js +++ b/server/api/documents.js @@ -114,7 +114,6 @@ router.post('documents.create', auth(), async ctx => { if (!ownerCollection) throw httpErrors.BadRequest(); - // FIXME: should we validate the existance of parentDocument? let parentDocumentObj = {}; if (parentDocument && ownerCollection.type === 'atlas') { parentDocumentObj = await Document.findOne({ From 9220978639555cee4109e299dd80ec5dc4691fdf Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Tue, 6 Jun 2017 22:34:29 -0700 Subject: [PATCH 11/11] changes per tom's comments --- frontend/scenes/Document/Document.js | 6 +----- frontend/scenes/Document/components/Menu.js | 10 ++++------ server/presenters.js | 2 +- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/frontend/scenes/Document/Document.js b/frontend/scenes/Document/Document.js index 77e14a780..c8b0631d8 100644 --- a/frontend/scenes/Document/Document.js +++ b/frontend/scenes/Document/Document.js @@ -117,11 +117,7 @@ type Props = { /> : Edit} - + ); diff --git a/frontend/scenes/Document/components/Menu.js b/frontend/scenes/Document/components/Menu.js index 6d050aa49..6e044b59b 100644 --- a/frontend/scenes/Document/components/Menu.js +++ b/frontend/scenes/Document/components/Menu.js @@ -11,7 +11,6 @@ import DocumentStore from '../DocumentStore'; type Props = { history: Object, document: DocumentType, - collectionTree: ?Object, store: DocumentStore, }; @@ -19,8 +18,9 @@ type Props = { props: Props; onCreateDocument = () => { - invariant(this.props.collectionTree, 'collectionTree is not available'); - this.props.history.push(`${this.props.collectionTree.url}/new`); + // Disabled until created a better API + // invariant(this.props.collectionTree, 'collectionTree is not available'); + // this.props.history.push(`${this.props.collectionTree.url}/new`); }; onCreateChild = () => { @@ -57,7 +57,6 @@ type Props = { const document = get(this.props, 'document'); if (document) { const collection = document.collection; - debugger; const allowDelete = collection && collection.type === 'atlas' && @@ -77,9 +76,8 @@ type Props = { {allowDelete && Delete} ); - } else { - return
; } + return null; } } diff --git a/server/presenters.js b/server/presenters.js index 10dc20807..75d0b0e75 100644 --- a/server/presenters.js +++ b/server/presenters.js @@ -47,7 +47,7 @@ export async function presentDocument(ctx, document, options) { id: document.atlasId, }, })); - return await presentCollection(ctx, collection); + return presentCollection(ctx, collection); }); }