Skip to content

Commit

Permalink
rename remove_perm --> remove_perms
Browse files Browse the repository at this point in the history
  • Loading branch information
RuthShryock committed Aug 29, 2024
1 parent 1ab648d commit 65bca19
Show file tree
Hide file tree
Showing 15 changed files with 42 additions and 42 deletions.
2 changes: 1 addition & 1 deletion kpi/management/commands/sync_kobocat_xforms.py
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ def _sync_permissions(asset, xform):
for p in perms_to_assign:
asset.assign_perms(user_obj, KPI_PKS_TO_CODENAMES[p], skip_kc=True)
for p in perms_to_revoke:
asset.remove_perm(user_obj, KPI_PKS_TO_CODENAMES[p], skip_kc=True)
asset.remove_perms(user_obj, KPI_PKS_TO_CODENAMES[p], skip_kc=True)
if all_revoked and FROM_KC_ONLY_PERMISSION.pk in all_kpi_perms:
# This user's KPI access came only from this script, and now all KC
# permissions have been removed. Purge all KPI grant permissions,
Expand Down
4 changes: 2 additions & 2 deletions kpi/mixins/object_permission.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def copy_permissions_from(self, source_object):
perm_queryset = self.permissions.exclude(user_id=self.owner_id)
# The bulk delete below (i.e.: `perm_queryset.delete()`) does not
# remove permissions in KoBoCAT.
# We could loop through `self.permissions` and call `remove_perm`
# We could loop through `self.permissions` and call `remove_perms`
# for each permission but it would have probably a performance hit
# with assets with lots of permissions.
# Let's use PostgreSQL specific function `ArrayAgg` to retrieve all
Expand Down Expand Up @@ -689,7 +689,7 @@ def has_perms(self, user_obj: User, perms: list, all_: bool = True) -> bool:

@transaction.atomic
@kc_transaction_atomic
def remove_perm(self, user_obj, perm, defer_recalc=False, skip_kc=False):
def remove_perms(self, user_obj, perm, defer_recalc=False, skip_kc=False):
"""
Revoke the given `perm` or list of permissions on this object from
`user_obj`. By default, recalculate descendant objects'
Expand Down
4 changes: 2 additions & 2 deletions kpi/serializers/v2/asset_permission_assignment.py
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ def create(self, validated_data):

# Perform the removals for each user
for user_pk, permissions in user_permissions_to_remove.items():
asset.remove_perm(
asset.remove_perms(
user_obj=user_pk_to_obj_cache[user_pk],
perm=permissions
)
Expand Down Expand Up @@ -450,7 +450,7 @@ def get_set_of_incoming_assignments(
# TODO: refactor permission assignment code so that it does not
# always require a fully-fledged `User` object? Until then, keep a
# stupid object cache thing because `assign_perms()` and
# `remove_perm()` REQUIRE user objects
# `remove_perms()` REQUIRE user objects
user_pk_to_obj_cache[
incoming_assignment['user'].pk
] = incoming_assignment['user']
Expand Down
8 changes: 4 additions & 4 deletions kpi/tests/api/v1/test_api_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def test_anon_cannot_list_permissions(self):
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertListEqual(response.data['results'], [])

self.asset.remove_perm(self.anon, 'view_asset')
self.asset.remove_perms(self.anon, 'view_asset')
self.assertFalse(self.anon.has_perm('view_asset', self.asset))

def test_user_sees_relevant_permissions_on_assigned_objects(self):
Expand Down Expand Up @@ -103,7 +103,7 @@ def test_user_sees_relevant_permissions_on_assigned_objects(self):
sorted(relevant_obj_perms.values_list('uid', flat=True)),
)

self.asset.remove_perm(self.anotheruser, 'view_asset')
self.asset.remove_perms(self.anotheruser, 'view_asset')
self.assertFalse(self.anotheruser.has_perm('view_asset', self.asset))

def test_user_cannot_see_permissions_on_unassigned_objects(self):
Expand All @@ -126,7 +126,7 @@ def test_user_cannot_see_permissions_on_unassigned_objects(self):
)
)

self.asset.remove_perm(self.anotheruser, 'view_asset')
self.asset.remove_perms(self.anotheruser, 'view_asset')
self.assertFalse(self.anotheruser.has_perm('view_asset', self.asset))

def test_superuser_sees_all_permissions(self):
Expand All @@ -146,5 +146,5 @@ def test_superuser_sees_all_permissions(self):
sorted(ObjectPermission.objects.values_list('uid', flat=True))
)

self.asset.remove_perm(self.anotheruser, 'view_asset')
self.asset.remove_perms(self.anotheruser, 'view_asset')
self.assertFalse(self.anotheruser.has_perm('view_asset', self.asset))
4 changes: 2 additions & 2 deletions kpi/tests/api/v2/test_api_asset_permission_assignment.py
Original file line number Diff line number Diff line change
Expand Up @@ -967,9 +967,9 @@ def test_bulk_remove_perm_reduces_queries(self):
with self.assertNumQueries(56):
for user in [self.someuser, self.anotheruser]:
for perm in permissions:
self.asset.remove_perm(user, perm)
self.asset.remove_perms(user, perm)

# Bulk remove permissions
with self.assertNumQueries(42):
for user in [self.someuser, self.anotheruser]:
self.asset.remove_perm(user, permissions)
self.asset.remove_perms(user, permissions)
4 changes: 2 additions & 2 deletions kpi/tests/api/v2/test_api_assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -832,7 +832,7 @@ def test_versions_public_access(self):
response2 = self.client.get(self._version_detail_url)
self.assertEqual(response2.status_code, status.HTTP_200_OK)
# public access regular user
self.asset.remove_perm(user, PERM_VIEW_ASSET)
self.asset.remove_perms(user, PERM_VIEW_ASSET)
self.client.login(username='anotheruser', password='anotheruser')
response3 = self.client.get(self.version_list_url)
self.assertEqual(response3.status_code, status.HTTP_200_OK)
Expand Down Expand Up @@ -1020,7 +1020,7 @@ def test_report_submissions(self):
self.assertEqual(response.status_code, status.HTTP_200_OK)
# Test that a user with the permissions to view submissions can
# access the data
self.asset.remove_perm(anotheruser, PERM_PARTIAL_SUBMISSIONS)
self.asset.remove_perms(anotheruser, PERM_PARTIAL_SUBMISSIONS)
self.asset.assign_perms(anotheruser, PERM_VIEW_SUBMISSIONS)
response = self.client.get(report_url)
self.assertEqual(response.status_code, status.HTTP_200_OK)
Expand Down
2 changes: 1 addition & 1 deletion kpi/tests/api/v2/test_api_paired_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ def test_create_trivial_case(self):
self.client.delete(response.data['url'])

# Try with 'partial_submissions' permission too.
self.source_asset.remove_perm(self.anotheruser, PERM_VIEW_SUBMISSIONS)
self.source_asset.remove_perms(self.anotheruser, PERM_VIEW_SUBMISSIONS)
partial_perms = {
PERM_VIEW_SUBMISSIONS: [{
'_submitted_by': {'$in': [
Expand Down
10 changes: 5 additions & 5 deletions kpi/tests/api/v2/test_api_submissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,7 @@ def test_list_submissions_asset_publicly_shared_as_authenticated_user(self):
self.asset.assign_perms(anonymous_user, PERM_VIEW_SUBMISSIONS)
response = self.client.get(self.submission_list_url, {"format": "json"})
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.asset.remove_perm(anonymous_user, PERM_VIEW_SUBMISSIONS)
self.asset.remove_perms(anonymous_user, PERM_VIEW_SUBMISSIONS)

def test_list_submissions_asset_publicly_shared_and_shared_with_user_as_anotheruser(self):
"""
Expand Down Expand Up @@ -659,10 +659,10 @@ def test_list_submissions_asset_publicly_shared_and_shared_with_user_as_anotheru
assert PERM_VIEW_SUBMISSIONS in self.asset.get_perms(self.anotheruser)

# resetting permissions of asset
self.asset.remove_perm(self.anotheruser, PERM_VIEW_ASSET)
self.asset.remove_perm(self.anotheruser, PERM_CHANGE_ASSET)
self.asset.remove_perm(anonymous_user, PERM_VIEW_ASSET)
self.asset.remove_perm(anonymous_user, PERM_VIEW_SUBMISSIONS)
self.asset.remove_perms(self.anotheruser, PERM_VIEW_ASSET)
self.asset.remove_perms(self.anotheruser, PERM_CHANGE_ASSET)
self.asset.remove_perms(anonymous_user, PERM_VIEW_ASSET)
self.asset.remove_perms(anonymous_user, PERM_VIEW_SUBMISSIONS)

def test_list_query_elem_match(self):
"""
Expand Down
6 changes: 3 additions & 3 deletions kpi/tests/test_assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,7 @@ def grant_and_revoke_standalone(self, user, perm):
self.asset.assign_perms(user, perm)
self.assertEqual(user.has_perm(perm, self.asset), True)
# Revoke
self.asset.remove_perm(user, perm)
self.asset.remove_perms(user, perm)
self.assertEqual(user.has_perm(perm, self.asset), False)

def test_user_view_permission(self):
Expand All @@ -623,7 +623,7 @@ def grant_and_revoke_parent(self, user, perm):
self.coll.assign_perms(user, perm)
self.assertEqual(user.has_perm(perm, self.asset_in_coll), True)
# Revoke
self.coll.remove_perm(user, perm)
self.coll.remove_perms(user, perm)
self.assertEqual(user.has_perm(perm, self.asset_in_coll), False)

def test_user_inherited_view_permission(self):
Expand Down Expand Up @@ -746,7 +746,7 @@ def test_anonymous_view_permission_on_standalone_asset(self):
self.assertTrue(AnonymousUser().has_perm(
PERM_VIEW_ASSET, self.asset))
# Revoke
self.asset.remove_perm(AnonymousUser(), PERM_VIEW_ASSET)
self.asset.remove_perms(AnonymousUser(), PERM_VIEW_ASSET)
self.assertFalse(AnonymousUser().has_perm(
PERM_VIEW_ASSET, self.asset))

Expand Down
8 changes: 4 additions & 4 deletions kpi/tests/test_collections.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ def grant_and_revoke_standalone(self, user, perm):
coll.assign_perms(user, perm)
self.assertEqual(user.has_perm(perm, coll), True)
# Revoke
coll.remove_perm(user, perm)
coll.remove_perms(user, perm)
self.assertEqual(user.has_perm(perm, coll), False)

def test_user_view_permission_on_standalone_collection(self):
Expand All @@ -282,7 +282,7 @@ def grant_and_revoke_parent(self, user, perm):
self.assertEqual(user.has_perm(perm, self.parent_coll), True)
self.assertEqual(user.has_perm(perm, self.child_coll), True)
# Revoke
self.parent_coll.remove_perm(user, perm)
self.parent_coll.remove_perms(user, perm)
self.assertEqual(user.has_perm(perm, self.parent_coll), False)
self.assertEqual(user.has_perm(perm, self.child_coll), False)

Expand All @@ -300,7 +300,7 @@ def grant_and_revoke_child(self, user, perm):
self.assertEqual(user.has_perm(perm, self.parent_coll), False)
self.assertEqual(user.has_perm(perm, self.child_coll), True)
# Revoke
self.child_coll.remove_perm(user, perm)
self.child_coll.remove_perms(user, perm)
self.assertEqual(user.has_perm(perm, self.parent_coll), False)
self.assertEqual(user.has_perm(perm, self.child_coll), False)

Expand Down Expand Up @@ -508,7 +508,7 @@ def test_anonymous_view_permission_on_standalone_collection(self):
self.assertTrue(AnonymousUser().has_perm(
PERM_VIEW_ASSET, self.standalone_coll))
# Revoke
self.standalone_coll.remove_perm(AnonymousUser(), PERM_VIEW_ASSET)
self.standalone_coll.remove_perms(AnonymousUser(), PERM_VIEW_ASSET)
self.assertFalse(AnonymousUser().has_perm(
PERM_VIEW_ASSET, self.standalone_coll))

Expand Down
4 changes: 2 additions & 2 deletions kpi/tests/test_mock_data_exports.py
Original file line number Diff line number Diff line change
Expand Up @@ -1191,8 +1191,8 @@ def test_anotheruser_can_export_when_submissions_publicly_shared(self):
unable to export submission data.
"""
# resetting permissions of `anotheruser` to have no permissions
self.asset.remove_perm(self.anotheruser, PERM_PARTIAL_SUBMISSIONS)
self.asset.remove_perm(self.anotheruser, PERM_VIEW_ASSET)
self.asset.remove_perms(self.anotheruser, PERM_PARTIAL_SUBMISSIONS)
self.asset.remove_perms(self.anotheruser, PERM_VIEW_ASSET)

anonymous_user = get_anonymous_user()

Expand Down
2 changes: 1 addition & 1 deletion kpi/tests/test_paired_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ def test_cannot_retrieve_source_if_source_stop_data_sharing(self):
def test_cannot_retrieve_source_if_source_remove_perm(self):
source = self.paired_data.get_source()
self.assertEqual(source, self.source_asset)
self.source_asset.remove_perm(
self.source_asset.remove_perms(
self.paired_data.asset.owner, PERM_VIEW_SUBMISSIONS
)
source = self.paired_data.get_source(force=True)
Expand Down
22 changes: 11 additions & 11 deletions kpi/tests/test_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def _test_remove_perm(self, obj, perm_name_prefix, user):
"""
perm_name = self._get_perm_name(perm_name_prefix, obj)
self.assertTrue(user.has_perm(perm_name, obj))
obj.remove_perm(user, perm_name)
obj.remove_perms(user, perm_name)
self.assertFalse(user.has_perm(perm_name, obj))

def _test_add_inherited_perm(self, ancestor_collection, perm_name_prefix,
Expand Down Expand Up @@ -123,7 +123,7 @@ def _test_add_and_remove_perm(self, obj, perm_name_prefix, user):
"""
self._test_add_perm(obj, perm_name_prefix, user)
remove_perm_name = self._get_perm_name(perm_name_prefix, obj)
obj.remove_perm(user, remove_perm_name)
obj.remove_perms(user, remove_perm_name)
self.assertFalse(user.has_perm(remove_perm_name, obj))

def _test_add_remove_inherited_perm(self, ancestor_asset,
Expand Down Expand Up @@ -154,7 +154,7 @@ def _test_add_remove_inherited_perm(self, ancestor_asset,
ancestor_perm_name = self._get_perm_name(
perm_name_prefix, ancestor_asset
)
ancestor_asset.remove_perm(user, ancestor_perm_name)
ancestor_asset.remove_perms(user, ancestor_perm_name)
self.assertFalse(user.has_perm(descendant_perm_name, descendant_obj))


Expand Down Expand Up @@ -236,7 +236,7 @@ def test_remove_submission_permission(self):
asset.assign_perms(grantee, codename)
self.assertTrue(grantee.has_perm(codename, asset))
for codename in reversed(codenames):
asset.remove_perm(grantee, codename)
asset.remove_perms(grantee, codename)
self.assertFalse(grantee.has_perm(codename, asset))

def test_add_asset_inherited_permission(self):
Expand Down Expand Up @@ -278,9 +278,9 @@ def test_implied_asset_grant_permissions(self):
self.assertListEqual(
sorted(asset.get_perms(grantee)), sorted(expected))
# Wipe the slate
asset.remove_perm(grantee, explicit)
asset.remove_perms(grantee, explicit)
for i in implied:
asset.remove_perm(grantee, i)
asset.remove_perms(grantee, i)

def test_remove_implied_asset_permissions(self):
"""
Expand All @@ -303,10 +303,10 @@ def test_remove_implied_asset_permissions(self):
self.assertListEqual(
sorted(asset.get_perms(grantee)), sorted(expected_perms)
)
asset.remove_perm(grantee, PERM_VIEW_ASSET)
asset.remove_perms(grantee, PERM_VIEW_ASSET)
# `add_submissions` does not imply `view_asset` anymore.
self.assertListEqual(asset.get_perms(grantee), [PERM_ADD_SUBMISSIONS])
asset.remove_perm(grantee, PERM_ADD_SUBMISSIONS)
asset.remove_perms(grantee, PERM_ADD_SUBMISSIONS)
self.assertListEqual(asset.get_perms(grantee), [])

asset.assign_perms(grantee, PERM_VALIDATE_SUBMISSIONS)
Expand All @@ -318,7 +318,7 @@ def test_remove_implied_asset_permissions(self):
self.assertListEqual(
sorted(asset.get_perms(grantee)), sorted(expected_perms)
)
asset.remove_perm(grantee, PERM_VIEW_ASSET)
asset.remove_perms(grantee, PERM_VIEW_ASSET)
self.assertListEqual(asset.get_perms(grantee), [])

def test_implied_asset_deny_permissions(self):
Expand All @@ -344,7 +344,7 @@ def test_implied_asset_deny_permissions(self):
sorted(asset.get_perms(grantee)), [PERM_CHANGE_ASSET, PERM_VIEW_ASSET])

# Removing `view_asset` should deny all child permissions
asset.remove_perm(grantee, PERM_VIEW_ASSET)
asset.remove_perms(grantee, PERM_VIEW_ASSET)
self.assertListEqual(asset.get_perms(grantee), [])

# Check that there actually deny permissions
Expand Down Expand Up @@ -719,7 +719,7 @@ def test_remove_partial_submission_permission(self):
self.assertTrue(asset.asset_partial_permissions.count() == 1)

# Then remove it
asset.remove_perm(grantee, PERM_PARTIAL_SUBMISSIONS)
asset.remove_perms(grantee, PERM_PARTIAL_SUBMISSIONS)
self.assertFalse(grantee.has_perm(PERM_PARTIAL_SUBMISSIONS, asset))
self.assertTrue(asset.asset_partial_permissions.count() == 0)

Expand Down
2 changes: 1 addition & 1 deletion kpi/views/v1/object_permission.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def perform_destroy(self, instance):
self.request.user, PERM_MANAGE_ASSET
):
raise exceptions.PermissionDenied()
instance.content_object.remove_perm(
instance.content_object.remove_perms(
instance.user,
instance.permission.codename
)
2 changes: 1 addition & 1 deletion kpi/views/v2/asset_permission_assignment.py
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ def destroy(self, request, *args, **kwargs):
)

codename = object_permission.permission.codename
self.asset.remove_perm(user, codename)
self.asset.remove_perms(user, codename)
return Response(status=status.HTTP_204_NO_CONTENT)

def get_serializer_context(self):
Expand Down

0 comments on commit 65bca19

Please sign in to comment.