perf: Requesting less db columns when calculating collection permissions (#3498)

perf: Not looping collection documentStructure for unpublish permission calculation

server/models/Document.ts

@@ -98,11 +98,12 @@ export const DOCUMENT_VERSION = 2;
   },
 }))
 @Scopes(() => ({
-  withCollection: (userId: string, paranoid = true) => {
+  withCollectionPermissions: (userId: string, paranoid = true) => {
     if (userId) {
       return {
         include: [
           {
+            attributes: ["id", "permission", "sharing", "teamId", "deletedAt"],
             model: Collection.scope({
               method: ["withMembership", userId],
             }),
@@ -116,8 +117,10 @@ export const DOCUMENT_VERSION = 2;
     return {
       include: [
         {
+          attributes: ["id", "permission", "sharing", "teamId", "deletedAt"],
           model: Collection,
           as: "collection",
+          paranoid,
         },
       ],
     };
@@ -127,6 +130,14 @@ export const DOCUMENT_VERSION = 2;
       exclude: ["state"],
     },
   },
+  withCollection: {
+    include: [
+      {
+        model: Collection,
+        as: "collection",
+      },
+    ],
+  },
   withState: {
     attributes: {
       // resets to include the state column
@@ -390,7 +401,7 @@ class Document extends ParanoidModel {
 
   static defaultScopeWithUser(userId: string) {
     const collectionScope: Readonly<ScopeOptions> = {
-      method: ["withCollection", userId],
+      method: ["withCollectionPermissions", userId],
     };
     const viewScope: Readonly<ScopeOptions> = {
       method: ["withViews", userId],
@@ -410,7 +421,7 @@ class Document extends ParanoidModel {
       "withoutState",
       "withDrafts",
       {
-        method: ["withCollection", options.userId, options.paranoid],
+        method: ["withCollectionPermissions", options.userId, options.paranoid],
       },
       {
         method: ["withViews", options.userId],
@@ -670,7 +681,7 @@ class Document extends ParanoidModel {
         method: ["withViews", user.id],
       },
       {
-        method: ["withCollection", user.id],
+        method: ["withCollectionPermissions", user.id],
       },
     ]).findAll({
       where: {

server/models/Share.ts

@@ -30,7 +30,7 @@ import Fix from "./decorators/Fix";
   ],
 }))
 @Scopes(() => ({
-  withCollection: (userId: string) => {
+  withCollectionPermissions: (userId: string) => {
     return {
       include: [
         {
@@ -39,6 +39,13 @@ import Fix from "./decorators/Fix";
           as: "document",
           include: [
             {
+              attributes: [
+                "id",
+                "permission",
+                "sharing",
+                "teamId",
+                "deletedAt",
+              ],
               model: Collection.scope({
                 method: ["withMembership", userId],
               }),

server/policies/document.ts

@@ -1,6 +1,5 @@
 import invariant from "invariant";
 import { Document, Revision, User, Team } from "@server/models";
-import { NavigationNode } from "~/types";
 import { allow, _cannot as cannot } from "./cancan";
 
 allow(User, "createDocument", Team, (user, team) => {
@@ -327,18 +326,5 @@ allow(User, "unpublish", Document, (user, document) => {
   if (cannot(user, "update", document.collection)) {
     return false;
   }
-  const documentID = document.id;
-
-  const hasChild = (documents: NavigationNode[]): boolean =>
-    documents.some((doc) => {
-      if (doc.id === documentID) {
-        return doc.children.length > 0;
-      }
-      return hasChild(doc.children);
-    });
-
-  return (
-    !hasChild(document.collection.documentStructure || []) &&
-    user.teamId === document.teamId
-  );
+  return user.teamId === document.teamId;
 });

server/routes/api/documents.ts

@@ -175,7 +175,7 @@ router.post("documents.archived", auth(), pagination(), async (ctx) => {
   const { user } = ctx.state;
   const collectionIds = await user.collectionIds();
   const collectionScope: Readonly<ScopeOptions> = {
-    method: ["withCollection", user.id],
+    method: ["withCollectionPermissions", user.id],
   };
   const viewScope: Readonly<ScopeOptions> = {
     method: ["withViews", user.id],
@@ -221,7 +221,7 @@ router.post("documents.deleted", auth(), pagination(), async (ctx) => {
     paranoid: false,
   });
   const collectionScope: Readonly<ScopeOptions> = {
-    method: ["withCollection", user.id],
+    method: ["withCollectionPermissions", user.id],
   };
   const viewScope: Readonly<ScopeOptions> = {
     method: ["withViews", user.id],
@@ -359,7 +359,7 @@ router.post("documents.drafts", auth(), pagination(), async (ctx) => {
   }
 
   const collectionScope: Readonly<ScopeOptions> = {
-    method: ["withCollection", user.id],
+    method: ["withCollectionPermissions", user.id],
   };
   const documents = await Document.scope([
     "defaultScope",
@@ -710,7 +710,7 @@ router.post("documents.search_titles", auth(), pagination(), async (ctx) => {
       method: ["withViews", user.id],
     },
     {
-      method: ["withCollection", user.id],
+      method: ["withCollectionPermissions", user.id],
     },
   ]).findAll({
     where: {
@@ -1219,6 +1219,11 @@ router.post("documents.unpublish", auth(), async (ctx) => {
   });
   authorize(user, "unpublish", document);
 
+  const childDocumentIds = await document.getChildDocumentIds();
+  if (childDocumentIds.length > 0) {
+    throw InvalidRequestError("Cannot unpublish document with child documents");
+  }
+
   await document.unpublish(user.id);
   await Event.create({
     name: "documents.unpublish",

server/routes/api/shares.ts

@@ -17,7 +17,7 @@ router.post("shares.info", auth(), async (ctx) => {
   const { user } = ctx.state;
   const shares = [];
   const share = await Share.scope({
-    method: ["withCollection", user.id],
+    method: ["withCollectionPermissions", user.id],
   }).findOne({
     where: id
       ? {
@@ -58,13 +58,17 @@ router.post("shares.info", auth(), async (ctx) => {
   }
 
   if (documentId) {
-    const document = await Document.scope("withCollection").findByPk(
-      documentId
-    );
+    const document = await Document.unscoped()
+      .scope("withCollection")
+      .findOne({
+        where: {
+          id: documentId,
+        },
+      });
     const parentIds = document?.collection?.getDocumentParents(documentId);
     const parentShare = parentIds
       ? await Share.scope({
-          method: ["withCollection", user.id],
+          method: ["withCollectionPermissions", user.id],
         }).findOne({
           where: {
             documentId: parentIds,
@@ -177,7 +181,7 @@ router.post("shares.update", auth(), async (ctx) => {
 
   // fetch the share with document and collection.
   const share = await Share.scope({
-    method: ["withCollection", user.id],
+    method: ["withCollectionPermissions", user.id],
   }).findByPk(id);
 
   authorize(user, "update", share);