From a0f9e66797f33bb6785bfa44c9c2117562d7f817 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Fri, 27 Oct 2023 17:59:45 -0300 Subject: [PATCH 1/7] feat: add update taxonomy orgs rest api --- .../core/djangoapps/content_tagging/api.py | 3 + .../rest_api/v1/serializers.py | 28 +++++ .../rest_api/v1/tests/test_views.py | 116 ++++++++++++++++++ .../content_tagging/rest_api/v1/views.py | 27 +++- .../core/djangoapps/content_tagging/rules.py | 1 + 5 files changed, 173 insertions(+), 2 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index a8630e29e527..88cdf0d03eda 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -58,6 +58,9 @@ def set_taxonomy_orgs( If not `all_orgs`, the taxonomy is associated with each org in the `orgs` list. If that list is empty, the taxonomy is not associated with any orgs. """ + if taxonomy.system_defined: + raise ValueError("Cannot set orgs for a system-defined taxonomy") + TaxonomyOrg.objects.filter( taxonomy=taxonomy, rel_type=relationship, diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py index 0a7ff92409ae..6f47bdf7de59 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py @@ -2,6 +2,8 @@ API Serializers for content tagging org """ +from __future__ import annotations + from rest_framework import serializers, fields from openedx_tagging.core.tagging.rest_api.v1.serializers import ( @@ -21,3 +23,29 @@ class TaxonomyOrgListQueryParamsSerializer(TaxonomyListQueryParamsSerializer): queryset=Organization.objects.all(), required=False, ) + + +class TaxonomyUpdateOrgBodySerializer(serializers.Serializer): + """ + Serializer for the body params for the update orgs action + """ + + orgs: fields.Field = serializers.SlugRelatedField( + many=True, + slug_field="short_name", + queryset=Organization.objects.all(), + required=False, + ) + + all_orgs: fields.Field = serializers.BooleanField(required=False) + + def validate(self, attrs: dict) -> dict: + """ + Validate the serializer data + """ + if bool(attrs.get("orgs") is not None) == bool(attrs.get("all_orgs")): + raise serializers.ValidationError( + "You must specify either orgs or all_orgs, but not both." + ) + + return attrs diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index d37b5df26c2a..1cf0fa9fdc6a 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -40,6 +40,7 @@ TAXONOMY_ORG_LIST_URL = "/api/content_tagging/v1/taxonomies/" TAXONOMY_ORG_DETAIL_URL = "/api/content_tagging/v1/taxonomies/{pk}/" +TAXONOMY_ORG_UPDATE_ORG_URL = "/api/content_tagging/v1/taxonomies/{pk}/orgs/" OBJECT_TAG_UPDATE_URL = "/api/content_tagging/v1/object_tags/{object_id}/?taxonomy={taxonomy_id}" TAXONOMY_TEMPLATE_URL = "/api/content_tagging/v1/taxonomies/import/{filename}" @@ -1044,6 +1045,121 @@ def _test_api_call(self, **kwargs) -> None: assert response.status_code == status.HTTP_404_NOT_FOUND +@skip_unless_cms +@ddt.ddt +class TestTaxonomyUpdateOrg(TestTaxonomyObjectsMixin, APITestCase): + """ + Test cases for updating orgs from taxonomies + """ + + def test_update_org(self) -> None: + """ + Tests that taxonomy admin can add/remove orgs from a taxonomy + """ + url = TAXONOMY_ORG_UPDATE_ORG_URL.format(pk=self.tA1.pk) + self.client.force_authenticate(user=self.staff) + + response = self.client.put(url, {"orgs": [self.orgB.short_name, self.orgX.short_name]}, format="json") + assert response.status_code == status.HTTP_200_OK + + # Check that the orgs were updated + taxonomy_orgs = TaxonomyOrg.objects.filter(taxonomy=self.tA1) + assert taxonomy_orgs.count() == 2 + assert taxonomy_orgs[0].org == self.orgB + assert taxonomy_orgs[1].org == self.orgX + + def test_update_all_org(self) -> None: + """ + Tests that taxonomy admin can associate a taxonomy to all orgs + """ + url = TAXONOMY_ORG_UPDATE_ORG_URL.format(pk=self.tA1.pk) + self.client.force_authenticate(user=self.staff) + + response = self.client.put(url, {"all_orgs": True}, format="json") + assert response.status_code == status.HTTP_200_OK + + # Check that the orgs were updated + taxonomy_orgs = TaxonomyOrg.objects.filter(taxonomy=self.tA1) + assert taxonomy_orgs.count() == 1 + assert taxonomy_orgs[0].org is None + + def test_update_no_org(self) -> None: + """ + Tests that taxonomy admin can associate a taxonomy no orgs + """ + url = TAXONOMY_ORG_UPDATE_ORG_URL.format(pk=self.tA1.pk) + self.client.force_authenticate(user=self.staff) + + response = self.client.put(url, {"orgs": []}, format="json") + + assert response.status_code == status.HTTP_200_OK + + # Check that the orgs were updated + taxonomy_orgs = TaxonomyOrg.objects.filter(taxonomy=self.tA1) + assert taxonomy_orgs.count() == 0 + + @ddt.data( + (True, ["orgX"], "Using both all_orgs and orgs parameters should throw error"), + (False, None, "Using neither all_orgs or orgs parameter should throw error"), + (None, None, "Using neither all_orgs or orgs parameter should throw error"), + (False, 'InvalidOrg', "Passing an invalid org should throw error"), + ) + @ddt.unpack + def test_update_org_invalid_inputs(self, all_orgs: bool, orgs: list[str], reason: str) -> None: + """ + Tests if passing both or none of all_orgs and orgs parameters throws error + """ + url = TAXONOMY_ORG_UPDATE_ORG_URL.format(pk=self.tA1.pk) + self.client.force_authenticate(user=self.staff) + + # Set body cleaning empty values + body = {k: v for k, v in {"all_orgs": all_orgs, "orgs": orgs}.items() if v is not None} + response = self.client.put(url, body, format="json") + assert response.status_code == status.HTTP_400_BAD_REQUEST, reason + + # Check that the orgs didn't change + taxonomy_orgs = TaxonomyOrg.objects.filter(taxonomy=self.tA1) + assert taxonomy_orgs.count() == 1 + assert taxonomy_orgs[0].org == self.orgA + + def test_update_org_system_defined(self) -> None: + """ + Tests that is not possible to change the orgs associated with a system defined taxonomy + """ + url = TAXONOMY_ORG_UPDATE_ORG_URL.format(pk=self.st1.pk) + self.client.force_authenticate(user=self.staff) + + response = self.client.put(url, {"orgs": [self.orgA.short_name]}, format="json") + assert response.status_code in [status.HTTP_403_FORBIDDEN, status.HTTP_400_BAD_REQUEST] + + # Check that the orgs didn't change + taxonomy_orgs = TaxonomyOrg.objects.filter(taxonomy=self.st1) + assert taxonomy_orgs.count() == 1 + assert taxonomy_orgs[0].org is None + + @ddt.data( + "staffA", + "content_creatorA", + "instructorA", + "library_staffA", + "course_instructorA", + "course_staffA", + "library_userA", + ) + def test_update_org_no_perm(self, user_attr: str) -> None: + url = TAXONOMY_ORG_UPDATE_ORG_URL.format(pk=self.tA1.pk) + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + response = self.client.put(url, {"orgs": []}, format="json") + assert response.status_code == status.HTTP_403_FORBIDDEN + + # Check that the orgs didn't change + taxonomy_orgs = TaxonomyOrg.objects.filter(taxonomy=self.tA1) + assert taxonomy_orgs.count() == 1 + assert taxonomy_orgs[0].org == self.orgA + + class TestObjectTagMixin(TestTaxonomyObjectsMixin): """ Sets up data for testing ObjectTags. diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py index 6f1f7dd73a9c..3f15d06c6d9e 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py @@ -3,15 +3,18 @@ """ from openedx_tagging.core.tagging.rest_api.v1.views import ObjectTagView, TaxonomyView - +from rest_framework.decorators import action +from rest_framework.exceptions import PermissionDenied +from rest_framework.response import Response from ...api import ( create_taxonomy, get_taxonomies, get_taxonomies_for_org, + set_taxonomy_orgs, ) from ...rules import get_admin_orgs -from .serializers import TaxonomyOrgListQueryParamsSerializer +from .serializers import TaxonomyOrgListQueryParamsSerializer, TaxonomyUpdateOrgBodySerializer from .filters import ObjectTagTaxonomyOrgFilterBackend, UserOrgFilterBackend @@ -61,6 +64,26 @@ def perform_create(self, serializer): user_admin_orgs = get_admin_orgs(self.request.user) serializer.instance = create_taxonomy(**serializer.validated_data, orgs=user_admin_orgs) + @action(detail=True, methods=["put"]) + def orgs(self, request, **_kwargs) -> Response: + """ + Export a taxonomy. + """ + taxonomy = self.get_object() + perm = "oel_tagging.update_orgs" + if not request.user.has_perm(perm, taxonomy): + raise PermissionDenied("You do not have permission to update the orgs associated with this taxonomy.") + body = TaxonomyUpdateOrgBodySerializer( + data=request.data, + ) + body.is_valid(raise_exception=True) + orgs = body.validated_data.get("orgs") + all_orgs: bool = body.validated_data.get("all_orgs", False) + + set_taxonomy_orgs(taxonomy=taxonomy, all_orgs=all_orgs, orgs=orgs) + + return Response() + class ObjectTagOrgView(ObjectTagView): """ diff --git a/openedx/core/djangoapps/content_tagging/rules.py b/openedx/core/djangoapps/content_tagging/rules.py index 5206f6204534..cf2eafc73f58 100644 --- a/openedx/core/djangoapps/content_tagging/rules.py +++ b/openedx/core/djangoapps/content_tagging/rules.py @@ -282,6 +282,7 @@ def can_change_taxonomy_tag(user: UserType, tag: oel_tagging.Tag | None = None) rules.set_perm("oel_tagging.delete_taxonomy", can_change_taxonomy) rules.set_perm("oel_tagging.view_taxonomy", can_view_taxonomy) rules.set_perm("oel_tagging.export_taxonomy", can_view_taxonomy) +rules.add_perm("oel_tagging.update_orgs", oel_tagging.is_taxonomy_admin) # Tag rules.set_perm("oel_tagging.add_tag", can_change_taxonomy_tag) From 243585a4e6e83098943192517c22125b6985b697 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Wed, 1 Nov 2023 17:46:30 -0300 Subject: [PATCH 2/7] test: add permission change test --- .../rest_api/v1/tests/test_views.py | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index 1cf0fa9fdc6a..0475c8c42f50 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -1159,6 +1159,29 @@ def test_update_org_no_perm(self, user_attr: str) -> None: assert taxonomy_orgs.count() == 1 assert taxonomy_orgs[0].org == self.orgA + def test_update_org_check_permissions(self) -> None: + url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tB1.pk) + self.client.force_authenticate(user=self.staffA) + + response = self.client.put(url, {"name": "new name"}, format="json") + + # User staffA can't update metadata from a taxonomy from orgB + assert response.status_code == status.HTTP_404_NOT_FOUND + + url = TAXONOMY_ORG_UPDATE_ORG_URL.format(pk=self.tB1.pk) + self.client.force_authenticate(user=self.staff) + + # Add the taxonomy tB1 to orgA + response = self.client.put(url, {"orgs": [self.orgA.short_name]}, format="json") + + url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tB1.pk) + self.client.force_authenticate(user=self.staffA) + + response = self.client.put(url, {"name": "new name"}, format="json") + + # Now staffA can change the metadata from a tB1 because it's associated with orgA + assert response.status_code == status.HTTP_200_OK + class TestObjectTagMixin(TestTaxonomyObjectsMixin): """ From 1933c87a05e15b7e552b03bab526115098e6ca4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Wed, 1 Nov 2023 19:03:17 -0300 Subject: [PATCH 3/7] test: add new permissions change test --- .../rest_api/v1/tests/test_views.py | 62 ++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index 0475c8c42f50..13ed3043e583 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -1147,6 +1147,9 @@ def test_update_org_system_defined(self) -> None: "library_userA", ) def test_update_org_no_perm(self, user_attr: str) -> None: + """ + Tests that only taxonomy admins can associate orgs to taxonomies + """ url = TAXONOMY_ORG_UPDATE_ORG_URL.format(pk=self.tA1.pk) user = getattr(self, user_attr) self.client.force_authenticate(user=user) @@ -1159,7 +1162,10 @@ def test_update_org_no_perm(self, user_attr: str) -> None: assert taxonomy_orgs.count() == 1 assert taxonomy_orgs[0].org == self.orgA - def test_update_org_check_permissions(self) -> None: + def test_update_org_check_permissions_orgA(self) -> None: + """ + Tests that adding an org to a taxonomy allow org level admins to edit it + """ url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tB1.pk) self.client.force_authenticate(user=self.staffA) @@ -1182,6 +1188,60 @@ def test_update_org_check_permissions(self) -> None: # Now staffA can change the metadata from a tB1 because it's associated with orgA assert response.status_code == status.HTTP_200_OK + def test_update_org_check_permissions_all_orgs(self) -> None: + """ + Tests that adding an org to all orgs only let taxonomy global admins to edit it + """ + url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tA1.pk) + self.client.force_authenticate(user=self.staffA) + + response = self.client.put(url, {"name": "new name"}, format="json") + + # User staffA can update metadata from a taxonomy from orgA + assert response.status_code == status.HTTP_200_OK + + url = TAXONOMY_ORG_UPDATE_ORG_URL.format(pk=self.tB1.pk) + self.client.force_authenticate(user=self.staff) + + # Add the taxonomy tA1 to all orgs + response = self.client.put(url, {"all_orgs": True}, format="json") + + url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tB1.pk) + self.client.force_authenticate(user=self.staffA) + + response = self.client.put(url, {"name": "new name"}, format="json") + + # Now staffA can't change the metadata from a tA1 because only global taxonomy admins can edit all orgs + # taxonomies + assert response.status_code == status.HTTP_403_FORBIDDEN + + def test_update_org_check_permissions_no_orgs(self) -> None: + """ + Tests that remove all orgs from a taxonomy only let taxonomy global admins to edit it + """ + url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tA1.pk) + self.client.force_authenticate(user=self.staffA) + + response = self.client.put(url, {"name": "new name"}, format="json") + + # User staffA can update metadata from a taxonomy from orgA + assert response.status_code == status.HTTP_200_OK + + url = TAXONOMY_ORG_UPDATE_ORG_URL.format(pk=self.tB1.pk) + self.client.force_authenticate(user=self.staff) + + # Remove all orgs from tA1 + response = self.client.put(url, {"orgs": []}, format="json") + + url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tB1.pk) + self.client.force_authenticate(user=self.staffA) + + response = self.client.put(url, {"name": "new name"}, format="json") + + # Now staffA can't change the metadata from a tA1 because only global taxonomy admins can edit no orgs + # taxonomies + assert response.status_code == status.HTTP_404_NOT_FOUND + class TestObjectTagMixin(TestTaxonomyObjectsMixin): """ From 1222c7d3108b70105bcd7be3e6cf4846feb7108b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Fri, 3 Nov 2023 09:02:17 -0300 Subject: [PATCH 4/7] docs: fix wrong docstring --- openedx/core/djangoapps/content_tagging/rest_api/v1/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py index 3f15d06c6d9e..4f9b21393ebc 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py @@ -67,7 +67,7 @@ def perform_create(self, serializer): @action(detail=True, methods=["put"]) def orgs(self, request, **_kwargs) -> Response: """ - Export a taxonomy. + Update the orgs associated with taxonomies. """ taxonomy = self.get_object() perm = "oel_tagging.update_orgs" From 8c5ea9994e643e6fd14a9af98c700ae211ce3b14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Fri, 3 Nov 2023 17:37:42 -0300 Subject: [PATCH 5/7] feat: add org to taxonomy serializer --- .../rest_api/v1/serializers.py | 26 +++++++++++++ .../rest_api/v1/tests/test_views.py | 38 +++++++++---------- .../content_tagging/rest_api/v1/views.py | 9 +++-- 3 files changed, 51 insertions(+), 22 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py index 6f47bdf7de59..a7a0adc6022c 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py @@ -8,6 +8,7 @@ from openedx_tagging.core.tagging.rest_api.v1.serializers import ( TaxonomyListQueryParamsSerializer, + TaxonomySerializer, ) from organizations.models import Organization @@ -49,3 +50,28 @@ def validate(self, attrs: dict) -> dict: ) return attrs + + +class OrgListField(serializers.RelatedField): + """ + Serializer to return the list of orgs for a taxonomy + """ + def to_representation(self, value): + """ + Return the Organization short_name, not the TaxonomyOrg object + """ + return value.org.short_name if value.org else None + + +class TaxonomyOrgSerializer(TaxonomySerializer): + """ + Serializer for Taxonomy objects inclusing the associated orgs + """ + + orgs = OrgListField(many=True, read_only=True, source="taxonomyorg_set") + + class Meta: + model = TaxonomySerializer.Meta.model + fields = TaxonomySerializer.Meta.fields + ["orgs"] + read_only_fields = ["orgs"] + diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index 13ed3043e583..895cd0cba87e 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -455,7 +455,7 @@ def test_create_taxonomy(self, user_attr: str, expected_status: int) -> None: # Also checks if the taxonomy was associated with the org if user_attr == "staffA": - assert TaxonomyOrg.objects.filter(taxonomy=response.data["id"], org=self.orgA).exists() + assert response.data["orgs"] == [self.orgA.short_name] @ddt.ddt @@ -1063,10 +1063,9 @@ def test_update_org(self) -> None: assert response.status_code == status.HTTP_200_OK # Check that the orgs were updated - taxonomy_orgs = TaxonomyOrg.objects.filter(taxonomy=self.tA1) - assert taxonomy_orgs.count() == 2 - assert taxonomy_orgs[0].org == self.orgB - assert taxonomy_orgs[1].org == self.orgX + url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tA1.pk) + response = self.client.get(url) + assert response.data["orgs"] == [self.orgB.short_name, self.orgX.short_name] def test_update_all_org(self) -> None: """ @@ -1079,9 +1078,9 @@ def test_update_all_org(self) -> None: assert response.status_code == status.HTTP_200_OK # Check that the orgs were updated - taxonomy_orgs = TaxonomyOrg.objects.filter(taxonomy=self.tA1) - assert taxonomy_orgs.count() == 1 - assert taxonomy_orgs[0].org is None + url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tA1.pk) + response = self.client.get(url) + assert response.data["orgs"] == [None] def test_update_no_org(self) -> None: """ @@ -1095,8 +1094,9 @@ def test_update_no_org(self) -> None: assert response.status_code == status.HTTP_200_OK # Check that the orgs were updated - taxonomy_orgs = TaxonomyOrg.objects.filter(taxonomy=self.tA1) - assert taxonomy_orgs.count() == 0 + url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tA1.pk) + response = self.client.get(url) + assert response.data["orgs"] == [] @ddt.data( (True, ["orgX"], "Using both all_orgs and orgs parameters should throw error"), @@ -1118,9 +1118,9 @@ def test_update_org_invalid_inputs(self, all_orgs: bool, orgs: list[str], reason assert response.status_code == status.HTTP_400_BAD_REQUEST, reason # Check that the orgs didn't change - taxonomy_orgs = TaxonomyOrg.objects.filter(taxonomy=self.tA1) - assert taxonomy_orgs.count() == 1 - assert taxonomy_orgs[0].org == self.orgA + url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tA1.pk) + response = self.client.get(url) + assert response.data["orgs"] == [self.orgA.short_name] def test_update_org_system_defined(self) -> None: """ @@ -1133,9 +1133,9 @@ def test_update_org_system_defined(self) -> None: assert response.status_code in [status.HTTP_403_FORBIDDEN, status.HTTP_400_BAD_REQUEST] # Check that the orgs didn't change - taxonomy_orgs = TaxonomyOrg.objects.filter(taxonomy=self.st1) - assert taxonomy_orgs.count() == 1 - assert taxonomy_orgs[0].org is None + url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.st1.pk) + response = self.client.get(url) + assert response.data["orgs"] == [None] @ddt.data( "staffA", @@ -1158,9 +1158,9 @@ def test_update_org_no_perm(self, user_attr: str) -> None: assert response.status_code == status.HTTP_403_FORBIDDEN # Check that the orgs didn't change - taxonomy_orgs = TaxonomyOrg.objects.filter(taxonomy=self.tA1) - assert taxonomy_orgs.count() == 1 - assert taxonomy_orgs[0].org == self.orgA + url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tA1.pk) + response = self.client.get(url) + assert response.data["orgs"] == [self.orgA.short_name] def test_update_org_check_permissions_orgA(self) -> None: """ diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py index 4f9b21393ebc..e5a4d6079ff2 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py @@ -14,7 +14,7 @@ set_taxonomy_orgs, ) from ...rules import get_admin_orgs -from .serializers import TaxonomyOrgListQueryParamsSerializer, TaxonomyUpdateOrgBodySerializer +from .serializers import TaxonomyOrgListQueryParamsSerializer, TaxonomyOrgSerializer, TaxonomyUpdateOrgBodySerializer from .filters import ObjectTagTaxonomyOrgFilterBackend, UserOrgFilterBackend @@ -39,6 +39,7 @@ class TaxonomyOrgView(TaxonomyView): """ filter_backends = [UserOrgFilterBackend] + serializer_class = TaxonomyOrgSerializer def get_queryset(self): """ @@ -53,9 +54,11 @@ def get_queryset(self): 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) + queryset = get_taxonomies_for_org(enabled, org) else: - return get_taxonomies(enabled) + queryset = get_taxonomies(enabled) + + return queryset.prefetch_related("taxonomyorg_set") def perform_create(self, serializer): """ From 79a95bc5bb572db4fcb124578715f3c860a81fea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Fri, 3 Nov 2023 19:56:45 -0300 Subject: [PATCH 6/7] style: fix pylint --- .../core/djangoapps/content_tagging/rest_api/v1/serializers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py index a7a0adc6022c..3a1297d57ccf 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py @@ -74,4 +74,3 @@ class Meta: model = TaxonomySerializer.Meta.model fields = TaxonomySerializer.Meta.fields + ["orgs"] read_only_fields = ["orgs"] - From e2e8f768a33ba145e8f0ca5dab21485b2b68759e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Mon, 6 Nov 2023 10:34:28 -0300 Subject: [PATCH 7/7] feat: add all_orgs field --- .../rest_api/v1/serializers.py | 30 ++++++++++--------- .../rest_api/v1/tests/test_views.py | 8 +++-- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py index 3a1297d57ccf..2784aef3f000 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py @@ -52,25 +52,27 @@ def validate(self, attrs: dict) -> dict: return attrs -class OrgListField(serializers.RelatedField): - """ - Serializer to return the list of orgs for a taxonomy - """ - def to_representation(self, value): - """ - Return the Organization short_name, not the TaxonomyOrg object - """ - return value.org.short_name if value.org else None - - class TaxonomyOrgSerializer(TaxonomySerializer): """ Serializer for Taxonomy objects inclusing the associated orgs """ - orgs = OrgListField(many=True, read_only=True, source="taxonomyorg_set") + orgs = serializers.SerializerMethodField() + all_orgs = serializers.SerializerMethodField() + + def get_orgs(self, obj) -> list[str]: + """ + Return the list of orgs for the taxonomy. + """ + return [taxonomy_org.org.short_name for taxonomy_org in obj.taxonomyorg_set.all() if taxonomy_org.org] + + def get_all_orgs(self, obj) -> bool: + """ + Return True if the taxonomy is associated with all orgs. + """ + return obj.taxonomyorg_set.filter(org__isnull=True).exists() class Meta: model = TaxonomySerializer.Meta.model - fields = TaxonomySerializer.Meta.fields + ["orgs"] - read_only_fields = ["orgs"] + fields = TaxonomySerializer.Meta.fields + ["orgs", "all_orgs"] + read_only_fields = ["orgs", "all_orgs"] diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index 895cd0cba87e..91c3c473dc4e 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -1066,6 +1066,7 @@ def test_update_org(self) -> None: url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tA1.pk) response = self.client.get(url) assert response.data["orgs"] == [self.orgB.short_name, self.orgX.short_name] + assert not response.data["all_orgs"] def test_update_all_org(self) -> None: """ @@ -1080,7 +1081,8 @@ def test_update_all_org(self) -> None: # Check that the orgs were updated url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tA1.pk) response = self.client.get(url) - assert response.data["orgs"] == [None] + assert response.data["orgs"] == [] + assert response.data["all_orgs"] def test_update_no_org(self) -> None: """ @@ -1097,6 +1099,7 @@ def test_update_no_org(self) -> None: url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.tA1.pk) response = self.client.get(url) assert response.data["orgs"] == [] + assert not response.data["all_orgs"] @ddt.data( (True, ["orgX"], "Using both all_orgs and orgs parameters should throw error"), @@ -1135,7 +1138,8 @@ def test_update_org_system_defined(self) -> None: # Check that the orgs didn't change url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.st1.pk) response = self.client.get(url) - assert response.data["orgs"] == [None] + assert response.data["orgs"] == [] + assert response.data["all_orgs"] @ddt.data( "staffA",