Simplify user/group permissions destroying to only accept a single permission (to follow same cadence)

Author: schloerke

integration/tests/posit/connect/test_content_item_permissions.py

@@ -1,3 +1,5 @@
+import pytest
+
 from posit import connect
 from posit.connect.content import ContentItem
 
@@ -59,11 +61,9 @@ def assert_permissions_match_guids(permissions, objs_with_guid):
         )
 
         # Remove permissions (and from some that isn't an owner)
-        destroyed_permissions = self.content.permissions.destroy(self.user_aron, self.user_bill)
-        assert_permissions_match_guids(
-            destroyed_permissions,
-            [self.user_aron],
-        )
+        self.content.permissions.destroy(self.user_aron)
+        with pytest.raises(ValueError):
+            self.content.permissions.destroy(self.user_bill)
 
         # Prove they have been removed
         assert_permissions_match_guids(

src/posit/connect/permissions.py

@@ -111,8 +111,15 @@ def find(self, **kwargs) -> List[Permission]:
         """
         path = f"v1/content/{self.content_guid}/permissions"
         url = self.params.url + path
-        response = self.params.session.get(url, json=kwargs)
+        response = self.params.session.get(url)
+        kwargs_items = kwargs.items()
         results = response.json()
+        if len(kwargs_items) > 0:
+            results = [
+                result
+                for result in results
+                if isinstance(result, dict) and (result.items() >= kwargs_items)
+            ]
         return [Permission(self.params, **result) for result in results]
 
     def find_one(self, **kwargs) -> Permission | None:
@@ -142,25 +149,18 @@ def get(self, uid: str) -> Permission:
         response = self.params.session.get(url)
         return Permission(self.params, **response.json())
 
-    def destroy(self, *permissions: str | Group | User | Permission) -> list[Permission]:
-        """Remove supplied content item permissions.
+    def destroy(self, permission: str | Group | User | Permission, /) -> None:
+        """Remove supplied content item permission.
 
-        Removes all provided permissions from the content item's permissions. If a permission isn't
-        found, it is silently ignored.
+        Removes provided permission from the content item's permissions.
 
         Parameters
         ----------
-        *permissions : str | Group | User | Permission
-            The content item permissions to remove. If a `str` is received, it is compared against
+        permission : str | Group | User | Permission
+            The content item permission to remove. If a `str` is received, it is compared against
             the `Permissions`'s `principal_guid`. If a `Group` or `User` is received, the associated
             `Permission` will be removed.
 
-        Returns
-        -------
-        list[Permission]
-            The removed permissions. If a permission is not found, there is nothing to remove and 
-            it is not included in the returned list.
-
         Examples
         --------
         ```python
@@ -175,51 +175,46 @@ def destroy(self, *permissions: str | Group | User | Permission) -> list[Permiss
         ############################
 
         client = connect.Client()
+        content_item = client.content.get(content_guid)
 
         # Remove a single permission by principal_guid
-        client.content.get(content_guid).permissions.destroy(principal_guid)
+        content_item.permissions.destroy(principal_guid)
 
         # Remove by user (if principal_guid is a user)
         user = client.users.get(principal_guid)
-        client.content.get(content_guid).permissions.destroy(user)
+        content_item.permissions.destroy(user)
 
         # Remove by group (if principal_guid is a group)
         group = client.groups.get(principal_guid)
-        client.content.get(content_guid).permissions.destroy(group)
+        content_item.permissions.destroy(group)
 
         # Remove all groups with a matching prefix name
         groups = client.groups.find(prefix=group_name_prefix)
-        client.content.get(content_guid).permissions.destroy(*groups)
+        for group in groups:
+            content_item.permissions.destroy(group)
 
         # Confirm new permissions
-        client.content.get(content_guid).permissions.find()
+        content_item.permissions.find()
         ```
         """
         from .groups import Group
         from .users import User
 
-        if len(permissions) == 0:
-            raise ValueError("Expected at least one `permission` to remove")
-
-        principal_guids: set[str] = set()
-
-        for arg in permissions:
-            if isinstance(arg, str):
-                principal_guid = arg
-            elif isinstance(arg, (Group, User)):
-                principal_guid: str = arg["guid"]
-            elif isinstance(arg, Permission):
-                principal_guid: str = arg["principal_guid"]
-            else:
-                raise TypeError(
-                    f"destroy() expected argument type 'str', 'User', 'Group', or 'Permission' but got '{type(arg).__name__}'",
-                )
-            principal_guids.add(principal_guid)
-
-        destroyed_permissions: list[Permission] = []
-        for permission in self.find():
-            if permission["principal_guid"] in principal_guids:
-                permission.destroy()
-                destroyed_permissions.append(permission)
-
-        return destroyed_permissions
+        if isinstance(permission, str):
+            permission_obj = self.get(permission)
+        elif isinstance(permission, (Group, User)):
+            principal_guid: str = permission["guid"]
+            permission_obj = self.find_one(
+                principal_guid=principal_guid,
+            )
+            print("Barret!", permission, principal_guid, permission_obj)
+            if permission_obj is None:
+                raise ValueError(f"Permission with principal_guid '{principal_guid}' not found.")
+        elif isinstance(permission, Permission):
+            permission_obj = permission
+        else:
+            raise TypeError(
+                f"destroy() expected `permission=` to have type `str | User | Group | Permission`. Received `{permission}` of type `{type(permission)}`.",
+            )
+
+        permission_obj.destroy()

src/posit/connect/users.py

@@ -2,7 +2,7 @@
 
 from __future__ import annotations
 
-from typing import TYPE_CHECKING, List, Literal, Optional, overload
+from typing import TYPE_CHECKING, List, Literal
 
 from typing_extensions import NotRequired, Required, TypedDict, Unpack
 
@@ -169,26 +169,14 @@ def __init__(self, ctx: Context, user_guid: str) -> None:
         self._ctx: Context = ctx
         self._user_guid: str = user_guid
 
-    @overload
-    def add(self, group: Group, /) -> None: ...
-    @overload
-    def add(self, /, *, group_guid: str) -> None: ...
-
-    def add(self, group: Optional[Group] = None, /, *, group_guid: Optional[str] = None) -> None:
+    def add(self, group: str | Group) -> None:
         """
         Add the user to the specified group.
 
         Parameters
         ----------
-        group : Group
-            The groups to which the user will be added. Only one of `group=` or `group_guid=` can
-            be provided.
-        group_guid : str
-            The unique identifier (guid) of the group to which the user will be added.
-
-        Returns
-        -------
-        None
+        group : str | Group
+            The group guid or `Group` object to which the user will be added.
 
         Examples
         --------
@@ -212,57 +200,35 @@ def add(self, group: Optional[Group] = None, /, *, group_guid: Optional[str] = N
             user.groups.add(group)
 
         # Add the user to a group by GUID
-        user.groups.add(group_guid="GROUP_GUID_HERE")
+        user.groups.add("GROUP_GUID_HERE")
         ```
 
         See Also
         --------
         * https://docs.posit.co/connect/api/#post-/v1/groups/-group_guid-/members
         """
-        if group is not None:
-            from .groups import Group
-
-            if group_guid:
-                raise ValueError("Only one of `group=` or `group_guid` may be be provided.")
+        from .groups import Group
 
-            if not isinstance(group, Group):
-                raise TypeError(
-                    f"`group=` is not an instance of Group. Received {group}",
-                )
+        if isinstance(group, Group):
             group.members.add(user_guid=self._user_guid)
             return
 
-        if not group_guid:
-            raise ValueError("Only one of `group=` or `group_guid` may be be provided.")
-        group = self._ctx.client.groups.get(group_guid)
-        group.members.add(user_guid=self._user_guid)
+        if not isinstance(group, str):
+            raise TypeError(f"`group=` must be a `str | Group`. Received {group}")
+        if not group:
+            raise ValueError("`group=` must not be empty.")
 
-    @overload
-    def delete(self, group: Group, /) -> None: ...
-    @overload
-    def delete(self, /, *, group_guid: str) -> None: ...
+        group_obj = self._ctx.client.groups.get(group)
+        group_obj.members.add(user_guid=self._user_guid)
 
-    def delete(
-        self,
-        group: Optional[Group] = None,
-        /,
-        *,
-        group_guid: Optional[str] = None,
-    ) -> None:
+    def delete(self, group: str | Group) -> None:
         """
         Remove the user from the specified group.
 
         Parameters
         ----------
-        group : Group
-            The groups to which the user will be added. Only one of `group=` or `group_guid=` can
-            be provided.
-        group_guid : str
-            The unique identifier (guid) of the group from which the user will be removed.
-
-        Returns
-        -------
-        None
+        group : str | Group
+            The group to which the user will be added.
 
         Examples
         --------
@@ -286,33 +252,26 @@ def delete(
             user.groups.delete(group)
 
         # Remove the user from a group by GUID
-        user.groups.delete(group_guid="GROUP_GUID_HERE")
+        user.groups.delete("GROUP_GUID_HERE")
         ```
 
         See Also
         --------
         * https://docs.posit.co/connect/api/#delete-/v1/groups/-group_guid-/members/-user_guid-
         """
-        if group is not None:
-            from .groups import Group
-
-            if group_guid:
-                raise ValueError("Only one of `group=` or `group_guid=` may be be provided.")
+        from .groups import Group
 
-            if not isinstance(group, Group):
-                raise TypeError(
-                    f"`group=` is not an instance of Group. Received {group}",
-                )
+        if isinstance(group, Group):
             group.members.delete(user_guid=self._user_guid)
             return
 
-        if not isinstance(group_guid, str):
-            raise TypeError("`group_guid=` must be a string. Received {user}")
-        if not group_guid:
-            raise ValueError("`group_guid=` must not be empty.")
+        if not isinstance(group, str):
+            raise TypeError(f"`group=` must be a `str | Group`. Received {group}")
+        if not group:
+            raise ValueError("`group=` must not be empty.")
 
-        group = self._ctx.client.groups.get(group_guid)
-        group.members.delete(user_guid=self._user_guid)
+        group_obj = self._ctx.client.groups.get(group)
+        group_obj.members.delete(user_guid=self._user_guid)
 
     def find(self) -> List[Group]:
         """

tests/posit/connect/__api__/v1/content/f2f37341-e21d-3d80-c698-a935ad614066/permissions.json

@@ -1,16 +1,16 @@
 [
-    {
-        "id": 94,
-        "content_guid": "f2f37341-e21d-3d80-c698-a935ad614066",
-        "principal_guid": "78974391-d89f-4f11-916a-ba50cfe993db",
-        "principal_type": "user",
-        "role": "owner"
-    },
-    {
-        "id": 59,
-        "content_guid": "f2f37341-e21d-3d80-c698-a935ad614066",
-        "principal_guid": "75b95fc0-ae02-4d85-8732-79a845143eed",
-        "principal_type": "group",
-        "role": "viewer"
-    }
+  {
+    "id": 94,
+    "content_guid": "f2f37341-e21d-3d80-c698-a935ad614066",
+    "principal_guid": "20a79ce3-6e87-4522-9faf-be24228800a4",
+    "principal_type": "user",
+    "role": "owner"
+  },
+  {
+    "id": 59,
+    "content_guid": "f2f37341-e21d-3d80-c698-a935ad614066",
+    "principal_guid": "6f300623-1e0c-48e6-a473-ddf630c0c0c3",
+    "principal_type": "group",
+    "role": "viewer"
+  }
 ]

tests/posit/connect/__api__/v1/content/f2f37341-e21d-3d80-c698-a935ad614066/permissions/59.json

@@ -0,0 +1,7 @@
+{
+  "id": 59,
+  "content_guid": "f2f37341-e21d-3d80-c698-a935ad614066",
+  "principal_guid": "6f300623-1e0c-48e6-a473-ddf630c0c0c3",
+  "principal_type": "group",
+  "role": "viewer"
+}

tests/posit/connect/__api__/v1/content/f2f37341-e21d-3d80-c698-a935ad614066/permissions/94.json

@@ -1,7 +1,7 @@
 {
-    "id": 94,
-    "content_guid": "f2f37341-e21d-3d80-c698-a935ad614066",
-    "principal_guid": "78974391-d89f-4f11-916a-ba50cfe993db",
-    "principal_type": "user",
-    "role": "owner"
+  "id": "94",
+  "content_guid": "f2f37341-e21d-3d80-c698-a935ad614066",
+  "principal_guid": "20a79ce3-6e87-4522-9faf-be24228800a4",
+  "principal_type": "user",
+  "role": "owner"
 }