Move collection description rendering to JSON (#6944)

* First pass, moving collection description rendering to JSON

* tsc

* docs

* refactor

* test

app/components/CollectionDescription.tsx

@@ -65,7 +65,7 @@ function CollectionDescription({ collection }: Props) {
       debounce(async (getValue) => {
         try {
           await collection.save({
-            description: getValue(),
+            data: getValue(false),
           });
           setDirty(false);
         } catch (err) {
@@ -109,7 +109,7 @@ function CollectionDescription({ collection }: Props) {
             >
               <Editor
                 key={key}
-                defaultValue={collection.description || ""}
+                defaultValue={collection.data}
                 onChange={handleChange}
                 placeholder={placeholder}
                 readOnly={!isEditing}

app/models/Collection.ts

@@ -1,10 +1,10 @@
 import invariant from "invariant";
-import trim from "lodash/trim";
 import { action, computed, observable, reaction, runInAction } from "mobx";
 import {
   CollectionPermission,
   FileOperationFormat,
   NavigationNode,
+  type ProsemirrorData,
 } from "@shared/types";
 import { sortNavigationNodes } from "@shared/utils/collections";
 import type CollectionsStore from "~/stores/CollectionsStore";
@@ -27,34 +27,56 @@ export default class Collection extends ParanoidModel {
   @observable
   id: string;
 
+  /**
+   * The name of the collection.
+   */
   @Field
   @observable
   name: string;
 
   @Field
-  @observable
+  @observable.shallow
-  description: string;
+  data: ProsemirrorData;
 
+  /**
+   * An emoji to use as the collection icon.
+   */
   @Field
   @observable
   icon: string;
 
+  /**
+   * A color to use for the collection icon and other highlights.
+   */
   @Field
   @observable
   color: string;
 
+  /**
+   * The default permission for workspace users.
+   */
   @Field
   @observable
   permission?: CollectionPermission;
 
+  /**
+   * Whether public sharing is enabled for the collection. Note this can also be disabled at the
+   * workspace level.
+   */
   @Field
   @observable
   sharing: boolean;
 
+  /**
+   * The sort index for the collection.
+   */
   @Field
   @observable
   index: string;
 
+  /**
+   * The sort field and direction for documents in the collection.
+   */
   @Field
   @observable
   sort: {
@@ -112,9 +134,8 @@ export default class Collection extends ParanoidModel {
     return !this.permission;
   }
 
-  @computed
   get hasDescription(): boolean {
-    return !!trim(this.description, "\\").trim();
+    return !!this.data;
   }
 
   @computed

plugins/webhooks/server/tasks/DeliverWebhookTask.ts

@@ -428,7 +428,7 @@ export default class DeliverWebhookTask extends BaseTask<Props> {
       subscription,
       payload: {
         id: event.collectionId,
-        model: model && presentCollection(model),
+        model: model && (await presentCollection(undefined, model)),
       },
     });
   }
@@ -454,7 +454,8 @@ export default class DeliverWebhookTask extends BaseTask<Props> {
       payload: {
         id: event.modelId,
         model: model && presentMembership(model),
-        collection: model && presentCollection(model.collection!),
+        collection:
+          model && (await presentCollection(undefined, model.collection!)),
         user: model && presentUser(model.user),
       },
     });
@@ -481,7 +482,8 @@ export default class DeliverWebhookTask extends BaseTask<Props> {
       payload: {
         id: event.modelId,
         model: model && presentCollectionGroupMembership(model),
-        collection: model && presentCollection(model.collection!),
+        collection:
+          model && (await presentCollection(undefined, model.collection!)),
         group: model && presentGroup(model.group),
       },
     });

server/migrations/20240524234042-add-content-to-collection.js

@@ -0,0 +1,14 @@
+"use strict";
+
+module.exports = {
+  async up(queryInterface, Sequelize) {
+    await queryInterface.addColumn("collections", "content", {
+      type: Sequelize.JSONB,
+      allowNull: true,
+    });
+  },
+
+  async down(queryInterface) {
+    await queryInterface.removeColumn("collections", "content");
+  },
+};

server/models/Collection.ts

@@ -32,7 +32,7 @@ import {
   BeforeDestroy,
 } from "sequelize-typescript";
 import isUUID from "validator/lib/isUUID";
-import type { CollectionSort } from "@shared/types";
+import type { CollectionSort, ProsemirrorData } from "@shared/types";
 import { CollectionPermission, NavigationNode } from "@shared/types";
 import { UrlHelper } from "@shared/utils/UrlHelper";
 import { sortNavigationNodes } from "@shared/utils/collections";
@@ -49,6 +49,7 @@ import User from "./User";
 import UserMembership from "./UserMembership";
 import ParanoidModel from "./base/ParanoidModel";
 import Fix from "./decorators/Fix";
+import { DocumentHelper } from "./helpers/DocumentHelper";
 import IsHexColor from "./validators/IsHexColor";
 import Length from "./validators/Length";
 import NotContainsUrl from "./validators/NotContainsUrl";
@@ -163,6 +164,12 @@ class Collection extends ParanoidModel<
   @Column
   name: string;
 
+  /**
+   * The content of the collection as Markdown.
+   *
+   * @deprecated Use `content` instead, or `DocumentHelper.toMarkdown` if exporting lossy markdown.
+   * This column will be removed in a future migration.
+   */
   @Length({
     max: CollectionValidation.maxDescriptionLength,
     msg: `description must be ${CollectionValidation.maxDescriptionLength} characters or less`,
@@ -170,6 +177,12 @@ class Collection extends ParanoidModel<
   @Column
   description: string | null;
 
+  /**
+   * The content of the collection as JSON, this is a snapshot at the last time the state was saved.
+   */
+  @Column(DataType.JSONB)
+  content: ProsemirrorData | null;
+
   @Length({
     max: 50,
     msg: `icon must be 50 characters or less`,
@@ -260,6 +273,10 @@ class Collection extends ParanoidModel<
     if (model.icon === "collection") {
       model.icon = null;
     }
+
+    if (!model.content) {
+      model.content = await DocumentHelper.toJSON(model);
+    }
   }
 
   @BeforeDestroy

server/models/helpers/DocumentHelper.tsx

@@ -11,7 +11,7 @@ import { ProsemirrorData } from "@shared/types";
 import { parser, serializer, schema } from "@server/editor";
 import { addTags } from "@server/logging/tracer";
 import { trace } from "@server/logging/tracing";
-import { Document, Revision } from "@server/models";
+import { Collection, Document, Revision } from "@server/models";
 import diff from "@server/utils/diff";
 import { ProsemirrorHelper } from "./ProsemirrorHelper";
 import { TextHelper } from "./TextHelper";
@@ -43,7 +43,7 @@ export class DocumentHelper {
    * @param document The document or revision to convert
    * @returns The document content as a Prosemirror Node
    */
-  static toProsemirror(document: Document | Revision) {
+  static toProsemirror(document: Document | Revision | Collection) {
     if ("content" in document && document.content) {
       return Node.fromJSON(schema, document.content);
     }
@@ -52,7 +52,10 @@ export class DocumentHelper {
       Y.applyUpdate(ydoc, document.state);
       return Node.fromJSON(schema, yDocToProsemirrorJSON(ydoc, "default"));
     }
-    return parser.parse(document.text) || Node.fromJSON(schema, {});
+
+    const text =
+      document instanceof Collection ? document.description : document.text;
+    return parser.parse(text ?? "") || Node.fromJSON(schema, {});
   }
 
   /**
@@ -64,7 +67,7 @@ export class DocumentHelper {
    * @returns The document content as a plain JSON object
    */
   static async toJSON(
-    document: Document | Revision,
+    document: Document | Revision | Collection,
     options?: {
       /** The team context */
       teamId: string;
@@ -83,6 +86,8 @@ export class DocumentHelper {
       const ydoc = new Y.Doc();
       Y.applyUpdate(ydoc, document.state);
       doc = Node.fromJSON(schema, yDocToProsemirrorJSON(ydoc, "default"));
+    } else if (document instanceof Collection) {
+      doc = parser.parse(document.description ?? "");
     } else {
       doc = parser.parse(document.text);
     }
@@ -123,12 +128,12 @@ export class DocumentHelper {
   }
 
   /**
-   * Returns the document as Markdown. This is a lossy conversion and should nly be used for export.
+   * Returns the document as Markdown. This is a lossy conversion and should only be used for export.
    *
    * @param document The document or revision to convert
    * @returns The document title and content as a Markdown string
    */
-  static toMarkdown(document: Document | Revision) {
+  static toMarkdown(document: Document | Revision | Collection) {
     const text = serializer
       .serialize(DocumentHelper.toProsemirror(document))
       .replace(/\n\\(\n|$)/g, "\n\n")
@@ -138,6 +143,10 @@ export class DocumentHelper {
       .replace(/’/g, "'")
       .trim();
 
+    if (document instanceof Collection) {
+      return text;
+    }
+
     const title = `${document.emoji ? document.emoji + " " : ""}${
       document.title
     }`;

server/presenters/collection.ts

@@ -1,13 +1,21 @@
 import { colorPalette } from "@shared/utils/collections";
 import Collection from "@server/models/Collection";
+import { DocumentHelper } from "@server/models/helpers/DocumentHelper";
+import { APIContext } from "@server/types";
+
+export default async function presentCollection(
+  ctx: APIContext | undefined,
+  collection: Collection
+) {
+  const asData = !ctx || Number(ctx?.headers["x-api-version"] ?? 0) >= 3;
 
-export default function presentCollection(collection: Collection) {
   return {
     id: collection.id,
     url: collection.url,
     urlId: collection.urlId,
     name: collection.name,
-    description: collection.description,
+    data: asData ? await DocumentHelper.toJSON(collection) : undefined,
+    description: asData ? undefined : collection.description,
     sort: collection.sort,
     icon: collection.icon,
     index: collection.index,

server/queues/processors/WebsocketsProcessor.ts

@@ -182,7 +182,7 @@ export default class WebsocketsProcessor {
               ? `team-${collection.teamId}`
               : `user-${collection.createdById}`
           )
-          .emit(event.name, presentCollection(collection));
+          .emit(event.name, await presentCollection(undefined, collection));
 
         return socketio
           .to(
@@ -210,7 +210,7 @@ export default class WebsocketsProcessor {
               ? `collection-${event.collectionId}`
               : `team-${collection.teamId}`
           )
-          .emit(event.name, presentCollection(collection));
+          .emit(event.name, await presentCollection(undefined, collection));
       }
 
       case "collections.delete": {

server/queues/tasks/ExportJSONTask.ts

@@ -1,7 +1,6 @@
 import JSZip from "jszip";
 import omit from "lodash/omit";
 import { NavigationNode } from "@shared/types";
-import { parser } from "@server/editor";
 import env from "@server/env";
 import Logger from "@server/logging/Logger";
 import {
@@ -63,10 +62,10 @@ export default class ExportJSONTask extends ExportTask {
   ) {
     const output: CollectionJSONExport = {
       collection: {
-        ...omit(presentCollection(collection), ["url"]),
+        ...omit(await presentCollection(undefined, collection), [
-        description: collection.description
+          "url",
-          ? parser.parse(collection.description)
+          "description",
-          : null,
+        ]),
         documentStructure: collection.documentStructure,
       },
       documents: {},

server/queues/tasks/ImportJSONTask.ts

@@ -129,15 +129,14 @@ export default class ImportJSONTask extends ImportTask {
       }
 
       const collectionId = uuidv4();
+      const data = item.collection.description ?? item.collection.data;
+
       output.collections.push({
         ...item.collection,
         description:
-          item.collection.description &&
+          data && typeof data === "object"
-          typeof item.collection.description === "object"
+            ? serializer.serialize(Node.fromJSON(schema, data))
-            ? serializer.serialize(
+            : data,
-                Node.fromJSON(schema, item.collection.description)
-              )
-            : item.collection.description,
         id: collectionId,
         externalId: item.collection.id,
       });

server/routes/api/collections/collections.test.ts

@@ -1318,6 +1318,48 @@ describe("#collections.update", () => {
     expect(body.policies.length).toBe(1);
   });
 
+  it("allows editing description", async () => {
+    const team = await buildTeam();
+    const admin = await buildAdmin({ teamId: team.id });
+    const collection = await buildCollection({ teamId: team.id });
+    const res = await server.post("/api/collections.update", {
+      body: {
+        token: admin.getJwtToken(),
+        id: collection.id,
+        description: "Test",
+      },
+    });
+    const body = await res.json();
+    expect(res.status).toEqual(200);
+    expect(body.data.description).toBe("Test");
+
+    await collection.reload();
+
+    expect(collection.description).toBe("Test");
+    expect(collection.content).toBeTruthy();
+  });
+
+  it("allows editing data", async () => {
+    const team = await buildTeam();
+    const admin = await buildAdmin({ teamId: team.id });
+    const collection = await buildCollection({ teamId: team.id });
+    const res = await server.post("/api/collections.update", {
+      body: {
+        token: admin.getJwtToken(),
+        id: collection.id,
+        data: {
+          content: [
+            { content: [{ text: "Test", type: "text" }], type: "paragraph" },
+          ],
+          type: "doc",
+        },
+      },
+    });
+    const body = await res.json();
+    expect(res.status).toEqual(200);
+    expect(body.data.description).toBe("Test");
+  });
+
   it("allows editing sort", async () => {
     const team = await buildTeam();
     const admin = await buildAdmin({ teamId: team.id });

server/routes/api/collections/collections.ts

@@ -10,6 +10,7 @@ import {
 import collectionDestroyer from "@server/commands/collectionDestroyer";
 import collectionExporter from "@server/commands/collectionExporter";
 import teamUpdater from "@server/commands/teamUpdater";
+import { parser } from "@server/editor";
 import auth from "@server/middlewares/authentication";
 import { rateLimiter } from "@server/middlewares/rateLimiter";
 import { transaction } from "@server/middlewares/transaction";
@@ -25,6 +26,7 @@ import {
   Attachment,
   FileOperation,
 } from "@server/models";
+import { DocumentHelper } from "@server/models/helpers/DocumentHelper";
 import { authorize } from "@server/policies";
 import {
   presentCollection,
@@ -48,8 +50,10 @@ router.post(
   "collections.create",
   auth(),
   validate(T.CollectionsCreateSchema),
+  transaction(),
   async (ctx: APIContext<T.CollectionsCreateReq>) => {
-    const { name, color, description, permission, sharing, icon, sort } =
+    const { transaction } = ctx.state;
+    const { name, color, description, data, permission, sharing, icon, sort } =
       ctx.input.body;
     let { index } = ctx.input.body;
 
@@ -69,6 +73,7 @@ router.post(
           Sequelize.literal('"collection"."index" collate "C"'),
           ["updatedAt", "DESC"],
         ],
+        transaction,
       });
 
       index = fractionalIndex(
@@ -78,9 +83,10 @@ router.post(
     }
 
     index = await removeIndexCollision(user.teamId, index);
-    const collection = await Collection.create({
+    const collection = Collection.build({
       name,
-      description,
+      content: data,
+      description: data ? undefined : description,
       icon,
       color,
       teamId: user.teamId,
@@ -90,24 +96,38 @@ router.post(
       sort,
       index,
     });
-    await Event.create({
+
-      name: "collections.create",
+    if (data) {
-      collectionId: collection.id,
+      collection.description = DocumentHelper.toMarkdown(collection);
-      teamId: collection.teamId,
+    }
-      actorId: user.id,
+
-      data: {
+    await collection.save({ transaction });
-        name,
+
+    await Event.create(
+      {
+        name: "collections.create",
+        collectionId: collection.id,
+        teamId: collection.teamId,
+        actorId: user.id,
+        data: {
+          name,
+        },
+        ip: ctx.request.ip,
       },
-      ip: ctx.request.ip,
+      {
-    });
+        transaction,
+      }
+    );
     // we must reload the collection to get memberships for policy presenter
     const reloaded = await Collection.scope({
       method: ["withMembership", user.id],
-    }).findByPk(collection.id);
+    }).findByPk(collection.id, {
+      transaction,
+    });
     invariant(reloaded, "collection not found");
 
     ctx.body = {
-      data: presentCollection(reloaded),
+      data: await presentCollection(ctx, reloaded),
       policies: presentPolicies(user, [reloaded]),
     };
   }
@@ -127,7 +147,7 @@ router.post(
     authorize(user, "read", collection);
 
     ctx.body = {
-      data: presentCollection(collection),
+      data: await presentCollection(ctx, collection),
       policies: presentPolicies(user, [collection]),
     };
   }
@@ -636,8 +656,17 @@ router.post(
   transaction(),
   async (ctx: APIContext<T.CollectionsUpdateReq>) => {
     const { transaction } = ctx.state;
-    const { id, name, description, icon, permission, color, sort, sharing } =
+    const {
-      ctx.input.body;
+      id,
+      name,
+      description,
+      data,
+      icon,
+      permission,
+      color,
+      sort,
+      sharing,
+    } = ctx.input.body;
 
     const { user } = ctx.state.auth;
     const collection = await Collection.scope({
@@ -675,6 +704,14 @@ router.post(
 
     if (description !== undefined) {
       collection.description = description;
+      collection.content = description
+        ? parser.parse(description)?.toJSON()
+        : null;
+    }
+
+    if (data !== undefined) {
+      collection.content = data;
+      collection.description = DocumentHelper.toMarkdown(collection);
     }
 
     if (icon !== undefined) {
@@ -759,7 +796,7 @@ router.post(
     }
 
     ctx.body = {
-      data: presentCollection(collection),
+      data: await presentCollection(ctx, collection),
       policies: presentPolicies(user, [collection]),
     };
   }
@@ -811,7 +848,9 @@ router.post(
 
     ctx.body = {
       pagination: { ...ctx.state.pagination, total },
-      data: collections.map(presentCollection),
+      data: await Promise.all(
+        collections.map((collection) => presentCollection(ctx, collection))
+      ),
       policies: presentPolicies(user, collections),
     };
   }

server/routes/api/collections/schema.ts

@@ -6,7 +6,7 @@ import { IconLibrary } from "@shared/utils/IconLibrary";
 import { colorPalette } from "@shared/utils/collections";
 import { Collection } from "@server/models";
 import { ValidateColor, ValidateIndex } from "@server/validation";
-import { BaseSchema } from "../schema";
+import { BaseSchema, ProsemirrorSchema } from "../schema";
 
 function zodEnumFromObjectKeys<
   TI extends Record<string, any>,
@@ -16,6 +16,11 @@ function zodEnumFromObjectKeys<
   return z.enum([firstKey, ...otherKeys]);
 }
 
+const BaseIdSchema = z.object({
+  /** Id of the collection to be updated */
+  id: z.string(),
+});
+
 export const CollectionsCreateSchema = BaseSchema.extend({
   body: z.object({
     name: z.string(),
@@ -24,6 +29,7 @@ export const CollectionsCreateSchema = BaseSchema.extend({
       .regex(ValidateColor.regex, { message: ValidateColor.message })
       .default(randomElement(colorPalette)),
     description: z.string().nullish(),
+    data: ProsemirrorSchema.nullish(),
     permission: z
       .nativeEnum(CollectionPermission)
       .nullish()
@@ -49,17 +55,13 @@ export const CollectionsCreateSchema = BaseSchema.extend({
 export type CollectionsCreateReq = z.infer<typeof CollectionsCreateSchema>;
 
 export const CollectionsInfoSchema = BaseSchema.extend({
-  body: z.object({
+  body: BaseIdSchema,
-    id: z.string().uuid(),
-  }),
 });
 
 export type CollectionsInfoReq = z.infer<typeof CollectionsInfoSchema>;
 
 export const CollectionsDocumentsSchema = BaseSchema.extend({
-  body: z.object({
+  body: BaseIdSchema,
-    id: z.string().uuid(),
-  }),
 });
 
 export type CollectionsDocumentsReq = z.infer<
@@ -82,8 +84,7 @@ export const CollectionsImportSchema = BaseSchema.extend({
 export type CollectionsImportReq = z.infer<typeof CollectionsImportSchema>;
 
 export const CollectionsAddGroupSchema = BaseSchema.extend({
-  body: z.object({
+  body: BaseIdSchema.extend({
-    id: z.string().uuid(),
     groupId: z.string().uuid(),
     permission: z
       .nativeEnum(CollectionPermission)
@@ -94,8 +95,7 @@ export const CollectionsAddGroupSchema = BaseSchema.extend({
 export type CollectionsAddGroupsReq = z.infer<typeof CollectionsAddGroupSchema>;
 
 export const CollectionsRemoveGroupSchema = BaseSchema.extend({
-  body: z.object({
+  body: BaseIdSchema.extend({
-    id: z.string().uuid(),
     groupId: z.string().uuid(),
   }),
 });
@@ -105,8 +105,7 @@ export type CollectionsRemoveGroupReq = z.infer<
 >;
 
 export const CollectionsGroupMembershipsSchema = BaseSchema.extend({
-  body: z.object({
+  body: BaseIdSchema.extend({
-    id: z.string().uuid(),
     query: z.string().optional(),
     permission: z.nativeEnum(CollectionPermission).optional(),
   }),
@@ -117,8 +116,7 @@ export type CollectionsGroupMembershipsReq = z.infer<
 >;
 
 export const CollectionsAddUserSchema = BaseSchema.extend({
-  body: z.object({
+  body: BaseIdSchema.extend({
-    id: z.string().uuid(),
     userId: z.string().uuid(),
     permission: z.nativeEnum(CollectionPermission).optional(),
   }),
@@ -127,8 +125,7 @@ export const CollectionsAddUserSchema = BaseSchema.extend({
 export type CollectionsAddUserReq = z.infer<typeof CollectionsAddUserSchema>;
 
 export const CollectionsRemoveUserSchema = BaseSchema.extend({
-  body: z.object({
+  body: BaseIdSchema.extend({
-    id: z.string().uuid(),
     userId: z.string().uuid(),
   }),
 });
@@ -138,8 +135,7 @@ export type CollectionsRemoveUserReq = z.infer<
 >;
 
 export const CollectionsMembershipsSchema = BaseSchema.extend({
-  body: z.object({
+  body: BaseIdSchema.extend({
-    id: z.string().uuid(),
     query: z.string().optional(),
     permission: z.nativeEnum(CollectionPermission).optional(),
   }),
@@ -150,8 +146,7 @@ export type CollectionsMembershipsReq = z.infer<
 >;
 
 export const CollectionsExportSchema = BaseSchema.extend({
-  body: z.object({
+  body: BaseIdSchema.extend({
-    id: z.string().uuid(),
     format: z
       .nativeEnum(FileOperationFormat)
       .default(FileOperationFormat.MarkdownZip),
@@ -175,10 +170,10 @@ export type CollectionsExportAllReq = z.infer<
 >;
 
 export const CollectionsUpdateSchema = BaseSchema.extend({
-  body: z.object({
+  body: BaseIdSchema.extend({
-    id: z.string().uuid(),
     name: z.string().optional(),
     description: z.string().nullish(),
+    data: ProsemirrorSchema.nullish(),
     icon: zodEnumFromObjectKeys(IconLibrary.mapping).nullish(),
     permission: z.nativeEnum(CollectionPermission).nullish(),
     color: z
@@ -206,16 +201,13 @@ export const CollectionsListSchema = BaseSchema.extend({
 export type CollectionsListReq = z.infer<typeof CollectionsListSchema>;
 
 export const CollectionsDeleteSchema = BaseSchema.extend({
-  body: z.object({
+  body: BaseIdSchema,
-    id: z.string().uuid(),
-  }),
 });
 
 export type CollectionsDeleteReq = z.infer<typeof CollectionsDeleteSchema>;
 
 export const CollectionsMoveSchema = BaseSchema.extend({
-  body: z.object({
+  body: BaseIdSchema.extend({
-    id: z.string().uuid(),
     index: z
       .string()
       .regex(ValidateIndex.regex, { message: ValidateIndex.message })

server/routes/api/documents/documents.ts

@@ -1172,7 +1172,7 @@ router.post(
           documents.map((document) => presentDocument(ctx, document))
         ),
         collections: await Promise.all(
-          collections.map((collection) => presentCollection(collection))
+          collections.map((collection) => presentCollection(ctx, collection))
         ),
       },
       policies: collectionChanged ? presentPolicies(user, documents) : [],

server/types.ts

@@ -10,6 +10,7 @@ import {
   DocumentPermission,
   JSONValue,
   UnfurlResourceType,
+  ProsemirrorData,
 } from "@shared/types";
 import { BaseSchema } from "@server/routes/api/schema";
 import { AccountProvisionerResult } from "./commands/accountProvisioner";
@@ -494,7 +495,8 @@ export type CollectionJSONExport = {
     id: string;
     urlId: string;
     name: string;
-    description: Record<string, any> | null;
+    data?: ProsemirrorData | null;
+    description?: ProsemirrorData | null;
     permission?: CollectionPermission | null;
     color: string;
     icon?: string | null;