Reduce number of queries required for content tagging endpoints

openedx/core/djangoapps/content_tagging/api.py

@@ -84,7 +84,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.
@@ -97,7 +97,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(

openedx/core/djangoapps/content_tagging/models/base.py

@@ -67,19 +67,24 @@ def get_relationships(
 
     @classmethod
     def get_organizations(
-        cls, taxonomy: Taxonomy, rel_type: RelType
-    ) -> list[Organization]:
+        cls, taxonomy: Taxonomy, rel_type=RelType.OWNER,
+    ) -> tuple[bool, list[Organization]]:
         """
-        Returns the list of Organizations which have the given relationship to the taxonomy.
+        Returns a tuple containing:
+        * bool: flag indicating whether "all organizations" have the given relationship to the taxonomy
+        * orgs: list of Organizations which have the given relationship to the taxonomy
         """
-        rels = cls.objects.filter(
-            taxonomy=taxonomy,
-            rel_type=rel_type,
-        )
-        # A relationship with org=None means all Organizations
-        if rels.filter(org=None).exists():
-            return list(Organization.objects.all())
-        return [rel.org for rel in rels]
+        is_all_org = False
+        orgs = []
+        # Iterate over the taxonomyorgs instead of filtering to take advantage of prefetched data.
+        for taxonomy_org in taxonomy.taxonomyorg_set.all():
+            if taxonomy_org.rel_type == rel_type:
+                if taxonomy_org.org is None:
+                    is_all_org = True
+                else:
+                    orgs.append(taxonomy_org.org)
+
+        return (is_all_org, orgs)
 
 
 class ContentObjectTag(ObjectTag):

openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py

@@ -4,7 +4,6 @@
 
 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,40 +13,30 @@
 
 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')
+from ...models import TaxonomyOrg
 
 
 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):
     """
@@ -86,14 +75,21 @@ class TaxonomyOrgSerializer(TaxonomySerializer):
     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]
+        """
+        return [
+            taxonomy_org.org.short_name for taxonomy_org in obj.taxonomyorg_set.all()
+            if taxonomy_org.org and taxonomy_org.rel_type == TaxonomyOrg.RelType.OWNER
+        ]
 
     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()
+        is_all_orgs = False
+        for taxonomy_org in obj.taxonomyorg_set.all():
+            if taxonomy_org.org_id is None and taxonomy_org.rel_type == TaxonomyOrg.RelType.OWNER:
+                return True
+        return False
 
     class Meta:
         model = TaxonomySerializer.Meta.model

openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py

@@ -494,15 +494,15 @@ def test_list_taxonomy_query_count(self):
         """
         Test how many queries are used when retrieving taxonomies and permissions
         """
-        url = TAXONOMY_ORG_LIST_URL + f'?org=${self.orgA.short_name}&enabled=true'
+        url = TAXONOMY_ORG_LIST_URL + f'?org={self.orgA.short_name}&enabled=true'
 
         self.client.force_authenticate(user=self.staff)
-        with self.assertNumQueries(16):  # TODO Why so many queries?
+        with self.assertNumQueries(11):
             response = self.client.get(url)
 
         assert response.status_code == 200
         assert response.data["can_add_taxonomy"]
-        assert len(response.data["results"]) == 2
+        assert len(response.data["results"]) == 4
         for taxonomy in response.data["results"]:
             if taxonomy["system_defined"]:
                 assert not taxonomy["can_change_taxonomy"]

openedx/core/djangoapps/content_tagging/rest_api/v1/views.py

@@ -5,7 +5,7 @@
 from openedx_tagging.core.tagging.rest_api.v1.views import ObjectTagView, TaxonomyView
 from rest_framework import status
 from rest_framework.decorators import action
-from rest_framework.exceptions import PermissionDenied, ValidationError
+from rest_framework.exceptions import PermissionDenied
 from rest_framework.request import Request
 from rest_framework.response import Response
 
@@ -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)
@@ -71,7 +66,8 @@ def get_queryset(self):
         else:
             queryset = get_taxonomies(enabled)
 
-        return queryset.prefetch_related("taxonomyorg_set")
+        # Prefetch tag_set so we can serialize the tag counts
+        return queryset.prefetch_related("taxonomyorg_set__org", "tag_set")
 
     def perform_create(self, serializer):
         """

openedx/core/djangoapps/content_tagging/rules.py

@@ -141,21 +141,12 @@ def can_view_taxonomy(user: UserType, taxonomy: oel_tagging.Taxonomy) -> bool:
     if oel_tagging.is_taxonomy_admin(user):
         return True
 
-    is_all_org = TaxonomyOrg.objects.filter(
-        taxonomy=taxonomy,
-        org=None,
-        rel_type=TaxonomyOrg.RelType.OWNER,
-    ).exists()
+    is_all_org, taxonomy_orgs = TaxonomyOrg.get_organizations(taxonomy)
 
     # Enabled all-org taxonomies can be viewed by any registred user
     if is_all_org:
         return taxonomy.enabled
 
-    taxonomy_orgs = TaxonomyOrg.get_organizations(
-        taxonomy=taxonomy,
-        rel_type=TaxonomyOrg.RelType.OWNER,
-    )
-
     # Org-level staff can view any taxonomy that is associated with one of their orgs.
     if is_org_admin(user, taxonomy_orgs):
         return True
@@ -191,21 +182,12 @@ def can_change_taxonomy(user: UserType, taxonomy: oel_tagging.Taxonomy) -> bool:
     if oel_tagging.is_taxonomy_admin(user):
         return True
 
-    is_all_org = TaxonomyOrg.objects.filter(
-        taxonomy=taxonomy,
-        org=None,
-        rel_type=TaxonomyOrg.RelType.OWNER,
-    ).exists()
+    is_all_org, taxonomy_orgs = TaxonomyOrg.get_organizations(taxonomy)
 
     # Only taxonomy admins can edit all org taxonomies
     if is_all_org:
         return False
 
-    taxonomy_orgs = TaxonomyOrg.get_organizations(
-        taxonomy=taxonomy,
-        rel_type=TaxonomyOrg.RelType.OWNER,
-    )
-
     # Org-level staff can edit any taxonomy that is associated with one of their orgs.
     if is_org_admin(user, taxonomy_orgs):
         return True

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 == [

requirements/constraints.txt

@@ -108,7 +108,7 @@ libsass==0.10.0
 click==8.1.6
 
 # pinning this version to avoid updates while the library is being developed
-openedx-learning==0.5.1
+openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@jill/tagging-less-queries
 
 # Open AI version 1.0.0 dropped support for openai.ChatCompletion which is currently in use in enterprise.
 openai<=0.28.1

requirements/edx/base.txt

@@ -782,7 +782,7 @@ openedx-filters==1.6.0
     # via
     #   -r requirements/edx/kernel.in
     #   lti-consumer-xblock
-openedx-learning==0.5.1
+openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@jill/tagging-less-queries
     # via
     #   -c requirements/edx/../constraints.txt
     #   -r requirements/edx/kernel.in

requirements/edx/development.txt

@@ -1313,7 +1313,7 @@ openedx-filters==1.6.0
     #   -r requirements/edx/doc.txt
     #   -r requirements/edx/testing.txt
     #   lti-consumer-xblock
-openedx-learning==0.5.1
+openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@jill/tagging-less-queries
     # via
     #   -c requirements/edx/../constraints.txt
     #   -r requirements/edx/doc.txt

requirements/edx/doc.txt

@@ -924,7 +924,7 @@ openedx-filters==1.6.0
     # via
     #   -r requirements/edx/base.txt
     #   lti-consumer-xblock
-openedx-learning==0.5.1
+openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@jill/tagging-less-queries
     # via
     #   -c requirements/edx/../constraints.txt
     #   -r requirements/edx/base.txt

requirements/edx/testing.txt

@@ -982,7 +982,7 @@ openedx-filters==1.6.0
     # via
     #   -r requirements/edx/base.txt
     #   lti-consumer-xblock
-openedx-learning==0.5.1
+openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@jill/tagging-less-queries
     # via
     #   -c requirements/edx/../constraints.txt
     #   -r requirements/edx/base.txt