Skip to content

Commit

Permalink
fix: make tree sorting case insensitive
Browse files Browse the repository at this point in the history
  • Loading branch information
bradenmacdonald committed Oct 25, 2023
1 parent 941f473 commit cd40639
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 3 deletions.
6 changes: 3 additions & 3 deletions openedx_tagging/core/tagging/models/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from django.core.exceptions import ValidationError
from django.db import models
from django.db.models import F, Q, Value
from django.db.models.functions import Concat
from django.db.models.functions import Concat, Lower
from django.utils.functional import cached_property
from django.utils.module_loading import import_string
from django.utils.translation import gettext_lazy as _
Expand Down Expand Up @@ -462,7 +462,7 @@ def _get_filtered_tags_deep(
# Add the "depth" to each tag:
qs = Tag.annotate_depth(qs)
# Add the "lineage" as a field called "sort_key" to sort them in order correctly:
qs = qs.annotate(sort_key=Concat(
qs = qs.annotate(sort_key=Lower(Concat(
# 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.
Expand All @@ -472,7 +472,7 @@ def _get_filtered_tags_deep(
F("value"),
Value("\t"), # We also need the '\t' separator character at the end, or MySQL will sort things wrong
output_field=models.CharField(),
))
)))
# Add the parent value
qs = qs.annotate(parent_value=F("parent__value"))
qs = qs.annotate(_id=F("id")) # ID has an underscore to encourage use of 'value' rather than this internal ID
Expand Down
37 changes: 37 additions & 0 deletions tests/openedx_tagging/core/tagging/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,43 @@ def test_pathological_tree_sort(self) -> None:
" 123 (111) (used: 0, children: 0)",
]

def test_case_insensitive_sort(self) -> None:
"""
Make sure the sorting is case-insensitive
"""
# pylint: disable=unused-variable
taxonomy = api.create_taxonomy("Sort Test")
root1 = Tag.objects.create(taxonomy=taxonomy, value="ALPHABET")
child1_1 = Tag.objects.create(taxonomy=taxonomy, value="Android", parent=root1)
child1_2 = Tag.objects.create(taxonomy=taxonomy, value="abacus", parent=root1)
child1_3 = Tag.objects.create(taxonomy=taxonomy, value="aardvark", parent=root1)
child1_4 = Tag.objects.create(taxonomy=taxonomy, value="ANVIL", parent=root1)

root2 = Tag.objects.create(taxonomy=taxonomy, value="abstract")
child2_1 = Tag.objects.create(taxonomy=taxonomy, value="Andes", parent=root2)
child2_2 = Tag.objects.create(taxonomy=taxonomy, value="azores islands", parent=root2)

result = pretty_format_tags(taxonomy.get_filtered_tags())
assert result == [
"abstract (None) (used: 0, children: 2)",
" Andes (abstract) (used: 0, children: 0)",
" azores islands (abstract) (used: 0, children: 0)",
"ALPHABET (None) (used: 0, children: 4)",
" aardvark (ALPHABET) (used: 0, children: 0)",
" abacus (ALPHABET) (used: 0, children: 0)",
" Android (ALPHABET) (used: 0, children: 0)",
" ANVIL (ALPHABET) (used: 0, children: 0)",
]

# And it's case insensitive when getting only a single level:
result = pretty_format_tags(taxonomy.get_filtered_tags(parent_tag_value="ALPHABET", depth=1))
assert result == [
" aardvark (ALPHABET) (used: 0, children: 0)",
" abacus (ALPHABET) (used: 0, children: 0)",
" Android (ALPHABET) (used: 0, children: 0)",
" ANVIL (ALPHABET) (used: 0, children: 0)",
]


class TestFilteredTagsFreeTextTaxonomy(TestCase):
"""
Expand Down

0 comments on commit cd40639

Please sign in to comment.