From 82cdebfb66ac1144c0d85acfdeb1915000b16375 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Sun, 2 Mar 2025 08:07:30 -0500 Subject: [PATCH] Add `name` column to revisions (#8603) * fix: Flaky test * Migration, model interface * Add policies to revisions * Add revisions.update endpoint * tests * lint --- app/models/Revision.ts | 3 + .../20250301234423-add-name-to-revisions.js | 15 +++++ server/models/Revision.ts | 20 +++++-- server/policies/index.ts | 1 + server/policies/revision.ts | 11 ++++ server/presenters/revision.ts | 1 + server/routes/api/revisions/revisions.test.ts | 56 +++++++++++++++++++ server/routes/api/revisions/revisions.ts | 35 +++++++++++- server/routes/api/revisions/schema.ts | 14 +++++ server/routes/api/shares/shares.test.ts | 1 + shared/validations.ts | 5 ++ 11 files changed, 155 insertions(+), 7 deletions(-) create mode 100644 server/migrations/20250301234423-add-name-to-revisions.js create mode 100644 server/policies/revision.ts diff --git a/app/models/Revision.ts b/app/models/Revision.ts index 50be73d49d..5dee0e950a 100644 --- a/app/models/Revision.ts +++ b/app/models/Revision.ts @@ -19,6 +19,9 @@ class Revision extends Model { /** The document title when the revision was created */ title: string; + /** An optional name for the revision */ + name: string | null; + /** Prosemirror data of the content when revision was created */ data: ProsemirrorData; diff --git a/server/migrations/20250301234423-add-name-to-revisions.js b/server/migrations/20250301234423-add-name-to-revisions.js new file mode 100644 index 0000000000..f04ad392a9 --- /dev/null +++ b/server/migrations/20250301234423-add-name-to-revisions.js @@ -0,0 +1,15 @@ +"use strict"; + +/** @type {import('sequelize-cli').Migration} */ +module.exports = { + async up(queryInterface, Sequelize) { + await queryInterface.addColumn("revisions", "name", { + type: Sequelize.STRING, + allowNull: true, + }); + }, + + async down(queryInterface) { + await queryInterface.removeColumn("revisions", "name"); + }, +}; diff --git a/server/models/Revision.ts b/server/models/Revision.ts index abe7bc28f3..4b0de27611 100644 --- a/server/models/Revision.ts +++ b/server/models/Revision.ts @@ -15,7 +15,7 @@ import { Length as SimpleLength, } from "sequelize-typescript"; import type { ProsemirrorData } from "@shared/types"; -import { DocumentValidation } from "@shared/validations"; +import { DocumentValidation, RevisionValidation } from "@shared/validations"; import Document from "./Document"; import User from "./User"; import IdModel from "./base/IdModel"; @@ -42,6 +42,7 @@ class Revision extends IdModel< @Column(DataType.SMALLINT) version?: number | null; + /** The editor version at the time of the revision */ @SimpleLength({ max: 255, msg: `editorVersion must be 255 characters or less`, @@ -49,6 +50,7 @@ class Revision extends IdModel< @Column editorVersion: string; + /** The document title at the time of the revision */ @Length({ max: DocumentValidation.maxTitleLength, msg: `Revision title must be ${DocumentValidation.maxTitleLength} characters or less`, @@ -56,6 +58,14 @@ class Revision extends IdModel< @Column title: string; + /** An optional name for the revision */ + @Length({ + max: RevisionValidation.maxNameLength, + msg: `Revision name must be ${RevisionValidation.maxNameLength} characters or less`, + }) + @Column + name: string | null; + /** * The content of the revision as Markdown. * @@ -65,13 +75,11 @@ class Revision extends IdModel< @Column(DataType.TEXT) text: string; - /** - * The content of the revision as JSON. - */ + /** The content of the revision as JSON. */ @Column(DataType.JSONB) content: ProsemirrorData | null; - /** An icon to use as the document icon. */ + /** The icon at the time of the revision. */ @Length({ max: 50, msg: `icon must be 50 characters or less`, @@ -79,7 +87,7 @@ class Revision extends IdModel< @Column icon: string | null; - /** The color of the icon. */ + /** The color at the time of the revision. */ @IsHexColor @Column color: string | null; diff --git a/server/policies/index.ts b/server/policies/index.ts index 82e5cf5ba4..70d5f4f8c8 100644 --- a/server/policies/index.ts +++ b/server/policies/index.ts @@ -12,6 +12,7 @@ import "./fileOperation"; import "./integration"; import "./pins"; import "./reaction"; +import "./revision"; import "./searchQuery"; import "./share"; import "./star"; diff --git a/server/policies/revision.ts b/server/policies/revision.ts new file mode 100644 index 0000000000..06e931868e --- /dev/null +++ b/server/policies/revision.ts @@ -0,0 +1,11 @@ +import { User, Revision } from "@server/models"; +import { allow } from "./cancan"; +import { and, isTeamMutable, or } from "./utils"; + +allow(User, ["update"], Revision, (actor, revision) => + and( + // + or(actor.id === revision?.userId, actor.isAdmin), + isTeamMutable(actor) + ) +); diff --git a/server/presenters/revision.ts b/server/presenters/revision.ts index 5bf6250245..2461839625 100644 --- a/server/presenters/revision.ts +++ b/server/presenters/revision.ts @@ -12,6 +12,7 @@ async function presentRevision(revision: Revision, diff?: string) { id: revision.id, documentId: revision.documentId, title: strippedTitle, + name: revision.name, data: await DocumentHelper.toJSON(revision), icon: revision.icon ?? emoji, color: revision.color, diff --git a/server/routes/api/revisions/revisions.test.ts b/server/routes/api/revisions/revisions.test.ts index 7db8e71874..9958648891 100644 --- a/server/routes/api/revisions/revisions.test.ts +++ b/server/routes/api/revisions/revisions.test.ts @@ -1,5 +1,6 @@ import { UserMembership, Revision } from "@server/models"; import { + buildAdmin, buildCollection, buildDocument, buildUser, @@ -42,6 +43,61 @@ describe("#revisions.info", () => { }); }); +describe("#revisions.update", () => { + it("should update a document revision", async () => { + const user = await buildUser(); + const document = await buildDocument({ + userId: user.id, + teamId: user.teamId, + }); + const revision = await Revision.createFromDocument(document); + + const res = await server.post("/api/revisions.update", { + body: { + token: user.getJwtToken(), + id: revision.id, + name: "new name", + }, + }); + const body = await res.json(); + expect(res.status).toEqual(200); + expect(body.data.name).toEqual("new name"); + }); + + it("should allow an admin to update a document revision", async () => { + const admin = await buildAdmin(); + const document = await buildDocument({ + teamId: admin.teamId, + }); + const revision = await Revision.createFromDocument(document); + + const res = await server.post("/api/revisions.update", { + body: { + token: admin.getJwtToken(), + id: revision.id, + name: "new name", + }, + }); + const body = await res.json(); + expect(res.status).toEqual(200); + expect(body.data.name).toEqual("new name"); + }); + + it("should require authorization", async () => { + const document = await buildDocument(); + const revision = await Revision.createFromDocument(document); + const user = await buildUser(); + const res = await server.post("/api/revisions.update", { + body: { + token: user.getJwtToken(), + id: revision.id, + name: "new name", + }, + }); + expect(res.status).toEqual(403); + }); +}); + describe("#revisions.diff", () => { it("should return the document HTML if no previous revision", async () => { const user = await buildUser(); diff --git a/server/routes/api/revisions/revisions.ts b/server/routes/api/revisions/revisions.ts index f09855d234..54c27ec7f3 100644 --- a/server/routes/api/revisions/revisions.ts +++ b/server/routes/api/revisions/revisions.ts @@ -4,11 +4,12 @@ import { RevisionHelper } from "@shared/utils/RevisionHelper"; import slugify from "@shared/utils/slugify"; import { ValidationError } from "@server/errors"; import auth from "@server/middlewares/authentication"; +import { transaction } from "@server/middlewares/transaction"; import validate from "@server/middlewares/validate"; import { Document, Revision } from "@server/models"; import { DocumentHelper } from "@server/models/helpers/DocumentHelper"; import { authorize } from "@server/policies"; -import { presentRevision } from "@server/presenters"; +import { presentPolicies, presentRevision } from "@server/presenters"; import { APIContext } from "@server/types"; import pagination from "../middlewares/pagination"; import * as T from "./schema"; @@ -57,6 +58,36 @@ router.post( includeStyles: false, }) ), + policies: presentPolicies(user, [after]), + }; + } +); + +router.post( + "revisions.update", + auth(), + validate(T.RevisionsUpdateSchema), + transaction(), + async (ctx: APIContext) => { + const { id, name } = ctx.input.body; + const { user } = ctx.state.auth; + const { transaction } = ctx.state; + + const revision = await Revision.findByPk(id, { + rejectOnEmpty: true, + }); + const document = await Document.findByPk(revision.documentId, { + userId: user.id, + }); + authorize(user, "update", document); + authorize(user, "update", revision); + + revision.name = name; + await revision.save({ transaction }); + + ctx.body = { + data: await presentRevision(revision), + policies: presentPolicies(user, [revision]), }; } ); @@ -110,6 +141,7 @@ router.post( ctx.body = { data: content, + policies: presentPolicies(user, [revision]), }; } ); @@ -144,6 +176,7 @@ router.post( ctx.body = { pagination: ctx.state.pagination, data, + policies: presentPolicies(user, revisions), }; } ); diff --git a/server/routes/api/revisions/schema.ts b/server/routes/api/revisions/schema.ts index 85c88974d5..d7b5224527 100644 --- a/server/routes/api/revisions/schema.ts +++ b/server/routes/api/revisions/schema.ts @@ -1,5 +1,6 @@ import isEmpty from "lodash/isEmpty"; import { z } from "zod"; +import { RevisionValidation } from "@shared/validations"; import { Revision } from "@server/models"; import { BaseSchema } from "@server/routes/api/schema"; @@ -25,6 +26,19 @@ export const RevisionsDiffSchema = BaseSchema.extend({ export type RevisionsDiffReq = z.infer; +export const RevisionsUpdateSchema = BaseSchema.extend({ + body: z.object({ + id: z.string().uuid(), + + name: z + .string() + .min(RevisionValidation.minNameLength) + .max(RevisionValidation.maxNameLength), + }), +}); + +export type RevisionsUpdateReq = z.infer; + export const RevisionsListSchema = z.object({ body: z.object({ direction: z diff --git a/server/routes/api/shares/shares.test.ts b/server/routes/api/shares/shares.test.ts index 993ce345c1..fe8b9f435a 100644 --- a/server/routes/api/shares/shares.test.ts +++ b/server/routes/api/shares/shares.test.ts @@ -62,6 +62,7 @@ describe("#shares.list", () => { const document = await buildDocument({ userId: user.id, teamId: user.teamId, + title: "hardcoded", }); await buildShare({ documentId: document.id, diff --git a/shared/validations.ts b/shared/validations.ts index d00e8898b1..a0359c9857 100644 --- a/shared/validations.ts +++ b/shared/validations.ts @@ -51,6 +51,11 @@ export const DocumentValidation = { maxStateLength: 1500 * 1024, }; +export const RevisionValidation = { + minNameLength: 1, + maxNameLength: 255, +}; + export const PinValidation = { /** The maximum number of pinned documents on an individual collection or home screen */ max: 8,