Merge pull request #1094 from garden-co/jazz-612-give-the-ability-to-…

…reader-writer-and-writeonly-roles-to-set feat: give the ability to extend a group to users that have any access to the parent
garden-co · Jan 2, 2025 · 99b9605 · 99b9605
2 parents 830f0c0 + 10ea733
commit 99b9605
Show file tree

Hide file tree

Showing 6 changed files with 126 additions and 75 deletions.
diff --git a/.changeset/tidy-dolphins-drum.md b/.changeset/tidy-dolphins-drum.md
@@ -0,0 +1,5 @@
+---
+"cojson": patch
+---
+
+Give the ability to extend a group to accounts with reader, writer and writeOnly access level to the parent group and not only admins. The account still needs to be an admin on the child group to be able to extend it.
diff --git a/packages/cojson/src/coValues/group.ts b/packages/cojson/src/coValues/group.ts
@@ -518,9 +518,20 @@ export class RawGroup<
   }
 
   extend(parent: RawGroup) {
-    if (parent.myRole() !== "admin" || this.myRole() !== "admin") {
+    if (this.myRole() !== "admin") {
       throw new Error(
-        "To extend a group, the current account must have admin role in both groups",
+        "To extend a group, the current account must be an admin in the child group",
+      );
+    }
+
+    if (
+      parent.myRole() !== "admin" &&
+      parent.myRole() !== "writer" &&
+      parent.myRole() !== "reader" &&
+      parent.myRole() !== "writeOnly"
+    ) {
+      throw new Error(
+        "To extend a group, the current account must have access to the parent group",
       );
     }
 

diff --git a/packages/cojson/src/permissions.ts b/packages/cojson/src/permissions.ts
@@ -311,8 +311,15 @@ function determineValidTransactionsForGroup(
       validTransactions.push({ txID: { sessionID, txIndex }, tx });
       continue;
     } else if (isChildExtension(change.key)) {
-      if (memberState[transactor] !== "admin") {
-        logPermissionError("Only admins can set child extensions");
+      if (
+        memberState[transactor] !== "admin" &&
+        memberState[transactor] !== "writer" &&
+        memberState[transactor] !== "reader" &&
+        memberState[transactor] !== "writeOnly"
+      ) {
+        logPermissionError(
+          "Only admins, writers, readers and writeOnly can set child extensions",
+        );
         continue;
       }
       validTransactions.push({ txID: { sessionID, txIndex }, tx });

diff --git a/packages/cojson/src/tests/group.test.ts b/packages/cojson/src/tests/group.test.ts
@@ -528,4 +528,85 @@ describe("writeOnly", () => {
     // The writer role should be able to see the edits from the admin
     expect(mapOnNode2.get("test")).toEqual("Written from the writeOnly member");
   });
+
+  test("a user should be able to extend a group when his role on the parent group is writer", async () => {
+    const { node1, node2 } = await createTwoConnectedNodes("server", "server");
+
+    const group = node1.node.createGroup();
+    group.addMember(
+      await loadCoValueOrFail(node1.node, node2.accountID),
+      "writer",
+    );
+
+    await group.core.waitForSync();
+
+    const groupOnNode2 = await loadCoValueOrFail(node2.node, group.id);
+
+    const childGroup = node2.node.createGroup();
+    childGroup.extend(groupOnNode2);
+
+    const map = childGroup.createMap();
+    map.set("test", "Written from node2");
+
+    await map.core.waitForSync();
+    await childGroup.core.waitForSync();
+
+    const mapOnNode2 = await loadCoValueOrFail(node2.node, map.id);
+
+    expect(mapOnNode2.get("test")).toEqual("Written from node2");
+  });
+
+  test("a user should be able to extend a group when his role on the parent group is reader", async () => {
+    const { node1, node2 } = await createTwoConnectedNodes("server", "server");
+
+    const group = node1.node.createGroup();
+    group.addMember(
+      await loadCoValueOrFail(node1.node, node2.accountID),
+      "reader",
+    );
+
+    await group.core.waitForSync();
+
+    const groupOnNode2 = await loadCoValueOrFail(node2.node, group.id);
+
+    const childGroup = node2.node.createGroup();
+    childGroup.extend(groupOnNode2);
+
+    const map = childGroup.createMap();
+    map.set("test", "Written from node2");
+
+    await map.core.waitForSync();
+    await childGroup.core.waitForSync();
+
+    const mapOnNode2 = await loadCoValueOrFail(node2.node, map.id);
+
+    expect(mapOnNode2.get("test")).toEqual("Written from node2");
+  });
+
+  test("a user should be able to extend a group when his role on the parent group is writeOnly", async () => {
+    const { node1, node2 } = await createTwoConnectedNodes("server", "server");
+
+    const group = node1.node.createGroup();
+    group.addMember(
+      await loadCoValueOrFail(node1.node, node2.accountID),
+      "writeOnly",
+    );
+
+    await group.core.waitForSync();
+
+    const groupOnNode2 = await loadCoValueOrFail(node2.node, group.id);
+
+    const childGroup = node2.node.createGroup();
+    childGroup.extend(groupOnNode2);
+
+    const map = childGroup.createMap();
+    map.set("test", "Written from node2");
+
+    await map.core.waitForSync();
+    await childGroup.core.waitForSync();
+
+    const mapOnNode2 = await loadCoValueOrFail(node2.node, map.id);
+
+    expect(mapOnNode2.get("test")).toEqual("Written from node2");
+  });
 });
diff --git a/packages/cojson/src/tests/permissions.test.ts b/packages/cojson/src/tests/permissions.test.ts
@@ -2157,21 +2157,17 @@ test("Admins can set child extensions when the admin role is inherited", async (
   );
 });
 
-test("Writers, readers and invitees can not set child extensions", () => {
+test("Writers, readers and writeOnly can set child extensions", () => {
   const { group, node } = newGroupHighLevel();
   const childGroup = node.createGroup();
 
   const writer = node.createAccount();
   const reader = node.createAccount();
-  const adminInvite = node.createAccount();
-  const writerInvite = node.createAccount();
-  const readerInvite = node.createAccount();
+  const writeOnly = node.createAccount();
 
   group.addMember(writer, "writer");
   group.addMember(reader, "reader");
-  group.addMember(adminInvite, "adminInvite");
-  group.addMember(writerInvite, "writerInvite");
-  group.addMember(readerInvite, "readerInvite");
+  group.addMember(writeOnly, "writeOnly");
 
   const groupAsWriter = expectGroup(
     group.core
@@ -2180,7 +2176,7 @@ test("Writers, readers and invitees can not set child extensions", () => {
   );
 
   groupAsWriter.set(`child_${childGroup.id}`, "extend", "trusting");
-  expect(groupAsWriter.get(`child_${childGroup.id}`)).toBeUndefined();
+  expect(groupAsWriter.get(`child_${childGroup.id}`)).toEqual("extend");
 
   const groupAsReader = expectGroup(
     group.core
@@ -2189,7 +2185,20 @@ test("Writers, readers and invitees can not set child extensions", () => {
   );
 
   groupAsReader.set(`child_${childGroup.id}`, "extend", "trusting");
-  expect(groupAsReader.get(`child_${childGroup.id}`)).toBeUndefined();
+  expect(groupAsReader.get(`child_${childGroup.id}`)).toEqual("extend");
+});
+
+test("Invitees can not set child extensions", () => {
+  const { group, node } = newGroupHighLevel();
+  const childGroup = node.createGroup();
+
+  const adminInvite = node.createAccount();
+  const writerInvite = node.createAccount();
+  const readerInvite = node.createAccount();
+
+  group.addMember(adminInvite, "adminInvite");
+  group.addMember(writerInvite, "writerInvite");
+  group.addMember(readerInvite, "readerInvite");
 
   const groupAsAdminInvite = expectGroup(
     group.core

diff --git a/packages/jazz-tools/src/tests/groupsAndAccounts.test.ts b/packages/jazz-tools/src/tests/groupsAndAccounts.test.ts
@@ -166,68 +166,6 @@ describe("Group inheritance", () => {
     expect(mapAsReaderAfterUpdate?.title).toBe("In Grand Child");
   });
 
-  test("Group inheritance should fail if the current account doesn't have admin role in both groups", async () => {
-    const me = await Account.create({
-      creationProps: { name: "Hermes Puggington" },
-      crypto: Crypto,
-    });
-
-    const other = await Account.createAs(me, {
-      creationProps: { name: "Another user" },
-    });
-
-    const parentGroup = Group.create({ owner: me });
-    parentGroup.addMember(other, "writer");
-    const group = Group.create({ owner: me });
-    group.addMember(other, "admin");
-
-    const parentGroupOnTheOtherSide = await Group.load(
-      parentGroup.id,
-      other,
-      {},
-    );
-    const groupOnTheOtherSide = await Group.load(group.id, other, {});
-
-    if (!groupOnTheOtherSide || !parentGroupOnTheOtherSide) {
-      throw new Error("CoValue not available");
-    }
-
-    expect(() => groupOnTheOtherSide.extend(parentGroupOnTheOtherSide)).toThrow(
-      "To extend a group, the current account must have admin role in both groups",
-    );
-  });
-
-  test("Group inheritance should work if the current account has admin role in both groups", async () => {
-    const me = await Account.create({
-      creationProps: { name: "Hermes Puggington" },
-      crypto: Crypto,
-    });
-
-    const other = await Account.createAs(me, {
-      creationProps: { name: "Another user" },
-    });
-
-    const parentGroup = Group.create({ owner: me });
-    parentGroup.addMember(other, "admin");
-    const group = Group.create({ owner: me });
-    group.addMember(other, "admin");
-
-    const parentGroupOnTheOtherSide = await Group.load(
-      parentGroup.id,
-      other,
-      {},
-    );
-    const groupOnTheOtherSide = await Group.load(group.id, other, {});
-
-    if (!groupOnTheOtherSide || !parentGroupOnTheOtherSide) {
-      throw new Error("CoValue not available");
-    }
-
-    expect(() =>
-      groupOnTheOtherSide.extend(parentGroupOnTheOtherSide),
-    ).not.toThrow();
-  });
-
   test("waitForSync should resolve when the value is uploaded", async () => {
     const { clientNode, serverNode, clientAccount } = await setupTwoNodes();