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

@@ -77,8 +77,9 @@ const DocumentBreadcrumb: React.FC<Props> = ({
   }
 
   const path = React.useMemo(
-    () => collection?.pathToDocument?.(document.id).slice(0, -1) || [],
+    () => collection?.pathToDocument(document.id).slice(0, -1) || [],
-    [collection, document]
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+    [collection, document, document.collectionId, document.parentDocumentId]
   );
 
   const items = React.useMemo(() => {

app/components/Sidebar/components/CollectionLink.tsx

@@ -27,7 +27,7 @@ import { useStarredContext } from "./StarredContext";
 type Props = {
   collection: Collection;
   expanded?: boolean;
-  onDisclosureClick: (ev: React.MouseEvent<HTMLButtonElement>) => void;
+  onDisclosureClick: (ev?: React.MouseEvent<HTMLButtonElement>) => void;
   activeDocument: Document | undefined;
   isDraggingAnyCollection?: boolean;
 };
@@ -62,7 +62,7 @@ const CollectionLink: React.FC<Props> = ({
   // 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<Props> = ({
       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<Props> = ({
           ),
         });
       } else {
-        documents.move(id, collection.id);
+        await documents.move(id, collection.id);
+
+        if (!expanded) {
+          onDisclosureClick();
+        }
       }
     },
     canDrop: () => canUpdate,

app/components/Sidebar/components/DocumentLink.tsx

@@ -105,8 +105,7 @@ function InnerDocumentLink(
 
   const handleDisclosureClick = React.useCallback(
     (ev) => {
-      ev.preventDefault();
+      ev?.preventDefault();
-      ev.stopPropagation();
       setExpanded(!expanded);
     },
     [expanded]
@@ -150,14 +149,10 @@ function InnerDocumentLink(
     collect: (monitor) => ({
       isDragging: monitor.isDragging(),
     }),
-    canDrag: () => {
+    canDrag: () =>
-      return (
+      policies.abilities(node.id).move ||
-        !isDraft &&
+      policies.abilities(node.id).archive ||
-        (policies.abilities(node.id).move ||
+      policies.abilities(node.id).delete,
-          policies.abilities(node.id).archive ||
-          policies.abilities(node.id).delete)
-      );
-    },
   });
 
   const hoverExpanding = React.useRef<ReturnType<typeof setTimeout>>();
@@ -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<DragObject>().id),
-      !!pathToNode &&
-      !pathToNode.includes(monitor.getItem<DragObject>().id),
     hover: (_item, monitor) => {
       // Enables expansion of document children when hovering over the document
       // for more than half a second.

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);
   }, []);
 

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 &&
+          document.parentDocumentId &&
-          node?.id === 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

server/commands/documentMover.ts

@@ -67,43 +67,45 @@ async function documentMover({
 
     invariant(newCollection, "collection should exist");
 
-    // Remove the document from the current collection
+    if (document.publishedAt) {
-    const response = await collection?.removeDocumentInStructure(document, {
+      // Remove the document from the current collection
-      transaction,
+      const response = await collection?.removeDocumentInStructure(document, {
-    });
+        transaction,
+      });
 
-    const documentJson = response?.[0];
+      const documentJson = response?.[0];
-    const fromIndex = response?.[1] || 0;
+      const fromIndex = response?.[1] || 0;
 
-    if (!documentJson) {
+      if (!documentJson) {
-      throw ValidationError("The document was not found in the collection");
+        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
+    if (collection && document.publishedAt) {
-    await newCollection.addDocumentToStructure(document, toIndex, {
-      documentJson,
-      transaction,
-    });
-
-    if (collection) {
       result.collections.push(collection);
     }
 

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);

server/policies/document.ts

@@ -174,14 +174,7 @@ allow(User, "move", Document, (user, document) => {
   if (document.deletedAt) {
     return false;
   }
-  if (!document.publishedAt) {
+  if (document.collection && cannot(user, "update", document.collection)) {
-    return false;
-  }
-  invariant(
-    document.collection,
-    "collection is missing, did you forget to include in the query scope?"
-  );
-  if (cannot(user, "update", document.collection)) {
     return false;
   }
   return user.teamId === document.teamId;