From 8e7823eb344e9d8a5c561e33c2c642f0cbc453df Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Wed, 7 Feb 2024 13:25:52 +1030 Subject: [PATCH] fix: filter list of taxonomies by org without fetching the Organization --- .../core/djangoapps/content_tagging/api.py | 3 +- .../rest_api/v1/serializers.py | 36 +++++++------------ .../rest_api/v1/tests/test_views.py | 2 +- .../content_tagging/rest_api/v1/views.py | 5 --- .../content_tagging/tests/test_api.py | 4 +-- 5 files changed, 16 insertions(+), 34 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index 70aa7e2150da..3db70f96b4d4 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -82,7 +82,7 @@ def set_taxonomy_orgs( def get_taxonomies_for_org( enabled=True, - org_owner: Organization | None = None, + org_short_name: str | None = None, ) -> QuerySet: """ Generates a list of the enabled Taxonomies available for the given org, sorted by name. @@ -95,7 +95,6 @@ def get_taxonomies_for_org( If you want the disabled Taxonomies, pass enabled=False. If you want all Taxonomies (both enabled and disabled), pass enabled=None. """ - org_short_name = org_owner.short_name if org_owner else None return oel_tagging.get_taxonomies(enabled=enabled).filter( Exists( TaxonomyOrg.get_relationships( 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 12433f8a381b..2f526dedc301 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py @@ -15,39 +15,27 @@ 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 = OptionalSlugRelatedField( - slug_field="short_name", - queryset=Organization.objects.all(), + org: fields.Field = serializers.CharField( required=False, ) unassigned: fields.Field = serializers.BooleanField(required=False) + def validate(self, attrs: dict) -> dict: + """ + Validate the serializer data + """ + if "org" in attrs and "unassigned" in attrs: + raise serializers.ValidationError( + "'org' and 'unassigned' params cannot be both defined" + ) + + return attrs + class TaxonomyUpdateOrgBodySerializer(serializers.Serializer): """ 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 7dc101966fa4..855a34a3f926 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 @@ -494,7 +494,7 @@ def test_list_taxonomy_query_count(self): url = TAXONOMY_ORG_LIST_URL + f'?org={self.orgA.short_name}&enabled=true' self.client.force_authenticate(user=self.staff) - with self.assertNumQueries(23): # TODO Why so many queries? + with self.assertNumQueries(21): # TODO Why so many queries? response = self.client.get(url) assert response.status_code == 200 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 151bc09f5d76..fff2404aa95b 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py @@ -56,13 +56,8 @@ 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) - unassigned = query_params.validated_data.get("unassigned", None) org = query_params.validated_data.get("org", None) - # Raise an error if both "org" and "unassigned" query params were provided - if "org" in query_params.validated_data and "unassigned" in query_params.validated_data: - raise ValidationError("'org' and 'unassigned' params cannot be both defined") - # If org filtering was requested, then use it, even if the org is invalid/None if "org" in query_params.validated_data: queryset = get_taxonomies_for_org(enabled, org) diff --git a/openedx/core/djangoapps/content_tagging/tests/test_api.py b/openedx/core/djangoapps/content_tagging/tests/test_api.py index 9a297be968b1..837e17e7c19c 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_api.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_api.py @@ -145,11 +145,11 @@ def test_get_taxonomies_enabled_subclasses(self): ) @ddt.unpack def test_get_taxonomies_for_org(self, org_attr, enabled, expected): - org_owner = getattr(self, org_attr) if org_attr else None + org_owner = getattr(self, org_attr).short_name if org_attr else None taxonomies = list( taxonomy.cast() for taxonomy in api.get_taxonomies_for_org( - org_owner=org_owner, enabled=enabled + org_short_name=org_owner, enabled=enabled ) ) assert taxonomies == [