From e67ac1215a98fd10ba70022c77cd65fc1033005e Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Fri, 6 Jan 2023 19:31:06 -0800 Subject: [PATCH] feat: Allow moving draft documents (#4652) * feat: Allow moving draft documents * Allow drag-n-drop move of draft documents * fix: Allow moving draft without a collection * fix: Allow moving draft without a collection --- app/components/DocumentBreadcrumb.tsx | 5 +- .../Sidebar/components/CollectionLink.tsx | 13 +++-- .../Sidebar/components/DocumentLink.tsx | 24 +++----- .../components/DraggableCollectionLink.tsx | 2 +- app/models/Collection.ts | 13 +++-- server/commands/documentMover.ts | 56 ++++++++++--------- server/policies/document.test.ts | 2 +- server/policies/document.ts | 9 +-- 8 files changed, 61 insertions(+), 63 deletions(-) diff --git a/app/components/DocumentBreadcrumb.tsx b/app/components/DocumentBreadcrumb.tsx index 78e6399d9..3a8b92892 100644 --- a/app/components/DocumentBreadcrumb.tsx +++ b/app/components/DocumentBreadcrumb.tsx @@ -77,8 +77,9 @@ const DocumentBreadcrumb: React.FC = ({ } const path = React.useMemo( - () => collection?.pathToDocument?.(document.id).slice(0, -1) || [], - [collection, document] + () => collection?.pathToDocument(document.id).slice(0, -1) || [], + // eslint-disable-next-line react-hooks/exhaustive-deps + [collection, document, document.collectionId, document.parentDocumentId] ); const items = React.useMemo(() => { diff --git a/app/components/Sidebar/components/CollectionLink.tsx b/app/components/Sidebar/components/CollectionLink.tsx index 83460d27d..09a189b7a 100644 --- a/app/components/Sidebar/components/CollectionLink.tsx +++ b/app/components/Sidebar/components/CollectionLink.tsx @@ -27,7 +27,7 @@ import { useStarredContext } from "./StarredContext"; type Props = { collection: Collection; expanded?: boolean; - onDisclosureClick: (ev: React.MouseEvent) => void; + onDisclosureClick: (ev?: React.MouseEvent) => void; activeDocument: Document | undefined; isDraggingAnyCollection?: boolean; }; @@ -62,7 +62,7 @@ const CollectionLink: React.FC = ({ // Drop to re-parent document const [{ isOver, canDrop }, drop] = useDrop({ accept: "document", - drop: (item: DragObject, monitor) => { + drop: async (item: DragObject, monitor) => { const { id, collectionId } = item; if (monitor.didDrop()) { return; @@ -81,7 +81,8 @@ const CollectionLink: React.FC = ({ if ( prevCollection && prevCollection.permission === null && - prevCollection.permission !== collection.permission + prevCollection.permission !== collection.permission && + !document?.isDraft ) { itemRef.current = item; @@ -97,7 +98,11 @@ const CollectionLink: React.FC = ({ ), }); } else { - documents.move(id, collection.id); + await documents.move(id, collection.id); + + if (!expanded) { + onDisclosureClick(); + } } }, canDrop: () => canUpdate, diff --git a/app/components/Sidebar/components/DocumentLink.tsx b/app/components/Sidebar/components/DocumentLink.tsx index 62bc8887f..bc7ce632a 100644 --- a/app/components/Sidebar/components/DocumentLink.tsx +++ b/app/components/Sidebar/components/DocumentLink.tsx @@ -105,8 +105,7 @@ function InnerDocumentLink( const handleDisclosureClick = React.useCallback( (ev) => { - ev.preventDefault(); - ev.stopPropagation(); + ev?.preventDefault(); setExpanded(!expanded); }, [expanded] @@ -150,14 +149,10 @@ function InnerDocumentLink( collect: (monitor) => ({ isDragging: monitor.isDragging(), }), - canDrag: () => { - return ( - !isDraft && - (policies.abilities(node.id).move || - policies.abilities(node.id).archive || - policies.abilities(node.id).delete) - ); - }, + canDrag: () => + policies.abilities(node.id).move || + policies.abilities(node.id).archive || + policies.abilities(node.id).delete, }); const hoverExpanding = React.useRef>(); @@ -174,19 +169,18 @@ function InnerDocumentLink( // Drop to re-parent const [{ isOverReparent, canDropToReparent }, dropToReparent] = useDrop({ accept: "document", - drop: (item: DragObject, monitor) => { + drop: async (item: DragObject, monitor) => { if (monitor.didDrop()) { return; } if (!collection) { return; } - documents.move(item.id, collection.id, node.id); + await documents.move(item.id, collection.id, node.id); + setExpanded(true); }, canDrop: (_item, monitor) => - !isDraft && - !!pathToNode && - !pathToNode.includes(monitor.getItem().id), + !!pathToNode && !pathToNode.includes(monitor.getItem().id), hover: (_item, monitor) => { // Enables expansion of document children when hovering over the document // for more than half a second. diff --git a/app/components/Sidebar/components/DraggableCollectionLink.tsx b/app/components/Sidebar/components/DraggableCollectionLink.tsx index b2f17ab4c..00c1fd47e 100644 --- a/app/components/Sidebar/components/DraggableCollectionLink.tsx +++ b/app/components/Sidebar/components/DraggableCollectionLink.tsx @@ -91,7 +91,7 @@ function DraggableCollectionLink({ }, [collection.id, ui.activeCollectionId, locationStateStarred]); const handleDisclosureClick = React.useCallback((ev) => { - ev.preventDefault(); + ev?.preventDefault(); setExpanded((e) => !e); }, []); diff --git a/app/models/Collection.ts b/app/models/Collection.ts index bd52b708e..21569f0e4 100644 --- a/app/models/Collection.ts +++ b/app/models/Collection.ts @@ -171,8 +171,11 @@ export default class Collection extends ParanoidModel { } pathToDocument(documentId: string) { - let path: NavigationNode[] | undefined; + let path: NavigationNode[] | undefined = []; const document = this.store.rootStore.documents.get(documentId); + if (!document) { + return path; + } const travelNodes = ( nodes: NavigationNode[], @@ -187,8 +190,8 @@ export default class Collection extends ParanoidModel { } if ( - document?.parentDocumentId && - node?.id === document?.parentDocumentId + document.parentDocumentId && + node.id === document.parentDocumentId ) { path = [...newPath, document.asNavigationNode]; return; @@ -199,10 +202,10 @@ export default class Collection extends ParanoidModel { }; if (this.documents) { - travelNodes(this.documents, []); + travelNodes(this.documents, path); } - return path || []; + return path; } @action diff --git a/server/commands/documentMover.ts b/server/commands/documentMover.ts index e37c72ff0..36f038821 100644 --- a/server/commands/documentMover.ts +++ b/server/commands/documentMover.ts @@ -67,43 +67,45 @@ async function documentMover({ invariant(newCollection, "collection should exist"); - // Remove the document from the current collection - const response = await collection?.removeDocumentInStructure(document, { - transaction, - }); + if (document.publishedAt) { + // Remove the document from the current collection + const response = await collection?.removeDocumentInStructure(document, { + transaction, + }); - const documentJson = response?.[0]; - const fromIndex = response?.[1] || 0; + const documentJson = response?.[0]; + const fromIndex = response?.[1] || 0; - if (!documentJson) { - throw ValidationError("The document was not found in the collection"); + if (!documentJson) { + throw ValidationError("The document was not found in the collection"); + } + + // if we're reordering from within the same parent + // the original and destination collection are the same, + // so when the initial item is removed above, the list will reduce by 1. + // We need to compensate for this when reordering + const toIndex = + index !== undefined && + document.parentDocumentId === parentDocumentId && + document.collectionId === collectionId && + fromIndex < index + ? index - 1 + : index; + + // Add the document and it's tree to the new collection + await newCollection.addDocumentToStructure(document, toIndex, { + documentJson, + transaction, + }); } - // if we're reordering from within the same parent - // the original and destination collection are the same, - // so when the initial item is removed above, the list will reduce by 1. - // We need to compensate for this when reordering - const toIndex = - index !== undefined && - document.parentDocumentId === parentDocumentId && - document.collectionId === collectionId && - fromIndex < index - ? index - 1 - : index; - // Update the properties on the document record document.collectionId = collectionId; document.parentDocumentId = parentDocumentId; document.lastModifiedById = user.id; document.updatedBy = user; - // Add the document and it's tree to the new collection - await newCollection.addDocumentToStructure(document, toIndex, { - documentJson, - transaction, - }); - - if (collection) { + if (collection && document.publishedAt) { result.collections.push(collection); } diff --git a/server/policies/document.test.ts b/server/policies/document.test.ts index 627837556..9646286ac 100644 --- a/server/policies/document.test.ts +++ b/server/policies/document.test.ts @@ -128,7 +128,7 @@ describe("no collection", () => { expect(abilities.createChildDocument).toEqual(false); expect(abilities.delete).toEqual(true); expect(abilities.download).toEqual(true); - expect(abilities.move).toEqual(false); + expect(abilities.move).toEqual(true); expect(abilities.permanentDelete).toEqual(false); expect(abilities.pin).toEqual(false); expect(abilities.pinToHome).toEqual(false); diff --git a/server/policies/document.ts b/server/policies/document.ts index ac5e612a7..f7ee39f69 100644 --- a/server/policies/document.ts +++ b/server/policies/document.ts @@ -174,14 +174,7 @@ allow(User, "move", Document, (user, document) => { if (document.deletedAt) { return false; } - if (!document.publishedAt) { - return false; - } - invariant( - document.collection, - "collection is missing, did you forget to include in the query scope?" - ); - if (cannot(user, "update", document.collection)) { + if (document.collection && cannot(user, "update", document.collection)) { return false; } return user.teamId === document.teamId;