From e404955394676776a26e89320fa17659eed3f07e Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Mon, 18 Nov 2019 18:51:32 -0800 Subject: [PATCH] feat: Trash (#1082) * wip: trash * Enable restoration of deleted documents * update Trash icon * Add endpoint to trigger garbage collection * fix: account for drafts * fix: Archived documents should be deletable * fix: Missing delete cascade * bump: upgrade rich-markdown-editor --- app/components/Sidebar/Main.js | 19 ++++- .../Sidebar/components/SidebarLink.js | 1 + app/components/SocketProvider.js | 5 +- app/menus/DocumentMenu.js | 15 ++-- app/models/Document.js | 10 +++ app/routes.js | 2 + app/scenes/Document/Document.js | 21 +++++- app/scenes/DocumentDelete.js | 4 +- app/scenes/Trash.js | 38 ++++++++++ app/stores/DocumentsStore.js | 13 ++++ package.json | 4 +- server/api/documents.js | 62 +++++++++++++++- server/api/index.js | 2 + server/api/utils.js | 31 ++++++++ server/api/utils.test.js | 74 +++++++++++++++++++ .../20191118023010-cascade-delete.js | 22 ++++++ server/models/Document.js | 6 ++ server/models/Event.js | 1 + server/policies/document.js | 22 +++++- yarn.lock | 24 +++--- 20 files changed, 346 insertions(+), 30 deletions(-) create mode 100644 app/scenes/Trash.js create mode 100644 server/api/utils.js create mode 100644 server/api/utils.test.js create mode 100644 server/migrations/20191118023010-cascade-delete.js diff --git a/app/components/Sidebar/Main.js b/app/components/Sidebar/Main.js index 22020ceb..d77ea2e7 100644 --- a/app/components/Sidebar/Main.js +++ b/app/components/Sidebar/Main.js @@ -8,6 +8,7 @@ import { EditIcon, SearchIcon, StarredIcon, + TrashIcon, PlusIcon, } from 'outline-icons'; @@ -111,7 +112,10 @@ class MainSidebar extends React.Component { } active={ - documents.active ? !documents.active.publishedAt : undefined + documents.active + ? !documents.active.publishedAt && + !documents.active.isDeleted + : undefined } /> @@ -125,7 +129,18 @@ class MainSidebar extends React.Component { exact={false} label="Archive" active={ - documents.active ? documents.active.isArchived : undefined + documents.active + ? documents.active.isArchived && !documents.active.isDeleted + : undefined + } + /> + } + exact={false} + label="Trash" + active={ + documents.active ? documents.active.isDeleted : undefined } /> {can.invite && ( diff --git a/app/components/Sidebar/components/SidebarLink.js b/app/components/Sidebar/components/SidebarLink.js index cdaf4b7a..5877cd88 100644 --- a/app/components/Sidebar/components/SidebarLink.js +++ b/app/components/Sidebar/components/SidebarLink.js @@ -158,6 +158,7 @@ const Label = styled.div` position: relative; width: 100%; max-height: 4.4em; + line-height: 1.6; `; const Disclosure = styled(CollapsedIcon)` diff --git a/app/components/SocketProvider.js b/app/components/SocketProvider.js index 53a68122..6823357a 100644 --- a/app/components/SocketProvider.js +++ b/app/components/SocketProvider.js @@ -59,7 +59,10 @@ class SocketProvider extends React.Component { let document = documents.get(documentId) || {}; if (event.event === 'documents.delete') { - documents.remove(documentId); + const document = documents.get(documentId); + if (document) { + document.deletedAt = documentDescriptor.updatedAt; + } continue; } diff --git a/app/menus/DocumentMenu.js b/app/menus/DocumentMenu.js index 228ad64a..7cc473e5 100644 --- a/app/menus/DocumentMenu.js +++ b/app/menus/DocumentMenu.js @@ -132,6 +132,7 @@ class DocumentMenu extends React.Component { const can = policies.abilities(document.id); const canShareDocuments = can.share && auth.team && auth.team.sharing; + const canViewHistory = can.read && !can.restore; return ( { onOpen={onOpen} onClose={onClose} > - {can.unarchive && ( + {(can.unarchive || can.restore) && ( Restore @@ -176,11 +177,13 @@ class DocumentMenu extends React.Component { Share link… )} -
- {can.read && ( - - Document history - + {canViewHistory && ( + +
+ + Document history + + )} {can.update && ( { const res = await client.post('/shares.create', { documentId: this.id }); diff --git a/app/routes.js b/app/routes.js index cb192f7f..994c5085 100644 --- a/app/routes.js +++ b/app/routes.js @@ -6,6 +6,7 @@ import Dashboard from 'scenes/Dashboard'; import Starred from 'scenes/Starred'; import Drafts from 'scenes/Drafts'; import Archive from 'scenes/Archive'; +import Trash from 'scenes/Trash'; import Collection from 'scenes/Collection'; import KeyedDocument from 'scenes/Document/KeyedDocument'; import DocumentNew from 'scenes/DocumentNew'; @@ -49,6 +50,7 @@ export default function Routes() { + diff --git a/app/scenes/Document/Document.js b/app/scenes/Document/Document.js index fddeb555..e1d8ece7 100644 --- a/app/scenes/Document/Document.js +++ b/app/scenes/Document/Document.js @@ -400,10 +400,25 @@ class DocumentScene extends React.Component { /> )} - {document.archivedAt && ( + {document.archivedAt && + !document.deletedAt && ( + + Archived by {document.updatedBy.name}{' '} + + )} + {document.deletedAt && ( - Archived by {document.updatedBy.name}{' '} - )} {
Are you sure about that? Deleting the{' '} - {document.title} document is permanent, and will - delete all of its history, and any child documents. + {document.title} document will delete all of its + history, and any child documents. {!document.isDraft && !document.isArchived && ( diff --git a/app/scenes/Trash.js b/app/scenes/Trash.js new file mode 100644 index 00000000..931b3a5a --- /dev/null +++ b/app/scenes/Trash.js @@ -0,0 +1,38 @@ +// @flow +import * as React from 'react'; +import { observer, inject } from 'mobx-react'; + +import CenteredContent from 'components/CenteredContent'; +import Empty from 'components/Empty'; +import PageTitle from 'components/PageTitle'; +import Heading from 'components/Heading'; +import PaginatedDocumentList from 'components/PaginatedDocumentList'; +import Subheading from 'components/Subheading'; +import DocumentsStore from 'stores/DocumentsStore'; + +type Props = { + documents: DocumentsStore, +}; + +@observer +class Trash extends React.Component { + render() { + const { documents } = this.props; + + return ( + + + Trash + Documents} + empty={Trash is empty at the moment.} + showCollection + /> + + ); + } +} + +export default inject('documents')(Trash); diff --git a/app/stores/DocumentsStore.js b/app/stores/DocumentsStore.js index e02e8c52..5519be68 100644 --- a/app/stores/DocumentsStore.js +++ b/app/stores/DocumentsStore.js @@ -121,6 +121,14 @@ export default class DocumentsStore extends BaseStore { ); } + @computed + get deleted(): Document[] { + return filter( + orderBy(this.orderedData, 'deletedAt', 'desc'), + d => d.deletedAt + ); + } + @computed get starredAlphabetical(): Document[] { return naturalSort(this.starred, 'title'); @@ -189,6 +197,11 @@ export default class DocumentsStore extends BaseStore { return this.fetchNamedPage('archived', options); }; + @action + fetchDeleted = async (options: ?PaginationParams): Promise<*> => { + return this.fetchNamedPage('deleted', options); + }; + @action fetchRecentlyUpdated = async (options: ?PaginationParams): Promise<*> => { return this.fetchNamedPage('list', options); diff --git a/package.json b/package.json index d9adb726..ff666a21 100644 --- a/package.json +++ b/package.json @@ -118,7 +118,7 @@ "mobx-react": "^5.4.2", "natural-sort": "^1.0.0", "nodemailer": "^4.4.0", - "outline-icons": "^1.9.0", + "outline-icons": "^1.10.0", "oy-vey": "^0.10.0", "pg": "^6.1.5", "pg-hstore": "2.3.2", @@ -139,7 +139,7 @@ "react-router-dom": "^4.3.1", "react-waypoint": "^7.3.1", "redis": "^2.6.2", - "rich-markdown-editor": "^9.8.9", + "rich-markdown-editor": "^9.10.0", "sequelize": "^5.21.1", "sequelize-cli": "^5.5.0", "sequelize-encrypted": "^0.1.0", diff --git a/server/api/documents.js b/server/api/documents.js index 653a4479..05fe33db 100644 --- a/server/api/documents.js +++ b/server/api/documents.js @@ -187,6 +187,46 @@ router.post('documents.archived', auth(), pagination(), async ctx => { }; }); +router.post('documents.deleted', auth(), pagination(), async ctx => { + const { sort = 'deletedAt' } = ctx.body; + let direction = ctx.body.direction; + if (direction !== 'ASC') direction = 'DESC'; + + const user = ctx.state.user; + const collectionIds = await user.collectionIds(); + + const collectionScope = { method: ['withCollection', user.id] }; + const documents = await Document.scope(collectionScope).findAll({ + where: { + teamId: user.teamId, + collectionId: collectionIds, + deletedAt: { + [Op.ne]: null, + }, + }, + include: [ + { model: User, as: 'createdBy', paranoid: false }, + { model: User, as: 'updatedBy', paranoid: false }, + ], + paranoid: false, + order: [[sort, direction]], + offset: ctx.state.pagination.offset, + limit: ctx.state.pagination.limit, + }); + + const data = await Promise.all( + documents.map(document => presentDocument(document)) + ); + + const policies = presentPolicies(user, documents); + + ctx.body = { + pagination: ctx.state.pagination, + data, + policies, + }; +}); + router.post('documents.viewed', auth(), pagination(), async ctx => { let { sort = 'updatedAt', direction } = ctx.body; if (direction !== 'ASC') direction = 'DESC'; @@ -409,9 +449,27 @@ router.post('documents.restore', auth(), async ctx => { ctx.assertPresent(id, 'id is required'); const user = ctx.state.user; - const document = await Document.findByPk(id, { userId: user.id }); + const document = await Document.findByPk(id, { + userId: user.id, + paranoid: false, + }); - if (document.archivedAt) { + if (document.deletedAt) { + authorize(user, 'restore', document); + + // restore a previously deleted document + await document.unarchive(user.id); + + await Event.create({ + name: 'documents.restore', + documentId: document.id, + collectionId: document.collectionId, + teamId: document.teamId, + actorId: user.id, + data: { title: document.title }, + ip: ctx.request.ip, + }); + } else if (document.archivedAt) { authorize(user, 'unarchive', document); // restore a previously archived document diff --git a/server/api/index.js b/server/api/index.js index a396d35c..4fc45dfb 100644 --- a/server/api/index.js +++ b/server/api/index.js @@ -15,6 +15,7 @@ import shares from './shares'; import team from './team'; import integrations from './integrations'; import notificationSettings from './notificationSettings'; +import utils from './utils'; import { NotFoundError } from '../errors'; import errorHandling from './middlewares/errorHandling'; @@ -47,6 +48,7 @@ router.use('/', shares.routes()); router.use('/', team.routes()); router.use('/', integrations.routes()); router.use('/', notificationSettings.routes()); +router.use('/', utils.routes()); router.post('*', ctx => { ctx.throw(new NotFoundError('Endpoint not found')); }); diff --git a/server/api/utils.js b/server/api/utils.js new file mode 100644 index 00000000..d35e7b1d --- /dev/null +++ b/server/api/utils.js @@ -0,0 +1,31 @@ +// @flow +import Router from 'koa-router'; +import subDays from 'date-fns/sub_days'; +import { AuthenticationError } from '../errors'; +import { Document } from '../models'; +import { Op } from '../sequelize'; + +const router = new Router(); + +router.post('utils.gc', async ctx => { + const { token } = ctx.body; + + if (process.env.UTILS_SECRET !== token) { + throw new AuthenticationError('Invalid secret token'); + } + + await Document.scope('withUnpublished').destroy({ + where: { + deletedAt: { + [Op.lt]: subDays(new Date(), 30), + }, + }, + force: true, + }); + + ctx.body = { + success: true, + }; +}); + +export default router; diff --git a/server/api/utils.test.js b/server/api/utils.test.js new file mode 100644 index 00000000..38b43357 --- /dev/null +++ b/server/api/utils.test.js @@ -0,0 +1,74 @@ +/* eslint-disable flowtype/require-valid-file-annotation */ +import TestServer from 'fetch-test-server'; +import subDays from 'date-fns/sub_days'; +import app from '../app'; +import { Document } from '../models'; +import { sequelize } from '../sequelize'; +import { flushdb } from '../test/support'; +import { buildDocument } from '../test/factories'; + +const server = new TestServer(app.callback()); + +beforeEach(flushdb); +afterAll(server.close); + +describe('#utils.gc', async () => { + it('should destroy documents deleted more than 30 days ago', async () => { + const document = await buildDocument({ + publishedAt: new Date(), + }); + + await sequelize.query( + `UPDATE documents SET "deletedAt" = '${subDays( + new Date(), + 60 + ).toISOString()}' WHERE id = '${document.id}'` + ); + + const res = await server.post('/api/utils.gc', { + body: { + token: process.env.UTILS_SECRET, + }, + }); + const reloaded = await Document.scope().findOne({ + where: { + id: document.id, + }, + paranoid: false, + }); + expect(res.status).toEqual(200); + expect(reloaded).toBe(null); + }); + + it('should destroy draft documents deleted more than 30 days ago', async () => { + const document = await buildDocument({ + publishedAt: undefined, + }); + + await sequelize.query( + `UPDATE documents SET "deletedAt" = '${subDays( + new Date(), + 60 + ).toISOString()}' WHERE id = '${document.id}'` + ); + + const res = await server.post('/api/utils.gc', { + body: { + token: process.env.UTILS_SECRET, + }, + }); + const reloaded = await Document.scope().findOne({ + where: { + id: document.id, + }, + paranoid: false, + }); + expect(res.status).toEqual(200); + expect(reloaded).toBe(null); + }); + + it('should require authentication', async () => { + const res = await server.post('/api/utils.gc'); + expect(res.status).toEqual(401); + }); +}); diff --git a/server/migrations/20191118023010-cascade-delete.js b/server/migrations/20191118023010-cascade-delete.js new file mode 100644 index 00000000..3b686f80 --- /dev/null +++ b/server/migrations/20191118023010-cascade-delete.js @@ -0,0 +1,22 @@ +const tableName = 'revisions'; +const constraintName = 'revisions_documentId_fkey'; + +module.exports = { + up: async (queryInterface, Sequelize) => { + await queryInterface.sequelize.query(`alter table "${tableName}" drop constraint "${constraintName}"`) + await queryInterface.sequelize.query( + `alter table "${tableName}" + add constraint "${constraintName}" foreign key("documentId") references "documents" ("id") + on delete cascade` + ); + }, + + down: async (queryInterface, Sequelize) => { + await queryInterface.sequelize.query(`alter table "${tableName}" drop constraint "${constraintName}"`) + await queryInterface.sequelize.query( + `alter table "${tableName}"\ + add constraint "${constraintName}" foreign key("documentId") references "documents" ("id") + on delete no action` + ); + }, +}; \ No newline at end of file diff --git a/server/models/Document.js b/server/models/Document.js index 8760b897..c52752d1 100644 --- a/server/models/Document.js +++ b/server/models/Document.js @@ -147,9 +147,11 @@ Document.associate = models => { }); Document.hasMany(models.Backlink, { as: 'backlinks', + onDelete: 'cascade', }); Document.hasMany(models.Star, { as: 'starred', + onDelete: 'cascade', }); Document.hasMany(models.View, { as: 'views', @@ -514,6 +516,10 @@ Document.prototype.unarchive = async function(userId) { await collection.addDocumentToStructure(this); this.collection = collection; + if (this.deletedAt) { + await this.restore(); + } + this.archivedAt = null; this.lastModifiedById = userId; await this.save(); diff --git a/server/models/Event.js b/server/models/Event.js index 9211c760..ab8f10cb 100644 --- a/server/models/Event.js +++ b/server/models/Event.js @@ -56,6 +56,7 @@ Event.ACTIVITY_EVENTS = [ 'documents.pin', 'documents.unpin', 'documents.delete', + 'documents.restore', 'collections.create', 'collections.delete', ]; diff --git a/server/policies/document.js b/server/policies/document.js index d86797ba..18576b96 100644 --- a/server/policies/document.js +++ b/server/policies/document.js @@ -18,6 +18,7 @@ allow(User, ['read', 'download'], Document, (user, document) => { allow(User, ['share'], Document, (user, document) => { if (document.archivedAt) return false; + if (document.deletedAt) return false; // existance of collection option is not required here to account for share tokens if (document.collection && cannot(user, 'read', document.collection)) { @@ -29,6 +30,7 @@ allow(User, ['share'], Document, (user, document) => { allow(User, ['star', 'unstar'], Document, (user, document) => { if (document.archivedAt) return false; + if (document.deletedAt) return false; if (!document.publishedAt) return false; invariant( @@ -47,6 +49,7 @@ allow(User, 'update', Document, (user, document) => { ); if (cannot(user, 'update', document.collection)) return false; if (document.archivedAt) return false; + if (document.deletedAt) return false; return user.teamId === document.teamId; }); @@ -58,6 +61,7 @@ allow(User, ['move', 'pin', 'unpin'], Document, (user, document) => { ); if (cannot(user, 'update', document.collection)) return false; if (document.archivedAt) return false; + if (document.deletedAt) return false; if (!document.publishedAt) return false; return user.teamId === document.teamId; @@ -65,15 +69,26 @@ allow(User, ['move', 'pin', 'unpin'], Document, (user, document) => { allow(User, 'delete', Document, (user, document) => { // unpublished drafts can always be deleted - if (!document.publishedAt && user.teamId === document.teamId) { + if ( + !document.deletedAt && + !document.publishedAt && + user.teamId === document.teamId + ) { return true; } // allow deleting document without a collection - if (document.collection && cannot(user, 'update', document.collection)) + if (document.collection && cannot(user, 'update', document.collection)) { return false; - if (document.archivedAt) return false; + } + if (document.deletedAt) return false; + + return user.teamId === document.teamId; +}); + +allow(User, 'restore', Document, (user, document) => { + if (!document.deletedAt) return false; return user.teamId === document.teamId; }); @@ -86,6 +101,7 @@ allow(User, 'archive', Document, (user, document) => { if (!document.publishedAt) return false; if (document.archivedAt) return false; + if (document.deletedAt) return false; return user.teamId === document.teamId; }); diff --git a/yarn.lock b/yarn.lock index c6027e00..a167b6e1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7120,10 +7120,10 @@ osenv@^0.1.4: os-homedir "^1.0.0" os-tmpdir "^1.0.0" -outline-icons@^1.9.0: - version "1.9.0" - resolved "https://registry.yarnpkg.com/outline-icons/-/outline-icons-1.9.0.tgz#e17d998272209846aa3277ad7ed7063fc4dae984" - integrity sha512-Uzh1aP9Js+9ieOrvZpyPjFOaVBjpC+OFzF1pRi5jemYpH6kiA236i7itv0OdLX6KFDpybP6b6OWrCVDle5RXLQ== +outline-icons@^1.10.0: + version "1.10.0" + resolved "https://registry.yarnpkg.com/outline-icons/-/outline-icons-1.10.0.tgz#3c8e6957429e2b04c9d0fc72fe72e473813ce5bd" + integrity sha512-1o3SnjzawEIh+QkZ6GHxPckuV+Tk5m5R2tjGY0CtosF3YA7JbgQ2jQrZdQsrqLzLa1j07f1bTEbAjGdbnunLpg== oy-vey@^0.10.0: version "0.10.0" @@ -8440,10 +8440,10 @@ retry-as-promised@^3.2.0: dependencies: any-promise "^1.3.0" -rich-markdown-editor@^9.8.9: - version "9.8.9" - resolved "https://registry.yarnpkg.com/rich-markdown-editor/-/rich-markdown-editor-9.8.9.tgz#ef9ee4d884988eca4ebc415495827a64fb0d7815" - integrity sha512-7MX2Y4MX0v81GW5vtnwPAIF6h1IPI1YE1Ex0kYUuTb+ugoLt+kM/zDc9uPL+Ix7jyr1TljDzfgBbDuP+sR2JjQ== +rich-markdown-editor@^9.10.0: + version "9.10.0" + resolved "https://registry.yarnpkg.com/rich-markdown-editor/-/rich-markdown-editor-9.10.0.tgz#df59720ed969c1288d672d58f239664a64dd1465" + integrity sha512-9FGzjPExSYb/T0Rp5YEkz8qgEQkiwprAzlg4J6aj5acFZlaon2QZ4sNO/2feBaT3IBfAbnPNZEb+JeuZpL7s7w== dependencies: "@domoinc/slate-edit-table" "^0.22.2" "@tommoor/slate-edit-list" "0.19.0-0" @@ -8458,7 +8458,7 @@ rich-markdown-editor@^9.8.9: eslint-plugin-prettier "^2.6.0" golery-slate-prism "0.6.0-golery.2" lodash "^4.17.11" - outline-icons "^1.9.0" + outline-icons "^1.10.0" prismjs "^1.16.0" react-autosize-textarea "^6.0.0" react-keydown "^1.9.10" @@ -8467,6 +8467,7 @@ rich-markdown-editor@^9.8.9: slate "^0.45.0" slate-collapse-on-escape "^0.8.1" slate-drop-or-paste-images "^0.9.1" + slate-instant-replace "^0.1.13" slate-md-serializer "5.4.4" slate-paste-linkify "^0.7.0" slate-react "^0.21.20" @@ -8843,6 +8844,11 @@ slate-hotkeys@^0.2.9: is-hotkey "0.1.4" slate-dev-environment "^0.2.2" +slate-instant-replace@^0.1.13: + version "0.1.13" + resolved "https://registry.yarnpkg.com/slate-instant-replace/-/slate-instant-replace-0.1.13.tgz#03a2c908253c399a1ca5d6922a93867eb186c69b" + integrity sha512-jesj33+TUgrmcVKGBy6QgqORaAq55hSuFmBOv70iNjif0CHqK9CapWfHOLg8N8HWw+VAX0YpSaqy/lAQSSmwWg== + slate-md-serializer@5.4.4: version "5.4.4" resolved "https://registry.yarnpkg.com/slate-md-serializer/-/slate-md-serializer-5.4.4.tgz#b0b55f7ab1dc9ed2159c6f97852594a81a5b76e9"