Deleting a collection should detach associated drafts from it (#5082)

Co-authored-by: Tom Moor <tom.moor@gmail.com>

server/commands/documentCreator.ts

@@ -109,7 +109,7 @@ export default async function documentCreator({
   );
 
   if (publish) {
-    await document.publish(user.id, { transaction });
+    await document.publish(user.id, collectionId!, { transaction });
     await Event.create(
       {
         name: "documents.publish",

server/commands/documentLoader.ts

@@ -126,7 +126,9 @@ export default async function loadDocument({
     if (canReadDocument) {
       // Cannot use document.collection here as it does not include the
       // documentStructure by default through the relationship.
-      collection = await Collection.findByPk(document.collectionId);
+      if (document.collectionId) {
+        collection = await Collection.findByPk(document.collectionId);
+      }
       if (!collection) {
         throw NotFoundError("Collection could not be found for document");
       }
@@ -146,7 +148,9 @@ export default async function loadDocument({
     }
 
     // It is possible to disable sharing at the collection so we must check
-    collection = await Collection.findByPk(document.collectionId);
+    if (document.collectionId) {
+      collection = await Collection.findByPk(document.collectionId);
+    }
     invariant(collection, "collection not found");
 
     if (!collection.sharing) {

server/commands/documentMover.test.ts

@@ -153,4 +153,27 @@ describe("documentMover", () => {
     expect(response.documents[0].collection?.id).toEqual(newCollection.id);
     expect(response.documents[0].updatedBy.id).toEqual(user.id);
   });
+
+  it("should detach document from collection and move it to drafts", async () => {
+    const { document, user, collection } = await seed();
+
+    const response = await sequelize.transaction(async (transaction) =>
+      documentMover({
+        user,
+        document,
+        collectionId: null,
+        index: 0,
+        ip,
+        transaction,
+      })
+    );
+
+    expect(response.collections[0].id).toBe(collection.id);
+    expect(response.collections.length).toEqual(1);
+    expect(response.documents.length).toEqual(1);
+
+    expect(response.documents[0].collection).toBeNull();
+    expect(response.documents[0].updatedBy.id).toEqual(user.id);
+    expect(response.documents[0].publishedAt).toBeNull();
+  });
 });

server/commands/documentMover.ts

@@ -5,12 +5,19 @@ import { User, Document, Collection, Pin, Event } from "@server/models";
 import pinDestroyer from "./pinDestroyer";
 
 type Props = {
+  /** User attempting to move the document */
   user: User;
+  /** Document which is being moved */
   document: Document;
-  collectionId: string;
+  /** Destination collection to which the document is moved */
+  collectionId: string | null;
+  /** ID of parent under which the document is moved */
   parentDocumentId?: string | null;
+  /** Position of moved document within document structure */
   index?: number;
+  /** The IP address of the user moving the document */
   ip: string;
+  /** The database transaction to run within */
   transaction?: Transaction;
 };
 
@@ -23,7 +30,7 @@ type Result = {
 async function documentMover({
   user,
   document,
-  collectionId,
+  collectionId = null,
   parentDocumentId = null,
   // convert undefined to null so parentId comparison treats them as equal
   index,
@@ -43,6 +50,7 @@ async function documentMover({
   }
 
   if (document.template) {
+    invariant(collectionId, "collectionId should exist");
     document.collectionId = collectionId;
     document.parentDocumentId = null;
     document.lastModifiedById = user.id;
@@ -51,20 +59,23 @@ async function documentMover({
     result.documents.push(document);
   } else {
     // Load the current and the next collection upfront and lock them
-    const collection = await Collection.findByPk(document.collectionId, {
+    const collection = await Collection.findByPk(document.collectionId!, {
       transaction,
       lock: Transaction.LOCK.UPDATE,
       paranoid: false,
     });
 
-    let newCollection = collectionChanged
-      ? await Collection.findByPk(collectionId, {
+    let newCollection = collection;
+    if (collectionChanged) {
+      if (collectionId) {
+        newCollection = await Collection.findByPk(collectionId, {
           transaction,
           lock: Transaction.LOCK.UPDATE,
-        })
-      : collection;
-
-    invariant(newCollection, "collection should exist");
+        });
+      } else {
+        newCollection = null;
+      }
+    }
 
     if (document.publishedAt) {
       // Remove the document from the current collection
@@ -99,11 +110,13 @@ async function documentMover({
       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 (newCollection) {
+        // Add the document and it's tree to the new collection
+        await newCollection.addDocumentToStructure(document, toIndex, {
+          documentJson,
+          transaction,
+        });
+      }
     } else {
       document.collectionId = collectionId;
       document.parentDocumentId = parentDocumentId;
@@ -118,30 +131,48 @@ async function documentMover({
     // If the collection has changed then we also need to update the properties
     // on all of the documents children to reflect the new collectionId
     if (collectionChanged) {
-      // Reload the collection to get relationship data
-      newCollection = await Collection.scope({
-        method: ["withMembership", user.id],
-      }).findByPk(collectionId, {
-        transaction,
-      });
-      invariant(newCollection, "collection should exist");
-
-      result.collections.push(newCollection);
-
       // Efficiently find the ID's of all the documents that are children of
       // the moved document and update in one query
       const childDocumentIds = await document.getChildDocumentIds();
-      await Document.update(
-        {
-          collectionId: newCollection.id,
-        },
-        {
+
+      if (collectionId) {
+        // Reload the collection to get relationship data
+        newCollection = await Collection.scope({
+          method: ["withMembership", user.id],
+        }).findByPk(collectionId, {
           transaction,
-          where: {
-            id: childDocumentIds,
+        });
+        invariant(newCollection, "collection should exist");
+        result.collections.push(newCollection);
+
+        await Document.update(
+          {
+            collectionId: newCollection.id,
           },
-        }
-      );
+          {
+            transaction,
+            where: {
+              id: childDocumentIds,
+            },
+          }
+        );
+      } else {
+        // document will be moved to drafts
+        document.publishedAt = null;
+
+        // point children's parent to moved document's parent
+        await Document.update(
+          {
+            parentDocumentId: document.parentDocumentId,
+          },
+          {
+            transaction,
+            where: {
+              id: childDocumentIds,
+            },
+          }
+        );
+      }
 
       // We must reload from the database to get the relationship data
       const documents = await Document.findAll({

server/commands/documentUpdater.ts

@@ -74,7 +74,7 @@ export default async function documentUpdater({
     if (!document.collectionId) {
       document.collectionId = collectionId as string;
     }
-    await document.publish(user.id, { transaction });
+    await document.publish(user.id, collectionId!, { transaction });
 
     await Event.create(
       {

server/commands/pinCreator.ts

@@ -11,7 +11,7 @@ type Props = {
   /** The document to pin */
   documentId: string;
   /** The collection to pin the document in. If no collection is provided then it will be pinned to home */
-  collectionId?: string | undefined;
+  collectionId?: string | null;
   /** The index to pin the document at. If no index is provided then it will be pinned to the end of the collection */
   index?: string;
   /** The IP address of the user creating the pin */