From 6b10c7308cd441518da97241ebc1ee7cf1ff3294 Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Sun, 24 Sep 2017 14:55:03 -0700 Subject: [PATCH 1/6] Fixed styling on document move --- .../DocumentMove/components/PathToDocument.js | 49 ++++++++++++------- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/frontend/scenes/Document/components/DocumentMove/components/PathToDocument.js b/frontend/scenes/Document/components/DocumentMove/components/PathToDocument.js index 1c2ec5ff2..df2f16973 100644 --- a/frontend/scenes/Document/components/DocumentMove/components/PathToDocument.js +++ b/frontend/scenes/Document/components/DocumentMove/components/PathToDocument.js @@ -13,27 +13,38 @@ import Document from 'models/Document'; import DocumentsStore from 'stores/DocumentsStore'; const ResultWrapper = styled.div` -display: flex; -margin-bottom: 10px; + display: flex; + margin-bottom: 10px; -color: ${color.text}; -cursor: default; + color: ${color.text}; + cursor: default; +`; + +const StyledChevronIcon = styled(ChevronIcon)` + padding-top: 2px; + width: 24px; + height: 24px; `; const ResultWrapperLink = ResultWrapper.withComponent('a').extend` -padding-top: 3px; -padding-left: 5px; + height: 32px; + padding-top: 3px; + padding-left: 5px; -&:hover, -&:active, -&:focus { - margin-left: 0px; - border-radius: 2px; - background: ${color.black}; - color: ${color.smokeLight}; - outline: none; - cursor: pointer; -} + &:hover, + &:active, + &:focus { + margin-left: 0px; + border-radius: 2px; + background: ${color.black}; + color: ${color.smokeLight}; + outline: none; + cursor: pointer; + + ${StyledChevronIcon} svg { + fill: ${color.smokeLight}; + } + } `; type Props = { @@ -79,16 +90,16 @@ type Props = { {this.resultDocument && {' '} - + {' '} {this.resultDocument.pathToDocument .map(doc => {doc.title}) - .reduce((prev, curr) => [prev, , curr])} + .reduce((prev, curr) => [prev, , curr])} } {document && {' '} - + {' '}{document.title} } From 9fc1731f99e3e36d1b59d3d16eedfb419dca7c50 Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Wed, 27 Sep 2017 00:11:26 -0700 Subject: [PATCH 2/6] draft --- .../components/DocumentMove/DocumentMove.js | 31 ++++++++++---- .../DocumentMove/components/PathToDocument.js | 7 ++-- frontend/stores/CollectionsStore.js | 42 +++++++++++-------- 3 files changed, 51 insertions(+), 29 deletions(-) diff --git a/frontend/scenes/Document/components/DocumentMove/DocumentMove.js b/frontend/scenes/Document/components/DocumentMove/DocumentMove.js index 1f7ebfbf7..a6c39a654 100644 --- a/frontend/scenes/Document/components/DocumentMove/DocumentMove.js +++ b/frontend/scenes/Document/components/DocumentMove/DocumentMove.js @@ -90,6 +90,26 @@ type Props = { render() { const { document, documents } = this.props; + let resultSet; + + resultSet = this.resultIds.filter(docId => { + const resultDoc = documents.getById(docId); + + if (document && resultDoc) { + return ( + // Exclude the document if it's on the path to a potential new path + !resultDoc.pathToDocument.map(doc => doc.id).includes(document.id) && + // Exclude if the same path, e.g the last one before the current + _.last(resultDoc.pathToDocument).id !== document.parentDocumentId + ); + } + return true; + }); + + // Prepend root if document does have a parent document + resultSet = document.parentDocumentId + ? _.concat(null, resultSet) + : this.resultIds; return ( @@ -115,18 +135,13 @@ type Props = { mode={ArrowKeyNavigation.mode.VERTICAL} defaultActiveChildIndex={0} > - this.setFirstDocumentRef(ref)} - onSuccess={this.handleClose} - /> - {this.resultIds.map((documentId, index) => ( + {resultSet.map((documentId, index) => ( index === 0 && this.setFirstDocumentRef(ref)} onSuccess={this.handleClose} /> ))} diff --git a/frontend/scenes/Document/components/DocumentMove/components/PathToDocument.js b/frontend/scenes/Document/components/DocumentMove/components/PathToDocument.js index df2f16973..12cfdaa59 100644 --- a/frontend/scenes/Document/components/DocumentMove/components/PathToDocument.js +++ b/frontend/scenes/Document/components/DocumentMove/components/PathToDocument.js @@ -74,16 +74,17 @@ type Props = { }; render() { - const { document, onSuccess, ref } = this.props; + const { document, documentId, onSuccess, ref } = this.props; // $FlowIssue we'll always have a document - const { collection } = document || this.resultDocument; + const { collection } = documentId ? this.resultDocument : document; const Component = onSuccess ? ResultWrapperLink : ResultWrapper; + // Exclude document when it's part of the path and not the preview return ( {collection.name} diff --git a/frontend/stores/CollectionsStore.js b/frontend/stores/CollectionsStore.js index d90c0985a..d1db0f315 100644 --- a/frontend/stores/CollectionsStore.js +++ b/frontend/stores/CollectionsStore.js @@ -5,7 +5,6 @@ import { action, runInAction, ObservableArray, - autorunAsync, } from 'mobx'; import ApiClient, { client } from 'utils/ApiClient'; import _ from 'lodash'; @@ -17,8 +16,6 @@ import ErrorsStore from 'stores/ErrorsStore'; import CacheStore from 'stores/CacheStore'; import UiStore from 'stores/UiStore'; -const COLLECTION_CACHE_KEY = 'COLLECTION_CACHE_KEY'; - type Options = { teamId: string, cache: CacheStore, @@ -41,6 +38,30 @@ class CollectionsStore { : undefined; } + /** + * List of paths to each of the documents, where paths are composed of id and title/name pairs + */ + @computed get pathsToDocuments(): ?[[{ id: string, title: string }]] { + let results = []; + const travelDocuments = (documentList, path) => + documentList.forEach(document => { + const { id, title } = document; + const node = { id, title }; + results.push(_.concat(path, node)); + travelDocuments(document.children, _.concat(path, [node])); + }); + + if (this.isLoaded) { + this.data.forEach(collection => { + const { id, name } = collection; + const node = { id, title: name }; + travelDocuments(collection.documents, [node]); + }); + } + + return results; + } + /* Actions */ @action fetchAll = async (): Promise<*> => { @@ -99,21 +120,6 @@ class CollectionsStore { this.teamId = options.teamId; this.cache = options.cache; this.ui = options.ui; - // - // this.cache.getItem(COLLECTION_CACHE_KEY).then(data => { - // if (data) { - // this.data.replace(data.map(collection => new Collection(collection))); - // this.isLoaded = true; - // } - // }); - - autorunAsync('CollectionsStore.persists', () => { - if (this.data.length > 0) - this.cache.setItem( - COLLECTION_CACHE_KEY, - this.data.map(collection => collection.data) - ); - }); } } From 466f6a509bd7b81e5477a7750b4d90f9c04d6b95 Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Sun, 1 Oct 2017 18:36:44 -0700 Subject: [PATCH 3/6] Refactored moving --- .../components/DocumentMove/DocumentMove.js | 199 ++++++++++-------- .../DocumentMove/components/PathToDocument.js | 50 ++--- frontend/stores/CollectionsStore.js | 15 +- package.json | 44 ++-- yarn.lock | 4 + 5 files changed, 167 insertions(+), 145 deletions(-) diff --git a/frontend/scenes/Document/components/DocumentMove/DocumentMove.js b/frontend/scenes/Document/components/DocumentMove/DocumentMove.js index a6c39a654..7c60ea6b4 100644 --- a/frontend/scenes/Document/components/DocumentMove/DocumentMove.js +++ b/frontend/scenes/Document/components/DocumentMove/DocumentMove.js @@ -1,9 +1,10 @@ // @flow import React, { Component } from 'react'; import ReactDOM from 'react-dom'; -import { observable, action } from 'mobx'; +import { observable, computed, action } from 'mobx'; import { observer, inject } from 'mobx-react'; import { withRouter } from 'react-router'; +import { Search } from 'js-search'; import ArrowKeyNavigation from 'boundless-arrow-key-navigation'; import _ from 'lodash'; import styled from 'styled-components'; @@ -17,25 +18,89 @@ import PathToDocument from './components/PathToDocument'; import Document from 'models/Document'; import DocumentsStore from 'stores/DocumentsStore'; +import CollectionsStore from 'stores/CollectionsStore'; type Props = { match: Object, history: Object, document: Document, documents: DocumentsStore, + collections: CollectionsStore, }; +type DocumentResult = { + id: string, + title: string, + type: 'document' | 'collection', +} + +type SearchResult = DocumentResult & { + path: Array, +} + @observer class DocumentMove extends Component { props: Props; firstDocument: HTMLElement; + @observable searchTerm: ?string; @observable isSaving: boolean; - @observable resultIds: Array = []; // Document IDs - @observable searchTerm: ?string = null; - @observable isFetching = false; - componentDidMount() { - this.setDefaultResult(); + @computed + get searchIndex() { + const { document, collections } = this.props; + const paths = collections.pathsToDocuments; + const index = new Search('id'); + index.addIndex('title'); + + // Build index + paths.forEach(path => { + // TMP: For now, exclude paths to other collections + if (_.first(path).id !== document.collection.id) return; + + const tail = _.last(path); + index.addDocuments([{ + ...tail, + path: path, + }]); + }); + + return index; + } + + @computed get results(): Array { + const { document, collections } = this.props; + + let results = []; + if (collections.isLoaded) { + if (this.searchTerm) { + // Search by + results = this.searchIndex.search(this.searchTerm); + } else { + // Default results, root of the current collection + results = document.collection.documents.map( + doc => collections.getPathForDocument(doc.id) + ); + } + } + + if (document.parentDocumentId) { + // Add root if document does have a parent document + results = [ + collections.getPathForDocument(document.collection.id), + ...results, + ] + } else { + // Exclude root from search results if document is already at the root + results = results.filter(result => + result.id !== document.collection.id); + } + + // Exclude document if on the path to result, or the same result + results = results.filter(result => { + return !result.path.map(doc => doc.id).includes(document.parentDocumentId); + }); + + return results; } handleKeyDown = ev => { @@ -53,101 +118,57 @@ type Props = { this.props.history.push(this.props.document.url); }; - handleFilter = (ev: SyntheticInputEvent) => { - this.searchTerm = ev.target.value; - this.updateSearchResults(); + handleFilter = (e: SyntheticInputEvent) => { + this.searchTerm = e.target.value; }; - updateSearchResults = _.debounce(() => { - this.search(); - }, 250); - setFirstDocumentRef = ref => { this.firstDocument = ref; }; - @action setDefaultResult() { - this.resultIds = this.props.document.collection.documents.map( - doc => doc.id - ); - } - - @action search = async () => { - this.isFetching = true; - - if (this.searchTerm) { - try { - this.resultIds = await this.props.documents.search(this.searchTerm); - } catch (e) { - console.error('Something went wrong'); - } - } else { - this.setDefaultResult(); - } - - this.isFetching = false; - }; - render() { - const { document, documents } = this.props; - let resultSet; - - resultSet = this.resultIds.filter(docId => { - const resultDoc = documents.getById(docId); - - if (document && resultDoc) { - return ( - // Exclude the document if it's on the path to a potential new path - !resultDoc.pathToDocument.map(doc => doc.id).includes(document.id) && - // Exclude if the same path, e.g the last one before the current - _.last(resultDoc.pathToDocument).id !== document.parentDocumentId - ); - } - return true; - }); - - // Prepend root if document does have a parent document - resultSet = document.parentDocumentId - ? _.concat(null, resultSet) - : this.resultIds; + const { document, documents, collections } = this.props; return ( -
- - - -
- -
- - - + {collections.isLoaded ? ( - - {resultSet.map((documentId, index) => ( - index === 0 && this.setFirstDocumentRef(ref)} - onSuccess={this.handleClose} +
+ + + +
+ +
+ + - ))} - + + + + {this.results.map((result, index) => ( + index === 0 && this.setFirstDocumentRef(ref)} + onClick={ () => 'move here' } + /> + ))} + + +
-
+ ) :
loading
} ); } @@ -163,4 +184,4 @@ const StyledArrowKeyNavigation = styled(ArrowKeyNavigation)` flex: 1; `; -export default withRouter(inject('documents')(DocumentMove)); +export default withRouter(inject('documents', 'collections')(DocumentMove)); diff --git a/frontend/scenes/Document/components/DocumentMove/components/PathToDocument.js b/frontend/scenes/Document/components/DocumentMove/components/PathToDocument.js index 12cfdaa59..69b717f2b 100644 --- a/frontend/scenes/Document/components/DocumentMove/components/PathToDocument.js +++ b/frontend/scenes/Document/components/DocumentMove/components/PathToDocument.js @@ -9,9 +9,6 @@ import { color } from 'styles/constants'; import Flex from 'components/Flex'; import ChevronIcon from 'components/Icon/ChevronIcon'; -import Document from 'models/Document'; -import DocumentsStore from 'stores/DocumentsStore'; - const ResultWrapper = styled.div` display: flex; margin-bottom: 10px; @@ -48,59 +45,36 @@ const ResultWrapperLink = ResultWrapper.withComponent('a').extend` `; type Props = { - documentId?: string, - onSuccess?: Function, - documents: DocumentsStore, - document?: Document, + result: Object, + document: Document, + onClick?: Function, ref?: Function, - selectable?: boolean, }; @observer class PathToDocument extends React.Component { props: Props; - get resultDocument(): ?Document { - const { documentId } = this.props; - if (documentId) return this.props.documents.getById(documentId); - } - - handleSelect = async (event: SyntheticEvent) => { - const { document, onSuccess } = this.props; - - invariant(onSuccess && document, 'onSuccess unavailable'); - event.preventDefault(); - await document.move(this.resultDocument ? this.resultDocument.id : null); - onSuccess(); - }; - render() { - const { document, documentId, onSuccess, ref } = this.props; + const { result, document, onClick, ref } = this.props; // $FlowIssue we'll always have a document - const { collection } = documentId ? this.resultDocument : document; - const Component = onSuccess ? ResultWrapperLink : ResultWrapper; + const Component = onClick ? ResultWrapperLink : ResultWrapper; + + if (!result) return
; - // Exclude document when it's part of the path and not the preview return ( - {collection.name} - {this.resultDocument && - - {' '} - - {' '} - {this.resultDocument.pathToDocument - .map(doc => {doc.title}) - .reduce((prev, curr) => [prev, , curr])} - } + {result.path + .map(doc => {doc.title}) + .reduce((prev, curr) => [prev, , curr])} {document && {' '} - + {' '}{document.title} } diff --git a/frontend/stores/CollectionsStore.js b/frontend/stores/CollectionsStore.js index d1db0f315..8540e29a1 100644 --- a/frontend/stores/CollectionsStore.js +++ b/frontend/stores/CollectionsStore.js @@ -46,7 +46,7 @@ class CollectionsStore { const travelDocuments = (documentList, path) => documentList.forEach(document => { const { id, title } = document; - const node = { id, title }; + const node = { id, title, type: 'document' }; results.push(_.concat(path, node)); travelDocuments(document.children, _.concat(path, [node])); }); @@ -54,7 +54,8 @@ class CollectionsStore { if (this.isLoaded) { this.data.forEach(collection => { const { id, name } = collection; - const node = { id, title: name }; + const node = { id, title: name, type: 'collection' }; + results.push([node]); travelDocuments(collection.documents, [node]); }); } @@ -62,6 +63,16 @@ class CollectionsStore { return results; } + getPathForDocument(documentId: string): Object { + const result = this.pathsToDocuments.find(path => _.last(path).id === documentId); + + const tail = _.last(result); + return { + ...tail, + path: result, + } + } + /* Actions */ @action fetchAll = async (): Promise<*> => { diff --git a/package.json b/package.json index 5af1067db..11e75b1e5 100644 --- a/package.json +++ b/package.json @@ -4,14 +4,11 @@ "main": "index.js", "scripts": { "clean": "rimraf dist", - "build:webpack": - "NODE_ENV=production webpack --config webpack.config.prod.js", - "build:analyze": - "NODE_ENV=production webpack --config webpack.config.prod.js --json | webpack-bundle-size-analyzer", + "build:webpack": "NODE_ENV=production webpack --config webpack.config.prod.js", + "build:analyze": "NODE_ENV=production webpack --config webpack.config.prod.js --json | webpack-bundle-size-analyzer", "build": "npm run clean && npm run build:webpack", "start": "node index.js", - "dev": - "NODE_ENV=development DEBUG=sql,cache,presenters ./node_modules/.bin/nodemon --inspect --watch server index.js", + "dev": "NODE_ENV=development DEBUG=sql,cache,presenters ./node_modules/.bin/nodemon --inspect --watch server index.js", "lint": "npm run lint:flow && npm run lint:js", "lint:js": "eslint frontend", "lint:flow": "flow", @@ -21,24 +18,39 @@ "sequelize:migrate": "sequelize db:migrate", "test": "npm run test:frontend && npm run test:server", "test:frontend": "jest", - "test:server": - "jest --config=server/.jestconfig.json --runInBand --forceExit", + "test:server": "jest --config=server/.jestconfig.json --runInBand --forceExit", "precommit": "lint-staged" }, "lint-staged": { - "*.js": ["eslint --fix", "git add"] + "*.js": [ + "eslint --fix", + "git add" + ] }, "jest": { "verbose": false, - "roots": ["frontend"], + "roots": [ + "frontend" + ], "moduleNameMapper": { "^.*[.](s?css|css)$": "/__mocks__/styleMock.js", "^.*[.](gif|ttf|eot|svg)$": "/__test__/fileMock.js" }, - "moduleFileExtensions": ["js", "jsx", "json"], - "moduleDirectories": ["node_modules"], - "modulePaths": ["frontend"], - "setupFiles": ["/setupJest.js", "/__mocks__/window.js"] + "moduleFileExtensions": [ + "js", + "jsx", + "json" + ], + "moduleDirectories": [ + "node_modules" + ], + "modulePaths": [ + "frontend" + ], + "setupFiles": [ + "/setupJest.js", + "/__mocks__/window.js" + ] }, "engines": { "node": ">= 7.6" @@ -95,6 +107,7 @@ "imports-loader": "0.6.5", "invariant": "^2.2.2", "isomorphic-fetch": "2.2.1", + "js-search": "^1.4.2", "js-tree": "1.1.0", "json-loader": "0.5.4", "jsonwebtoken": "7.0.1", @@ -157,8 +170,7 @@ "string-hash": "^1.1.0", "style-loader": "^0.18.2", "styled-components": "^2.0.0", - "truncate-html": - "https://github.com/jorilallo/truncate-html/tarball/master", + "truncate-html": "https://github.com/jorilallo/truncate-html/tarball/master", "url-loader": "0.5.7", "uuid": "2.0.2", "validator": "5.2.0", diff --git a/yarn.lock b/yarn.lock index eaabe2248..8afd54995 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4858,6 +4858,10 @@ js-beautify@^1.6.11: mkdirp "~0.5.0" nopt "~3.0.1" +js-search@^1.4.2: + version "1.4.2" + resolved "https://registry.yarnpkg.com/js-search/-/js-search-1.4.2.tgz#59a91e117d6badb20bf0d7643ba7577d5a81d7e2" + js-string-escape@1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/js-string-escape/-/js-string-escape-1.0.1.tgz#e2625badbc0d67c7533e9edc1068c587ae4137ef" From ccec69d431acba1691a1dcba00cb02204c02000b Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Sun, 1 Oct 2017 23:42:17 -0700 Subject: [PATCH 4/6] Final fixes to document move --- .../components/DocumentMove/DocumentMove.js | 148 +++++++++--------- .../DocumentMove/components/PathToDocument.js | 43 +++-- frontend/stores/CollectionsStore.js | 30 ++-- server/api/documents.js | 2 +- 4 files changed, 125 insertions(+), 98 deletions(-) diff --git a/frontend/scenes/Document/components/DocumentMove/DocumentMove.js b/frontend/scenes/Document/components/DocumentMove/DocumentMove.js index 7c60ea6b4..78ffcdbae 100644 --- a/frontend/scenes/Document/components/DocumentMove/DocumentMove.js +++ b/frontend/scenes/Document/components/DocumentMove/DocumentMove.js @@ -1,7 +1,7 @@ // @flow import React, { Component } from 'react'; import ReactDOM from 'react-dom'; -import { observable, computed, action } from 'mobx'; +import { observable, computed } from 'mobx'; import { observer, inject } from 'mobx-react'; import { withRouter } from 'react-router'; import { Search } from 'js-search'; @@ -18,7 +18,7 @@ import PathToDocument from './components/PathToDocument'; import Document from 'models/Document'; import DocumentsStore from 'stores/DocumentsStore'; -import CollectionsStore from 'stores/CollectionsStore'; +import CollectionsStore, { type DocumentPath } from 'stores/CollectionsStore'; type Props = { match: Object, @@ -28,16 +28,6 @@ type Props = { collections: CollectionsStore, }; -type DocumentResult = { - id: string, - title: string, - type: 'document' | 'collection', -} - -type SearchResult = DocumentResult & { - path: Array, -} - @observer class DocumentMove extends Component { props: Props; firstDocument: HTMLElement; @@ -45,8 +35,7 @@ type SearchResult = DocumentResult & { @observable searchTerm: ?string; @observable isSaving: boolean; - @computed - get searchIndex() { + @computed get searchIndex() { const { document, collections } = this.props; const paths = collections.pathsToDocuments; const index = new Search('id'); @@ -55,49 +44,53 @@ type SearchResult = DocumentResult & { // Build index paths.forEach(path => { // TMP: For now, exclude paths to other collections - if (_.first(path).id !== document.collection.id) return; + if (_.first(path.path).id !== document.collection.id) return; - const tail = _.last(path); - index.addDocuments([{ - ...tail, - path: path, - }]); + index.addDocuments([path]); }); return index; } - @computed get results(): Array { + @computed get results(): Array { const { document, collections } = this.props; let results = []; if (collections.isLoaded) { if (this.searchTerm) { - // Search by + // Search by results = this.searchIndex.search(this.searchTerm); } else { // Default results, root of the current collection - results = document.collection.documents.map( - doc => collections.getPathForDocument(doc.id) - ); + results = []; + document.collection.documents.forEach(doc => { + const path = collections.getPathForDocument(doc.id); + if (doc && path) { + results.push(path); + } + }); } } - if (document.parentDocumentId) { + if (document && document.parentDocumentId) { // Add root if document does have a parent document - results = [ - collections.getPathForDocument(document.collection.id), - ...results, - ] - } else { - // Exclude root from search results if document is already at the root - results = results.filter(result => - result.id !== document.collection.id); + const rootPath = collections.getPathForDocument(document.collection.id); + if (rootPath) { + results = [rootPath, ...results]; + } + } + + // Exclude root from search results if document is already at the root + if (!document.parentDocumentId) { + results = results.filter(result => result.id !== document.collection.id); } // Exclude document if on the path to result, or the same result results = results.filter(result => { - return !result.path.map(doc => doc.id).includes(document.parentDocumentId); + return ( + !result.path.map(doc => doc.id).includes(document.id) && + !result.path.map(doc => doc.id).includes(document.parentDocumentId) + ); }); return results; @@ -126,49 +119,58 @@ type SearchResult = DocumentResult & { this.firstDocument = ref; }; + renderPathToCurrentDocument() { + const { collections, document } = this.props; + const result = collections.getPathForDocument(document.id); + if (result) { + return ; + } + } + render() { - const { document, documents, collections } = this.props; + const { document, collections } = this.props; return ( - {collections.isLoaded ? ( - -
- - - -
+ {document && collections.isLoaded + ? +
+ + {this.renderPathToCurrentDocument()} + +
-
- - - - - - {this.results.map((result, index) => ( - index === 0 && this.setFirstDocumentRef(ref)} - onClick={ () => 'move here' } - /> - ))} - - -
- - ) :
loading
} +
+ + + + + + {this.results.map((result, index) => ( + + index === 0 && this.setFirstDocumentRef(ref)} + onSuccess={this.handleClose} + /> + ))} + + +
+ + :
} ); } diff --git a/frontend/scenes/Document/components/DocumentMove/components/PathToDocument.js b/frontend/scenes/Document/components/DocumentMove/components/PathToDocument.js index 69b717f2b..2e5bab449 100644 --- a/frontend/scenes/Document/components/DocumentMove/components/PathToDocument.js +++ b/frontend/scenes/Document/components/DocumentMove/components/PathToDocument.js @@ -1,14 +1,16 @@ // @flow import React from 'react'; import { observer } from 'mobx-react'; -import _ from 'lodash'; import invariant from 'invariant'; +import _ from 'lodash'; import styled from 'styled-components'; import { color } from 'styles/constants'; import Flex from 'components/Flex'; import ChevronIcon from 'components/Icon/ChevronIcon'; +import Document from 'models/Document'; + const ResultWrapper = styled.div` display: flex; margin-bottom: 10px; @@ -46,28 +48,41 @@ const ResultWrapperLink = ResultWrapper.withComponent('a').extend` type Props = { result: Object, - document: Document, - onClick?: Function, + document?: Document, + onSuccess?: Function, ref?: Function, }; @observer class PathToDocument extends React.Component { props: Props; - render() { - const { result, document, onClick, ref } = this.props; - // $FlowIssue we'll always have a document - const Component = onClick ? ResultWrapperLink : ResultWrapper; + handleClick = async (ev: SyntheticEvent) => { + ev.preventDefault(); + const { document, result, onSuccess } = this.props; - if (!result) return
; + invariant(onSuccess && document, 'onSuccess unavailable'); + + if (result.type === 'document') { + await document.move(result.id); + } else if ( + result.type === 'collection' && + result.id === document.collection.id + ) { + await document.move(null); + } else { + throw new Error('Not implemented yet'); + } + onSuccess(); + }; + + render() { + const { result, document, ref } = this.props; + const Component = document ? ResultWrapperLink : ResultWrapper; + + if (!result) return
; return ( - + {result.path .map(doc => {doc.title}) .reduce((prev, curr) => [prev, , curr])} diff --git a/frontend/stores/CollectionsStore.js b/frontend/stores/CollectionsStore.js index 8540e29a1..7231560f7 100644 --- a/frontend/stores/CollectionsStore.js +++ b/frontend/stores/CollectionsStore.js @@ -22,6 +22,16 @@ type Options = { ui: UiStore, }; +type DocumentPathItem = { + id: string, + title: string, + type: 'document' | 'collection', +}; + +export type DocumentPath = DocumentPathItem & { + path: Array, +}; + class CollectionsStore { @observable data: ObservableArray = observable.array([]); @observable isLoaded: boolean = false; @@ -41,7 +51,7 @@ class CollectionsStore { /** * List of paths to each of the documents, where paths are composed of id and title/name pairs */ - @computed get pathsToDocuments(): ?[[{ id: string, title: string }]] { + @computed get pathsToDocuments(): Array { let results = []; const travelDocuments = (documentList, path) => documentList.forEach(document => { @@ -60,17 +70,17 @@ class CollectionsStore { }); } - return results; + return results.map(result => { + const tail = _.last(result); + return { + ...tail, + path: result, + }; + }); } - getPathForDocument(documentId: string): Object { - const result = this.pathsToDocuments.find(path => _.last(path).id === documentId); - - const tail = _.last(result); - return { - ...tail, - path: result, - } + getPathForDocument(documentId: string): ?DocumentPath { + return this.pathsToDocuments.find(path => path.id === documentId); } /* Actions */ diff --git a/server/api/documents.js b/server/api/documents.js index b617b29c5..9d9892550 100644 --- a/server/api/documents.js +++ b/server/api/documents.js @@ -264,7 +264,7 @@ router.post('documents.move', auth(), async ctx => { // Set parent document if (parentDocument) { const parent = await Document.findById(parentDocument); - if (parent.atlasId !== document.atlasId) + if (!parent || parent.atlasId !== document.atlasId) throw httpErrors.BadRequest( 'Invalid parentDocument (must be same collection)' ); From 9900cc2f9f22a17367c4856fe73237081b393a29 Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Sun, 1 Oct 2017 23:45:27 -0700 Subject: [PATCH 5/6] minor refactor --- .../components/DocumentMove/DocumentMove.js | 77 +++++++++---------- 1 file changed, 38 insertions(+), 39 deletions(-) diff --git a/frontend/scenes/Document/components/DocumentMove/DocumentMove.js b/frontend/scenes/Document/components/DocumentMove/DocumentMove.js index 78ffcdbae..f2978e5c3 100644 --- a/frontend/scenes/Document/components/DocumentMove/DocumentMove.js +++ b/frontend/scenes/Document/components/DocumentMove/DocumentMove.js @@ -58,7 +58,7 @@ type Props = { let results = []; if (collections.isLoaded) { if (this.searchTerm) { - // Search by + // Search by the keyword results = this.searchIndex.search(this.searchTerm); } else { // Default results, root of the current collection @@ -132,45 +132,44 @@ type Props = { return ( - {document && collections.isLoaded - ? -
- - {this.renderPathToCurrentDocument()} - -
+ {document && + collections.isLoaded && + +
+ + {this.renderPathToCurrentDocument()} + +
-
- - - - - - {this.results.map((result, index) => ( - - index === 0 && this.setFirstDocumentRef(ref)} - onSuccess={this.handleClose} - /> - ))} - - -
- - :
} +
+ + + + + + {this.results.map((result, index) => ( + index === 0 && this.setFirstDocumentRef(ref)} + onSuccess={this.handleClose} + /> + ))} + + +
+ } ); } From 037967c56f40bd5b2e28af21dc2578e7cbd8ee33 Mon Sep 17 00:00:00 2001 From: Jori Lallo Date: Sat, 7 Oct 2017 16:03:02 -0700 Subject: [PATCH 6/6] Addressed review comments --- .../components/DocumentMove/DocumentMove.js | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/frontend/scenes/Document/components/DocumentMove/DocumentMove.js b/frontend/scenes/Document/components/DocumentMove/DocumentMove.js index f2978e5c3..53eeed8ec 100644 --- a/frontend/scenes/Document/components/DocumentMove/DocumentMove.js +++ b/frontend/scenes/Document/components/DocumentMove/DocumentMove.js @@ -42,17 +42,19 @@ type Props = { index.addIndex('title'); // Build index + const indexeableDocuments = []; paths.forEach(path => { // TMP: For now, exclude paths to other collections if (_.first(path.path).id !== document.collection.id) return; - index.addDocuments([path]); + indexeableDocuments.push(path); }); + index.addDocuments(indexeableDocuments); return index; } - @computed get results(): Array { + @computed get results(): DocumentPath[] { const { document, collections } = this.props; let results = []; @@ -86,12 +88,11 @@ type Props = { } // Exclude document if on the path to result, or the same result - results = results.filter(result => { - return ( + results = results.filter( + result => !result.path.map(doc => doc.id).includes(document.id) && !result.path.map(doc => doc.id).includes(document.parentDocumentId) - ); - }); + ); return results; } @@ -145,7 +146,7 @@ type Props = {