From 4a99f9f38676d6901e872870eec9fe3d3131d025 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Mon, 3 Apr 2023 21:05:22 -0400 Subject: [PATCH] fix: Mentions do not show any options in public collections (#5150) * Mentions do not show any options in public collections * Avoid reset data while loading --- app/editor/components/MentionMenu.tsx | 36 +++++++-------- server/routes/api/documents/documents.test.ts | 45 ++++++------------- server/routes/api/documents/documents.ts | 30 ++++++++----- 3 files changed, 50 insertions(+), 61 deletions(-) diff --git a/app/editor/components/MentionMenu.tsx b/app/editor/components/MentionMenu.tsx index 3bd7b5904..571575ab3 100644 --- a/app/editor/components/MentionMenu.tsx +++ b/app/editor/components/MentionMenu.tsx @@ -42,7 +42,7 @@ function MentionMenu({ search, isActive, ...rest }: Props) { const location = useLocation(); const documentId = parseDocumentSlug(location.pathname); const { view } = useEditor(); - const { data, request } = useRequest( + const { data, loading, request } = useRequest( React.useCallback( () => documentId @@ -59,24 +59,24 @@ function MentionMenu({ search, isActive, ...rest }: Props) { }, [request, isActive]); React.useEffect(() => { - if (data) { - setItems( - data.map((user) => ({ - name: "mention", - user, - title: user.name, - appendSpace: true, - attrs: { - id: v4(), - type: MentionType.User, - modelId: user.id, - actorId: auth.user?.id, - label: user.name, - }, - })) - ); + if (data && !loading) { + const items = data.map((user) => ({ + name: "mention", + user, + title: user.name, + appendSpace: true, + attrs: { + id: v4(), + type: MentionType.User, + modelId: user.id, + actorId: auth.user?.id, + label: user.name, + }, + })); + + setItems(items); } - }, [auth.user?.id, data]); + }, [auth.user?.id, loading, data]); const clearSearch = () => { const { state, dispatch } = view; diff --git a/server/routes/api/documents/documents.test.ts b/server/routes/api/documents/documents.test.ts index a57e7cfa4..53b7ce601 100644 --- a/server/routes/api/documents/documents.test.ts +++ b/server/routes/api/documents/documents.test.ts @@ -3082,15 +3082,15 @@ describe("#documents.unpublish", () => { }); describe("#documents.users", () => { - it("should return document users", async () => { + it("should return all users when collection is not private", async () => { const user = await buildUser(); const collection = await buildCollection({ teamId: user.teamId, - createdById: user.id, + userId: user.id, }); const document = await buildDocument({ collectionId: collection.id, - createdById: user.id, + userId: user.id, teamId: user.teamId, }); const [alan, bret, ken] = await Promise.all([ @@ -3108,28 +3108,6 @@ describe("#documents.users", () => { }), ]); - // add people to collection - await Promise.all([ - CollectionUser.create({ - collectionId: collection.id, - userId: alan.id, - permission: CollectionPermission.Read, - createdById: user.id, - }), - CollectionUser.create({ - collectionId: collection.id, - userId: bret.id, - permission: CollectionPermission.Read, - createdById: user.id, - }), - CollectionUser.create({ - collectionId: collection.id, - userId: ken.id, - permission: CollectionPermission.Read, - createdById: user.id, - }), - ]); - const res = await server.post("/api/documents.users", { body: { token: user.getJwtToken(), @@ -3139,23 +3117,25 @@ describe("#documents.users", () => { const body = await res.json(); expect(res.status).toBe(200); - expect(body.data.length).toBe(3); + expect(body.data.length).toBe(4); const memberIds = body.data.map((u: User) => u.id); expect(memberIds).toContain(alan.id); expect(memberIds).toContain(bret.id); expect(memberIds).toContain(ken.id); + expect(memberIds).toContain(user.id); }); it("should return document users with names matching the search query", async () => { const user = await buildUser(); const collection = await buildCollection({ teamId: user.teamId, - createdById: user.id, + userId: user.id, + permission: null, }); const document = await buildDocument({ collectionId: collection.id, - createdById: user.id, + userId: user.id, teamId: user.teamId, }); const [alan, bret, ken, jamie] = await Promise.all([ @@ -3248,7 +3228,7 @@ describe("#documents.users", () => { expect(body.data[0].name).toBe(alan.name); expect(anotherRes.status).toBe(200); - expect(anotherBody.data.length).toBe(3); + expect(anotherBody.data.length).toBe(4); const memberIds = anotherBody.data.map((u: User) => u.id); const memberNames = anotherBody.data.map((u: User) => u.name); expect(memberIds).toContain(bret.id); @@ -3263,11 +3243,12 @@ describe("#documents.users", () => { const user = await buildUser(); const collection = await buildCollection({ teamId: user.teamId, - createdById: user.id, + userId: user.id, + permission: null, }); const document = await buildDocument({ collectionId: collection.id, - createdById: user.id, + userId: user.id, teamId: user.teamId, }); const [alan, bret, ken] = await Promise.all([ @@ -3320,7 +3301,7 @@ describe("#documents.users", () => { const body = await res.json(); expect(res.status).toBe(200); - expect(body.data.length).toBe(2); + expect(body.data.length).toBe(3); const memberIds = body.data.map((u: User) => u.id); expect(memberIds).not.toContain(alan.id); diff --git a/server/routes/api/documents/documents.ts b/server/routes/api/documents/documents.ts index b8258c3c6..9746578f0 100644 --- a/server/routes/api/documents/documents.ts +++ b/server/routes/api/documents/documents.ts @@ -452,20 +452,28 @@ router.post( let users: User[] = []; let total = 0; + let where: WhereOptions = { + teamId: document.teamId, + suspendedAt: { + [Op.is]: null, + }, + }; if (document.collectionId) { - const memberIds = await Collection.membershipUserIds( - document.collectionId - ); + const collection = await document.$get("collection"); + + if (!collection?.permission) { + const memberIds = await Collection.membershipUserIds( + document.collectionId + ); + where = { + ...where, + id: { + [Op.in]: memberIds, + }, + }; + } - let where: WhereOptions = { - id: { - [Op.in]: memberIds, - }, - suspendedAt: { - [Op.is]: null, - }, - }; if (query) { where = { ...where,