Allow drafts to be created without requiring a collection (#4175)

* feat(server): allow document to be created without collectionId

* fix(server): policies for a draft doc without collection

* fix(app): hide share button for drafts

* feat(server): permissions around publishing a draft

* fix(server): return drafts without collection

* fix(server): handle draft deletion

* fix(server): show drafts in deleted docs

* fix(server): allow drafts without collection to be restored

* feat(server): return drafts in search results

* fix: use buildDraftDocument for drafts

* fix: remove isDraftWithoutCollection

* fix: do not return drafts for team

* fix: put invariants

* fix: query clause

* fix: check only for undefined

* fix: restore includeDrafts clause as it was before

server/routes/api/documents.test.ts

@@ -14,7 +14,9 @@ import {
   buildCollection,
   buildUser,
   buildDocument,
+  buildDraftDocument,
   buildViewer,
+  buildTeam,
 } from "@server/test/factories";
 import { seed, getTestServer } from "@server/test/support";
 
@@ -865,6 +867,30 @@ describe("#documents.drafts", () => {
     expect(body.data.length).toEqual(1);
   });
 
+  it("should return drafts, including ones without collectionIds", async () => {
+    const drafts = [];
+    const { user, document } = await seed();
+    document.publishedAt = null;
+    await document.save();
+    drafts.push(document);
+    const draftDocument = await buildDraftDocument({
+      title: "draft title",
+      text: "draft text",
+      createdById: user.id,
+      teamId: user.teamId,
+    });
+    drafts.push(draftDocument);
+
+    const res = await server.post("/api/documents.drafts", {
+      body: {
+        token: user.getJwtToken(),
+      },
+    });
+    const body = await res.json();
+    expect(res.status).toEqual(200);
+    expect(body.data.length).toEqual(drafts.length);
+  });
+
   it("should not return documents in private collections not a member of", async () => {
     const { user, document, collection } = await seed();
     document.publishedAt = null;
@@ -980,6 +1006,29 @@ describe("#documents.search", () => {
     expect(body.data[0].document.id).toEqual(share.documentId);
   });
 
+  it("should not return drafts using shareId", async () => {
+    const { user, document } = await seed();
+    document.publishedAt = null;
+    await document.save();
+    const share = await buildShare({
+      documentId: document.id,
+      includeChildDocuments: true,
+      teamId: user.teamId,
+      userId: user.id,
+    });
+    const res = await server.post("/api/documents.search", {
+      body: {
+        token: user.getJwtToken(),
+        shareId: share.id,
+        includeDrafts: true,
+        query: "test",
+      },
+    });
+    const body = await res.json();
+    expect(res.status).toEqual(200);
+    expect(body.data.length).toEqual(0);
+  });
+
   it("should not allow search if child documents are not included", async () => {
     const findableDocument = await buildDocument({
       title: "search term",
@@ -1140,7 +1189,7 @@ describe("#documents.search", () => {
       body: {
         token: user.getJwtToken(),
         query: "search term",
-        includeDrafts: "true",
+        includeDrafts: true,
       },
     });
     const body = await res.json();
@@ -1149,6 +1198,27 @@ describe("#documents.search", () => {
     expect(body.data[0].document.id).toEqual(document.id);
   });
 
+  it("should return drafts without collection too", async () => {
+    const user = await buildUser();
+    await buildDraftDocument({
+      title: "some title",
+      text: "some text",
+      createdById: user.id,
+      userId: user.id,
+      teamId: user.teamId,
+    });
+    const res = await server.post("/api/documents.search", {
+      body: {
+        token: user.getJwtToken(),
+        includeDrafts: true,
+        query: "text",
+      },
+    });
+    const body = await res.json();
+    expect(res.status).toEqual(200);
+    expect(body.data.length).toEqual(1);
+  });
+
   it("should not return draft documents created by other users", async () => {
     const user = await buildUser();
     await buildDocument({
@@ -1161,7 +1231,7 @@ describe("#documents.search", () => {
       body: {
         token: user.getJwtToken(),
         query: "search term",
-        includeDrafts: "true",
+        includeDrafts: true,
       },
     });
     const body = await res.json();
@@ -1200,7 +1270,7 @@ describe("#documents.search", () => {
       body: {
         token: user.getJwtToken(),
         query: "search term",
-        includeArchived: "true",
+        includeArchived: true,
       },
     });
     const body = await res.json();
@@ -1470,6 +1540,30 @@ describe("#documents.deleted", () => {
     expect(body.data.length).toEqual(1);
   });
 
+  it("should return deleted documents, including drafts without collection", async () => {
+    const { user } = await seed();
+    const document = await buildDocument({
+      userId: user.id,
+      teamId: user.teamId,
+    });
+    const draftDocument = await buildDraftDocument({
+      userId: user.id,
+      teamId: user.teamId,
+    });
+    await Promise.all([
+      document.delete(user.id),
+      draftDocument.delete(user.id),
+    ]);
+    const res = await server.post("/api/documents.deleted", {
+      body: {
+        token: user.getJwtToken(),
+      },
+    });
+    const body = await res.json();
+    expect(res.status).toEqual(200);
+    expect(body.data.length).toEqual(2);
+  });
+
   it("should not return documents in private collections not a member of", async () => {
     const { user } = await seed();
     const collection = await buildCollection({
@@ -1646,6 +1740,24 @@ describe("#documents.restore", () => {
     expect(body.data.deletedAt).toEqual(null);
   });
 
+  it("should allow restore of trashed drafts without collection", async () => {
+    const { user } = await seed();
+    const document = await buildDraftDocument({
+      userId: user.id,
+      teamId: user.teamId,
+    });
+    await document.delete(user.id);
+    const res = await server.post("/api/documents.restore", {
+      body: {
+        token: user.getJwtToken(),
+        id: document.id,
+      },
+    });
+    const body = await res.json();
+    expect(res.status).toEqual(200);
+    expect(body.data.deletedAt).toEqual(null);
+  });
+
   it("should allow restore of trashed documents with collectionId", async () => {
     const { user, document } = await seed();
     const collection = await buildCollection({
@@ -1864,6 +1976,81 @@ describe("#documents.create", () => {
     expect(body.policies[0].abilities.update).toEqual(true);
   });
 
+  it("should create a draft document not belonging to any collection", async () => {
+    const team = await buildTeam();
+    const user = await buildUser({ teamId: team.id });
+    const res = await server.post("/api/documents.create", {
+      body: {
+        token: user.getJwtToken(),
+        title: "draft document",
+        text: "draft document without collection",
+      },
+    });
+    const body = await res.json();
+
+    expect(res.status).toEqual(200);
+    expect(body.data.title).toBe("draft document");
+    expect(body.data.text).toBe("draft document without collection");
+    expect(body.data.collectionId).toBeNull();
+  });
+
+  it("should not allow creating a template without a collection", async () => {
+    const team = await buildTeam();
+    const user = await buildUser({ teamId: team.id });
+    const res = await server.post("/api/documents.create", {
+      body: {
+        template: true,
+        token: user.getJwtToken(),
+        title: "template",
+        text: "template without collection",
+      },
+    });
+    const body = await res.json();
+
+    expect(body.message).toBe(
+      "collectionId is required to create a nested doc or a template"
+    );
+    expect(res.status).toEqual(400);
+  });
+
+  it("should not allow publishing without specifying the collection", async () => {
+    const team = await buildTeam();
+    const user = await buildUser({ teamId: team.id });
+    const res = await server.post("/api/documents.create", {
+      body: {
+        token: user.getJwtToken(),
+        title: "title",
+        text: "text",
+        publish: true,
+      },
+    });
+    const body = await res.json();
+
+    expect(body.message).toBe(
+      "collectionId is required to publish a draft without collection"
+    );
+    expect(res.status).toEqual(400);
+  });
+
+  it("should not allow creating a nested doc without a collection", async () => {
+    const team = await buildTeam();
+    const user = await buildUser({ teamId: team.id });
+    const res = await server.post("/api/documents.create", {
+      body: {
+        token: user.getJwtToken(),
+        parentDocumentId: "d7a4eb73-fac1-4028-af45-d7e34d54db8e",
+        title: "nested doc",
+        text: "nested doc without collection",
+      },
+    });
+    const body = await res.json();
+
+    expect(body.message).toBe(
+      "collectionId is required to create a nested doc or a template"
+    );
+    expect(res.status).toEqual(400);
+  });
+
   it("should not allow very long titles", async () => {
     const { user, collection } = await seed();
     const res = await server.post("/api/documents.create", {
@@ -1969,6 +2156,73 @@ describe("#documents.update", () => {
     expect(events.length).toEqual(1);
   });
 
+  it("should not allow publishing a draft without specifying the collection", async () => {
+    const { user, team } = await seed();
+    const document = await buildDraftDocument({
+      teamId: team.id,
+    });
+    const res = await server.post("/api/documents.update", {
+      body: {
+        token: user.getJwtToken(),
+        id: document.id,
+        title: "Updated title",
+        text: "Updated text",
+        lastRevision: document.revisionCount,
+        publish: true,
+      },
+    });
+    const body = await res.json();
+    expect(res.status).toEqual(400);
+    expect(body.message).toBe(
+      "collectionId is required to publish a draft without collection"
+    );
+  });
+
+  it("should successfully publish a draft", async () => {
+    const { user, team, collection } = await seed();
+    const document = await buildDraftDocument({
+      title: "title",
+      text: "text",
+      teamId: team.id,
+    });
+
+    const res = await server.post("/api/documents.update", {
+      body: {
+        token: user.getJwtToken(),
+        id: document.id,
+        title: "Updated title",
+        text: "Updated text",
+        lastRevision: document.revisionCount,
+        collectionId: collection.id,
+        publish: true,
+      },
+    });
+    const body = await res.json();
+    expect(res.status).toEqual(200);
+    expect(body.data.collectionId).toBe(collection.id);
+    expect(body.data.title).toBe("Updated title");
+    expect(body.data.text).toBe("Updated text");
+  });
+
+  it("should not allow publishing by another collection's user", async () => {
+    const { user, document, collection } = await seed();
+    const anotherTeam = await buildTeam();
+    user.teamId = anotherTeam.id;
+    await user.save();
+    const res = await server.post("/api/documents.update", {
+      body: {
+        token: user.getJwtToken(),
+        id: document.id,
+        title: "Updated title",
+        text: "Updated text",
+        lastRevision: document.revisionCount,
+        collectionId: collection.id,
+        publish: true,
+      },
+    });
+    expect(res.status).toEqual(403);
+  });
+
   it("should not add template to collection structure when publishing", async () => {
     const user = await buildUser();
     const collection = await buildCollection({
@@ -2282,6 +2536,31 @@ describe("#documents.delete", () => {
     expect(body.success).toEqual(true);
   });
 
+  it("should delete a draft without collection", async () => {
+    const { user } = await seed();
+    const document = await buildDraftDocument({
+      teamId: user.teamId,
+      deletedAt: null,
+    });
+    const res = await server.post("/api/documents.delete", {
+      body: {
+        token: user.getJwtToken(),
+        id: document.id,
+      },
+    });
+
+    const body = await res.json();
+    expect(res.status).toEqual(200);
+    expect(body.success).toEqual(true);
+
+    const deletedDoc = await Document.findByPk(document.id, {
+      paranoid: false,
+    });
+    expect(deletedDoc).toBeDefined();
+    expect(deletedDoc).not.toBeNull();
+    expect(deletedDoc?.deletedAt).toBeTruthy();
+  });
+
   it("should allow permanently deleting a document", async () => {
     const user = await buildUser();
     const document = await buildDocument({

server/routes/api/documents.ts

@@ -44,6 +44,7 @@ import {
   assertPresent,
   assertPositiveInteger,
   assertNotEmpty,
+  assertBoolean,
 } from "@server/validation";
 import env from "../../env";
 import pagination from "./middlewares/pagination";
@@ -244,7 +245,9 @@ router.post(
     ]).findAll({
       where: {
         teamId: user.teamId,
-        collectionId: collectionIds,
+        collectionId: {
+          [Op.or]: [{ [Op.in]: collectionIds }, { [Op.is]: null }],
+        },
         deletedAt: {
           [Op.ne]: null,
         },
@@ -353,9 +356,11 @@ router.post("documents.drafts", auth(), pagination(), async (ctx) => {
   const collectionIds = collectionId
     ? [collectionId]
     : await user.collectionIds();
-  const where: WhereOptions<Document> = {
+  const where: WhereOptions = {
     createdById: user.id,
-    collectionId: collectionIds,
+    collectionId: {
+      [Op.or]: [{ [Op.in]: collectionIds }, { [Op.is]: null }],
+    },
     publishedAt: {
       [Op.is]: null,
     },
@@ -425,7 +430,7 @@ router.post(
             document: serializedDocument,
             sharedTree:
               share && share.includeChildDocuments
-                ? collection.getDocumentTree(share.documentId)
+                ? collection?.getDocumentTree(share.documentId)
                 : undefined,
           }
         : serializedDocument;
@@ -516,13 +521,15 @@ router.post("documents.restore", auth({ member: true }), async (ctx) => {
   // be caught as a 403 on the authorize call below. Otherwise we're checking here
   // that the original collection still exists and advising to pass collectionId
   // if not.
-  if (!collectionId && !collection) {
+  if (document.collection && !collectionId && !collection) {
     throw ValidationError(
       "Unable to restore to original collection, it may have been deleted"
     );
   }
 
-  authorize(user, "update", collection);
+  if (document.collection) {
+    authorize(user, "update", collection);
+  }
 
   if (document.deletedAt) {
     authorize(user, "restore", document);
@@ -655,6 +662,14 @@ router.post(
     } = ctx.request.body;
     assertNotEmpty(query, "query is required");
 
+    if (includeDrafts) {
+      assertBoolean(includeDrafts);
+    }
+
+    if (includeArchived) {
+      assertBoolean(includeArchived);
+    }
+
     const { offset, limit } = ctx.state.pagination;
     const snippetMinWords = parseInt(
       ctx.request.body.snippetMinWords || 20,
@@ -687,8 +702,8 @@ router.post(
       invariant(team, "Share must belong to a team");
 
       response = await Document.searchForTeam(team, query, {
-        includeArchived: includeArchived === "true",
-        includeDrafts: includeDrafts === "true",
+        includeArchived,
+        includeDrafts,
         collectionId: document.collectionId,
         share,
         dateFilter,
@@ -728,8 +743,8 @@ router.post(
       }
 
       response = await Document.searchForUser(user, query, {
-        includeArchived: includeArchived === "true",
-        includeDrafts: includeDrafts === "true",
+        includeArchived,
+        includeDrafts,
         collaboratorIds,
         collectionId,
         dateFilter,
@@ -827,6 +842,7 @@ router.post("documents.update", auth(), async (ctx) => {
     publish,
     lastRevision,
     templateId,
+    collectionId,
     append,
   } = ctx.request.body;
   const editorVersion = ctx.headers["x-editor-version"] as string | undefined;
@@ -834,24 +850,39 @@ router.post("documents.update", auth(), async (ctx) => {
   if (append) {
     assertPresent(text, "Text is required while appending");
   }
+
+  if (collectionId) {
+    assertUuid(collectionId, "collectionId must be an uuid");
+  }
+
   const { user } = ctx.state;
 
   let collection: Collection | null | undefined;
 
-  const document = await sequelize.transaction(async (transaction) => {
-    const document = await Document.findByPk(id, {
-      userId: user.id,
-      includeState: true,
-      transaction,
-    });
-    authorize(user, "update", document);
+  const document = await Document.findByPk(id, {
+    userId: user.id,
+    includeState: true,
+  });
+  authorize(user, "update", document);
 
-    collection = document.collection;
-
-    if (lastRevision && lastRevision !== document.revisionCount) {
-      throw InvalidRequestError("Document has changed since last revision");
+  if (publish) {
+    if (!document.collectionId) {
+      assertPresent(
+        collectionId,
+        "collectionId is required to publish a draft without collection"
+      );
+      collection = await Collection.findByPk(collectionId);
+    } else {
+      collection = document.collection;
     }
+    authorize(user, "publish", collection);
+  }
 
+  if (lastRevision && lastRevision !== document.revisionCount) {
+    throw InvalidRequestError("Document has changed since last revision");
+  }
+
+  const updatedDocument = await sequelize.transaction(async (transaction) => {
     return documentUpdater({
       document,
       user,
@@ -859,6 +890,7 @@ router.post("documents.update", auth(), async (ctx) => {
       text,
       fullWidth,
       publish,
+      collectionId,
       append,
       templateId,
       editorVersion,
@@ -867,14 +899,12 @@ router.post("documents.update", auth(), async (ctx) => {
     });
   });
 
-  invariant(collection, "collection not found");
-
-  document.updatedBy = user;
-  document.collection = collection;
+  updatedDocument.updatedBy = user;
+  updatedDocument.collection = collection;
 
   ctx.body = {
-    data: await presentDocument(document),
-    policies: presentPolicies(user, [document]),
+    data: await presentDocument(updatedDocument),
+    policies: presentPolicies(user, [updatedDocument]),
   };
 });
 
@@ -1169,7 +1199,19 @@ router.post("documents.create", auth(), async (ctx) => {
     index,
   } = ctx.request.body;
   const editorVersion = ctx.headers["x-editor-version"] as string | undefined;
-  assertUuid(collectionId, "collectionId must be an uuid");
+
+  if (parentDocumentId || template || publish) {
+    assertPresent(
+      collectionId,
+      publish
+        ? "collectionId is required to publish a draft without collection"
+        : "collectionId is required to create a nested doc or a template"
+    );
+  }
+
+  if (collectionId) {
+    assertUuid(collectionId, "collectionId must be an uuid");
+  }
 
   if (parentDocumentId) {
     assertUuid(parentDocumentId, "parentDocumentId must be an uuid");
@@ -1180,15 +1222,19 @@ router.post("documents.create", auth(), async (ctx) => {
   }
   const { user } = ctx.state;
 
-  const collection = await Collection.scope({
-    method: ["withMembership", user.id],
-  }).findOne({
-    where: {
-      id: collectionId,
-      teamId: user.teamId,
-    },
-  });
-  authorize(user, "publish", collection);
+  let collection;
+
+  if (collectionId) {
+    collection = await Collection.scope({
+      method: ["withMembership", user.id],
+    }).findOne({
+      where: {
+        id: collectionId,
+        teamId: user.teamId,
+      },
+    });
+    authorize(user, "publish", collection);
+  }
 
   let parentDocument;
 
@@ -1196,7 +1242,7 @@ router.post("documents.create", auth(), async (ctx) => {
     parentDocument = await Document.findOne({
       where: {
         id: parentDocumentId,
-        collectionId: collection.id,
+        collectionId: collection?.id,
       },
     });
     authorize(user, "read", parentDocument, {

server/routes/api/hooks.ts

@@ -85,7 +85,7 @@ router.post("hooks.unfurl", async (ctx) => {
     unfurls[link.url] = {
       title: doc.title,
       text: doc.getSummary(),
-      color: doc.collection.color,
+      color: doc.collection?.color,
     };
   }
 
@@ -131,8 +131,8 @@ router.post("hooks.interactive", async (ctx) => {
     attachments: [
       presentSlackAttachment(
         document,
-        document.collection,
         team,
+        document.collection,
         document.getSummary()
       ),
     ],
@@ -303,8 +303,8 @@ router.post("hooks.slack", async (ctx) => {
       attachments.push(
         presentSlackAttachment(
           result.document,
-          result.document.collection,
           team,
+          result.document.collection,
           queryIsInTitle ? undefined : result.context,
           env.SLACK_MESSAGE_ACTIONS
             ? [