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/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/DocumentStore.js b/frontend/scenes/Document/DocumentStore.js index 9a5aa2da2..e8835ba6c 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); } diff --git a/frontend/scenes/Document/components/Menu.js b/frontend/scenes/Document/components/Menu.js index 38e493d8c..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 = () => { @@ -55,24 +55,29 @@ 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; + 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} + + ); + } + return null; } } 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, }; diff --git a/package.json b/package.json index 3baea1213..5e5470c0d 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", 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/api/documents.js b/server/api/documents.js index a8fa7982b..13857d8b1 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,47 +114,36 @@ 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)); - }); + 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, + }); + + if (ownerCollection.type === 'atlas') { + await ownerCollection.addDocumentToStructure(newDocument, index); + } ctx.body = { - data: await presentDocument(ctx, document, { + data: await presentDocument(ctx, newDocument, { includeCollection: true, includeCollaborators: true, + collection: ownerCollection, }), }; }); @@ -159,32 +151,72 @@ 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(); - // Update - // TODO: Add locking const collection = await Collection.findById(document.atlasId); if (collection.type === 'atlas') { - await collection.updateNavigationTree(); + await collection.updateDocument(document); } ctx.body = { data: await presentDocument(ctx, document, { includeCollection: true, includeCollaborators: true, + collection: collection, + }), + }; +}); + +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, }), }; }); @@ -200,17 +232,17 @@ 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) { - 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/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-documentStructure-migration.js b/server/migrations/20170603185012-add-collection-documentStructure-migration.js new file mode 100644 index 000000000..aa92416d2 --- /dev/null +++ b/server/migrations/20170603185012-add-collection-documentStructure-migration.js @@ -0,0 +1,14 @@ +module.exports = { + up: (queryInterface, Sequelize) => { + queryInterface.renameTable('atlases', 'collections'); + queryInterface.addColumn('collections', 'documentStructure', { + type: Sequelize.JSONB, + allowNull: true, + }); + }, + + down: (queryInterface, _Sequelize) => { + queryInterface.renameTable('collections', 'atlases'); + queryInterface.removeColumn('atlases', 'documentStructure'); + }, +}; diff --git a/server/models/Collection.js b/server/models/Collection.js index 01f8cf662..187c70364 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 + documentStructure: DataTypes.JSONB, }, { - tableName: 'atlases', + tableName: 'collections', paranoid: true, hooks: { beforeValidate: collection => { @@ -38,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, @@ -48,7 +50,7 @@ const Collection = sequelize.define( title: 'Introduction', text: '# Introduction\n\nLets get started...', }); - await collection.buildStructure(); + collection.documentStructure = [document.toJSON()]; await collection.save(); }, }, @@ -58,156 +60,114 @@ 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); + async addDocumentToStructure(document, index) { + if (!this.documentStructure) return; + + 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 (document.parentDocumentId === childDocument.id) { + childDocument.children.splice( + index || childDocument.children.length, + 0, + document.toJSON() + ); + } + return childDocument; + }); + } + + await this.save(); + return this; + }, + + async updateDocument(document) { + if (!this.documentStructure) return; + + const updateChildren = (children, document) => { + const id = document.id; + + if (_.find(children, { id })) { + children = children.map(childDocument => { + if (childDocument.id === id) { + childDocument = { + ...document.toJSON(), + children: childDocument.children, + }; + } + return childDocument; + }); } else { - node.children = node.children.map(childNode => { - return insertNode(childNode); + children = children.map(childDocument => { + return updateChildren(childDocument.children, id); }); } - - return node; + return children; }; - this.navigationTree = insertNode(this.navigationTree); - return this.navigationTree; + this.documentStructure = updateChildren( + this.documentStructure, + document + ); + + await 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; + if (!this.documentStructure) return; - 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(); + const deleteFromChildren = (children, id) => { + if (_.find(children, { id })) { + _.remove(children, { id }); + } else { + children = children.map(childDocument => { + return { + ...childDocument, + children: deleteFromChildren(childDocument.children, id), + }; + }); } - - return shouldDelete ? null : node; + return children; }; - this.navigationTree = await deleteNodeAndDocument( - this.navigationTree, + this.documentStructure = deleteFromChildren( + this.documentStructure, document.id ); + + await this.save(); + return this; }, }, } diff --git a/server/models/Document.js b/server/models/Document.js index 4e6e165e7..9e70cef35 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,16 @@ const Document = sequelize.define( const slugifiedTitle = slugify(this.title); 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 54846cf2f..75d0b0e75 100644 --- a/server/presenters.js +++ b/server/presenters.js @@ -40,12 +40,14 @@ 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); + const collection = + options.collection || + (await Collection.findOne({ + where: { + id: document.atlasId, + }, + })); + return presentCollection(ctx, collection); }); } @@ -92,8 +94,9 @@ export async function presentCollection( updatedAt: collection.updatedAt, }; - if (collection.type === 'atlas') - data.navigationTree = collection.navigationTree; + if (collection.type === 'atlas') { + data.documents = await collection.getDocumentsStructure(); + } if (includeRecentDocuments) { const documents = await Document.findAll({