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

@@ -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;

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);

server/routes/api/documents/documents.ts

@@ -452,20 +452,28 @@ router.post(
 
     let users: User[] = [];
     let total = 0;
+    let where: WhereOptions<User> = {
+      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<User> = {
-        id: {
-          [Op.in]: memberIds,
-        },
-        suspendedAt: {
-          [Op.is]: null,
-        },
-      };
       if (query) {
         where = {
           ...where,