feat: Allow moving templates between collections (#1454)

- Allow template move in document policy
- fix: Ensure that document is not added to collection structure in documentMover command
- fix: Moving a template should now show nested documents as options
- fix: Hitting 'm' should not allow moving a draft
- fix: Styling of seperators on move screen

app/components/PathToDocument.js

@@ -63,7 +63,7 @@ const Title = styled.span`
 `;
 
 const StyledGoToIcon = styled(GoToIcon)`
-  opacity: 0.25;
+  fill: ${(props) => props.theme.divider};
 `;
 
 const ResultWrapper = styled.div`

app/scenes/Document/components/Document.js

@@ -133,7 +133,7 @@ class DocumentScene extends React.Component<Props> {
     ev.preventDefault();
     const { document, abilities } = this.props;
 
-    if (abilities.update) {
+    if (abilities.move) {
       this.props.history.push(documentMoveUrl(document));
     }
   }

app/scenes/Document/components/DocumentMove.js

@@ -52,6 +52,7 @@ class DocumentMove extends React.Component<Props> {
   @computed
   get results(): DocumentPath[] {
     const { document, collections } = this.props;
+    const onlyShowCollections = document.isTemplate;
 
     let results = [];
     if (collections.isLoaded) {
@@ -62,17 +63,23 @@ class DocumentMove extends React.Component<Props> {
       }
     }
 
-    // Exclude root from search results if document is already at the root
-    if (!document.parentDocumentId) {
-      results = results.filter((result) => result.id !== document.collectionId);
-    }
+    if (onlyShowCollections) {
+      results = results.filter((result) => result.type === "collection");
+    } else {
+      // Exclude root from search results if document is already at the root
+      if (!document.parentDocumentId) {
+        results = results.filter(
+          (result) => result.id !== document.collectionId
+        );
+      }
 
-    // Exclude document if on the path to result, or the same result
-    results = results.filter(
-      (result) =>
-        !result.path.map((doc) => doc.id).includes(document.id) &&
-        last(result.path.map((doc) => doc.id)) !== document.parentDocumentId
-    );
+      // Exclude document if on the path to result, or the same result
+      results = results.filter(
+        (result) =>
+          !result.path.map((doc) => doc.id).includes(document.id) &&
+          last(result.path.map((doc) => doc.id)) !== document.parentDocumentId
+      );
+    }
 
     return results;
   }

server/commands/documentMover.js

@@ -22,80 +22,95 @@ export default async function documentMover({
   const result = { collections: [], documents: [] };
   const collectionChanged = collectionId !== document.collectionId;
 
-  try {
-    transaction = await sequelize.transaction();
+  if (document.template) {
+    if (!collectionChanged) {
+      return result;
+    }
 
-    // remove from original collection
-    const collection = await document.getCollection({ transaction });
-    const documentJson = await collection.removeDocumentInStructure(document, {
-      save: false,
-    });
-
-    // if the collection is the same then it will get saved below, this
-    // line prevents a pointless intermediate save from occurring.
-    if (collectionChanged) await collection.save({ transaction });
-
-    // add to new collection (may be the same)
     document.collectionId = collectionId;
-    document.parentDocumentId = parentDocumentId;
+    document.parentDocumentId = null;
 
-    const newCollection: Collection = collectionChanged
-      ? await Collection.findByPk(collectionId, { transaction })
-      : collection;
-    await newCollection.addDocumentToStructure(document, index, {
-      documentJson,
-    });
-    result.collections.push(collection);
-
-    // if collection does not remain the same loop through children and change their
-    // collectionId too. This includes archived children, otherwise their collection
-    // would be wrong once restored.
-    if (collectionChanged) {
-      result.collections.push(newCollection);
-
-      const loopChildren = async (documentId) => {
-        const childDocuments = await Document.findAll({
-          where: { parentDocumentId: documentId },
-        });
-
-        await Promise.all(
-          childDocuments.map(async (child) => {
-            await loopChildren(child.id);
-            await child.update({ collectionId }, { transaction });
-            child.collection = newCollection;
-            result.documents.push(child);
-          })
-        );
-      };
-
-      await loopChildren(document.id);
-    }
-
-    await document.save({ transaction });
+    await document.save();
     result.documents.push(document);
+  } else {
+    try {
+      transaction = await sequelize.transaction();
 
-    await transaction.commit();
+      // remove from original collection
+      const collection = await document.getCollection({ transaction });
+      const documentJson = await collection.removeDocumentInStructure(
+        document,
+        {
+          save: false,
+        }
+      );
 
-    await Event.create({
-      name: "documents.move",
-      actorId: user.id,
-      documentId: document.id,
-      collectionId,
-      teamId: document.teamId,
-      data: {
-        title: document.title,
-        collectionIds: result.collections.map((c) => c.id),
-        documentIds: result.documents.map((d) => d.id),
-      },
-      ip,
-    });
-  } catch (err) {
-    if (transaction) {
-      await transaction.rollback();
+      // if the collection is the same then it will get saved below, this
+      // line prevents a pointless intermediate save from occurring.
+      if (collectionChanged) await collection.save({ transaction });
+
+      // add to new collection (may be the same)
+      document.collectionId = collectionId;
+      document.parentDocumentId = parentDocumentId;
+
+      const newCollection: Collection = collectionChanged
+        ? await Collection.findByPk(collectionId, { transaction })
+        : collection;
+      await newCollection.addDocumentToStructure(document, index, {
+        documentJson,
+      });
+      result.collections.push(collection);
+
+      // if collection does not remain the same loop through children and change their
+      // collectionId too. This includes archived children, otherwise their collection
+      // would be wrong once restored.
+      if (collectionChanged) {
+        result.collections.push(newCollection);
+
+        const loopChildren = async (documentId) => {
+          const childDocuments = await Document.findAll({
+            where: { parentDocumentId: documentId },
+          });
+
+          await Promise.all(
+            childDocuments.map(async (child) => {
+              await loopChildren(child.id);
+              await child.update({ collectionId }, { transaction });
+              child.collection = newCollection;
+              result.documents.push(child);
+            })
+          );
+        };
+
+        await loopChildren(document.id);
+      }
+
+      await document.save({ transaction });
+      result.documents.push(document);
+
+      await transaction.commit();
+    } catch (err) {
+      if (transaction) {
+        await transaction.rollback();
+      }
+      throw err;
     }
-    throw err;
   }
 
+  await Event.create({
+    name: "documents.move",
+    actorId: user.id,
+    documentId: document.id,
+    collectionId,
+    teamId: document.teamId,
+    data: {
+      title: document.title,
+      collectionIds: result.collections.map((c) => c.id),
+      documentIds: result.documents.map((d) => d.id),
+    },
+    ip,
+  });
+
   // we need to send all updated models back to the client
   return result;
 }

server/policies/document.js

@@ -71,7 +71,21 @@ allow(User, "createChildDocument", Document, (user, document) => {
   return user.teamId === document.teamId;
 });
 
-allow(User, ["move", "pin", "unpin"], Document, (user, document) => {
+allow(User, "move", Document, (user, document) => {
+  if (document.archivedAt) return false;
+  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)) return false;
+
+  return user.teamId === document.teamId;
+});
+
+allow(User, ["pin", "unpin"], Document, (user, document) => {
   if (document.archivedAt) return false;
   if (document.deletedAt) return false;
   if (document.template) return false;
@@ -112,16 +126,16 @@ allow(User, "restore", Document, (user, document) => {
 });
 
 allow(User, "archive", Document, (user, document) => {
+  if (!document.publishedAt) return false;
+  if (document.archivedAt) return false;
+  if (document.deletedAt) 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;
 
-  if (!document.publishedAt) return false;
-  if (document.archivedAt) return false;
-  if (document.deletedAt) return false;
-
   return user.teamId === document.teamId;
 });