Skip to content

Commit

Permalink
fix: filter list of taxonomies by org without fetching the Organization
Browse files Browse the repository at this point in the history
  • Loading branch information
pomegranited committed Feb 7, 2024
1 parent 0a3ada3 commit 8e7823e
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 34 deletions.
3 changes: 1 addition & 2 deletions openedx/core/djangoapps/content_tagging/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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(
Expand Down
36 changes: 12 additions & 24 deletions openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 0 additions & 5 deletions openedx/core/djangoapps/content_tagging/rest_api/v1/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions openedx/core/djangoapps/content_tagging/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 == [
Expand Down

0 comments on commit 8e7823e

Please sign in to comment.