From 67f2b3cce4f42e134247b3a1466c2a98a06be66f Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Sun, 6 May 2018 22:13:52 -0700 Subject: [PATCH 1/3] API endpoint accepts autosave --- app/models/Document.js | 10 +++++----- app/scenes/Document/Document.js | 19 +++++++++++++------ server/api/documents.js | 4 ++-- server/api/documents.test.js | 27 ++++++++++++++++++++++++++- server/models/Document.js | 22 +++++++++++----------- 5 files changed, 57 insertions(+), 25 deletions(-) diff --git a/app/models/Document.js b/app/models/Document.js index 50d08f82f..d751423b2 100644 --- a/app/models/Document.js +++ b/app/models/Document.js @@ -11,6 +11,8 @@ import type { User } from 'types'; import BaseModel from './BaseModel'; import Collection from './Collection'; +type SaveOptions = { publish: boolean, done: boolean, autosave: boolean }; + class Document extends BaseModel { isSaving: boolean = false; hasPendingChanges: boolean = false; @@ -168,7 +170,7 @@ class Document extends BaseModel { }; @action - save = async (publish: boolean = false, done: boolean = false) => { + save = async (options: SaveOptions) => { if (this.isSaving) return this; this.isSaving = true; @@ -180,8 +182,7 @@ class Document extends BaseModel { title: this.title, text: this.text, lastRevision: this.revision, - publish, - done, + ...options, }); } else { const data = { @@ -189,8 +190,7 @@ class Document extends BaseModel { collection: this.collection.id, title: this.title, text: this.text, - publish, - done, + ...options, }; if (this.parentDocument) { data.parentDocument = this.parentDocument; diff --git a/app/scenes/Document/Document.js b/app/scenes/Document/Document.js index e1e7e7cab..fef5f64b8 100644 --- a/app/scenes/Document/Document.js +++ b/app/scenes/Document/Document.js @@ -1,6 +1,7 @@ // @flow import * as React from 'react'; import get from 'lodash/get'; +import debounce from 'lodash/debounce'; import styled from 'styled-components'; import breakpoint from 'styled-components-breakpoint'; import { observable } from 'mobx'; @@ -32,6 +33,7 @@ import CenteredContent from 'components/CenteredContent'; import PageTitle from 'components/PageTitle'; import Search from 'scenes/Search'; +const AUTOSAVE_INTERVAL = 3000; const DISCARD_CHANGES = ` You have unsaved changes. Are you sure you want to discard them? @@ -154,20 +156,20 @@ class DocumentScene extends React.Component { handleCloseMoveModal = () => (this.moveModalOpen = false); handleOpenMoveModal = () => (this.moveModalOpen = true); - onSave = async (options: { redirect?: boolean, publish?: boolean } = {}) => { - const { redirect, publish } = options; - + onSave = async ( + options: { redirect?: boolean, publish?: boolean, autosave?: boolean } = {} + ) => { let document = this.document; if (!document || !document.allowSave) return; this.editCache = null; this.isSaving = true; - this.isPublishing = !!publish; - document = await document.save(publish, redirect); + this.isPublishing = !!options.publish; + document = await document.save(options); this.isSaving = false; this.isPublishing = false; - if (redirect) { + if (options.redirect) { this.props.history.push(document.url); this.props.ui.setActiveDocument(document); } else if (this.props.newDocument) { @@ -176,6 +178,10 @@ class DocumentScene extends React.Component { } }; + autosave = debounce(async () => { + this.onSave({ redirect: false, autosave: true }); + }, AUTOSAVE_INTERVAL); + onImageUploadStart = () => { this.isLoading = true; }; @@ -189,6 +195,7 @@ class DocumentScene extends React.Component { if (!document) return; if (document.text.trim() === text.trim()) return; document.updateData({ text }, true); + this.autosave(); }; onDiscard = () => { diff --git a/server/api/documents.js b/server/api/documents.js index e16e46681..f17e7bbb9 100644 --- a/server/api/documents.js +++ b/server/api/documents.js @@ -334,7 +334,7 @@ router.post('documents.create', auth(), async ctx => { }); router.post('documents.update', auth(), async ctx => { - const { id, title, text, publish, done, lastRevision } = ctx.body; + const { id, title, text, publish, autosave, done, lastRevision } = ctx.body; ctx.assertPresent(id, 'id is required'); ctx.assertPresent(title || text, 'title or text is required'); @@ -355,7 +355,7 @@ router.post('documents.update', auth(), async ctx => { if (publish) { await document.publish(); } else { - await document.save(); + await document.save({ autosave }); if (document.publishedAt && done) { events.add({ name: 'documents.update', model: document }); diff --git a/server/api/documents.test.js b/server/api/documents.test.js index 8c8eb45d2..20280ffbe 100644 --- a/server/api/documents.test.js +++ b/server/api/documents.test.js @@ -1,7 +1,7 @@ /* eslint-disable flowtype/require-valid-file-annotation */ import TestServer from 'fetch-test-server'; import app from '..'; -import { Document, View, Star } from '../models'; +import { Document, View, Star, Revision } from '../models'; import { flushdb, seed } from '../test/support'; import { buildUser } from '../test/factories'; @@ -484,6 +484,31 @@ describe('#documents.update', async () => { expect(body.data.collection.documents[0].title).toBe('Updated title'); }); + it('should not create new version when autosave=true', async () => { + const { user, document } = await seed(); + + const res = await server.post('/api/documents.update', { + body: { + token: user.getJwtToken(), + id: document.id, + title: 'Updated title', + text: 'Updated text', + lastRevision: document.revision, + autosave: true, + }, + }); + + const prevRevisionRecords = await Revision.count(); + const body = await res.json(); + + expect(res.status).toEqual(200); + expect(body.data.title).toBe('Updated title'); + expect(body.data.text).toBe('Updated text'); + + const revisionRecords = await Revision.count(); + expect(revisionRecords).toBe(prevRevisionRecords); + }); + it('should fallback to a default title', async () => { const { user, document } = await seed(); diff --git a/server/models/Document.js b/server/models/Document.js index e47510f87..7bcb61d98 100644 --- a/server/models/Document.js +++ b/server/models/Document.js @@ -25,8 +25,9 @@ const slugify = text => remove: /[.]/g, }); -const createRevision = doc => { - // Create revision of the current (latest) +const createRevision = (doc, options = {}) => { + if (options.autosave) return; + return Revision.create({ title: doc.title, text: doc.text, @@ -204,15 +205,14 @@ Document.searchForUser = async ( LIMIT :limit OFFSET :offset; `; - const results = await sequelize - .query(sql, { - replacements: { - query, - limit, - offset, - }, - model: Document, - }) + const results = await sequelize.query(sql, { + replacements: { + query, + limit, + offset, + }, + model: Document, + }); const ids = results.map(document => document.id); // Second query to get views for the data From ba0a7b7f4a71ea9c1133b4026228743ddc02ee20 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Mon, 7 May 2018 22:08:47 -0700 Subject: [PATCH 2/3] Update docs Remove extra API request after document.update Cleanup --- .../Sidebar/components/SidebarLink.js | 3 +- app/models/Collection.js | 30 ++++++++++++++----- app/models/Document.js | 8 ++--- app/scenes/Document/Document.js | 6 ++-- app/scenes/Document/components/Actions.js | 7 +++-- app/utils/importFile.js | 2 +- server/pages/Api.js | 20 ++++++++++++- 7 files changed, 56 insertions(+), 20 deletions(-) diff --git a/app/components/Sidebar/components/SidebarLink.js b/app/components/Sidebar/components/SidebarLink.js index 42958eb53..5e9219bc9 100644 --- a/app/components/Sidebar/components/SidebarLink.js +++ b/app/components/Sidebar/components/SidebarLink.js @@ -2,7 +2,7 @@ import * as React from 'react'; import { observable, action } from 'mobx'; import { observer } from 'mobx-react'; -import { NavLink } from 'react-router-dom'; +import { withRouter, NavLink } from 'react-router-dom'; import { CollapsedIcon } from 'outline-icons'; import { color, fontWeight } from 'shared/styles/constants'; import styled from 'styled-components'; @@ -59,6 +59,7 @@ type Props = { active?: boolean, }; +@withRouter @observer class SidebarLink extends React.Component { @observable expanded: boolean = false; diff --git a/app/models/Collection.js b/app/models/Collection.js index 6184c8e0b..409909d00 100644 --- a/app/models/Collection.js +++ b/app/models/Collection.js @@ -3,6 +3,7 @@ import { extendObservable, action, computed, runInAction } from 'mobx'; import invariant from 'invariant'; import BaseModel from 'models/BaseModel'; +import Document from 'models/Document'; import { client } from 'utils/ApiClient'; import stores from 'stores'; import ErrorsStore from 'stores/ErrorsStore'; @@ -20,7 +21,7 @@ class Collection extends BaseModel { name: string; color: string; type: 'atlas' | 'journal'; - documents: Array; + documents: NavigationNode[]; updatedAt: string; url: string; @@ -51,6 +52,21 @@ class Collection extends BaseModel { return results; } + @action + updateDocument(document: Document) { + const travelDocuments = (documentList, path) => + documentList.forEach(d => { + if (d.id === document.id) { + d.title = document.title; + d.url = document.url; + } else { + travelDocuments(d.children); + } + }); + + travelDocuments(this.documents); + } + /* Actions */ @action @@ -123,7 +139,7 @@ class Collection extends BaseModel { extendObservable(this, data); } - constructor(collection: Object = {}) { + constructor(collection: $Shape) { super(); this.updateData(collection); @@ -133,11 +149,11 @@ class Collection extends BaseModel { if (data.collectionId === this.id) this.fetch(); }); this.on( - 'collections.update', - (data: { id: string, collection: Collection }) => { - // FIXME: calling this.updateData won't update the - // UI. Some mobx issue - if (data.id === this.id) this.fetch(); + 'documents.update', + (data: { collectionId: string, document: Document }) => { + if (data.collectionId === this.id) { + this.updateDocument(data.document); + } } ); this.on('documents.move', (data: { collectionId: string }) => { diff --git a/app/models/Document.js b/app/models/Document.js index d751423b2..975b64587 100644 --- a/app/models/Document.js +++ b/app/models/Document.js @@ -11,7 +11,7 @@ import type { User } from 'types'; import BaseModel from './BaseModel'; import Collection from './Collection'; -type SaveOptions = { publish: boolean, done: boolean, autosave: boolean }; +type SaveOptions = { publish?: boolean, done?: boolean, autosave?: boolean }; class Document extends BaseModel { isSaving: boolean = false; @@ -204,9 +204,9 @@ class Document extends BaseModel { this.hasPendingChanges = false; }); - this.emit('collections.update', { - id: this.collection.id, - collection: this.collection, + this.emit('documents.update', { + document: this, + collectionId: this.collection.id, }); } catch (e) { this.errors.add('Document failed saving'); diff --git a/app/scenes/Document/Document.js b/app/scenes/Document/Document.js index fef5f64b8..3d952d64a 100644 --- a/app/scenes/Document/Document.js +++ b/app/scenes/Document/Document.js @@ -157,7 +157,7 @@ class DocumentScene extends React.Component { handleOpenMoveModal = () => (this.moveModalOpen = true); onSave = async ( - options: { redirect?: boolean, publish?: boolean, autosave?: boolean } = {} + options: { done?: boolean, publish?: boolean, autosave?: boolean } = {} ) => { let document = this.document; if (!document || !document.allowSave) return; @@ -169,7 +169,7 @@ class DocumentScene extends React.Component { this.isSaving = false; this.isPublishing = false; - if (options.redirect) { + if (options.done) { this.props.history.push(document.url); this.props.ui.setActiveDocument(document); } else if (this.props.newDocument) { @@ -179,7 +179,7 @@ class DocumentScene extends React.Component { }; autosave = debounce(async () => { - this.onSave({ redirect: false, autosave: true }); + this.onSave({ done: false, autosave: true }); }, AUTOSAVE_INTERVAL); onImageUploadStart = () => { diff --git a/app/scenes/Document/components/Actions.js b/app/scenes/Document/components/Actions.js index b72fa7488..96768a9af 100644 --- a/app/scenes/Document/components/Actions.js +++ b/app/scenes/Document/components/Actions.js @@ -20,8 +20,9 @@ type Props = { savingIsDisabled: boolean, onDiscard: () => *, onSave: ({ - redirect?: boolean, + done?: boolean, publish?: boolean, + autosave?: boolean, }) => *, history: Object, }; @@ -36,11 +37,11 @@ class DocumentActions extends React.Component { }; handleSave = () => { - this.props.onSave({ redirect: true }); + this.props.onSave({ done: true }); }; handlePublish = () => { - this.props.onSave({ redirect: true, publish: true }); + this.props.onSave({ done: true, publish: true }); }; render() { diff --git a/app/utils/importFile.js b/app/utils/importFile.js index ccb1340cb..17d2d8046 100644 --- a/app/utils/importFile.js +++ b/app/utils/importFile.js @@ -29,7 +29,7 @@ const importFile = async ({ if (documentId) data.parentDocument = documentId; let document = new Document(data); - document = await document.save(true); + document = await document.save({ publish: true }); documents.add(document); resolve(document); }; diff --git a/server/pages/Api.js b/server/pages/Api.js index 11fc571f0..dee26dd06 100644 --- a/server/pages/Api.js +++ b/server/pages/Api.js @@ -425,7 +425,25 @@ export default function Pricing() { id="publish" description={ - Pass true to publish a draft + Pass true to publish a draft. + + } + /> + + Pass true to signify an autosave. This skips + creating a revision. + + } + /> + + Pass true to signify the end of an editing + session. This will trigger documents.update hooks. } /> From 94e63b6171133f48ad80c6330e1048dcefc030ee Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Mon, 7 May 2018 22:53:13 -0700 Subject: [PATCH 3/3] Account for draft being published. Need to reload the collection still in this scenario --- app/models/Collection.js | 3 +++ app/models/Document.js | 11 ++++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/app/models/Collection.js b/app/models/Collection.js index 409909d00..5051ac69b 100644 --- a/app/models/Collection.js +++ b/app/models/Collection.js @@ -156,6 +156,9 @@ class Collection extends BaseModel { } } ); + this.on('documents.publish', (data: { collectionId: string }) => { + if (data.collectionId === this.id) this.fetch(); + }); this.on('documents.move', (data: { collectionId: string }) => { if (data.collectionId === this.id) this.fetch(); }); diff --git a/app/models/Document.js b/app/models/Document.js index 975b64587..aa317dc3c 100644 --- a/app/models/Document.js +++ b/app/models/Document.js @@ -172,6 +172,8 @@ class Document extends BaseModel { @action save = async (options: SaveOptions) => { if (this.isSaving) return this; + + const wasDraft = !this.publishedAt; this.isSaving = true; try { @@ -208,8 +210,15 @@ class Document extends BaseModel { document: this, collectionId: this.collection.id, }); + + if (wasDraft && this.publishedAt) { + this.emit('documents.publish', { + id: this.id, + collectionId: this.collection.id, + }); + } } catch (e) { - this.errors.add('Document failed saving'); + this.errors.add('Document failed to save'); } finally { this.isSaving = false; }