Skip to content

Commit

Permalink
fix: feat: check permissions only if ?include_perms
Browse files Browse the repository at this point in the history
fixes type/testing issues with previous commit
  • Loading branch information
pomegranited committed Jan 11, 2024
1 parent 1e2d9b0 commit bb1a0a2
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 13 deletions.
4 changes: 2 additions & 2 deletions openedx_tagging/core/tagging/rest_api/v1/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ def get_can_tag_object(self, instance) -> Optional[bool]:
model = self._model
app_label = model._meta.app_label
perm_name = f'{app_label}.add_objecttag'
perm_object = ObjectTagPermissionItem(taxonomy=instance, object_id=None)
return request.user.has_perm(perm_name, perm_object)
perm_object = ObjectTagPermissionItem(taxonomy=instance, object_id="")
return request.user.has_perm(perm_name, perm_object) # type: ignore[arg-type]


class ObjectTagListQueryParamsSerializer(serializers.Serializer): # pylint: disable=abstract-method
Expand Down
24 changes: 13 additions & 11 deletions tests/openedx_tagging/core/tagging/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1197,7 +1197,7 @@ def test_get_counts_invalid_spec(self):
assert "Wildcard matches are only supported if the * is at the end." in str(result.content)


@ddt.data
@ddt.ddt
class TestTaxonomyTagsView(TestTaxonomyViewMixin):
"""
Tests the list/create/update/delete tags of taxonomy view
Expand Down Expand Up @@ -1442,16 +1442,16 @@ def test_small_query_count(self, include_perms):
expected_perm = True

self.client.force_authenticate(user=self.staff)
with self.assertNumQueries(1):
with self.assertNumQueries(6):
response = self.client.get(url)

assert response.status_code == status.HTTP_200_OK
assert response.data["can_add"] == expected_perm
assert len(response.data["results"]) == 3
for taxonomy in response.data["results"]:
assert taxonomy["can_change"] == expected_perm
assert taxonomy["can_delete"] == expected_perm
assert not taxonomy["can_tag_object"]
# TODO Permission checks are not run for TagData, only Tag instances.
assert taxonomy["can_change"] is None
assert taxonomy["can_delete"] is None

def test_empty_results(self):
"""
Expand Down Expand Up @@ -1486,12 +1486,14 @@ def test_large_taxonomy(self, include_perms):
self.client.force_authenticate(user=self.staff)

expected_perm = None
expected_next_params = 'include_counts=&page=2'
url = self.large_taxonomy_url + "?include_counts"
if include_perms:
url += "&include_perms"
expected_perm = True
expected_next_params = 'include_counts=&include_perms=&page=2'

with self.assertNumQueries(1):
with self.assertNumQueries(4):
response = self.client.get(url)

assert response.status_code == status.HTTP_200_OK
Expand Down Expand Up @@ -1524,19 +1526,20 @@ def test_large_taxonomy(self, include_perms):
f"rest_api/v1/taxonomies/{self.large_taxonomy.id}"
f"/tags/?parent_tag={quote_plus(results[0]['value'])}"
)
assert results[0].get("can_change") == expected_perm
assert results[0].get("can_delete") == expected_perm
assert not results[0].get("can_tag_object")
# TODO Permission checks are not run for TagData, only Tag instances.
assert results[0].get("can_change") is None
assert results[0].get("can_delete") is None

# Checking pagination values
assert data.get("next") == (
"http://testserver/tagging/"
f"rest_api/v1/taxonomies/{self.large_taxonomy.id}/tags/?include_counts=&page=2"
f"rest_api/v1/taxonomies/{self.large_taxonomy.id}/tags/?{expected_next_params}"
)
assert data.get("previous") is None
assert data.get("count") == self.root_tags_count
assert data.get("num_pages") == 6
assert data.get("current_page") == 1
assert data.get("can_add") == expected_perm

def test_next_page_large_taxonomy(self):
self._build_large_taxonomy()
Expand Down Expand Up @@ -1746,7 +1749,6 @@ def test_get_leaves_paginated(self):
]
next_url = response.data.get("next")
assert next_url is not None

response2 = self.client.get(next_url)
results2 = response2.data["results"]
assert pretty_format_tags(results2) == [
Expand Down

0 comments on commit bb1a0a2

Please sign in to comment.