diff --git a/server/models/Document.ts b/server/models/Document.ts index ca817a253..931e600c4 100644 --- a/server/models/Document.ts +++ b/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 = { method: ["withCollectionPermissions", userId], diff --git a/server/routes/api/documents/documents.test.ts b/server/routes/api/documents/documents.test.ts index b0863cce7..f8db3ca19 100644 --- a/server/routes/api/documents/documents.test.ts +++ b/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 diff --git a/server/routes/api/documents/documents.ts b/server/routes/api/documents/documents.ts index f4f3acea1..a87c5c3b0 100644 --- a/server/routes/api/documents/documents.ts +++ b/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, + [Op.in]: uniq([...memberIds, ...collectionMemberIds]), }, - }; - } + }, + collection?.permission + ? { + role: { + [Op.ne]: UserRole.Guest, + }, + } + : {}, + ], + }; - if (query) { - where = { - ...where, - name: { - [Op.iLike]: `%${query}%`, - }, - }; - } - - [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)),