From dad31a393a77340077ca40a7dfc599607e46c104 Mon Sep 17 00:00:00 2001 From: Jillian Date: Fri, 10 Nov 2023 05:53:12 +1030 Subject: [PATCH] fix: allows taxonomy list to "filter by org" even if org does not exist (#33686) If the user requests the taxonomies linked to a non-existent org, then we still want to see the non-org taxonomies. --- .../rest_api/v1/serializers.py | 24 ++++++++++++++++++- .../rest_api/v1/tests/test_views.py | 16 ++----------- .../content_tagging/rest_api/v1/views.py | 4 +++- 3 files changed, 28 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 2784aef3f000..058022ef7516 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py @@ -4,6 +4,7 @@ from __future__ import annotations +from django.core.exceptions import ObjectDoesNotExist from rest_framework import serializers, fields from openedx_tagging.core.tagging.rest_api.v1.serializers import ( @@ -14,12 +15,33 @@ from organizations.models import Organization +class OptionalSlugRelatedField(serializers.SlugRelatedField): + """ + Modifies the DRF serializer SlugRelatedField. + + Non-existent slug values are represented internally as an empty queryset, instead of throwing a validation error. + """ + + def to_internal_value(self, data): + """ + Returns the object related to the given slug value, or an empty queryset if not found. + """ + + queryset = self.get_queryset() + try: + return queryset.get(**{self.slug_field: data}) + except ObjectDoesNotExist: + return queryset.none() + except (TypeError, ValueError): + self.fail('invalid') + + class TaxonomyOrgListQueryParamsSerializer(TaxonomyListQueryParamsSerializer): """ Serializer for the query params for the GET view """ - org: fields.Field = serializers.SlugRelatedField( + org: fields.Field = OptionalSlugRelatedField( slug_field="short_name", queryset=Organization.objects.all(), required=False, 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 4e298188dbe1..ee6b3330a803 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 @@ -343,6 +343,8 @@ def test_list_taxonomy_enabled_filter(self, enabled_parameter: bool, expected_ta ("orgA", ["st1", "st2", "t1", "t2", "tA1", "tA2", "tBA1", "tBA2"]), ("orgB", ["st1", "st2", "t1", "t2", "tB1", "tB2", "tBA1", "tBA2"]), ("orgX", ["st1", "st2", "t1", "t2"]), + # Non-existent orgs are ignored + ("invalidOrg", ["st1", "st2", "t1", "t2"]), ) @ddt.unpack def test_list_taxonomy_org_filter(self, org_parameter: str, expected_taxonomies: list[str]) -> None: @@ -355,20 +357,6 @@ def test_list_taxonomy_org_filter(self, org_parameter: str, expected_taxonomies: expected_taxonomies=expected_taxonomies, ) - def test_list_taxonomy_invalid_org(self) -> None: - """ - Tests that using an invalid org in the filter will raise BAD_REQUEST - """ - url = TAXONOMY_ORG_LIST_URL - - self.client.force_authenticate(user=self.staff) - - 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", (), None), ("staffA", ["tA2", "tBA1", "tBA2"], 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 e5a4d6079ff2..90b2024e239a 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py @@ -52,8 +52,10 @@ def get_queryset(self): query_params = TaxonomyOrgListQueryParamsSerializer(data=self.request.query_params.dict()) query_params.is_valid(raise_exception=True) enabled = query_params.validated_data.get("enabled", None) + + # If org filtering was requested, then use it, even if the org is invalid/None org = query_params.validated_data.get("org", None) - if org: + if "org" in query_params.validated_data: queryset = get_taxonomies_for_org(enabled, org) else: queryset = get_taxonomies(enabled)