From 18b033873610f35e6317084a2ffd3b4b5d1cb25a Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Wed, 28 Feb 2018 23:28:36 -0800 Subject: [PATCH] Pinned documents (#608) * Migrations and API for pinned documents * Documentation * Add pin icon * Fin. * v0.2.0 * Remove pin from DocumentPreview, add general menu Add Pinned documents header * Tidy * Fixed: Drafts appearing on collection home --- app/components/DocumentList/DocumentList.js | 21 ++--- .../DocumentPreview/DocumentPreview.js | 58 +++++++++----- app/components/DropdownMenu/DropdownMenu.js | 44 ++++++----- app/components/Icon/PinIcon.js | 12 +++ app/menus/CollectionMenu.js | 6 +- app/menus/DocumentMenu.js | 36 ++++++--- app/models/Document.js | 29 ++++++- app/scenes/Collection/Collection.js | 50 +++++++++--- app/stores/DocumentsStore.js | 20 ++++- .../api/__snapshots__/documents.test.js.snap | 18 +++++ server/api/documents.js | 76 +++++++++++++++++-- server/api/documents.test.js | 60 +++++++++++++++ .../20180225203847-document-pinning.js | 13 ++++ server/models/Document.js | 18 +---- server/pages/Api.js | 38 +++++++++- server/presenters/document.js | 1 + 16 files changed, 399 insertions(+), 101 deletions(-) create mode 100644 app/components/Icon/PinIcon.js create mode 100644 server/migrations/20180225203847-document-pinning.js diff --git a/app/components/DocumentList/DocumentList.js b/app/components/DocumentList/DocumentList.js index db69417d..836c6dfe 100644 --- a/app/components/DocumentList/DocumentList.js +++ b/app/components/DocumentList/DocumentList.js @@ -8,24 +8,27 @@ class DocumentList extends React.Component { props: { documents: Document[], showCollection?: boolean, + limit?: number, }; render() { - const { documents, showCollection } = this.props; + const { limit, showCollection } = this.props; + const documents = limit + ? this.props.documents.splice(0, limit) + : this.props.documents; return ( - {documents && - documents.map(document => ( - - ))} + {documents.map(document => ( + + ))} ); } diff --git a/app/components/DocumentPreview/DocumentPreview.js b/app/components/DocumentPreview/DocumentPreview.js index 40f2d0a6..e0be3355 100644 --- a/app/components/DocumentPreview/DocumentPreview.js +++ b/app/components/DocumentPreview/DocumentPreview.js @@ -5,9 +5,11 @@ import { Link } from 'react-router-dom'; import Document from 'models/Document'; import styled from 'styled-components'; import { color } from 'shared/styles/constants'; +import Flex from 'shared/components/Flex'; import Highlight from 'components/Highlight'; import StarredIcon from 'components/Icon/StarredIcon'; import PublishingInfo from './components/PublishingInfo'; +import DocumentMenu from 'menus/DocumentMenu'; type Props = { document: Document, @@ -19,10 +21,8 @@ type Props = { const StyledStar = styled(({ solid, ...props }) => ( ))` - position: absolute; opacity: ${props => (props.solid ? '1 !important' : 0)}; transition: all 100ms ease-in-out; - margin-left: 2px; &:hover { transform: scale(1.1); @@ -32,6 +32,13 @@ const StyledStar = styled(({ solid, ...props }) => ( } `; +const StyledDocumentMenu = styled(DocumentMenu)` + position: absolute; + right: 16px; + top: 50%; + transform: translateY(-50%); +`; + const DocumentLink = styled(Link)` display: block; margin: 0 -16px; @@ -41,6 +48,11 @@ const DocumentLink = styled(Link)` max-height: 50vh; min-width: 100%; overflow: hidden; + position: relative; + + ${StyledDocumentMenu} { + opacity: 0; + } &:hover, &:active, @@ -49,7 +61,7 @@ const DocumentLink = styled(Link)` border: 2px solid ${color.smoke}; outline: none; - ${StyledStar} { + ${StyledStar}, ${StyledDocumentMenu} { opacity: 0.5; &:hover { @@ -61,11 +73,19 @@ const DocumentLink = styled(Link)` &:focus { border: 2px solid ${color.slateDark}; } +`; - h3 { - margin-top: 0; - margin-bottom: 0.25em; - } +const Heading = styled.h3` + display: flex; + align-items: center; + height: 24px; + margin-top: 0; + margin-bottom: 0.25em; +`; + +const Actions = styled(Flex)` + margin-left: 4px; + align-items: center; `; @observer @@ -95,19 +115,19 @@ class DocumentPreview extends Component { return ( -

+ - {document.publishedAt && - (document.starred ? ( - - - - ) : ( - - - - ))} -

+ {document.publishedAt && ( + + {document.starred ? ( + + ) : ( + + )} + + )} + + void, onClose?: () => void, children?: React.Element<*>, + className?: string, style?: Object, }; @@ -23,10 +24,9 @@ class DropdownMenu extends Component { @observable top: number; @observable right: number; - handleOpen = (openPortal: SyntheticEvent => void) => { + handleOpen = (openPortal: SyntheticEvent => *) => { return (ev: SyntheticMouseEvent) => { ev.preventDefault(); - ev.stopPropagation(); const currentTarget = ev.currentTarget; invariant(document.body, 'why you not here'); @@ -41,30 +41,34 @@ class DropdownMenu extends Component { }; render() { + const { className, label, children } = this.props; + return ( -
+
- {({ closePortal, openPortal, portal }) => [ - , - portal( - - {this.props.children} - - ), - ]} + {({ closePortal, openPortal, portal }) => ( + + + {portal( + { + ev.stopPropagation(); + closePortal(); + }} + style={this.props.style} + top={this.top} + right={this.right} + > + {children} + + )} + + )}
); diff --git a/app/components/Icon/PinIcon.js b/app/components/Icon/PinIcon.js new file mode 100644 index 00000000..0b759434 --- /dev/null +++ b/app/components/Icon/PinIcon.js @@ -0,0 +1,12 @@ +// @flow +import React from 'react'; +import Icon from './Icon'; +import type { Props } from './Icon'; + +export default function PinIcon(props: Props) { + return ( + + + + ); +} diff --git a/app/menus/CollectionMenu.js b/app/menus/CollectionMenu.js index 1a7edf0d..8702af9f 100644 --- a/app/menus/CollectionMenu.js +++ b/app/menus/CollectionMenu.js @@ -9,7 +9,6 @@ import Collection from 'models/Collection'; import UiStore from 'stores/UiStore'; import DocumentsStore from 'stores/DocumentsStore'; import MoreIcon from 'components/Icon/MoreIcon'; -import Flex from 'shared/components/Flex'; import { DropdownMenu, DropdownMenuItem } from 'components/DropdownMenu'; type Props = { @@ -80,15 +79,16 @@ class CollectionMenu extends Component { onClose={onClose} > {collection && ( - + New document Import document +
Edit… - + )} Delete… diff --git a/app/menus/DocumentMenu.js b/app/menus/DocumentMenu.js index 2e4f5607..d24200ae 100644 --- a/app/menus/DocumentMenu.js +++ b/app/menus/DocumentMenu.js @@ -15,44 +15,60 @@ class DocumentMenu extends Component { label?: React$Element, history: Object, document: Document, + className: string, }; - handleNewChild = () => { + handleNewChild = (ev: SyntheticEvent) => { const { history, document } = this.props; history.push( `${document.collection.url}/new?parentDocument=${document.id}` ); }; - handleDelete = () => { + handleDelete = (ev: SyntheticEvent) => { const { document } = this.props; this.props.ui.setActiveModal('document-delete', { document }); }; - handleMove = () => { + handleMove = (ev: SyntheticEvent) => { this.props.history.push(documentMoveUrl(this.props.document)); }; - handleStar = () => { + handlePin = (ev: SyntheticEvent) => { + this.props.document.pin(); + }; + + handleUnpin = (ev: SyntheticEvent) => { + this.props.document.unpin(); + }; + + handleStar = (ev: SyntheticEvent) => { this.props.document.star(); }; - handleUnstar = () => { + handleUnstar = (ev: SyntheticEvent) => { this.props.document.unstar(); }; - handleExport = () => { + handleExport = (ev: SyntheticEvent) => { this.props.document.download(); }; render() { - const { document, label } = this.props; + const { document, label, className } = this.props; const isDraft = !document.publishedAt; return ( - }> + } className={className}> {!isDraft && ( + {document.pinned ? ( + + Unpin + + ) : ( + Pin + )} {document.starred ? ( Unstar @@ -62,6 +78,7 @@ class DocumentMenu extends Component { Star )} +
Move… )} + Delete… +
Download Print - Delete… ); } diff --git a/app/models/Document.js b/app/models/Document.js index 80da8eab..0237b4f2 100644 --- a/app/models/Document.js +++ b/app/models/Document.js @@ -16,7 +16,7 @@ class Document extends BaseModel { hasPendingChanges: boolean = false; errors: ErrorsStore; - collaborators: Array; + collaborators: User[]; collection: $Shape; collectionId: string; firstViewedAt: ?string; @@ -24,18 +24,19 @@ class Document extends BaseModel { modifiedSinceViewed: ?boolean; createdAt: string; createdBy: User; + updatedAt: string; + updatedBy: User; html: string; id: string; team: string; emoji: string; private: boolean = false; starred: boolean = false; + pinned: boolean = false; text: string = ''; title: string = ''; parentDocument: ?string; publishedAt: ?string; - updatedAt: string; - updatedBy: User; url: string; views: number; revision: number; @@ -98,6 +99,28 @@ class Document extends BaseModel { /* Actions */ + @action + pin = async () => { + this.pinned = true; + try { + await client.post('/documents.pin', { id: this.id }); + } catch (e) { + this.pinned = false; + this.errors.add('Document failed to pin'); + } + }; + + @action + unpin = async () => { + this.pinned = false; + try { + await client.post('/documents.unpin', { id: this.id }); + } catch (e) { + this.pinned = true; + this.errors.add('Document failed to unpin'); + } + }; + @action star = async () => { this.starred = true; diff --git a/app/scenes/Collection/Collection.js b/app/scenes/Collection/Collection.js index 7e70c249..80bf0ead 100644 --- a/app/scenes/Collection/Collection.js +++ b/app/scenes/Collection/Collection.js @@ -17,6 +17,7 @@ import Actions, { Action, Separator } from 'components/Actions'; import CenteredContent from 'components/CenteredContent'; import CollectionIcon from 'components/Icon/CollectionIcon'; import NewDocumentIcon from 'components/Icon/NewDocumentIcon'; +import PinIcon from 'components/Icon/PinIcon'; import { ListPlaceholder } from 'components/LoadingPlaceholder'; import Button from 'components/Button'; import HelpText from 'components/HelpText'; @@ -55,16 +56,21 @@ class CollectionScene extends Component { loadContent = async (id: string) => { const { collections } = this.props; - const collection = collections.getById(id) || (await collections.fetch(id)); if (collection) { this.props.ui.setActiveCollection(collection); this.collection = collection; - await this.props.documents.fetchRecentlyModified({ - limit: 10, - collection: collection.id, - }); + + await Promise.all([ + this.props.documents.fetchRecentlyModified({ + limit: 10, + collection: id, + }), + this.props.documents.fetchPinned({ + collection: id, + }), + ]); } this.isFetching = false; @@ -132,10 +138,18 @@ class CollectionScene extends Component { return this.renderEmptyCollection(); } + const pinnedDocuments = this.collection + ? this.props.documents.pinnedInCollection(this.collection.id) + : []; + const recentDocuments = this.collection + ? this.props.documents.recentlyEditedInCollection(this.collection.id) + : []; + const hasPinnedDocuments = !!pinnedDocuments.length; + return ( {this.collection ? ( - + {' '} {this.collection.name} + + {hasPinnedDocuments && ( + + + Pinned + + + + )} + Recently edited - + {this.renderActions()} - + ) : ( )} @@ -161,6 +181,12 @@ class CollectionScene extends Component { } } +const TinyPinIcon = styled(PinIcon)` + position: relative; + top: 4px; + opacity: 0.8; +`; + const Heading = styled.h1` display: flex; diff --git a/app/stores/DocumentsStore.js b/app/stores/DocumentsStore.js index 4367b2d3..d161deaf 100644 --- a/app/stores/DocumentsStore.js +++ b/app/stores/DocumentsStore.js @@ -60,10 +60,19 @@ class DocumentsStore extends BaseStore { return docs; } - recentlyEditedIn(documentIds: string[]): Document[] { + pinnedInCollection(collectionId: string): Document[] { + return _.filter( + this.recentlyEditedInCollection(collectionId), + document => document.pinned + ); + } + + recentlyEditedInCollection(collectionId: string): Document[] { return _.orderBy( - _.filter(this.data.values(), document => - documentIds.includes(document.id) + _.filter( + this.data.values(), + document => + document.collectionId === collectionId && !!document.publishedAt ), 'updatedAt', 'desc' @@ -147,6 +156,11 @@ class DocumentsStore extends BaseStore { await this.fetchPage('drafts', options); }; + @action + fetchPinned = async (options: ?PaginationParams): Promise<*> => { + await this.fetchPage('pinned', options); + }; + @action search = async ( query: string, diff --git a/server/api/__snapshots__/documents.test.js.snap b/server/api/__snapshots__/documents.test.js.snap index 747d364a..eaf3a72b 100644 --- a/server/api/__snapshots__/documents.test.js.snap +++ b/server/api/__snapshots__/documents.test.js.snap @@ -17,6 +17,15 @@ Object { } `; +exports[`#documents.pin should require authentication 1`] = ` +Object { + "error": "authentication_required", + "message": "Authentication required", + "ok": false, + "status": 401, +} +`; + exports[`#documents.search should require authentication 1`] = ` Object { "error": "authentication_required", @@ -44,6 +53,15 @@ Object { } `; +exports[`#documents.unpin should require authentication 1`] = ` +Object { + "error": "authentication_required", + "message": "Authentication required", + "ok": false, + "status": 401, +} +`; + exports[`#documents.unstar should require authentication 1`] = ` Object { "error": "authentication_required", diff --git a/server/api/documents.js b/server/api/documents.js index 8b4a684a..1a9c20df 100644 --- a/server/api/documents.js +++ b/server/api/documents.js @@ -19,8 +19,7 @@ router.post('documents.list', auth(), pagination(), async ctx => { let where = { teamId: user.teamId }; if (collection) where = { ...where, atlasId: collection }; - const userId = user.id; - const starredScope = { method: ['withStarred', userId] }; + const starredScope = { method: ['withStarred', user.id] }; const documents = await Document.scope('defaultScope', starredScope).findAll({ where, order: [[sort, direction]], @@ -38,6 +37,36 @@ router.post('documents.list', auth(), pagination(), async ctx => { }; }); +router.post('documents.pinned', auth(), pagination(), async ctx => { + let { sort = 'updatedAt', direction, collection } = ctx.body; + if (direction !== 'ASC') direction = 'DESC'; + ctx.assertPresent(collection, 'collection is required'); + + const user = ctx.state.user; + const starredScope = { method: ['withStarred', user.id] }; + const documents = await Document.scope('defaultScope', starredScope).findAll({ + where: { + teamId: user.teamId, + atlasId: collection, + pinnedById: { + [Op.ne]: null, + }, + }, + order: [[sort, direction]], + offset: ctx.state.pagination.offset, + limit: ctx.state.pagination.limit, + }); + + const data = await Promise.all( + documents.map(document => presentDocument(ctx, document)) + ); + + ctx.body = { + pagination: ctx.state.pagination, + data, + }; +}); + router.post('documents.viewed', auth(), pagination(), async ctx => { let { sort = 'updatedAt', direction } = ctx.body; if (direction !== 'ASC') direction = 'DESC'; @@ -166,8 +195,7 @@ router.post('documents.search', auth(), pagination(), async ctx => { const { offset, limit } = ctx.state.pagination; ctx.assertPresent(query, 'query is required'); - const user = await ctx.state.user; - + const user = ctx.state.user; const documents = await Document.searchForUser(user, query, { offset, limit, @@ -183,13 +211,45 @@ router.post('documents.search', auth(), pagination(), async ctx => { }; }); +router.post('documents.pin', auth(), async ctx => { + const { id } = ctx.body; + ctx.assertPresent(id, 'id is required'); + const user = ctx.state.user; + const document = await Document.findById(id); + + authorize(user, 'update', document); + + document.pinnedById = user.id; + await document.save(); + + ctx.body = { + data: await presentDocument(ctx, document), + }; +}); + +router.post('documents.unpin', auth(), async ctx => { + const { id } = ctx.body; + ctx.assertPresent(id, 'id is required'); + const user = ctx.state.user; + const document = await Document.findById(id); + + authorize(user, 'update', document); + + document.pinnedById = null; + await document.save(); + + ctx.body = { + data: await presentDocument(ctx, document), + }; +}); + router.post('documents.star', auth(), async ctx => { const { id } = ctx.body; ctx.assertPresent(id, 'id is required'); - const user = await ctx.state.user; + const user = ctx.state.user; const document = await Document.findById(id); - authorize(ctx.state.user, 'read', document); + authorize(user, 'read', document); await Star.findOrCreate({ where: { documentId: document.id, userId: user.id }, @@ -199,10 +259,10 @@ router.post('documents.star', auth(), async ctx => { router.post('documents.unstar', auth(), async ctx => { const { id } = ctx.body; ctx.assertPresent(id, 'id is required'); - const user = await ctx.state.user; + const user = ctx.state.user; const document = await Document.findById(id); - authorize(ctx.state.user, 'read', document); + authorize(user, 'read', document); await Star.destroy({ where: { documentId: document.id, userId: user.id }, diff --git a/server/api/documents.test.js b/server/api/documents.test.js index dcd4a4f0..bcd5abb6 100644 --- a/server/api/documents.test.js +++ b/server/api/documents.test.js @@ -248,6 +248,66 @@ describe('#documents.starred', async () => { }); }); +describe('#documents.pin', async () => { + it('should pin the document', async () => { + const { user, document } = await seed(); + + const res = await server.post('/api/documents.pin', { + body: { token: user.getJwtToken(), id: document.id }, + }); + const body = await res.json(); + expect(body.data.pinned).toEqual(true); + }); + + it('should require authentication', async () => { + const res = await server.post('/api/documents.pin'); + const body = await res.json(); + + expect(res.status).toEqual(401); + expect(body).toMatchSnapshot(); + }); + + it('should require authorization', async () => { + const { document } = await seed(); + const user = await buildUser(); + const res = await server.post('/api/documents.pin', { + body: { token: user.getJwtToken(), id: document.id }, + }); + expect(res.status).toEqual(403); + }); +}); + +describe('#documents.unpin', async () => { + it('should unpin the document', async () => { + const { user, document } = await seed(); + document.pinnedBy = user; + await document.save(); + + const res = await server.post('/api/documents.unpin', { + body: { token: user.getJwtToken(), id: document.id }, + }); + const body = await res.json(); + expect(body.data.pinned).toEqual(false); + }); + + it('should require authentication', async () => { + const res = await server.post('/api/documents.unpin'); + const body = await res.json(); + + expect(res.status).toEqual(401); + expect(body).toMatchSnapshot(); + }); + + it('should require authorization', async () => { + const { document } = await seed(); + const user = await buildUser(); + const res = await server.post('/api/documents.unpin', { + body: { token: user.getJwtToken(), id: document.id }, + }); + expect(res.status).toEqual(403); + }); +}); + describe('#documents.star', async () => { it('should star the document', async () => { const { user, document } = await seed(); diff --git a/server/migrations/20180225203847-document-pinning.js b/server/migrations/20180225203847-document-pinning.js new file mode 100644 index 00000000..b0f153c9 --- /dev/null +++ b/server/migrations/20180225203847-document-pinning.js @@ -0,0 +1,13 @@ +'use strict'; + +module.exports = { + up: async (queryInterface, Sequelize) => { + await queryInterface.addColumn('documents', 'pinnedById', { + type: Sequelize.UUID, + }); + }, + + down: async (queryInterface, Sequelize) => { + await queryInterface.removeColumn('documents', 'pinnedById'); + } +}; diff --git a/server/models/Document.js b/server/models/Document.js index e5802601..878ac1d1 100644 --- a/server/models/Document.js +++ b/server/models/Document.js @@ -85,20 +85,6 @@ const Document = sequelize.define( revisionCount: { type: DataTypes.INTEGER, defaultValue: 0 }, publishedAt: DataTypes.DATE, parentDocumentId: DataTypes.UUID, - createdById: { - type: DataTypes.UUID, - allowNull: false, - references: { - model: 'users', - }, - }, - lastModifiedById: { - type: DataTypes.UUID, - allowNull: false, - references: { - model: 'users', - }, - }, collaboratorIds: DataTypes.ARRAY(DataTypes.UUID), }, { @@ -129,6 +115,10 @@ Document.associate = models => { as: 'updatedBy', foreignKey: 'lastModifiedById', }); + Document.belongsTo(models.User, { + as: 'pinnedBy', + foreignKey: 'pinnedById', + }); Document.hasMany(models.Revision, { as: 'revisions', onDelete: 'cascade', diff --git a/server/pages/Api.js b/server/pages/Api.js index f640b4db..43c09878 100644 --- a/server/pages/Api.js +++ b/server/pages/Api.js @@ -427,6 +427,34 @@ export default function Pricing() { + + + Pins a document to the collection home. The pinned document is + visible to all members of the team. + + + + + + + + + Unpins a document from the collection home. It will still remain + in the collection itself. + + + + + + Star (favorite) a document for authenticated user. @@ -442,7 +470,7 @@ export default function Pricing() { - Unstar as starred (favorited) a document for authenticated user. + Unstar a starred (favorited) document for authenticated user. + + Return pinned documents for a collection + + +