fix: Cannot mention users that have been explicitly added to document

server/models/Document.ts

@@ -194,6 +194,13 @@ type AdditionalFindOptions = {
       ],
     };
   },
+  withAllMemberships: {
+    include: [
+      {
+        association: "memberships",
+      },
+    ],
+  },
 }))
 @Table({ tableName: "documents", modelName: "document" })
 @Fix
@@ -535,6 +542,25 @@ class Document extends ParanoidModel<
   @HasMany(() => View)
   views: View[];
 
+  /**
+   * Returns an array of unique userIds that are members of a document via direct membership
+   *
+   * @param documentId
+   * @returns userIds
+   */
+  static async membershipUserIds(documentId: string) {
+    const document = await this.scope("withAllMemberships").findOne({
+      where: {
+        id: documentId,
+      },
+    });
+    if (!document) {
+      return [];
+    }
+
+    return document.memberships.map((membership) => membership.userId);
+  }
+
   static defaultScopeWithUser(userId: string) {
     const collectionScope: Readonly<ScopeOptions> = {
       method: ["withCollectionPermissions", userId],

server/routes/api/documents/documents.test.ts

@@ -4,6 +4,7 @@ import {
   CollectionPermission,
   DocumentPermission,
   StatusFilter,
+  UserRole,
 } from "@shared/types";
 import {
   Document,
@@ -3912,6 +3913,64 @@ describe("#documents.users", () => {
     expect(memberIds).toContain(user.id);
   });
 
+  it("should return collection users when collection is private", async () => {
+    const user = await buildUser();
+    const collection = await buildCollection({
+      teamId: user.teamId,
+      userId: user.id,
+      permission: null,
+    });
+    const document = await buildDocument({
+      collectionId: collection.id,
+      userId: user.id,
+      teamId: user.teamId,
+    });
+    const [alan, ken] = await Promise.all([
+      buildUser({
+        name: "Alan Kay",
+        teamId: user.teamId,
+      }),
+
+      buildUser({
+        name: "Ken",
+        teamId: user.teamId,
+      }),
+      buildUser({
+        name: "Bret Victor",
+        teamId: user.teamId,
+      }),
+    ]);
+
+    await UserMembership.create({
+      createdById: alan.id,
+      collectionId: collection.id,
+      userId: alan.id,
+      permission: CollectionPermission.ReadWrite,
+    });
+
+    await UserMembership.create({
+      createdById: ken.id,
+      documentId: document.id,
+      userId: ken.id,
+      permission: CollectionPermission.ReadWrite,
+    });
+
+    const res = await server.post("/api/documents.users", {
+      body: {
+        token: user.getJwtToken(),
+        id: document.id,
+      },
+    });
+    const body = await res.json();
+
+    expect(res.status).toBe(200);
+    expect(body.data.length).toBe(2);
+
+    const memberIds = body.data.map((u: User) => u.id);
+    expect(memberIds).toContain(alan.id);
+    expect(memberIds).toContain(ken.id);
+  });
+
   it("should return document users with names matching the search query", async () => {
     const user = await buildUser({
       // Ensure the generated name doesn't match
@@ -4028,7 +4087,7 @@ describe("#documents.users", () => {
     expect(memberNames).toContain(jamie.name);
   });
 
-  it("should not return suspended users", async () => {
+  it("should not return suspended or guest users", async () => {
     const user = await buildUser();
     const collection = await buildCollection({
       teamId: user.teamId,
@@ -4053,6 +4112,11 @@ describe("#documents.users", () => {
         name: "Ken Thompson",
         teamId: user.teamId,
       }),
+      buildUser({
+        name: "Guest",
+        teamId: user.teamId,
+        role: UserRole.Guest,
+      }),
     ]);
 
     // add people to collection

server/routes/api/documents/documents.ts

@@ -5,6 +5,7 @@ import invariant from "invariant";
 import JSZip from "jszip";
 import Router from "koa-router";
 import escapeRegExp from "lodash/escapeRegExp";
+import uniq from "lodash/uniq";
 import mime from "mime-types";
 import { Op, ScopeOptions, Sequelize, WhereOptions } from "sequelize";
 import { v4 as uuidv4 } from "uuid";
@@ -476,36 +477,46 @@ router.post(
       },
     };
 
-    if (document.collectionId) {
-      const collection = await document.$get("collection");
+    const [collection, memberIds, collectionMemberIds] = await Promise.all([
+      document.$get("collection"),
+      Document.membershipUserIds(document.id),
+      document.collectionId
+        ? Collection.membershipUserIds(document.collectionId)
+        : [],
+    ]);
 
-      if (!collection?.permission) {
-        const memberIds = await Collection.membershipUserIds(
-          document.collectionId
-        );
-        where = {
-          ...where,
+    where = {
+      ...where,
+      [Op.or]: [
+        {
           id: {
-            [Op.in]: memberIds,
-          },
-        };
-      }
-
-      if (query) {
-        where = {
-          ...where,
-          name: {
-            [Op.iLike]: `%${query}%`,
+            [Op.in]: uniq([...memberIds, ...collectionMemberIds]),
           },
-        };
-      }
+        },
+        collection?.permission
+          ? {
+              role: {
+                [Op.ne]: UserRole.Guest,
+              },
+            }
+          : {},
+      ],
+    };
 
-      [users, total] = await Promise.all([
-        User.findAll({ where, offset, limit }),
-        User.count({ where }),
-      ]);
+    if (query) {
+      where = {
+        ...where,
+        name: {
+          [Op.iLike]: `%${query}%`,
+        },
+      };
     }
 
+    [users, total] = await Promise.all([
+      User.findAll({ where, offset, limit }),
+      User.count({ where }),
+    ]);
+
     ctx.body = {
       pagination: { ...ctx.state.pagination, total },
       data: users.map((user) => presentUser(user)),