-
Notifications
You must be signed in to change notification settings - Fork 8
feat: Add Permissions.delete(*permissions) method
#339
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f737bc6
e498165
32e5024
85fdc1e
1b50382
1119e31
a9fbb36
9eaf38d
8269629
ecdf800
b2cd9d2
ba02668
c3261ed
2ce66b3
debb45a
19f25dd
11e4730
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| from posit import connect | ||
| from posit.connect.content import ContentItem | ||
|
|
||
|
|
||
| class TestContentPermissions: | ||
| content: ContentItem | ||
|
|
||
| @classmethod | ||
| def setup_class(cls): | ||
| cls.client = connect.Client() | ||
| cls.content = cls.client.content.create(name="example") | ||
|
|
||
| cls.user_aron = cls.client.users.create( | ||
| username="permission_aron", | ||
| email="permission_aron@example.com", | ||
| password="permission_s3cur3p@ssword", | ||
| ) | ||
| cls.user_bill = cls.client.users.create( | ||
| username="permission_bill", | ||
| email="permission_bill@example.com", | ||
| password="permission_s3cur3p@ssword", | ||
| ) | ||
|
|
||
| cls.group_friends = cls.client.groups.create(name="Friends") | ||
|
|
||
| @classmethod | ||
| def teardown_class(cls): | ||
| cls.content.delete() | ||
| assert cls.client.content.count() == 0 | ||
|
|
||
| cls.group_friends.delete() | ||
| assert cls.client.groups.count() == 0 | ||
|
|
||
| def test_permissions_add_destroy(self): | ||
| assert self.client.groups.count() == 1 | ||
| assert self.client.users.count() == 3 | ||
| assert self.content.permissions.find() == [] | ||
|
|
||
| # Add permissions | ||
| self.content.permissions.create( | ||
| principal_guid=self.user_aron["guid"], | ||
| principal_type="user", | ||
| role="viewer", | ||
| ) | ||
| self.content.permissions.create( | ||
| principal_guid=self.group_friends["guid"], | ||
| principal_type="group", | ||
| role="owner", | ||
| ) | ||
|
|
||
| def assert_permissions_match_guids(permissions, objs_with_guid): | ||
| for permission, obj_with_guid in zip(permissions, objs_with_guid): | ||
| assert permission["principal_guid"] == obj_with_guid["guid"] | ||
|
|
||
| # Prove they have been added | ||
| assert_permissions_match_guids( | ||
| self.content.permissions.find(), | ||
| [self.user_aron, self.group_friends], | ||
| ) | ||
|
|
||
| # 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], | ||
| ) | ||
|
|
||
| # Prove they have been removed | ||
| assert_permissions_match_guids( | ||
| self.content.permissions.find(), | ||
| [self.group_friends], | ||
| ) | ||
|
|
||
| # Remove the last permission | ||
| destroyed_permissions = self.content.permissions.destroy(self.group_friends) | ||
| assert_permissions_match_guids( | ||
| destroyed_permissions, | ||
| [self.group_friends], | ||
| ) | ||
|
|
||
| # Prove they have been removed | ||
| assert self.content.permissions.find() == [] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,16 +2,20 @@ | |
|
|
||
| from __future__ import annotations | ||
|
|
||
| from typing import List, overload | ||
| from typing import TYPE_CHECKING, List, overload | ||
|
|
||
| from requests.sessions import Session as Session | ||
|
|
||
| from .resources import Resource, ResourceParameters, Resources | ||
|
|
||
| if TYPE_CHECKING: | ||
| from .groups import Group | ||
| from .users import User | ||
|
|
||
|
|
||
| class Permission(Resource): | ||
| def delete(self) -> None: | ||
| """Delete the permission.""" | ||
| def destroy(self) -> None: | ||
| """Destroy the permission.""" | ||
| path = f"v1/content/{self['content_guid']}/permissions/{self['id']}" | ||
| url = self.params.url + path | ||
| self.params.session.delete(url) | ||
|
|
@@ -137,3 +141,85 @@ def get(self, uid: str) -> Permission: | |
| url = self.params.url + path | ||
| 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. | ||
|
|
||
| Removes all provided permissions from the content item's permissions. If a permission isn't | ||
| found, it is silently ignored. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| *permissions : str | Group | User | Permission | ||
| The content item permissions 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 | ||
| from posit import connect | ||
|
|
||
| #### User-defined inputs #### | ||
| # 1. specify the guid for the content item | ||
| content_guid = "CONTENT_GUID_HERE" | ||
| # 2. specify either the principal_guid or group name prefix | ||
| principal_guid = "USER_OR_GROUP_GUID_HERE" | ||
| group_name_prefix = "GROUP_NAME_PREFIX_HERE" | ||
| ############################ | ||
|
|
||
| client = connect.Client() | ||
|
|
||
| # Remove a single permission by principal_guid | ||
| client.content.get(content_guid).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) | ||
|
|
||
| # Remove by group (if principal_guid is a group) | ||
| group = client.groups.get(principal_guid) | ||
| client.content.get(content_guid).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) | ||
|
|
||
| # Confirm new permissions | ||
| client.content.get(content_guid).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 | ||
|
Comment on lines
+207
to
+208
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have other facilities for checking if a string is the right shape to be a GUID? That might be nice to check here since someone might try passing
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No. We do not have the functionality. I believe the thought process is to have the server return an error and we display the error.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nods that makes sense. What does the server return as the error in that case? Is it something we expect SDK authors to know how to recover from? Doing something more than ^^^ is probably scope creep, but I am curious what the ergonomics would be
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we instead change the approach of this method to always submit the delete API calls to the server? (rather than only the permissions we know exist.) Then the server will always display an error. And we wouldn't need to do any validation on our end.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AAAH right on line 223 it will silently be ignore if it's anything other than a matching GUID. HMMM I don't have a strong intuition which direction would be better here, pre-checking or always sending and failing. There certainly are complications with either.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For posterity, did you end up implementing this or does it still (silently) ignore non-GUIDs (or really anything that doesn't match a GUID already present if it's a string)?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It still silently ignores non existent permissions |
||
| 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) | ||
schloerke marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| return destroyed_permissions | ||
Uh oh!
There was an error while loading. Please reload this page.