From cd40639fe7dd53d43addd19eccca2e8863e3ee49 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Wed, 25 Oct 2023 12:11:32 -0700 Subject: [PATCH] fix: make tree sorting case insensitive --- openedx_tagging/core/tagging/models/base.py | 6 +-- .../core/tagging/test_models.py | 37 +++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/openedx_tagging/core/tagging/models/base.py b/openedx_tagging/core/tagging/models/base.py index 3c7830b5..82915cd3 100644 --- a/openedx_tagging/core/tagging/models/base.py +++ b/openedx_tagging/core/tagging/models/base.py @@ -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 _ @@ -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. @@ -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 diff --git a/tests/openedx_tagging/core/tagging/test_models.py b/tests/openedx_tagging/core/tagging/test_models.py index 6bea627c..c0c80fea 100644 --- a/tests/openedx_tagging/core/tagging/test_models.py +++ b/tests/openedx_tagging/core/tagging/test_models.py @@ -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): """