feat: refactor views to use new get_filtered_tags()

openedx_tagging/core/tagging/api.py

@@ -16,7 +16,7 @@
 from django.db.models import QuerySet
 from django.utils.translation import gettext as _
 
-from .data import TagData
+from .data import TagDataQuerySet
 from .models import ObjectTag, Tag, Taxonomy
 
 # Export this as part of the API
@@ -71,7 +71,7 @@ def get_taxonomies(enabled=True) -> QuerySet[Taxonomy]:
     return queryset.filter(enabled=enabled)
 
 
-def get_tags(taxonomy: Taxonomy) -> QuerySet[TagData]:
+def get_tags(taxonomy: Taxonomy) -> TagDataQuerySet:
     """
     Returns a QuerySet of all the tags in the given taxonomy.
 
@@ -81,7 +81,7 @@ def get_tags(taxonomy: Taxonomy) -> QuerySet[TagData]:
     return taxonomy.cast().get_filtered_tags()
 
 
-def get_root_tags(taxonomy: Taxonomy) -> QuerySet[TagData]:
+def get_root_tags(taxonomy: Taxonomy) -> TagDataQuerySet:
     """
     Returns a list of the root tags for the given taxonomy.
 
@@ -90,7 +90,7 @@ def get_root_tags(taxonomy: Taxonomy) -> QuerySet[TagData]:
     return taxonomy.cast().get_filtered_tags(depth=1)
 
 
-def search_tags(taxonomy: Taxonomy, search_term: str, exclude_object_id: str | None = None) -> QuerySet[TagData]:
+def search_tags(taxonomy: Taxonomy, search_term: str, exclude_object_id: str | None = None) -> TagDataQuerySet:
     """
     Returns a list of all tags that contains `search_term` of the given
     taxonomy, as well as their ancestors (so they can be displayed in a tree).
@@ -115,7 +115,7 @@ def search_tags(taxonomy: Taxonomy, search_term: str, exclude_object_id: str | N
 def get_children_tags(
     taxonomy: Taxonomy,
     parent_tag_value: str,
-) -> QuerySet[TagData]:
+) -> TagDataQuerySet:
     """
     Returns a QuerySet of children tags for the given parent tag.
 

openedx_tagging/core/tagging/data.py

@@ -3,7 +3,10 @@
 """
 from __future__ import annotations
 
-from typing import TypedDict
+from typing import Any, TypedDict, TYPE_CHECKING
+from typing_extensions import TypeAlias
+
+from django.db.models import QuerySet
 
 
 class TagData(TypedDict):
@@ -24,3 +27,13 @@ class TagData(TypedDict):
     usage_count: int
     # Internal database ID, if any. Generally should not be used; prefer 'value' which is unique within each taxonomy.
     _id: int | None
+
+
+if TYPE_CHECKING:
+    from django_stubs_ext import ValuesQuerySet
+    TagDataQuerySet: TypeAlias = ValuesQuerySet[Any, TagData]
+    # The following works better for pyright (provides proper VS Code autocompletions),
+    # but I can't find any way to specify different types for pyright vs mypy :/
+    # TagDataQuerySet: TypeAlias = QuerySet[TagData]
+else:
+    TagDataQuerySet = QuerySet[TagData]

openedx_tagging/core/tagging/import_export/parsers.py

@@ -10,7 +10,6 @@
 
 from django.utils.translation import gettext as _
 
-from ..api import get_tags
 from ..models import Taxonomy
 from .exceptions import EmptyCSVField, EmptyJSONField, FieldJSONError, InvalidFormat, TagParserError
 from .import_plan import TagItem

openedx_tagging/core/tagging/models/base.py

@@ -17,7 +17,8 @@
 
 from openedx_learning.lib.fields import MultiCollationTextField, case_insensitive_char_field, case_sensitive_char_field
 
-from ..data import TagData
+from ..data import TagDataQuerySet
+from .utils import ConcatNull
 
 log = logging.getLogger(__name__)
 
@@ -110,7 +111,7 @@ def get_lineage(self) -> Lineage:
             tag = tag.parent
             depth -= 1
         return lineage
-    
+
     @cached_property
     def num_ancestors(self) -> int:
         """
@@ -123,7 +124,7 @@ def num_ancestors(self) -> int:
             num_ancestors += 1
             tag = tag.parent
         return num_ancestors
-    
+
     @staticmethod
     def annotate_depth(qs: models.QuerySet) -> models.QuerySet:
         """
@@ -301,7 +302,7 @@ def get_filtered_tags(
         search_term: str | None = None,
         include_counts: bool = True,
         excluded_values: list[str] | None = None,
-    ) -> models.QuerySet[TagData]:
+    ) -> TagDataQuerySet:
         """
         Returns a filtered QuerySet of tag values.
         For free text or dynamic taxonomies, this will only return tag values
@@ -354,7 +355,7 @@ def _get_filtered_tags_free_text(
         self,
         search_term: str | None,
         include_counts: bool,
-    ) -> models.QuerySet[TagData]:
+    ) -> TagDataQuerySet:
         """
         Implementation of get_filtered_tags() for free text taxonomies.
         """
@@ -383,7 +384,7 @@ def _get_filtered_tags_one_level(
         parent_tag_value: str | None,
         search_term: str | None,
         include_counts: bool,
-    ) -> models.QuerySet[TagData]:
+    ) -> TagDataQuerySet:
         """
         Implementation of get_filtered_tags() for closed taxonomies, where
         depth=1. When depth=1, we're only looking at a single "level" of the
@@ -424,7 +425,7 @@ def _get_filtered_tags_deep(
         search_term: str | None,
         include_counts: bool,
         excluded_values: list[str] | None,
-    ) -> models.QuerySet[TagData]:
+    ) -> TagDataQuerySet:
         """
         Implementation of get_filtered_tags() for closed taxonomies, where
         we're including tags from multiple levels of the hierarchy.
@@ -462,9 +463,12 @@ def _get_filtered_tags_deep(
         qs = Tag.annotate_depth(qs)
         # Add the "lineage" field to sort them in order correctly:
         qs = qs.annotate(sort_key=Concat(
-            Coalesce(F("parent__parent__parent__value"), Value("")),
-            Coalesce(F("parent__parent__value"), Value("")),
-            Coalesce(F("parent__value"), Value("")),
+            # For a root tag, we want sort_key="RootValue" and for a depth=1 tag
+            # we want sort_key="RootValue\tValue". The following does that, since
+            # ConcatNull(...) returns NULL if any argument is NULL.
+            Coalesce(ConcatNull(F("parent__parent__parent__value"), Value("\t")), Value("")),
+            Coalesce(ConcatNull(F("parent__parent__value"), Value("\t")), Value("")),
+            Coalesce(ConcatNull(F("parent__value"), Value("\t")), Value("")),
             F("value"),
             output_field=models.CharField(),
         ))

openedx_tagging/core/tagging/models/utils.py

@@ -0,0 +1,24 @@
+"""
+Utilities for tagging and taxonomy models
+"""
+
+from django.db.models.expressions import Func
+
+
+class ConcatNull(Func):  # pylint: disable=abstract-method
+    """
+    Concatenate two arguments together. Like normal SQL but unlike Django's
+    "Concat", if either argument is NULL, the result will be NULL.
+    """
+
+    function = "CONCAT"
+
+    def as_sqlite(self, compiler, connection, **extra_context):
+        """ SQLite doesn't have CONCAT() but has a concatenation operator """
+        return super().as_sql(
+            compiler,
+            connection,
+            template="%(expressions)s",
+            arg_joiner=" || ",
+            **extra_context,
+        )

openedx_tagging/core/tagging/rest_api/v1/serializers.py

@@ -5,7 +5,8 @@
 from rest_framework import serializers
 from rest_framework.reverse import reverse
 
-from openedx_tagging.core.tagging.models import ObjectTag, Tag, Taxonomy
+from openedx_tagging.core.tagging.data import TagData
+from openedx_tagging.core.tagging.models import ObjectTag, Taxonomy
 
 
 class TaxonomyListQueryParamsSerializer(serializers.Serializer):  # pylint: disable=abstract-method
@@ -93,102 +94,41 @@ class ObjectTagUpdateQueryParamsSerializer(serializers.Serializer):  # pylint: d
     )
 
 
-class TagsSerializer(serializers.ModelSerializer):
+class TagDataSerializer(serializers.Serializer):
     """
-    Serializer for Tags
+    Serializer for TagData
 
     Adds a link to get the sub tags
     """
+    value = serializers.CharField()
+    external_id = serializers.CharField(allow_null=True)
+    child_count = serializers.IntegerField()
+    depth = serializers.IntegerField()
+    parent_value = serializers.CharField(allow_null=True)
+    usage_count = serializers.IntegerField(required=False)
+    # Internal database ID, if any. Generally should not be used; prefer 'value' which is unique within each taxonomy.
+    # Free text taxonomies never have '_id' for their tags.
+    _id = serializers.IntegerField(allow_null=True)
 
-    sub_tags_link = serializers.SerializerMethodField()
-    children_count = serializers.SerializerMethodField()
+    sub_tags_url = serializers.SerializerMethodField()
 
-    class Meta:
-        model = Tag
-        fields = (
-            "id",
-            "value",
-            "taxonomy_id",
-            "parent_id",
-            "sub_tags_link",
-            "children_count",
-        )
-
-    def get_sub_tags_link(self, obj):
+    def get_sub_tags_url(self, obj: TagData):
         """
         Returns URL for the list of child tags of the current tag.
         """
-        if obj.children.count():
-            query_params = f"?parent_tag_id={obj.id}"
+        if obj["child_count"] > 0 and "taxonomy_id" in self.context:
+            query_params = f"?parent_tag={obj['value']}"
+            request = self.context["request"]
+            url_namespace = request.resolver_match.namespace  # get the namespace, usually "oel_tagging"
             url = (
-                reverse("oel_tagging:taxonomy-tags", args=[str(obj.taxonomy_id)])
+                reverse(f"{url_namespace}:taxonomy-tags", args=[str(self.context["taxonomy_id"])])
                 + query_params
             )
-            request = self.context.get("request")
             return request.build_absolute_uri(url)
         return None
 
-    def get_children_count(self, obj):
-        """
-        Returns the number of child tags of the given tag.
-        """
-        return obj.children.count()
+    def update(self, instance, validated_data):
+        raise RuntimeError('`update()` is not supported by the TagData serializer.')
 
-
-class TagsWithSubTagsSerializer(serializers.ModelSerializer):
-    """
-    Serializer for Tags.
-
-    Represents a tree with a list of sub tags
-    """
-
-    sub_tags = serializers.SerializerMethodField()
-    children_count = serializers.SerializerMethodField()
-
-    class Meta:
-        model = Tag
-        fields = (
-            "id",
-            "value",
-            "taxonomy_id",
-            "sub_tags",
-            "children_count",
-        )
-
-    def get_sub_tags(self, obj):
-        """
-        Returns a serialized list of child tags for the given tag.
-        """
-        serializer = TagsWithSubTagsSerializer(
-            obj.children.all().order_by("value", "id"),
-            many=True,
-            read_only=True,
-        )
-        return serializer.data
-
-    def get_children_count(self, obj):
-        """
-        Returns the number of child tags of the given tag.
-        """
-        return obj.children.count()
-
-
-class TagsForSearchSerializer(TagsWithSubTagsSerializer):
-    """
-    Serializer for Tags
-
-    Used to filter sub tags of a given tag
-    """
-
-    def get_sub_tags(self, obj):
-        """
-        Returns a serialized list of child tags for the given tag.
-        """
-        serializer = TagsWithSubTagsSerializer(obj.sub_tags, many=True, read_only=True)
-        return serializer.data
-
-    def get_children_count(self, obj):
-        """
-        Returns the number of child tags of the given tag.
-        """
-        return len(obj.sub_tags)
+    def create(self, validated_data):
+        raise RuntimeError('`create()` is not supported by the TagData serializer.')