From ecc4a0d53de568770e4fd97756f33e78c7a11dca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Thu, 24 Aug 2023 14:32:07 -0300 Subject: [PATCH] feat: add taxonomies for org api (#32871) * feat: add taxonomies for org api * chore: run CI * feat: Add retrieve object_tags REST API (#577) * chore: update requirements --------- Co-authored-by: Yusuf Musleh --- cms/urls.py | 5 + openedx/features/content_tagging/api.py | 8 +- .../features/content_tagging/models/base.py | 2 +- .../content_tagging/rest_api/__init__.py | 0 .../features/content_tagging/rest_api/urls.py | 9 + .../content_tagging/rest_api/v1/__init__.py | 0 .../content_tagging/rest_api/v1/filters.py | 21 + .../rest_api/v1/serializers.py | 23 + .../rest_api/v1/tests/__init__.py | 0 .../rest_api/v1/tests/test_views.py | 641 ++++++++++++++++++ .../content_tagging/rest_api/v1/urls.py | 19 + .../content_tagging/rest_api/v1/views.py | 60 ++ openedx/features/content_tagging/rules.py | 55 +- .../content_tagging/tests/test_api.py | 12 +- .../content_tagging/tests/test_rules.py | 306 +++++---- openedx/features/content_tagging/urls.py | 10 + requirements/constraints.txt | 5 - requirements/edx/base.txt | 8 +- requirements/edx/development.txt | 5 +- requirements/edx/doc.txt | 8 +- requirements/edx/testing.txt | 8 +- 21 files changed, 1033 insertions(+), 172 deletions(-) create mode 100644 openedx/features/content_tagging/rest_api/__init__.py create mode 100644 openedx/features/content_tagging/rest_api/urls.py create mode 100644 openedx/features/content_tagging/rest_api/v1/__init__.py create mode 100644 openedx/features/content_tagging/rest_api/v1/filters.py create mode 100644 openedx/features/content_tagging/rest_api/v1/serializers.py create mode 100644 openedx/features/content_tagging/rest_api/v1/tests/__init__.py create mode 100644 openedx/features/content_tagging/rest_api/v1/tests/test_views.py create mode 100644 openedx/features/content_tagging/rest_api/v1/urls.py create mode 100644 openedx/features/content_tagging/rest_api/v1/views.py create mode 100644 openedx/features/content_tagging/urls.py diff --git a/cms/urls.py b/cms/urls.py index 2a0fd7051b..ffbd9f9298 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -332,3 +332,8 @@ urlpatterns.extend(get_plugin_url_patterns(ProjectType.CMS)) urlpatterns += [ path('api/contentstore/', include('cms.djangoapps.contentstore.rest_api.urls')) ] + +# Content tagging +urlpatterns += [ + path('api/content_tagging/', include(('openedx.features.content_tagging.urls'))), +] diff --git a/openedx/features/content_tagging/api.py b/openedx/features/content_tagging/api.py index f1bb43f66c..b2cb4653b2 100644 --- a/openedx/features/content_tagging/api.py +++ b/openedx/features/content_tagging/api.py @@ -101,20 +101,16 @@ def get_taxonomies_for_org( def get_content_tags( - object_id: str, taxonomy: Taxonomy = None, valid_only=True + object_id: str, taxonomy_id: str = None ) -> Iterator[ContentObjectTag]: """ Generates a list of content tags for a given object. Pass taxonomy to limit the returned object_tags to a specific taxonomy. - - Pass valid_only=False when displaying tags to content authors, so they can see invalid tags too. - Invalid tags will (probably) be hidden from learners. """ for object_tag in oel_tagging.get_object_tags( object_id=object_id, - taxonomy=taxonomy, - valid_only=valid_only, + taxonomy_id=taxonomy_id, ): yield ContentObjectTag.cast(object_tag) diff --git a/openedx/features/content_tagging/models/base.py b/openedx/features/content_tagging/models/base.py index 255d3852fa..16df0d3752 100644 --- a/openedx/features/content_tagging/models/base.py +++ b/openedx/features/content_tagging/models/base.py @@ -49,7 +49,7 @@ class TaxonomyOrg(models.Model): @classmethod def get_relationships( - cls, taxonomy: Taxonomy, rel_type: RelType, org_short_name: str = None + cls, taxonomy: Taxonomy, rel_type: RelType, org_short_name: Union[str, None] = None ) -> QuerySet: """ Returns the relationships of the given rel_type and taxonomy where: diff --git a/openedx/features/content_tagging/rest_api/__init__.py b/openedx/features/content_tagging/rest_api/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/features/content_tagging/rest_api/urls.py b/openedx/features/content_tagging/rest_api/urls.py new file mode 100644 index 0000000000..d7f012bb7b --- /dev/null +++ b/openedx/features/content_tagging/rest_api/urls.py @@ -0,0 +1,9 @@ +""" +Taxonomies API URLs. +""" + +from django.urls import path, include + +from .v1 import urls as v1_urls + +urlpatterns = [path("v1/", include(v1_urls))] diff --git a/openedx/features/content_tagging/rest_api/v1/__init__.py b/openedx/features/content_tagging/rest_api/v1/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/features/content_tagging/rest_api/v1/filters.py b/openedx/features/content_tagging/rest_api/v1/filters.py new file mode 100644 index 0000000000..ee8771d17e --- /dev/null +++ b/openedx/features/content_tagging/rest_api/v1/filters.py @@ -0,0 +1,21 @@ +""" +API Filters for content tagging org +""" + +from rest_framework.filters import BaseFilterBackend + +from ...rules import is_taxonomy_admin + + +class UserOrgFilterBackend(BaseFilterBackend): + """ + Taxonomy admin can see all taxonomies + Everyone else can see only enabled taxonomies + + """ + + def filter_queryset(self, request, queryset, _): + if is_taxonomy_admin(request.user): + return queryset + + return queryset.filter(enabled=True) diff --git a/openedx/features/content_tagging/rest_api/v1/serializers.py b/openedx/features/content_tagging/rest_api/v1/serializers.py new file mode 100644 index 0000000000..1771816a14 --- /dev/null +++ b/openedx/features/content_tagging/rest_api/v1/serializers.py @@ -0,0 +1,23 @@ +""" +API Serializers for content tagging org +""" + +from rest_framework import serializers + +from openedx_tagging.core.tagging.rest_api.v1.serializers import ( + TaxonomyListQueryParamsSerializer, +) + +from organizations.models import Organization + + +class TaxonomyOrgListQueryParamsSerializer(TaxonomyListQueryParamsSerializer): + """ + Serializer for the query params for the GET view + """ + + org = serializers.SlugRelatedField( + slug_field="short_name", + queryset=Organization.objects.all(), + required=False, + ) diff --git a/openedx/features/content_tagging/rest_api/v1/tests/__init__.py b/openedx/features/content_tagging/rest_api/v1/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/features/content_tagging/rest_api/v1/tests/test_views.py b/openedx/features/content_tagging/rest_api/v1/tests/test_views.py new file mode 100644 index 0000000000..a8177426eb --- /dev/null +++ b/openedx/features/content_tagging/rest_api/v1/tests/test_views.py @@ -0,0 +1,641 @@ +""" +Tests tagging rest api views +""" + +from urllib.parse import parse_qs, urlparse + +import ddt +from django.contrib.auth import get_user_model +from django.test.testcases import override_settings +from openedx_tagging.core.tagging.models import Taxonomy +from openedx_tagging.core.tagging.models.system_defined import SystemDefinedTaxonomy +from openedx_tagging.core.tagging.rest_api.v1.serializers import TaxonomySerializer +from organizations.models import Organization +from rest_framework import status +from rest_framework.test import APITestCase + +from common.djangoapps.student.auth import update_org_role +from common.djangoapps.student.roles import OrgContentCreatorRole +from openedx.core.djangolib.testing.utils import skip_unless_cms +from openedx.features.content_tagging.models import TaxonomyOrg + +User = get_user_model() + +TAXONOMY_ORG_LIST_URL = "/api/content_tagging/v1/taxonomies/" +TAXONOMY_ORG_DETAIL_URL = "/api/content_tagging/v1/taxonomies/{pk}/" + + +def check_taxonomy( + data, + pk, + name, + description=None, + enabled=True, + required=False, + allow_multiple=False, + allow_free_text=False, + system_defined=False, + visible_to_authors=True, + **_ +): + """ + Check the given data against the expected values. + """ + assert data["id"] == pk + assert data["name"] == name + assert data["description"] == description + assert data["enabled"] == enabled + assert data["required"] == required + assert data["allow_multiple"] == allow_multiple + assert data["allow_free_text"] == allow_free_text + assert data["system_defined"] == system_defined + assert data["visible_to_authors"] == visible_to_authors + + +class TestTaxonomyViewSetMixin: + """ + Sets up data for testing Content Taxonomies. + """ + + def setUp(self): + super().setUp() + self.user = User.objects.create( + username="user", + email="user@example.com", + ) + self.userS = User.objects.create( + username="staff", + email="staff@example.com", + is_staff=True, + ) + + self.orgA = Organization.objects.create(name="Organization A", short_name="orgA") + self.orgB = Organization.objects.create(name="Organization B", short_name="orgB") + self.orgX = Organization.objects.create(name="Organization X", short_name="orgX") + + self.userA = User.objects.create( + username="userA", + email="userA@example.com", + ) + update_org_role(self.userS, OrgContentCreatorRole, self.userA, [self.orgA.short_name]) + + # Orphaned taxonomy + self.ot1 = Taxonomy.objects.create(name="ot1", enabled=True) + self.ot2 = Taxonomy.objects.create(name="ot2", enabled=False) + + # System defined taxonomy + self.st1 = Taxonomy.objects.create(name="st1", enabled=True) + self.st1.taxonomy_class = SystemDefinedTaxonomy + self.st1.save() + TaxonomyOrg.objects.create( + taxonomy=self.st1, + rel_type=TaxonomyOrg.RelType.OWNER, + org=None, + ) + self.st2 = Taxonomy.objects.create(name="st2", enabled=False) + self.st2.taxonomy_class = SystemDefinedTaxonomy + self.st2.save() + TaxonomyOrg.objects.create( + taxonomy=self.st2, + rel_type=TaxonomyOrg.RelType.OWNER, + ) + + # Global taxonomy + self.t1 = Taxonomy.objects.create(name="t1", enabled=True) + TaxonomyOrg.objects.create( + taxonomy=self.t1, + rel_type=TaxonomyOrg.RelType.OWNER, + ) + self.t2 = Taxonomy.objects.create(name="t2", enabled=False) + TaxonomyOrg.objects.create( + taxonomy=self.t2, + rel_type=TaxonomyOrg.RelType.OWNER, + ) + + # OrgA taxonomy + self.tA1 = Taxonomy.objects.create(name="tA1", enabled=True) + TaxonomyOrg.objects.create( + taxonomy=self.tA1, + org=self.orgA, + rel_type=TaxonomyOrg.RelType.OWNER, + ) + self.tA2 = Taxonomy.objects.create(name="tA2", enabled=False) + TaxonomyOrg.objects.create( + taxonomy=self.tA2, + org=self.orgA, + rel_type=TaxonomyOrg.RelType.OWNER, + ) + + # OrgB taxonomy + self.tB1 = Taxonomy.objects.create(name="tB1", enabled=True) + TaxonomyOrg.objects.create( + taxonomy=self.tB1, + org=self.orgB, + rel_type=TaxonomyOrg.RelType.OWNER, + ) + self.tB2 = Taxonomy.objects.create(name="tB2", enabled=False) + TaxonomyOrg.objects.create( + taxonomy=self.tB2, + org=self.orgB, + rel_type=TaxonomyOrg.RelType.OWNER, + ) + + # OrgA and OrgB taxonomy + self.tC1 = Taxonomy.objects.create(name="tC1", enabled=True) + TaxonomyOrg.objects.create( + taxonomy=self.tC1, + org=self.orgA, + rel_type=TaxonomyOrg.RelType.OWNER, + ) + TaxonomyOrg.objects.create( + taxonomy=self.tC1, + org=self.orgB, + rel_type=TaxonomyOrg.RelType.OWNER, + ) + self.tC2 = Taxonomy.objects.create(name="tC2", enabled=False) + TaxonomyOrg.objects.create( + taxonomy=self.tC2, + org=self.orgA, + rel_type=TaxonomyOrg.RelType.OWNER, + ) + TaxonomyOrg.objects.create( + taxonomy=self.tC2, + org=self.orgB, + rel_type=TaxonomyOrg.RelType.OWNER, + ) + + +@skip_unless_cms +@ddt.ddt +@override_settings(FEATURES={"ENABLE_CREATOR_GROUP": True}) +class TestTaxonomyViewSet(TestTaxonomyViewSetMixin, APITestCase): + """ + Test cases for TaxonomyViewSet when ENABLE_CREATOR_GROUP is True + """ + + @ddt.data( + ("user", None, None, ("ot1", "st1", "t1", "tA1", "tB1", "tC1")), + ("userA", None, None, ("ot1", "st1", "t1", "tA1", "tB1", "tC1")), + ("userS", None, None, ("ot1", "ot2", "st1", "st2", "t1", "t2", "tA1", "tA2", "tB1", "tB2")), + # Default page_size=10, and so "tC1" and "tC2" appear on the second page + ("user", True, None, ("ot1", "st1", "t1", "tA1", "tB1", "tC1")), + ("userA", True, None, ("ot1", "st1", "t1", "tA1", "tB1", "tC1")), + ("userS", True, None, ("ot1", "st1", "t1", "tA1", "tB1", "tC1")), + ("user", False, None, ()), + ("userA", False, None, ()), + ("userS", False, None, ("ot2", "st2", "t2", "tA2", "tB2", "tC2")), + ("user", None, "orgA", ("st1", "t1", "tA1", "tC1")), + ("userA", None, "orgA", ("st1", "t1", "tA1", "tC1")), + ("userS", None, "orgA", ("st1", "st2", "t1", "t2", "tA1", "tA2", "tC1", "tC2")), + ("user", True, "orgA", ("st1", "t1", "tA1", "tC1")), + ("userA", True, "orgA", ("st1", "t1", "tA1", "tC1")), + ("userS", True, "orgA", ("st1", "t1", "tA1", "tC1")), + ("user", False, "orgA", ()), + ("userA", False, "orgA", ()), + ("userS", False, "orgA", ("st2", "t2", "tA2", "tC2")), + ("user", None, "orgX", ("st1", "t1")), + ("userA", None, "orgX", ("st1", "t1")), + ("userS", None, "orgX", ("st1", "st2", "t1", "t2")), + ("user", True, "orgX", ("st1", "t1")), + ("userA", True, "orgX", ("st1", "t1")), + ("userS", True, "orgX", ("st1", "t1")), + ("user", False, "orgX", ()), + ("userA", False, "orgX", ()), + ("userS", False, "orgX", ("st2", "t2")), + ) + @ddt.unpack + def test_list_taxonomy(self, user_attr, enabled_parameter, org_name, expected_taxonomies): + url = TAXONOMY_ORG_LIST_URL + + if user_attr: + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + # Set parameters cleaning empty values + query_params = {k: v for k, v in {"enabled": enabled_parameter, "org": org_name}.items() if v is not None} + + response = self.client.get(url, query_params, format="json") + + assert response.status_code == status.HTTP_200_OK + self.assertEqual(set(t["name"] for t in response.data["results"]), set(expected_taxonomies)) + + def test_list_taxonomy_invalid_org( + self, + ): + url = TAXONOMY_ORG_LIST_URL + + self.client.force_authenticate(user=self.userS) + + # Set parameters cleaning empty values + query_params = {"org": "invalidOrg"} + + response = self.client.get(url, query_params, format="json") + + assert response.status_code == status.HTTP_400_BAD_REQUEST + + @ddt.data( + ("user", ("tA1", "tB1", "tC1"), None), + ("userA", ("tA1", "tB1", "tC1"), None), + ("userS", ("st2", "t1", "t2"), "3"), + ) + @ddt.unpack + def test_list_taxonomy_pagination(self, user_attr, expected_taxonomies, expected_next_page): + url = TAXONOMY_ORG_LIST_URL + + if user_attr: + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + query_params = {"page_size": 3, "page": 2} + + response = self.client.get(url, query_params, format="json") + + assert response.status_code == status.HTTP_200_OK + self.assertEqual(set(t["name"] for t in response.data["results"]), set(expected_taxonomies)) + parsed_url = urlparse(response.data["next"]) + + next_page = parse_qs(parsed_url.query).get("page", [None])[0] + assert next_page == expected_next_page + + def test_list_invalid_page(self): + url = TAXONOMY_ORG_LIST_URL + + self.client.force_authenticate(user=self.user) + + query_params = {"page": 123123} + + response = self.client.get(url, query_params, format="json") + + assert response.status_code == status.HTTP_404_NOT_FOUND + + @ddt.data( + (None, "ot1", status.HTTP_403_FORBIDDEN), + (None, "ot2", status.HTTP_403_FORBIDDEN), + (None, "st1", status.HTTP_403_FORBIDDEN), + (None, "st2", status.HTTP_403_FORBIDDEN), + (None, "t1", status.HTTP_403_FORBIDDEN), + (None, "t2", status.HTTP_403_FORBIDDEN), + (None, "tA1", status.HTTP_403_FORBIDDEN), + (None, "tA2", status.HTTP_403_FORBIDDEN), + (None, "tB1", status.HTTP_403_FORBIDDEN), + (None, "tB2", status.HTTP_403_FORBIDDEN), + (None, "tC1", status.HTTP_403_FORBIDDEN), + (None, "tC2", status.HTTP_403_FORBIDDEN), + ("user", "ot1", status.HTTP_200_OK), + ("user", "ot2", status.HTTP_404_NOT_FOUND), + ("user", "st1", status.HTTP_200_OK), + ("user", "st2", status.HTTP_404_NOT_FOUND), + ("user", "t1", status.HTTP_200_OK), + ("user", "t2", status.HTTP_404_NOT_FOUND), + ("user", "tA1", status.HTTP_200_OK), + ("user", "tA2", status.HTTP_404_NOT_FOUND), + ("user", "tB1", status.HTTP_200_OK), + ("user", "tB2", status.HTTP_404_NOT_FOUND), + ("user", "tC1", status.HTTP_200_OK), + ("user", "tC2", status.HTTP_404_NOT_FOUND), + ("userA", "ot1", status.HTTP_200_OK), + ("userA", "ot2", status.HTTP_404_NOT_FOUND), + ("userA", "st1", status.HTTP_200_OK), + ("userA", "st2", status.HTTP_404_NOT_FOUND), + ("userA", "t1", status.HTTP_200_OK), + ("userA", "t2", status.HTTP_404_NOT_FOUND), + ("userA", "tA1", status.HTTP_200_OK), + ("userA", "tA2", status.HTTP_404_NOT_FOUND), + ("userA", "tB1", status.HTTP_200_OK), + ("userA", "tB2", status.HTTP_404_NOT_FOUND), + ("userA", "tC1", status.HTTP_200_OK), + ("userA", "tC2", status.HTTP_404_NOT_FOUND), + ("userS", "ot1", status.HTTP_200_OK), + ("userS", "ot2", status.HTTP_200_OK), + ("userS", "st1", status.HTTP_200_OK), + ("userS", "st2", status.HTTP_200_OK), + ("userS", "t1", status.HTTP_200_OK), + ("userS", "t2", status.HTTP_200_OK), + ("userS", "tA1", status.HTTP_200_OK), + ("userS", "tA2", status.HTTP_200_OK), + ("userS", "tB1", status.HTTP_200_OK), + ("userS", "tB2", status.HTTP_200_OK), + ("userS", "tC1", status.HTTP_200_OK), + ("userS", "tC2", status.HTTP_200_OK), + ) + @ddt.unpack + def test_detail_taxonomy(self, user_attr, taxonomy_attr, expected_status): + taxonomy = getattr(self, taxonomy_attr) + + url = TAXONOMY_ORG_DETAIL_URL.format(pk=taxonomy.pk) + + if user_attr: + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + response = self.client.get(url) + assert response.status_code == expected_status + + if status.is_success(expected_status): + check_taxonomy(response.data, taxonomy.pk, **(TaxonomySerializer(taxonomy.cast()).data)) + + @ddt.data( + (None, status.HTTP_403_FORBIDDEN), + ("user", status.HTTP_403_FORBIDDEN), + ("userA", status.HTTP_403_FORBIDDEN), + ("userS", status.HTTP_201_CREATED), + ) + @ddt.unpack + def test_create_taxonomy(self, user_attr, expected_status): + url = TAXONOMY_ORG_LIST_URL + + create_data = { + "name": "taxonomy_data", + "description": "This is a description", + "enabled": True, + "required": True, + "allow_multiple": True, + } + + if user_attr: + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + response = self.client.post(url, create_data, format="json") + assert response.status_code == expected_status + + # If we were able to create the taxonomy, check if it was created + if status.is_success(expected_status): + check_taxonomy(response.data, response.data["id"], **create_data) + url = TAXONOMY_ORG_DETAIL_URL.format(pk=response.data["id"]) + + response = self.client.get(url) + check_taxonomy(response.data, response.data["id"], **create_data) + + @ddt.data( + (None, "ot1", status.HTTP_403_FORBIDDEN), + (None, "ot2", status.HTTP_403_FORBIDDEN), + (None, "st1", status.HTTP_403_FORBIDDEN), + (None, "st2", status.HTTP_403_FORBIDDEN), + (None, "t1", status.HTTP_403_FORBIDDEN), + (None, "t2", status.HTTP_403_FORBIDDEN), + (None, "tA1", status.HTTP_403_FORBIDDEN), + (None, "tA2", status.HTTP_403_FORBIDDEN), + (None, "tB1", status.HTTP_403_FORBIDDEN), + (None, "tB2", status.HTTP_403_FORBIDDEN), + (None, "tC1", status.HTTP_403_FORBIDDEN), + (None, "tC2", status.HTTP_403_FORBIDDEN), + ("user", "ot1", status.HTTP_403_FORBIDDEN), + ("user", "ot2", status.HTTP_403_FORBIDDEN), + ("user", "st1", status.HTTP_403_FORBIDDEN), + ("user", "st2", status.HTTP_403_FORBIDDEN), + ("user", "t1", status.HTTP_403_FORBIDDEN), + ("user", "t2", status.HTTP_403_FORBIDDEN), + ("user", "tA1", status.HTTP_403_FORBIDDEN), + ("user", "tA2", status.HTTP_403_FORBIDDEN), + ("user", "tB1", status.HTTP_403_FORBIDDEN), + ("user", "tB2", status.HTTP_403_FORBIDDEN), + ("user", "tC1", status.HTTP_403_FORBIDDEN), + ("user", "tC2", status.HTTP_403_FORBIDDEN), + ("userA", "ot1", status.HTTP_403_FORBIDDEN), + ("userA", "ot2", status.HTTP_403_FORBIDDEN), + ("userA", "st1", status.HTTP_403_FORBIDDEN), + ("userA", "st2", status.HTTP_403_FORBIDDEN), + ("userA", "t1", status.HTTP_403_FORBIDDEN), + ("userA", "t2", status.HTTP_403_FORBIDDEN), + ("userA", "tA1", status.HTTP_403_FORBIDDEN), + ("userA", "tA2", status.HTTP_403_FORBIDDEN), + ("userA", "tB1", status.HTTP_403_FORBIDDEN), + ("userA", "tB2", status.HTTP_403_FORBIDDEN), + ("userA", "tC1", status.HTTP_403_FORBIDDEN), + ("userA", "tC2", status.HTTP_403_FORBIDDEN), + ("userS", "ot1", status.HTTP_200_OK), + ("userS", "ot2", status.HTTP_200_OK), + ("userS", "st1", status.HTTP_403_FORBIDDEN), + ("userS", "st2", status.HTTP_403_FORBIDDEN), + ("userS", "t1", status.HTTP_200_OK), + ("userS", "t2", status.HTTP_200_OK), + ("userS", "t1", status.HTTP_200_OK), + ("userS", "t2", status.HTTP_200_OK), + ("userS", "tA1", status.HTTP_200_OK), + ("userS", "tA2", status.HTTP_200_OK), + ("userS", "tB1", status.HTTP_200_OK), + ("userS", "tB2", status.HTTP_200_OK), + ("userS", "tC1", status.HTTP_200_OK), + ("userS", "tC2", status.HTTP_200_OK), + ) + @ddt.unpack + def test_update_taxonomy(self, user_attr, taxonomy_attr, expected_status): + taxonomy = getattr(self, taxonomy_attr) + + url = TAXONOMY_ORG_DETAIL_URL.format(pk=taxonomy.pk) + + if user_attr: + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + response = self.client.put(url, {"name": "new name"}, format="json") + assert response.status_code == expected_status + + # If we were able to update the taxonomy, check if the name changed + if status.is_success(expected_status): + response = self.client.get(url) + check_taxonomy( + response.data, + response.data["id"], + **{ + "name": "new name", + "description": taxonomy.description, + "enabled": taxonomy.enabled, + "required": taxonomy.required, + }, + ) + + @ddt.data( + (False, status.HTTP_403_FORBIDDEN), + (True, status.HTTP_403_FORBIDDEN), + ) + @ddt.unpack + def test_update_taxonomy_system_defined(self, update_value, expected_status): + """ + Test that we can't update system_defined field + """ + url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.st1.pk) + + self.client.force_authenticate(user=self.userS) + response = self.client.put(url, {"name": "new name", "system_defined": update_value}, format="json") + assert response.status_code == expected_status + + # Verify that system_defined has not changed + response = self.client.get(url) + assert response.data["system_defined"] is True + + @ddt.data( + (None, "ot1", status.HTTP_403_FORBIDDEN), + (None, "ot2", status.HTTP_403_FORBIDDEN), + (None, "st1", status.HTTP_403_FORBIDDEN), + (None, "st2", status.HTTP_403_FORBIDDEN), + (None, "t1", status.HTTP_403_FORBIDDEN), + (None, "t2", status.HTTP_403_FORBIDDEN), + (None, "tA1", status.HTTP_403_FORBIDDEN), + (None, "tA2", status.HTTP_403_FORBIDDEN), + (None, "tB1", status.HTTP_403_FORBIDDEN), + (None, "tB2", status.HTTP_403_FORBIDDEN), + (None, "tC1", status.HTTP_403_FORBIDDEN), + (None, "tC2", status.HTTP_403_FORBIDDEN), + ("user", "ot1", status.HTTP_403_FORBIDDEN), + ("user", "ot2", status.HTTP_403_FORBIDDEN), + ("user", "st1", status.HTTP_403_FORBIDDEN), + ("user", "st2", status.HTTP_403_FORBIDDEN), + ("user", "t1", status.HTTP_403_FORBIDDEN), + ("user", "t2", status.HTTP_403_FORBIDDEN), + ("user", "tA1", status.HTTP_403_FORBIDDEN), + ("user", "tA2", status.HTTP_403_FORBIDDEN), + ("user", "tB1", status.HTTP_403_FORBIDDEN), + ("user", "tB2", status.HTTP_403_FORBIDDEN), + ("user", "tC1", status.HTTP_403_FORBIDDEN), + ("user", "tC2", status.HTTP_403_FORBIDDEN), + ("userA", "ot1", status.HTTP_403_FORBIDDEN), + ("userA", "ot2", status.HTTP_403_FORBIDDEN), + ("userA", "st1", status.HTTP_403_FORBIDDEN), + ("userA", "st2", status.HTTP_403_FORBIDDEN), + ("userA", "t1", status.HTTP_403_FORBIDDEN), + ("userA", "t2", status.HTTP_403_FORBIDDEN), + ("userA", "tA1", status.HTTP_403_FORBIDDEN), + ("userA", "tA2", status.HTTP_403_FORBIDDEN), + ("userA", "tB1", status.HTTP_403_FORBIDDEN), + ("userA", "tB2", status.HTTP_403_FORBIDDEN), + ("userA", "tC1", status.HTTP_403_FORBIDDEN), + ("userA", "tC2", status.HTTP_403_FORBIDDEN), + ("userS", "ot1", status.HTTP_200_OK), + ("userS", "ot2", status.HTTP_200_OK), + ("userS", "st1", status.HTTP_403_FORBIDDEN), + ("userS", "st2", status.HTTP_403_FORBIDDEN), + ("userS", "t1", status.HTTP_200_OK), + ("userS", "t2", status.HTTP_200_OK), + ("userS", "tA1", status.HTTP_200_OK), + ("userS", "tA2", status.HTTP_200_OK), + ("userS", "tB1", status.HTTP_200_OK), + ("userS", "tB2", status.HTTP_200_OK), + ("userS", "tC1", status.HTTP_200_OK), + ("userS", "tC2", status.HTTP_200_OK), + ) + @ddt.unpack + def test_patch_taxonomy(self, user_attr, taxonomy_attr, expected_status): + taxonomy = getattr(self, taxonomy_attr) + + url = TAXONOMY_ORG_DETAIL_URL.format(pk=taxonomy.pk) + + if user_attr: + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + response = self.client.patch(url, {"name": "new name"}, format="json") + assert response.status_code == expected_status + + # If we were able to patch the taxonomy, check if the name changed + if status.is_success(expected_status): + response = self.client.get(url) + check_taxonomy( + response.data, + response.data["id"], + **{ + "name": "new name", + "description": taxonomy.description, + "enabled": taxonomy.enabled, + "required": taxonomy.required, + }, + ) + + @ddt.data( + (False, status.HTTP_403_FORBIDDEN), + (True, status.HTTP_403_FORBIDDEN), + ) + @ddt.unpack + def test_patch_taxonomy_system_defined(self, update_value, expected_status): + """ + Test that we can't patch system_defined field + """ + url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.st1.pk) + + self.client.force_authenticate(user=self.userS) + response = self.client.patch(url, {"name": "new name", "system_defined": update_value}, format="json") + assert response.status_code == expected_status + + # Verify that system_defined has not changed + response = self.client.get(url) + assert response.data["system_defined"] is True + + @ddt.data( + (None, "ot1", status.HTTP_403_FORBIDDEN), + (None, "ot2", status.HTTP_403_FORBIDDEN), + (None, "st1", status.HTTP_403_FORBIDDEN), + (None, "st2", status.HTTP_403_FORBIDDEN), + (None, "t1", status.HTTP_403_FORBIDDEN), + (None, "t2", status.HTTP_403_FORBIDDEN), + (None, "tA1", status.HTTP_403_FORBIDDEN), + (None, "tA2", status.HTTP_403_FORBIDDEN), + (None, "tB1", status.HTTP_403_FORBIDDEN), + (None, "tB2", status.HTTP_403_FORBIDDEN), + (None, "tC1", status.HTTP_403_FORBIDDEN), + (None, "tC2", status.HTTP_403_FORBIDDEN), + ("user", "ot1", status.HTTP_403_FORBIDDEN), + ("user", "ot2", status.HTTP_403_FORBIDDEN), + ("user", "st1", status.HTTP_403_FORBIDDEN), + ("user", "st2", status.HTTP_403_FORBIDDEN), + ("user", "t1", status.HTTP_403_FORBIDDEN), + ("user", "t2", status.HTTP_403_FORBIDDEN), + ("user", "tA1", status.HTTP_403_FORBIDDEN), + ("user", "tA2", status.HTTP_403_FORBIDDEN), + ("user", "tB1", status.HTTP_403_FORBIDDEN), + ("user", "tB2", status.HTTP_403_FORBIDDEN), + ("user", "tC1", status.HTTP_403_FORBIDDEN), + ("user", "tC2", status.HTTP_403_FORBIDDEN), + ("userA", "ot1", status.HTTP_403_FORBIDDEN), + ("userA", "ot2", status.HTTP_403_FORBIDDEN), + ("userA", "st1", status.HTTP_403_FORBIDDEN), + ("userA", "st2", status.HTTP_403_FORBIDDEN), + ("userA", "t1", status.HTTP_403_FORBIDDEN), + ("userA", "t2", status.HTTP_403_FORBIDDEN), + ("userA", "tA1", status.HTTP_403_FORBIDDEN), + ("userA", "tA2", status.HTTP_403_FORBIDDEN), + ("userA", "tB1", status.HTTP_403_FORBIDDEN), + ("userA", "tB2", status.HTTP_403_FORBIDDEN), + ("userA", "tC1", status.HTTP_403_FORBIDDEN), + ("userA", "tC2", status.HTTP_403_FORBIDDEN), + ("userS", "ot1", status.HTTP_204_NO_CONTENT), + ("userS", "ot2", status.HTTP_204_NO_CONTENT), + ("userS", "st1", status.HTTP_403_FORBIDDEN), + ("userS", "st2", status.HTTP_403_FORBIDDEN), + ("userS", "t1", status.HTTP_204_NO_CONTENT), + ("userS", "t2", status.HTTP_204_NO_CONTENT), + ("userS", "tA1", status.HTTP_204_NO_CONTENT), + ("userS", "tA2", status.HTTP_204_NO_CONTENT), + ("userS", "tB1", status.HTTP_204_NO_CONTENT), + ("userS", "tB2", status.HTTP_204_NO_CONTENT), + ("userS", "tC1", status.HTTP_204_NO_CONTENT), + ("userS", "tC2", status.HTTP_204_NO_CONTENT), + ) + @ddt.unpack + def test_delete_taxonomy(self, user_attr, taxonomy_attr, expected_status): + taxonomy = getattr(self, taxonomy_attr) + + url = TAXONOMY_ORG_DETAIL_URL.format(pk=taxonomy.pk) + + if user_attr: + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + response = self.client.delete(url) + assert response.status_code == expected_status + + # If we were able to delete the taxonomy, check that it's really gone + if status.is_success(expected_status): + response = self.client.get(url) + assert response.status_code == status.HTTP_404_NOT_FOUND + + +@skip_unless_cms +@ddt.ddt +@override_settings(FEATURES={"ENABLE_CREATOR_GROUP": False}) +class TestTaxonomyViewSetNoCreatorGroup(TestTaxonomyViewSet): # pylint: disable=test-inherits-tests + """ + Test cases for TaxonomyViewSet when ENABLE_CREATOR_GROUP is False + + The permissions are the same for when ENABLED_CREATOR_GRUP is True + """ diff --git a/openedx/features/content_tagging/rest_api/v1/urls.py b/openedx/features/content_tagging/rest_api/v1/urls.py new file mode 100644 index 0000000000..5c0bceb38e --- /dev/null +++ b/openedx/features/content_tagging/rest_api/v1/urls.py @@ -0,0 +1,19 @@ +""" +Taxonomies API v1 URLs. +""" + +from rest_framework.routers import DefaultRouter + +from django.urls.conf import path, include + +from openedx_tagging.core.tagging.rest_api.v1 import views as oel_tagging_views + +from . import views + +router = DefaultRouter() +router.register("taxonomies", views.TaxonomyOrgView, basename="taxonomy") +router.register("object_tags", oel_tagging_views.ObjectTagView, basename="object_tag") + +urlpatterns = [ + path('', include(router.urls)) +] diff --git a/openedx/features/content_tagging/rest_api/v1/views.py b/openedx/features/content_tagging/rest_api/v1/views.py new file mode 100644 index 0000000000..7bdfe7cd39 --- /dev/null +++ b/openedx/features/content_tagging/rest_api/v1/views.py @@ -0,0 +1,60 @@ +""" +Tagging Org API Views +""" + +from openedx_tagging.core.tagging.rest_api.v1.views import TaxonomyView + + +from ...api import ( + create_taxonomy, + get_taxonomies, + get_taxonomies_for_org, +) +from .serializers import TaxonomyOrgListQueryParamsSerializer +from .filters import UserOrgFilterBackend + + +class TaxonomyOrgView(TaxonomyView): + """ + View to list, create, retrieve, update, or delete Taxonomies. + This view extends the TaxonomyView to add Organization filters. + + Refer to TaxonomyView docstring for usage details. + + **Additional List Query Parameters** + * org (optional) - Filter by organization. + + **List Example Requests** + GET api/content_tagging/v1/taxonomies?org=orgA - Get all taxonomies for organization A + GET api/content_tagging/v1/taxonomies?org=orgA&enabled=true - Get all enabled taxonomies for organization A + + **List Query Returns** + * 200 - Success + * 400 - Invalid query parameter + * 403 - Permission denied + """ + + filter_backends = [UserOrgFilterBackend] + + def get_queryset(self): + """ + Return a list of taxonomies. + + Returns all taxonomies by default. + If you want the disabled taxonomies, pass enabled=False. + If you want the enabled taxonomies, pass enabled=True. + """ + query_params = TaxonomyOrgListQueryParamsSerializer(data=self.request.query_params.dict()) + query_params.is_valid(raise_exception=True) + enabled = query_params.validated_data.get("enabled", None) + org = query_params.validated_data.get("org", None) + if org: + return get_taxonomies_for_org(enabled, org) + else: + return get_taxonomies(enabled) + + def perform_create(self, serializer): + """ + Create a new taxonomy. + """ + serializer.instance = create_taxonomy(**serializer.validated_data) diff --git a/openedx/features/content_tagging/rules.py b/openedx/features/content_tagging/rules.py index 9ab7072bf9..df256b9331 100644 --- a/openedx/features/content_tagging/rules.py +++ b/openedx/features/content_tagging/rules.py @@ -11,13 +11,13 @@ from .models import TaxonomyOrg User = get_user_model() -def is_taxonomy_admin(user: User, taxonomy: oel_tagging.Taxonomy = None) -> bool: +def is_taxonomy_user(user: User, taxonomy: oel_tagging.Taxonomy = None) -> bool: """ - Returns True if the given user is a Taxonomy Admin for the given content taxonomy. + Returns True if the given user is a Taxonomy User for the given content taxonomy. - Global Taxonomy Admins include global staff and superusers, plus course creators who can create courses for any org. - Otherwise, a taxonomy must be provided to determine if the user is a org-level course creator for one of the - taxonomy's org owners. + Taxonomy users include global staff and superusers, plus course creators who can create courses for any org. + Otherwise, we need a taxonomy provided to determine if the user is an org-level course creator for one of the + orgs allowed to use this taxonomy. """ if oel_tagging.is_taxonomy_admin(user): return True @@ -35,27 +35,48 @@ def is_taxonomy_admin(user: User, taxonomy: oel_tagging.Taxonomy = None) -> bool return False +def is_taxonomy_admin(user: User) -> bool: + """ + Returns True if the given user is a Taxonomy Admin. + + Taxonomy Admins include global staff and superusers. + """ + return oel_tagging.is_taxonomy_admin(user) + + @rules.predicate def can_view_taxonomy(user: User, taxonomy: oel_tagging.Taxonomy = None) -> bool: """ - Anyone can view an enabled taxonomy, - but only taxonomy admins can view a disabled taxonomy. + Everyone can potentially view a taxonomy (taxonomy=None). The object permission must be checked + to determine if the user can view a specific taxonomy. + Only taxonomy admins can view a disabled taxonomy. """ - if taxonomy: - taxonomy = taxonomy.cast() - return (taxonomy and taxonomy.enabled) or is_taxonomy_admin(user, taxonomy) + if not taxonomy: + return True + + taxonomy = taxonomy.cast() + + return taxonomy.enabled or is_taxonomy_admin(user) + + +@rules.predicate +def can_add_taxonomy(user: User) -> bool: + """ + Only taxonomy admins can add taxonomies. + """ + return is_taxonomy_admin(user) @rules.predicate def can_change_taxonomy(user: User, taxonomy: oel_tagging.Taxonomy = None) -> bool: """ + Only taxonomy admins can change a taxonomies. Even taxonomy admins cannot change system taxonomies. """ if taxonomy: taxonomy = taxonomy.cast() - return is_taxonomy_admin(user, taxonomy) and ( - not taxonomy or (taxonomy and not taxonomy.system_defined) - ) + + return (not taxonomy or (not taxonomy.system_defined)) and is_taxonomy_admin(user) @rules.predicate @@ -67,7 +88,7 @@ def can_change_taxonomy_tag(user: User, tag: oel_tagging.Tag = None) -> bool: taxonomy = tag.taxonomy if tag else None if taxonomy: taxonomy = taxonomy.cast() - return is_taxonomy_admin(user, taxonomy) and ( + return is_taxonomy_admin(user) and ( not tag or not taxonomy or (taxonomy and not taxonomy.allow_free_text and not taxonomy.system_defined) @@ -77,18 +98,18 @@ def can_change_taxonomy_tag(user: User, tag: oel_tagging.Tag = None) -> bool: @rules.predicate def can_change_object_tag(user: User, object_tag: oel_tagging.ObjectTag = None) -> bool: """ - Taxonomy admins can create or modify object tags on enabled taxonomies. + Taxonomy users can create or modify object tags on enabled taxonomies. """ taxonomy = object_tag.taxonomy if object_tag else None if taxonomy: taxonomy = taxonomy.cast() - return is_taxonomy_admin(user, taxonomy) and ( + return is_taxonomy_user(user, taxonomy) and ( not object_tag or not taxonomy or (taxonomy and taxonomy.cast().enabled) ) # Taxonomy -rules.set_perm("oel_tagging.add_taxonomy", can_change_taxonomy) +rules.set_perm("oel_tagging.add_taxonomy", can_add_taxonomy) rules.set_perm("oel_tagging.change_taxonomy", can_change_taxonomy) rules.set_perm("oel_tagging.delete_taxonomy", can_change_taxonomy) rules.set_perm("oel_tagging.view_taxonomy", can_view_taxonomy) diff --git a/openedx/features/content_tagging/tests/test_api.py b/openedx/features/content_tagging/tests/test_api.py index f97958f9f6..263ae761ef 100644 --- a/openedx/features/content_tagging/tests/test_api.py +++ b/openedx/features/content_tagging/tests/test_api.py @@ -189,14 +189,12 @@ class TestAPITaxonomy(TestTaxonomyMixin, TestCase): object_tag_attr, ): taxonomy_id = getattr(self, taxonomy_attr).id - taxonomy = api.get_taxonomy(taxonomy_id) object_tag = getattr(self, object_tag_attr) - with self.assertNumQueries(1): + with self.assertNumQueries(2): valid_tags = list( api.get_content_tags( - taxonomy=taxonomy, + taxonomy_id=taxonomy_id, object_id=object_tag.object_id, - valid_only=True, ) ) assert len(valid_tags) == 1 @@ -219,14 +217,12 @@ class TestAPITaxonomy(TestTaxonomyMixin, TestCase): object_tag_attr, ): taxonomy_id = getattr(self, taxonomy_attr).id - taxonomy = api.get_taxonomy(taxonomy_id) object_tag = getattr(self, object_tag_attr) - with self.assertNumQueries(1): + with self.assertNumQueries(2): valid_tags = list( api.get_content_tags( - taxonomy=taxonomy, + taxonomy_id=taxonomy_id, object_id=object_tag.object_id, - valid_only=False, ) ) assert len(valid_tags) == 1 diff --git a/openedx/features/content_tagging/tests/test_rules.py b/openedx/features/content_tagging/tests/test_rules.py index 77dcc2270b..0bb0e53815 100644 --- a/openedx/features/content_tagging/tests/test_rules.py +++ b/openedx/features/content_tagging/tests/test_rules.py @@ -103,40 +103,69 @@ class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): # Taxonomy - @ddt.data( - ("oel_tagging.add_taxonomy", "taxonomy_all_orgs"), - ("oel_tagging.add_taxonomy", "taxonomy_both_orgs"), - ("oel_tagging.add_taxonomy", "taxonomy_disabled"), - ("oel_tagging.change_taxonomy", "taxonomy_all_orgs"), - ("oel_tagging.change_taxonomy", "taxonomy_both_orgs"), - ("oel_tagging.change_taxonomy", "taxonomy_disabled"), - ("oel_tagging.delete_taxonomy", "taxonomy_all_orgs"), - ("oel_tagging.delete_taxonomy", "taxonomy_both_orgs"), - ("oel_tagging.delete_taxonomy", "taxonomy_disabled"), - ) - @ddt.unpack - def test_change_taxonomy_all_orgs(self, perm, taxonomy_attr): - """Taxonomy administrators with course creator access for the taxonomy org""" - taxonomy = getattr(self, taxonomy_attr) - self._expected_users_have_perm(perm, taxonomy) - - @ddt.data( - ("oel_tagging.add_taxonomy", "taxonomy_one_org"), - ("oel_tagging.change_taxonomy", "taxonomy_one_org"), - ("oel_tagging.delete_taxonomy", "taxonomy_one_org"), - ) - @ddt.unpack - def test_change_taxonomy_org1(self, perm, taxonomy_attr): - taxonomy = getattr(self, taxonomy_attr) - self._expected_users_have_perm(perm, taxonomy, user_org2=False) - @ddt.data( "oel_tagging.add_taxonomy", "oel_tagging.change_taxonomy", "oel_tagging.delete_taxonomy", ) + def test_taxonomy_base_edit_permissions(self, perm): + """ + Test that only Staff & Superuser can call add/edit/delete taxonomies. + """ + assert self.superuser.has_perm(perm) + assert self.staff.has_perm(perm) + assert not self.user_all_orgs.has_perm(perm) + assert not self.user_both_orgs.has_perm(perm) + assert not self.user_org2.has_perm(perm) + assert not self.learner.has_perm(perm) + + @ddt.data( + "oel_tagging.view_taxonomy", + ) + def test_taxonomy_base_view_permissions(self, perm): + """ + Test that everyone can call view taxonomy. + """ + assert self.superuser.has_perm(perm) + assert self.staff.has_perm(perm) + assert self.user_all_orgs.has_perm(perm) + assert self.user_both_orgs.has_perm(perm) + assert self.user_org2.has_perm(perm) + assert self.learner.has_perm(perm) + + @ddt.data( + ("oel_tagging.change_taxonomy", "taxonomy_all_orgs"), + ("oel_tagging.change_taxonomy", "taxonomy_disabled"), + ("oel_tagging.change_taxonomy", "taxonomy_both_orgs"), + ("oel_tagging.change_taxonomy", "taxonomy_one_org"), + ("oel_tagging.change_taxonomy", "taxonomy_no_orgs"), + ("oel_tagging.delete_taxonomy", "taxonomy_all_orgs"), + ("oel_tagging.delete_taxonomy", "taxonomy_disabled"), + ("oel_tagging.delete_taxonomy", "taxonomy_both_orgs"), + ("oel_tagging.delete_taxonomy", "taxonomy_one_org"), + ("oel_tagging.delete_taxonomy", "taxonomy_no_orgs"), + ) + @ddt.unpack + def test_change_taxonomy(self, perm, taxonomy_attr): + """ + Test that only Staff & Superuser can edit/delete taxonomies. + """ + taxonomy = getattr(self, taxonomy_attr) + assert self.superuser.has_perm(perm, taxonomy) + assert self.staff.has_perm(perm, taxonomy) + assert not self.user_all_orgs.has_perm(perm, taxonomy) + assert not self.user_both_orgs.has_perm(perm, taxonomy) + assert not self.user_org2.has_perm(perm, taxonomy) + assert not self.learner.has_perm(perm, taxonomy) + + @ddt.data( + "oel_tagging.change_taxonomy", + "oel_tagging.delete_taxonomy", + ) def test_system_taxonomy(self, perm): - """Taxonomy administrators cannot edit system taxonomies""" + """ + Test that even taxonomy administrators cannot edit/delete system taxonomies. + """ system_taxonomy = api.create_taxonomy( name="System Languages", ) @@ -150,49 +179,40 @@ class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): assert not self.learner.has_perm(perm, system_taxonomy) @ddt.data( - (True, "taxonomy_all_orgs"), - (False, "taxonomy_all_orgs"), - (True, "taxonomy_both_orgs"), - (False, "taxonomy_both_orgs"), + "taxonomy_all_orgs", + "taxonomy_both_orgs", + "taxonomy_one_org", + "taxonomy_no_orgs", ) - @ddt.unpack - def test_view_taxonomy_enabled(self, enabled, taxonomy_attr): - """Anyone can see enabled taxonomies, but learners cannot see disabled taxonomies""" + def test_view_taxonomy_enabled(self, taxonomy_attr): + """ + Test that anyone can view enabled taxonomies. + """ taxonomy = getattr(self, taxonomy_attr) - taxonomy.enabled = enabled + taxonomy.enabled = True perm = "oel_tagging.view_taxonomy" - self._expected_users_have_perm(perm, taxonomy, learner_obj=enabled) - @ddt.data( - (True, "taxonomy_no_orgs"), - (False, "taxonomy_no_orgs"), - ) - @ddt.unpack - def test_view_taxonomy_no_orgs(self, enabled, taxonomy_attr): - """ - Enabled taxonomies with no org can be viewed by anyone. - Disabled taxonomies with no org can only be viewed by staff/superusers. - """ - taxonomy = getattr(self, taxonomy_attr) - taxonomy.enabled = enabled - perm = "oel_tagging.view_taxonomy" assert self.superuser.has_perm(perm, taxonomy) assert self.staff.has_perm(perm, taxonomy) - assert self.user_all_orgs.has_perm(perm, taxonomy) == enabled - assert self.user_both_orgs.has_perm(perm, taxonomy) == enabled - assert self.user_org2.has_perm(perm, taxonomy) == enabled - assert self.learner.has_perm(perm, taxonomy) == enabled + assert self.user_all_orgs.has_perm(perm, taxonomy) + assert self.user_both_orgs.has_perm(perm, taxonomy) + assert self.user_org2.has_perm(perm, taxonomy) + assert self.learner.has_perm(perm, taxonomy) @ddt.data( - ("oel_tagging.change_taxonomy", "taxonomy_no_orgs"), - ("oel_tagging.delete_taxonomy", "taxonomy_no_orgs"), + "taxonomy_all_orgs", + "taxonomy_both_orgs", + "taxonomy_one_org", + "taxonomy_no_orgs", ) - @ddt.unpack - def test_change_taxonomy_no_orgs(self, perm, taxonomy_attr): + def test_view_taxonomy_disabled(self, taxonomy_attr): """ - Taxonomies with no org can only be changed by staff and superusers. + Test that only Staff & Superuser can view disabled taxonomies. """ taxonomy = getattr(self, taxonomy_attr) + taxonomy.enabled = False + perm = "oel_tagging.view_taxonomy" + assert self.superuser.has_perm(perm, taxonomy) assert self.staff.has_perm(perm, taxonomy) assert not self.user_all_orgs.has_perm(perm, taxonomy) @@ -202,39 +222,115 @@ class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): # Tag - @ddt.data( - ("oel_tagging.add_tag", "tag_all_orgs"), - ("oel_tagging.add_tag", "tag_both_orgs"), - ("oel_tagging.add_tag", "tag_disabled"), - ("oel_tagging.change_tag", "tag_all_orgs"), - ("oel_tagging.change_tag", "tag_both_orgs"), - ("oel_tagging.change_tag", "tag_disabled"), - ("oel_tagging.delete_tag", "tag_all_orgs"), - ("oel_tagging.delete_tag", "tag_both_orgs"), - ("oel_tagging.delete_tag", "tag_disabled"), - ) - @ddt.unpack - def test_change_tag_all_orgs(self, perm, tag_attr): - """Taxonomy administrators can modify tags on non-free-text taxonomies""" - tag = getattr(self, tag_attr) - self._expected_users_have_perm(perm, tag) - - @ddt.data( - ("oel_tagging.add_tag", "tag_one_org"), - ("oel_tagging.change_tag", "tag_one_org"), - ("oel_tagging.delete_tag", "tag_one_org"), - ) - @ddt.unpack - def test_change_tag_org1(self, perm, tag_attr): - """Taxonomy administrators can modify tags on non-free-text taxonomies""" - tag = getattr(self, tag_attr) - self._expected_users_have_perm(perm, tag, user_org2=False) - @ddt.data( "oel_tagging.add_tag", "oel_tagging.change_tag", "oel_tagging.delete_tag", ) + def test_tag_base_edit_permissions(self, perm): + """ + Test that only Staff & Superuser can call add/edit/delete tags. + """ + assert self.superuser.has_perm(perm) + assert self.staff.has_perm(perm) + assert not self.user_all_orgs.has_perm(perm) + assert not self.user_both_orgs.has_perm(perm) + assert not self.user_org2.has_perm(perm) + assert not self.learner.has_perm(perm) + + @ddt.data( + "oel_tagging.view_tag", + ) + def test_tag_base_view_permissions(self, perm): + """ + Test that everyone can call view tag. + """ + assert self.superuser.has_perm(perm) + assert self.staff.has_perm(perm) + assert self.user_all_orgs.has_perm(perm) + assert self.user_both_orgs.has_perm(perm) + assert self.user_org2.has_perm(perm) + assert self.learner.has_perm(perm) + + @ddt.data( + ("oel_tagging.change_tag", "tag_all_orgs"), + ("oel_tagging.change_tag", "tag_disabled"), + ("oel_tagging.change_tag", "tag_both_orgs"), + ("oel_tagging.change_tag", "tag_one_org"), + ("oel_tagging.change_tag", "tag_no_orgs"), + ("oel_tagging.delete_tag", "tag_all_orgs"), + ("oel_tagging.delete_tag", "tag_disabled"), + ("oel_tagging.delete_tag", "tag_both_orgs"), + ("oel_tagging.delete_tag", "tag_one_org"), + ("oel_tagging.delete_tag", "tag_no_orgs"), + ) + @ddt.unpack + def test_change_tag(self, perm, tag_attr): + """ + Test that only Staff & Superuser can edit/delete taxonomies. + """ + tag = getattr(self, tag_attr) + assert self.superuser.has_perm(perm, tag) + assert self.staff.has_perm(perm, tag) + assert not self.user_all_orgs.has_perm(perm, tag) + assert not self.user_both_orgs.has_perm(perm, tag) + assert not self.user_org2.has_perm(perm, tag) + assert not self.learner.has_perm(perm, tag) + + @ddt.data( + "oel_tagging.change_tag", + "oel_tagging.delete_tag", + ) + def test_system_taxonomy_tag(self, perm): + """ + Test that even taxonomy administrators cannot edit/delete tags on system taxonomies. + """ + system_taxonomy = api.create_taxonomy( + name="System Languages", + ) + system_taxonomy.taxonomy_class = UserSystemDefinedTaxonomy + system_taxonomy = system_taxonomy.cast() + tag_system_taxonomy = Tag.objects.create( + taxonomy=system_taxonomy, + value="en", + ) + + assert self.superuser.has_perm(perm, tag_system_taxonomy) + assert not self.staff.has_perm(perm, tag_system_taxonomy) + assert not self.user_all_orgs.has_perm(perm, tag_system_taxonomy) + assert not self.user_both_orgs.has_perm(perm, tag_system_taxonomy) + assert not self.user_org2.has_perm(perm, tag_system_taxonomy) + assert not self.learner.has_perm(perm, tag_system_taxonomy) + + @ddt.data( + "oel_tagging.change_tag", + "oel_tagging.delete_tag", + ) + def test_free_text_taxonomy_tag(self, perm): + """ + Test that even taxonomy administrators cannot edit/delete tags on free text taxonomies. + """ + free_text_taxonomy = api.create_taxonomy( + name="Free text", + allow_free_text=True, + ) + + tag_free_text_taxonomy = Tag.objects.create( + taxonomy=free_text_taxonomy, + value="value1", + ) + + assert self.superuser.has_perm(perm, tag_free_text_taxonomy) + assert not self.staff.has_perm(perm, tag_free_text_taxonomy) + assert not self.user_all_orgs.has_perm(perm, tag_free_text_taxonomy) + assert not self.user_both_orgs.has_perm(perm, tag_free_text_taxonomy) + assert not self.user_org2.has_perm(perm, tag_free_text_taxonomy) + assert not self.learner.has_perm(perm, tag_free_text_taxonomy) + + @ddt.data( + "oel_tagging.change_tag", + "oel_tagging.delete_tag", + ) def test_tag_no_taxonomy(self, perm): """Taxonomy administrators can modify any Tag, even those with no Taxonnmy.""" tag = Tag() @@ -242,10 +338,9 @@ class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): # Global Taxonomy Admins can do pretty much anything assert self.superuser.has_perm(perm, tag) assert self.staff.has_perm(perm, tag) - assert self.user_all_orgs.has_perm(perm, tag) - # Org content creators are bound by a taxonomy's org restrictions, - # so if there's no taxonomy, they can't do anything to it. + # Everyone else can't do anything + assert not self.user_all_orgs.has_perm(perm, tag) assert not self.user_both_orgs.has_perm(perm, tag) assert not self.user_org2.has_perm(perm, tag) assert not self.learner.has_perm(perm, tag) @@ -403,26 +498,6 @@ class TestRulesTaxonomyNoCreatorGroup( user_org2=True, ) - @ddt.data( - "oel_tagging.add_tag", - "oel_tagging.change_tag", - "oel_tagging.delete_tag", - ) - def test_tag_no_taxonomy(self, perm): - """Taxonomy administrators can modify any Tag, even those with no Taxonnmy.""" - tag = Tag() - - # Global Taxonomy Admins can do pretty much anything - assert self.superuser.has_perm(perm, tag) - assert self.staff.has_perm(perm, tag) - assert self.user_all_orgs.has_perm(perm, tag) - - # Org content creators are bound by a taxonomy's org restrictions, - # but since there's no org restrictions enabled, anyone has these permissions. - assert self.user_both_orgs.has_perm(perm, tag) - assert self.user_org2.has_perm(perm, tag) - assert self.learner.has_perm(perm, tag) - @ddt.data( "oel_tagging.add_object_tag", "oel_tagging.change_object_tag", @@ -446,11 +521,6 @@ class TestRulesTaxonomyNoCreatorGroup( # Taxonomy @ddt.data( - ("oel_tagging.add_taxonomy", "taxonomy_all_orgs"), - ("oel_tagging.add_taxonomy", "taxonomy_both_orgs"), - ("oel_tagging.add_taxonomy", "taxonomy_disabled"), - ("oel_tagging.add_taxonomy", "taxonomy_one_org"), - ("oel_tagging.add_taxonomy", "taxonomy_no_orgs"), ("oel_tagging.change_taxonomy", "taxonomy_all_orgs"), ("oel_tagging.change_taxonomy", "taxonomy_both_orgs"), ("oel_tagging.change_taxonomy", "taxonomy_disabled"), @@ -470,17 +540,11 @@ class TestRulesTaxonomyNoCreatorGroup( Organization.objects.all().delete() taxonomy = getattr(self, taxonomy_attr) # Superusers & Staff always have access - assert self.superuser.has_perm(perm) assert self.superuser.has_perm(perm, taxonomy) - assert self.staff.has_perm(perm) assert self.staff.has_perm(perm, taxonomy) # But everyone else's object-level access is removed - assert self.user_all_orgs.has_perm(perm) assert not self.user_all_orgs.has_perm(perm, taxonomy) - assert self.user_both_orgs.has_perm(perm) assert not self.user_both_orgs.has_perm(perm, taxonomy) - assert self.user_org2.has_perm(perm) assert not self.user_org2.has_perm(perm, taxonomy) - assert self.learner.has_perm(perm) assert not self.learner.has_perm(perm, taxonomy) diff --git a/openedx/features/content_tagging/urls.py b/openedx/features/content_tagging/urls.py new file mode 100644 index 0000000000..b81c01e1b0 --- /dev/null +++ b/openedx/features/content_tagging/urls.py @@ -0,0 +1,10 @@ +""" +Content Tagging URLs +""" +from django.urls import path, include + +from .rest_api import urls + +urlpatterns = [ + path('', include(urls)), +] diff --git a/requirements/constraints.txt b/requirements/constraints.txt index a94ea0e02d..683a8a2799 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -131,8 +131,3 @@ click==8.1.6 # plz upgrade this in separate ticket redis==4.6.0 - -# openedx-learning new version has some changes which are breaking quality tests -# See https://github.com/openedx/openedx-learning/pull/68 for the changes. -# It needs to be updated in a separate issue. -openedx-learning==0.1.2 diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 6cd543e793..bb7ba0268d 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -43,6 +43,7 @@ attrs==23.1.0 # lti-consumer-xblock # openedx-blockstore # openedx-events + # openedx-learning # referencing babel==2.11.0 # via @@ -98,6 +99,7 @@ celery==5.3.1 # edx-celeryutils # edx-enterprise # event-tracking + # openedx-learning certifi==2023.7.22 # via # -r requirements/edx/paver.txt @@ -770,10 +772,8 @@ openedx-filters==1.5.0 # via # -r requirements/edx/kernel.in # lti-consumer-xblock -openedx-learning==0.1.2 - # via - # -c requirements/edx/../constraints.txt - # -r requirements/edx/kernel.in +openedx-learning==0.1.5 + # via -r requirements/edx/kernel.in openedx-mongodbproxy==0.2.0 # via -r requirements/edx/kernel.in optimizely-sdk==4.1.1 diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 08eafdea95..bc4f6e2a9a 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -92,6 +92,7 @@ attrs==23.1.0 # lti-consumer-xblock # openedx-blockstore # openedx-events + # openedx-learning # referencing babel==2.11.0 # via @@ -175,6 +176,7 @@ celery==5.3.1 # edx-celeryutils # edx-enterprise # event-tracking + # openedx-learning certifi==2023.7.22 # via # -r requirements/edx/doc.txt @@ -1299,9 +1301,8 @@ openedx-filters==1.5.0 # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # lti-consumer-xblock -openedx-learning==0.1.2 +openedx-learning==0.1.5 # via - # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt openedx-mongodbproxy==0.2.0 diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 7a4f2f81a2..052b61028b 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -60,6 +60,7 @@ attrs==23.1.0 # lti-consumer-xblock # openedx-blockstore # openedx-events + # openedx-learning # referencing babel==2.11.0 # via @@ -125,6 +126,7 @@ celery==5.3.1 # edx-celeryutils # edx-enterprise # event-tracking + # openedx-learning certifi==2023.7.22 # via # -r requirements/edx/base.txt @@ -910,10 +912,8 @@ openedx-filters==1.5.0 # via # -r requirements/edx/base.txt # lti-consumer-xblock -openedx-learning==0.1.2 - # via - # -c requirements/edx/../constraints.txt - # -r requirements/edx/base.txt +openedx-learning==0.1.5 + # via -r requirements/edx/base.txt openedx-mongodbproxy==0.2.0 # via -r requirements/edx/base.txt optimizely-sdk==4.1.1 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 6446f7dd79..043164cafc 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -66,6 +66,7 @@ attrs==23.1.0 # lti-consumer-xblock # openedx-blockstore # openedx-events + # openedx-learning # referencing babel==2.11.0 # via @@ -131,6 +132,7 @@ celery==5.3.1 # edx-celeryutils # edx-enterprise # event-tracking + # openedx-learning certifi==2023.7.22 # via # -r requirements/edx/base.txt @@ -979,10 +981,8 @@ openedx-filters==1.5.0 # via # -r requirements/edx/base.txt # lti-consumer-xblock -openedx-learning==0.1.2 - # via - # -c requirements/edx/../constraints.txt - # -r requirements/edx/base.txt +openedx-learning==0.1.5 + # via -r requirements/edx/base.txt openedx-mongodbproxy==0.2.0 # via -r requirements/edx/base.txt optimizely-sdk==4.1.1