chore: improve collections router

server/commands/teamUpdater.ts

@@ -1,7 +1,6 @@
 import { has } from "lodash";
 import { Transaction } from "sequelize";
 import { TeamPreference } from "@shared/types";
-import { sequelize } from "@server/database/sequelize";
 import env from "@server/env";
 import { Event, Team, TeamDomain, User } from "@server/models";
 
@@ -10,9 +9,16 @@ type TeamUpdaterProps = {
   ip?: string;
   user: User;
   team: Team;
+  transaction: Transaction;
 };
 
-const teamUpdater = async ({ params, user, team, ip }: TeamUpdaterProps) => {
+const teamUpdater = async ({
+  params,
+  user,
+  team,
+  ip,
+  transaction,
+}: TeamUpdaterProps) => {
   const {
     name,
     avatarUrl,
@@ -28,8 +34,6 @@ const teamUpdater = async ({ params, user, team, ip }: TeamUpdaterProps) => {
     preferences,
   } = params;
 
-  const transaction: Transaction = await sequelize.transaction();
-
   if (subdomain !== undefined && env.SUBDOMAINS_ENABLED) {
     team.subdomain = subdomain === "" ? null : subdomain;
   }
@@ -110,35 +114,31 @@ const teamUpdater = async ({ params, user, team, ip }: TeamUpdaterProps) => {
 
   const changes = team.changed();
 
-  try {
-    const savedTeam = await team.save({
-      transaction,
-    });
-    if (changes) {
-      const data = changes.reduce(
-        (acc, curr) => ({ ...acc, [curr]: team[curr] }),
-        {}
-      );
+  const savedTeam = await team.save({
+    transaction,
+  });
 
-      await Event.create(
-        {
-          name: "teams.update",
-          actorId: user.id,
-          teamId: user.teamId,
-          data,
-          ip,
-        },
-        {
-          transaction,
-        }
-      );
-    }
-    await transaction.commit();
-    return savedTeam;
-  } catch (error) {
-    await transaction.rollback();
-    throw error;
+  if (changes) {
+    const data = changes.reduce(
+      (acc, curr) => ({ ...acc, [curr]: team[curr] }),
+      {}
+    );
+
+    await Event.create(
+      {
+        name: "teams.update",
+        actorId: user.id,
+        teamId: user.teamId,
+        data,
+        ip,
+      },
+      {
+        transaction,
+      }
+    );
   }
+
+  return savedTeam;
 };
 
 export default teamUpdater;

server/routes/api/collections.ts

@@ -12,7 +12,6 @@ import {
 import { colorPalette } from "@shared/utils/collections";
 import collectionExporter from "@server/commands/collectionExporter";
 import teamUpdater from "@server/commands/teamUpdater";
-import { sequelize } from "@server/database/sequelize";
 import { ValidationError } from "@server/errors";
 import auth from "@server/middlewares/authentication";
 import { rateLimiter } from "@server/middlewares/rateLimiter";
@@ -168,7 +167,9 @@ router.post(
   "collections.import",
   rateLimiter(RateLimiterStrategy.TenPerHour),
   auth(),
+  transaction(),
   async (ctx: APIContext) => {
+    const { transaction } = ctx.state;
     const { attachmentId, format = FileOperationFormat.MarkdownZip } =
       ctx.request.body;
     assertUuid(attachmentId, "attachmentId is required");
@@ -181,37 +182,35 @@ router.post(
 
     assertIn(format, Object.values(FileOperationFormat), "Invalid format");
 
-    await sequelize.transaction(async (transaction) => {
-      const fileOperation = await FileOperation.create(
-        {
-          type: FileOperationType.Import,
-          state: FileOperationState.Creating,
-          format,
-          size: attachment.size,
-          key: attachment.key,
-          userId: user.id,
-          teamId: user.teamId,
-        },
-        {
-          transaction,
-        }
-      );
+    const fileOperation = await FileOperation.create(
+      {
+        type: FileOperationType.Import,
+        state: FileOperationState.Creating,
+        format,
+        size: attachment.size,
+        key: attachment.key,
+        userId: user.id,
+        teamId: user.teamId,
+      },
+      {
+        transaction,
+      }
+    );
 
-      await Event.create(
-        {
-          name: "fileOperations.create",
-          teamId: user.teamId,
-          actorId: user.id,
-          modelId: fileOperation.id,
-          data: {
-            type: FileOperationType.Import,
-          },
+    await Event.create(
+      {
+        name: "fileOperations.create",
+        teamId: user.teamId,
+        actorId: user.id,
+        modelId: fileOperation.id,
+        data: {
+          type: FileOperationType.Import,
         },
-        {
-          transaction,
-        }
-      );
-    });
+      },
+      {
+        transaction,
+      }
+    );
 
     ctx.body = {
       success: true,
@@ -561,7 +560,9 @@ router.post(
   "collections.export",
   rateLimiter(RateLimiterStrategy.TenPerHour),
   auth(),
+  transaction(),
   async (ctx: APIContext) => {
+    const { transaction } = ctx.state;
     const { id } = ctx.request.body;
     const {
       format = FileOperationFormat.MarkdownZip,
@@ -581,17 +582,15 @@ router.post(
     }).findByPk(id);
     authorize(user, "export", collection);
 
-    const fileOperation = await sequelize.transaction(async (transaction) =>
-      collectionExporter({
-        collection,
-        user,
-        team,
-        format,
-        includeAttachments,
-        ip: ctx.request.ip,
-        transaction,
-      })
-    );
+    const fileOperation = await collectionExporter({
+      collection,
+      user,
+      team,
+      format,
+      includeAttachments,
+      ip: ctx.request.ip,
+      transaction,
+    });
 
     ctx.body = {
       success: true,
@@ -606,7 +605,9 @@ router.post(
   "collections.export_all",
   rateLimiter(RateLimiterStrategy.FivePerHour),
   auth(),
+  transaction(),
   async (ctx: APIContext) => {
+    const { transaction } = ctx.state;
     const {
       format = FileOperationFormat.MarkdownZip,
       includeAttachments = true,
@@ -618,16 +619,14 @@ router.post(
     assertIn(format, Object.values(FileOperationFormat), "Invalid format");
     assertBoolean(includeAttachments, "includeAttachments must be a boolean");
 
-    const fileOperation = await sequelize.transaction(async (transaction) =>
-      collectionExporter({
-        user,
-        team,
-        format,
-        includeAttachments,
-        ip: ctx.request.ip,
-        transaction,
-      })
-    );
+    const fileOperation = await collectionExporter({
+      user,
+      team,
+      format,
+      includeAttachments,
+      ip: ctx.request.ip,
+      transaction,
+    });
 
     ctx.body = {
       success: true,
@@ -638,124 +637,147 @@ router.post(
   }
 );
 
-router.post("collections.update", auth(), async (ctx: APIContext) => {
-  const { id, name, description, icon, permission, color, sort, sharing } =
-    ctx.request.body;
+router.post(
+  "collections.update",
+  auth(),
+  transaction(),
+  async (ctx: APIContext) => {
+    const { transaction } = ctx.state;
+    const { id, name, description, icon, permission, color, sort, sharing } =
+      ctx.request.body;
 
-  if (color) {
-    assertHexColor(color, "Invalid hex value (please use format #FFFFFF)");
-  }
-
-  const { user } = ctx.state.auth;
-  const collection = await Collection.scope({
-    method: ["withMembership", user.id],
-  }).findByPk(id);
-  authorize(user, "update", collection);
-
-  // we're making this collection have no default access, ensure that the
-  // current user has an admin membership so that at least they can manage it.
-  if (
-    permission !== CollectionPermission.ReadWrite &&
-    collection.permission === CollectionPermission.ReadWrite
-  ) {
-    await CollectionUser.findOrCreate({
-      where: {
-        collectionId: collection.id,
-        userId: user.id,
-      },
-      defaults: {
-        permission: CollectionPermission.Admin,
-        createdById: user.id,
-      },
-    });
-  }
-
-  let privacyChanged = false;
-  let sharingChanged = false;
-
-  if (name !== undefined) {
-    collection.name = name.trim();
-  }
-
-  if (description !== undefined) {
-    collection.description = description;
-  }
-
-  if (icon !== undefined) {
-    collection.icon = icon;
-  }
-
-  if (color !== undefined) {
-    collection.color = color;
-  }
-
-  if (permission !== undefined) {
-    if (permission) {
-      assertCollectionPermission(permission);
+    if (color) {
+      assertHexColor(color, "Invalid hex value (please use format #FFFFFF)");
     }
-    privacyChanged = permission !== collection.permission;
-    collection.permission = permission ? permission : null;
-  }
 
-  if (sharing !== undefined) {
-    sharingChanged = sharing !== collection.sharing;
-    collection.sharing = sharing;
-  }
-
-  if (sort !== undefined) {
-    collection.sort = sort;
-  }
-
-  await collection.save();
-  await Event.create({
-    name: "collections.update",
-    collectionId: collection.id,
-    teamId: collection.teamId,
-    actorId: user.id,
-    data: {
-      name,
-    },
-    ip: ctx.request.ip,
-  });
-
-  if (privacyChanged || sharingChanged) {
-    await Event.create({
-      name: "collections.permission_changed",
-      collectionId: collection.id,
-      teamId: collection.teamId,
-      actorId: user.id,
-      data: {
-        privacyChanged,
-        sharingChanged,
-      },
-      ip: ctx.request.ip,
+    const { user } = ctx.state.auth;
+    const collection = await Collection.scope({
+      method: ["withMembership", user.id],
+    }).findByPk(id, {
+      transaction,
     });
-  }
-
-  // must reload to update collection membership for correct policy calculation
-  // if the privacy level has changed. Otherwise skip this query for speed.
-  if (privacyChanged || sharingChanged) {
-    await collection.reload();
-    const team = await Team.findByPk(user.teamId, { rejectOnEmpty: true });
+    authorize(user, "update", collection);
 
+    // we're making this collection have no default access, ensure that the
+    // current user has an admin membership so that at least they can manage it.
     if (
-      collection.permission === null &&
-      team?.defaultCollectionId === collection.id
+      permission !== CollectionPermission.ReadWrite &&
+      collection.permission === CollectionPermission.ReadWrite
     ) {
-      await teamUpdater({
-        params: { defaultCollectionId: null },
-        ip: ctx.request.ip,
-        user,
-        team,
+      await CollectionUser.findOrCreate({
+        where: {
+          collectionId: collection.id,
+          userId: user.id,
+        },
+        defaults: {
+          permission: CollectionPermission.Admin,
+          createdById: user.id,
+        },
+        transaction,
       });
     }
-  }
 
-  ctx.body = {
-    data: presentCollection(collection),
-    policies: presentPolicies(user, [collection]),
-  };
-});
+    let privacyChanged = false;
+    let sharingChanged = false;
+
+    if (name !== undefined) {
+      collection.name = name.trim();
+    }
+
+    if (description !== undefined) {
+      collection.description = description;
+    }
+
+    if (icon !== undefined) {
+      collection.icon = icon;
+    }
+
+    if (color !== undefined) {
+      collection.color = color;
+    }
+
+    if (permission !== undefined) {
+      if (permission) {
+        assertCollectionPermission(permission);
+      }
+      privacyChanged = permission !== collection.permission;
+      collection.permission = permission ? permission : null;
+    }
+
+    if (sharing !== undefined) {
+      sharingChanged = sharing !== collection.sharing;
+      collection.sharing = sharing;
+    }
+
+    if (sort !== undefined) {
+      collection.sort = sort;
+    }
+
+    await collection.save({ transaction });
+    await Event.create(
+      {
+        name: "collections.update",
+        collectionId: collection.id,
+        teamId: collection.teamId,
+        actorId: user.id,
+        data: {
+          name,
+        },
+        ip: ctx.request.ip,
+      },
+      {
+        transaction,
+      }
+    );
+
+    if (privacyChanged || sharingChanged) {
+      await Event.create(
+        {
+          name: "collections.permission_changed",
+          collectionId: collection.id,
+          teamId: collection.teamId,
+          actorId: user.id,
+          data: {
+            privacyChanged,
+            sharingChanged,
+          },
+          ip: ctx.request.ip,
+        },
+        {
+          transaction,
+        }
+      );
+    }
+
+    // must reload to update collection membership for correct policy calculation
+    // if the privacy level has changed. Otherwise skip this query for speed.
+    if (privacyChanged || sharingChanged) {
+      await collection.reload({ transaction });
+      const team = await Team.findByPk(user.teamId, {
+        transaction,
+        rejectOnEmpty: true,
+      });
+
+      if (
+        collection.permission === null &&
+        team?.defaultCollectionId === collection.id
+      ) {
+        await teamUpdater({
+          params: { defaultCollectionId: null },
+          ip: ctx.request.ip,
+          user,
+          team,
+          transaction,
+        });
+      }
+    }
+
+    ctx.body = {
+      data: presentCollection(collection),
+      policies: presentPolicies(user, [collection]),
+    };
+  }
+);
 
 router.post(
   "collections.list",
@@ -805,82 +827,112 @@ router.post(
   }
 );
 
-router.post("collections.delete", auth(), async (ctx: APIContext) => {
-  const { id } = ctx.request.body;
-  const { user } = ctx.state.auth;
-  assertUuid(id, "id is required");
+router.post(
+  "collections.delete",
+  auth(),
+  transaction(),
+  async (ctx: APIContext) => {
+    const { transaction } = ctx.state;
+    const { id } = ctx.request.body;
+    const { user } = ctx.state.auth;
+    assertUuid(id, "id is required");
 
-  const collection = await Collection.scope({
-    method: ["withMembership", user.id],
-  }).findByPk(id);
-  const team = await Team.findByPk(user.teamId);
-
-  authorize(user, "delete", collection);
-
-  const total = await Collection.count();
-  if (total === 1) {
-    throw ValidationError("Cannot delete last collection");
-  }
-
-  await collection.destroy();
-
-  if (team && team.defaultCollectionId === collection.id) {
-    await teamUpdater({
-      params: { defaultCollectionId: null },
-      ip: ctx.request.ip,
-      user,
-      team,
+    const collection = await Collection.scope({
+      method: ["withMembership", user.id],
+    }).findByPk(id, {
+      transaction,
     });
+    const team = await Team.findByPk(user.teamId);
+
+    authorize(user, "delete", collection);
+
+    const total = await Collection.count();
+    if (total === 1) {
+      throw ValidationError("Cannot delete last collection");
+    }
+
+    await collection.destroy({ transaction });
+
+    if (team && team.defaultCollectionId === collection.id) {
+      await teamUpdater({
+        params: { defaultCollectionId: null },
+        ip: ctx.request.ip,
+        user,
+        team,
+        transaction,
+      });
+    }
+
+    await Event.create(
+      {
+        name: "collections.delete",
+        collectionId: collection.id,
+        teamId: collection.teamId,
+        actorId: user.id,
+        data: {
+          name: collection.name,
+        },
+        ip: ctx.request.ip,
+      },
+      { transaction }
+    );
+
+    ctx.body = {
+      success: true,
+    };
   }
+);
 
-  await Event.create({
-    name: "collections.delete",
-    collectionId: collection.id,
-    teamId: collection.teamId,
-    actorId: user.id,
-    data: {
-      name: collection.name,
-    },
-    ip: ctx.request.ip,
-  });
+router.post(
+  "collections.move",
+  auth(),
+  transaction(),
+  async (ctx: APIContext) => {
+    const { transaction } = ctx.state;
+    const id = ctx.request.body.id;
+    let index = ctx.request.body.index;
+    assertPresent(index, "index is required");
+    assertIndexCharacters(index);
+    assertUuid(id, "id must be a uuid");
+    const { user } = ctx.state.auth;
 
-  ctx.body = {
-    success: true,
-  };
-});
+    const collection = await Collection.findByPk(id, {
+      transaction,
+    });
+    authorize(user, "move", collection);
 
-router.post("collections.move", auth(), async (ctx: APIContext) => {
-  const id = ctx.request.body.id;
-  let index = ctx.request.body.index;
-  assertPresent(index, "index is required");
-  assertIndexCharacters(index);
-  assertUuid(id, "id must be a uuid");
-  const { user } = ctx.state.auth;
+    index = await removeIndexCollision(user.teamId, index);
+    await collection.update(
+      {
+        index,
+      },
+      {
+        transaction,
+      }
+    );
+    await Event.create(
+      {
+        name: "collections.move",
+        collectionId: collection.id,
+        teamId: collection.teamId,
+        actorId: user.id,
+        data: {
+          index,
+        },
+        ip: ctx.request.ip,
+      },
+      {
+        transaction,
+      }
+    );
 
-  const collection = await Collection.findByPk(id);
-  authorize(user, "move", collection);
-
-  index = await removeIndexCollision(user.teamId, index);
-  await collection.update({
-    index,
-  });
-  await Event.create({
-    name: "collections.move",
-    collectionId: collection.id,
-    teamId: collection.teamId,
-    actorId: user.id,
-    data: {
-      index,
-    },
-    ip: ctx.request.ip,
-  });
-
-  ctx.body = {
-    success: true,
-    data: {
-      index,
-    },
-  };
-});
+    ctx.body = {
+      success: true,
+      data: {
+        index,
+      },
+    };
+  }
+);
 
 export default router;