From 28cb5aa3795bf9aa938da1a5a4ee09addae60b9b Mon Sep 17 00:00:00 2001 From: Hemachandar <132386067+hmacr@users.noreply.github.com> Date: Thu, 21 Nov 2024 05:44:11 +0530 Subject: [PATCH] Convert pin mutations to use auto event insertion (#7993) --- server/commands/pinCreator.test.ts | 29 ++++++++-------- server/commands/pinCreator.ts | 49 +++++++-------------------- server/commands/pinDestroyer.ts | 2 ++ server/commands/pinUpdater.ts | 53 ------------------------------ server/models/Pin.ts | 2 ++ server/routes/api/pins/pins.ts | 39 ++++++++++++++-------- 6 files changed, 57 insertions(+), 117 deletions(-) delete mode 100644 server/commands/pinUpdater.ts diff --git a/server/commands/pinCreator.test.ts b/server/commands/pinCreator.test.ts index 42da0c78d8..79f8d0e46c 100644 --- a/server/commands/pinCreator.test.ts +++ b/server/commands/pinCreator.test.ts @@ -1,10 +1,9 @@ import { Event } from "@server/models"; import { buildDocument, buildUser } from "@server/test/factories"; +import { withAPIContext } from "@server/test/support"; import pinCreator from "./pinCreator"; describe("pinCreator", () => { - const ip = "127.0.0.1"; - it("should create pin to home", async () => { const user = await buildUser(); const document = await buildDocument({ @@ -12,11 +11,13 @@ describe("pinCreator", () => { teamId: user.teamId, }); - const pin = await pinCreator({ - documentId: document.id, - user, - ip, - }); + const pin = await withAPIContext(user, (ctx) => + pinCreator({ + ctx, + user, + documentId: document.id, + }) + ); const event = await Event.findLatest({ teamId: user.teamId, @@ -36,12 +37,14 @@ describe("pinCreator", () => { teamId: user.teamId, }); - const pin = await pinCreator({ - documentId: document.id, - collectionId: document.collectionId, - user, - ip, - }); + const pin = await withAPIContext(user, (ctx) => + pinCreator({ + ctx, + user, + documentId: document.id, + collectionId: document.collectionId, + }) + ); const event = await Event.findLatest({ teamId: user.teamId, diff --git a/server/commands/pinCreator.ts b/server/commands/pinCreator.ts index cab9caa5c5..6c5adeb04a 100644 --- a/server/commands/pinCreator.ts +++ b/server/commands/pinCreator.ts @@ -2,10 +2,12 @@ import fractionalIndex from "fractional-index"; import { Sequelize, Op, WhereOptions } from "sequelize"; import { PinValidation } from "@shared/validations"; import { ValidationError } from "@server/errors"; -import { Pin, User, Event } from "@server/models"; -import { sequelize } from "@server/storage/database"; +import { Pin, User } from "@server/models"; +import { APIContext } from "@server/types"; type Props = { + /** The request context */ + ctx: APIContext; /** The user creating the pin */ user: User; /** The document to pin */ @@ -14,8 +16,6 @@ type Props = { 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 */ - ip: string; }; /** @@ -26,10 +26,10 @@ type Props = { * @returns Pin The pin that was created */ export default async function pinCreator({ + ctx, user, documentId, collectionId, - ip, ...rest }: Props): Promise { let { index } = rest; @@ -62,38 +62,13 @@ export default async function pinCreator({ index = fractionalIndex(pins.length ? pins[0].index : null, null); } - const transaction = await sequelize.transaction(); - let pin; - - try { - pin = await Pin.create( - { - createdById: user.id, - teamId: user.teamId, - collectionId, - documentId, - index, - }, - { transaction } - ); - - await Event.create( - { - name: "pins.create", - modelId: pin.id, - teamId: user.teamId, - actorId: user.id, - documentId, - collectionId, - ip, - }, - { transaction } - ); - await transaction.commit(); - } catch (err) { - await transaction.rollback(); - throw err; - } + const pin = await Pin.createWithCtx(ctx, { + createdById: user.id, + teamId: user.teamId, + collectionId, + documentId, + index, + }); return pin; } diff --git a/server/commands/pinDestroyer.ts b/server/commands/pinDestroyer.ts index 594f4bcbf1..af59e095d0 100644 --- a/server/commands/pinDestroyer.ts +++ b/server/commands/pinDestroyer.ts @@ -13,6 +13,8 @@ type Props = { }; /** + * @deprecated use pin.destroyWithCtx instead. This will be removed once document routes migrate to auto event insertion using APIContext. + * * This command destroys a document pin. This just removes the pin itself and * does not touch the document * diff --git a/server/commands/pinUpdater.ts b/server/commands/pinUpdater.ts deleted file mode 100644 index e87173f481..0000000000 --- a/server/commands/pinUpdater.ts +++ /dev/null @@ -1,53 +0,0 @@ -import { Event, Pin, User } from "@server/models"; -import { sequelize } from "@server/storage/database"; - -type Props = { - /** The user updating the pin */ - user: User; - /** The existing pin */ - pin: Pin; - /** The index to pin the document at */ - index: string; - /** The IP address of the user creating the pin */ - ip: string; -}; - -/** - * This command updates a "pinned" document. A pin can only be moved to a new - * index (reordered) once created. - * - * @param Props The properties of the pin to update - * @returns Pin The updated pin - */ -export default async function pinUpdater({ - user, - pin, - index, - ip, -}: Props): Promise { - const transaction = await sequelize.transaction(); - - try { - pin.index = index; - await pin.save({ transaction }); - - await Event.create( - { - name: "pins.update", - modelId: pin.id, - teamId: user.teamId, - actorId: user.id, - documentId: pin.documentId, - collectionId: pin.collectionId, - ip, - }, - { transaction } - ); - await transaction.commit(); - } catch (err) { - await transaction.rollback(); - throw err; - } - - return pin; -} diff --git a/server/models/Pin.ts b/server/models/Pin.ts index 0234194a6a..bebd850358 100644 --- a/server/models/Pin.ts +++ b/server/models/Pin.ts @@ -20,6 +20,8 @@ class Pin extends IdModel< InferAttributes, Partial> > { + static eventNamespace = "pins"; + @Length({ max: 256, msg: `index must be 256 characters or less`, diff --git a/server/routes/api/pins/pins.ts b/server/routes/api/pins/pins.ts index c359866ff2..015661a532 100644 --- a/server/routes/api/pins/pins.ts +++ b/server/routes/api/pins/pins.ts @@ -1,9 +1,8 @@ import Router from "koa-router"; -import { Sequelize, Op } from "sequelize"; +import { Sequelize, Op, Transaction } from "sequelize"; import pinCreator from "@server/commands/pinCreator"; -import pinDestroyer from "@server/commands/pinDestroyer"; -import pinUpdater from "@server/commands/pinUpdater"; import auth from "@server/middlewares/authentication"; +import { transaction } from "@server/middlewares/transaction"; import validate from "@server/middlewares/validate"; import { Collection, Document, Pin } from "@server/models"; import { authorize } from "@server/policies"; @@ -22,18 +21,21 @@ router.post( "pins.create", auth(), validate(T.PinsCreateSchema), + transaction(), async (ctx: APIContext) => { const { documentId, collectionId, index } = ctx.input.body; const { user } = ctx.state.auth; + const { transaction } = ctx.state; const document = await Document.findByPk(documentId, { userId: user.id, + transaction, }); authorize(user, "read", document); if (collectionId) { const collection = await Collection.scope({ method: ["withMembership", user.id], - }).findByPk(collectionId); + }).findByPk(collectionId, { transaction }); authorize(user, "update", collection); authorize(user, "pin", document); } else { @@ -41,10 +43,10 @@ router.post( } const pin = await pinCreator({ + ctx, user, documentId, collectionId, - ip: ctx.request.ip, index, }); @@ -108,13 +110,20 @@ router.post( "pins.update", auth(), validate(T.PinsUpdateSchema), + transaction(), async (ctx: APIContext) => { const { id, index } = ctx.input.body; const { user } = ctx.state.auth; - let pin = await Pin.findByPk(id, { rejectOnEmpty: true }); + const { transaction } = ctx.state; + const pin = await Pin.findByPk(id, { + transaction, + lock: Transaction.LOCK.UPDATE, + rejectOnEmpty: true, + }); const document = await Document.findByPk(pin.documentId, { userId: user.id, + transaction, }); if (pin.collectionId) { @@ -123,12 +132,7 @@ router.post( authorize(user, "update", pin); } - pin = await pinUpdater({ - user, - pin, - ip: ctx.request.ip, - index, - }); + await pin.updateWithCtx(ctx, { index }); ctx.body = { data: presentPin(pin), @@ -141,14 +145,21 @@ router.post( "pins.delete", auth(), validate(T.PinsDeleteSchema), + transaction(), async (ctx: APIContext) => { const { id } = ctx.input.body; + const { transaction } = ctx.state; const { user } = ctx.state.auth; - const pin = await Pin.findByPk(id, { rejectOnEmpty: true }); + const pin = await Pin.findByPk(id, { + transaction, + lock: Transaction.LOCK.UPDATE, + rejectOnEmpty: true, + }); const document = await Document.findByPk(pin.documentId, { userId: user.id, + transaction, }); if (pin.collectionId) { @@ -157,7 +168,7 @@ router.post( authorize(user, "delete", pin); } - await pinDestroyer({ user, pin, ip: ctx.request.ip }); + await pin.destroyWithCtx(ctx); ctx.body = { success: true,