TP2000-1651 Fix sorting on list views

certificates/jinja2/includes/certificates/tabs/measures.jinja

@@ -44,18 +44,16 @@
     ]) or "" }}
   {% endfor %}
 
-  {% set base_url = url('certificate-ui-detail-measures', kwargs={"certificate_type__sid":object.certificate_type.sid, "sid":object.sid}) %}
-
   {% set commodity_code %}
-    {{ create_sortable_anchor(request, "goods_nomenclature", "Commodity code", base_url) }}
+    {{ create_sortable_anchor(request, "Commodity code", sorting_urls["goods_nomenclature"]) }}
   {% endset %}
 
   {% set geo_area %}
-    {{ create_sortable_anchor(request, "geo_area", "Geographical area", base_url) }}
+    {{ create_sortable_anchor(request, "Geographical area", sorting_urls["geo_area"]) }}
   {% endset %}
 
   {% set start_date %}
-    {{ create_sortable_anchor(request, "start_date", "Start date", base_url) }}
+    {{ create_sortable_anchor(request, "Start date", sorting_urls["start_date"]) }}
   {% endset %}
 
   {% if paginator.count > 0 %}

certificates/tests/test_views.py

@@ -373,6 +373,7 @@ def test_certificate_detail_measures_view_sorting_commodity(valid_user_client):
         )
         measures.append(measure)
     commodity_codes = [measure.goods_nomenclature.item_id for measure in measures]
+    commodity_codes.sort()
 
     url = reverse(
         "certificate-ui-detail-measures",

certificates/views.py

@@ -168,6 +168,7 @@ class CertificateDetailMeasures(SortingMixin, WithPaginationListMixin, ListView)
     paginate_by = 20
     sort_by_fields = ["goods_nomenclature", "geo_area", "start_date"]
     custom_sorting = {
+        "goods_nomenclature": "goods_nomenclature__item_id",
         "geo_area": "geographical_area__area_id",
         "start_date": "valid_between",
     }

commodities/jinja2/includes/commodities/tabs/measures-declarable.jinja

@@ -62,22 +62,20 @@
   ]) or "" }}
 {% endfor %}
 
-{% set base_url = url('commodity-ui-detail-measures-declarable', args=[commodity.sid]) %}
-
 {% set comm_code %}
-  {{ create_sortable_anchor(request, "commodity", "Commodity code", base_url) }}
+  {{ create_sortable_anchor(request, "Commodity code", sorting_urls["commodity"]) }}
 {% endset %}
 
 {% set measure_type %}
-  {{ create_sortable_anchor(request, "measure_type", "Measure type", base_url) }}
+  {{ create_sortable_anchor(request, "Measure type", sorting_urls["measure_type"]) }}
 {% endset %}
 
 {% set geo_area %}
-  {{ create_sortable_anchor(request, "geo_area", "Geographical area", base_url) }}
+  {{ create_sortable_anchor(request, "Geographical area", sorting_urls["geo_area"]) }}
 {% endset %}
 
 {% set start_date %}
-  {{ create_sortable_anchor(request, "start_date", "Start date", base_url) }}
+  {{ create_sortable_anchor(request, "Start date", sorting_urls["start_date"]) }}
 {% endset %}
 
 {% if paginator.count > 0 %}

commodities/jinja2/includes/commodities/tabs/measures-defined.jinja

@@ -56,18 +56,16 @@
   ]) or "" }}
 {% endfor %}
 
-{% set base_url = url('commodity-ui-detail-measures-as-defined', args=[commodity.sid]) %}
-
 {% set measure_type %}
-  {{ create_sortable_anchor(request, "measure_type", "Measure type", base_url) }}
+  {{ create_sortable_anchor(request, "Measure type", sorting_urls["measure_type"]) }}
 {% endset %}
 
 {% set geo_area %}
-  {{ create_sortable_anchor(request, "geo_area", "Geographical area", base_url) }}
+  {{ create_sortable_anchor(request, "Geographical area", sorting_urls["geo_area"]) }}
 {% endset %}
 
 {% set start_date %}
-  {{ create_sortable_anchor(request, "start_date", "Start date", base_url) }}
+  {{ create_sortable_anchor(request, "Start date", sorting_urls["start_date"]) }}
 {% endset %}
 
 {% if paginator.count > 0 %}

common/jinja2/components/create_sortable_anchor.jinja

@@ -1,21 +1,11 @@
-
-{% macro create_sortable_anchor(request, sort_by, title, base_url, query_params=False, fragment="") %}
-
-  {% if query_params %}
-    {% set query_string_prefix = "&"%}
-  {% else %}
-    {% set query_string_prefix = "?"%}
-  {% endif %}
-
-  {% if request.GET.sort_by == sort_by and request.GET.ordered == "desc" %}
-
-      <a class="govuk-link govuk-link--no-visited-state sort-icon--down" href="{{ base_url }}{{query_string_prefix}}sort_by={{ sort_by }}&ordered=asc{{ fragment }}">
-        {{ title }} <img src="{{ static('common/images/sort_icon.svg') }}" alt="">
-      </a>
+{% macro create_sortable_anchor(request, title, url, fragment="") %}
+  {% if request.GET.ordered == "desc" %}
+    <a class="govuk-link govuk-link--no-visited-state sort-icon--down" href="{{ url }}{{ fragment }}">
+      {{ title }} <img src="{{ static('common/images/sort_icon.svg') }}" alt="">
+    </a>
   {% else %}
-      <a class="govuk-link govuk-link--no-visited-state sort-icon--up" href="{{ base_url }}{{query_string_prefix}}sort_by={{ sort_by }}&ordered=desc{{ fragment }}">
+    <a class="govuk-link govuk-link--no-visited-state sort-icon--up" href="{{ url }}{{ fragment }}">
       {{ title }} <img src="{{ static('common/images/sort_icon.svg') }}" alt="">
     </a>
   {% endif %}
-
 {% endmacro %}

common/util.py

@@ -47,7 +47,9 @@
 from django.db.models.functions.text import Lower
 from django.db.models.functions.text import Upper
 from django.db.transaction import atomic
+from django.http import HttpRequest
 from django.template import loader
+from django.urls import reverse
 from django.utils import timezone
 from lxml import etree
 from psycopg.types.range import DateRange
@@ -809,3 +811,11 @@ def get_related_names(instance, related_model) -> list[str]:
         if field.one_to_many and issubclass(field.related_model, related_model):
             related_names.append(field.get_accessor_name())
     return related_names
+
+
+def get_current_view_url(request: HttpRequest) -> str:
+    """Returns the resolved URL of the current view."""
+    view_name = request.resolver_match.view_name
+    args = request.resolver_match.args
+    kwargs = request.resolver_match.kwargs
+    return reverse(view_name, args=args, kwargs=kwargs)

common/views/mixins.py

@@ -8,11 +8,13 @@
 from django.db.models import Model
 from django.db.models import QuerySet
 from django.http import Http404
+from django.http import QueryDict
 
 from common.business_rules import BusinessRule
 from common.business_rules import BusinessRuleViolation
 from common.models import TrackedModel
 from common.pagination import build_pagination_list
+from common.util import get_current_view_url
 
 
 class WithPaginationListMixin:
@@ -162,3 +164,41 @@ def get_ordering(self):
 
         else:
             return None
+
+    def remove_sorting_params(self) -> QueryDict:
+        """Returns a cleaned copy of GET params having removed 'sort_by' and
+        'ordered'."""
+        query_params = self.request.GET.copy()
+        query_params.pop("sort_by", None)
+        query_params.pop("ordered", None)
+        return query_params
+
+    def build_sorting_urls(self) -> dict[str, str]:
+        """
+        Returns a dictionary mapping a sort_by_field to its sorting URL.
+
+        Appends `sort_by` and `ordered` query params to the current view URL for each field in `sort_by_fields`,
+        while keeping other query params (such as those used for filtering) in place.
+        """
+        urls = {}
+        view_url = get_current_view_url(self.request)
+        query_params = self.remove_sorting_params().urlencode()
+        ordering = self.get_ordering()
+
+        for sort_by in self.sort_by_fields:
+            if ordering and ordering.startswith("-"):
+                sorting_params = f"sort_by={sort_by}&ordered=asc"
+            else:
+                sorting_params = f"sort_by={sort_by}&ordered=desc"
+            urls[sort_by] = (
+                f"{view_url}?{query_params}&{sorting_params}"
+                if query_params
+                else f"{view_url}?{sorting_params}"
+            )
+
+        return urls
+
+    def get_context_data(self, **kwargs):
+        context = super().get_context_data(**kwargs)
+        context["sorting_urls"] = self.build_sorting_urls()
+        return context

footnotes/jinja2/includes/footnotes/tabs/measures.jinja

@@ -44,18 +44,16 @@
     ]) or "" }}
   {% endfor %}
 
-  {% set base_url = url('footnote-ui-detail-measures', kwargs={"footnote_type__footnote_type_id":object.footnote_type.footnote_type_id, "footnote_id":object.footnote_id}) %}
-
   {% set commodity_code %}
-    {{ create_sortable_anchor(request, "goods_nomenclature", "Commodity code", base_url) }}
+    {{ create_sortable_anchor(request, "Commodity code", sorting_urls["goods_nomenclature"]) }}
   {% endset %}
 
  {% set geo_area %}
-    {{ create_sortable_anchor(request, "geo_area", "Geographical area", base_url) }}
+    {{ create_sortable_anchor(request, "Geographical area", sorting_urls["geo_area"]) }}
   {% endset %}
 
   {% set start_date %}
-    {{ create_sortable_anchor(request, "start_date", "Start date", base_url) }}
+    {{ create_sortable_anchor(request, "Start date", sorting_urls["start_date"]) }}
   {% endset %}
 
   {% if paginator.count > 0 %}

footnotes/views.py

@@ -203,6 +203,7 @@ class FootnoteDetailMeasures(SortingMixin, WithPaginationListMixin, ListView):
     paginate_by = 20
     sort_by_fields = ["goods_nomenclature", "geo_area", "start_date"]
     custom_sorting = {
+        "goods_nomenclature": "goods_nomenclature__item_id",
         "geo_area": "geographical_area__area_id",
         "start_date": "valid_between",
     }

geo_areas/jinja2/includes/geo_areas/tabs/measures.jinja

@@ -40,14 +40,12 @@
     ]) or "" }}
   {% endfor %}
 
-  {% set base_url = url('geo_area-ui-detail-measures', kwargs={"sid": object.sid}) %}
-
   {% set commodity_code %}
-    {{ create_sortable_anchor(request, "goods_nomenclature", "Commodity code", base_url) }}
+    {{ create_sortable_anchor(request, "Commodity code", sorting_urls["goods_nomenclature"]) }}
   {% endset %}
 
   {% set start_date %}
-    {{ create_sortable_anchor(request, "start_date", "Start date", base_url) }}
+    {{ create_sortable_anchor(request, "Start date", sorting_urls["start_date"]) }}
   {% endset %}
 
   {% if paginator.count > 0 %}

geo_areas/views.py

@@ -148,6 +148,7 @@ class GeoAreaDetailMeasures(SortingMixin, WithPaginationListMixin, ListView):
     paginate_by = 20
     sort_by_fields = ["goods_nomenclature", "start_date"]
     custom_sorting = {
+        "goods_nomenclature": "goods_nomenclature__item_id",
         "start_date": "valid_between",
     }
 

measures/jinja2/includes/measures/list.jinja

@@ -13,23 +13,23 @@
 {%- endset %}
 
 {% set sid %}
-  {{ create_sortable_anchor(request, "sid", "SID", base_url, query_params) }}
+  {{ create_sortable_anchor(request, "SID", sorting_urls["sid"]) }}
 {% endset %}
 
 {% set measure_type %}
-  {{ create_sortable_anchor(request, "measure_type", "Type", base_url, query_params) }}
+  {{ create_sortable_anchor(request, "Type", sorting_urls["measure_type"]) }}
 {% endset %}
 
 {% set geo_area %}
-  {{ create_sortable_anchor(request, "geo_area", "Geographical area", base_url, query_params) }}
+  {{ create_sortable_anchor(request, "Geographical area", sorting_urls["geo_area"]) }}
 {% endset %}
 
 {% set start_date %}
-  {{ create_sortable_anchor(request, "start_date", "Start date", base_url, query_params) }}
+  {{ create_sortable_anchor(request, "Start date", sorting_urls["start_date"]) }}
 {% endset %}
 
 {% set end_date %}
-  {{ create_sortable_anchor(request, "end_date", "End date", base_url, query_params) }}
+  {{ create_sortable_anchor(request, "End date", sorting_urls["end_date"]) }}
 {% endset %}
 
 {# sets out form #}

measures/views/search.py

@@ -84,16 +84,6 @@ def get_form_kwargs(self):
         kwargs["objects"] = page.object_list
         return kwargs
 
-    def cleaned_query_params(self):
-        # Remove the sort_by and ordered params in order to stop them being duplicated in the base url
-        if "sort_by" and "ordered" in self.filterset.data:
-            cleaned_filterset = self.filterset.data.copy()
-            cleaned_filterset.pop("sort_by")
-            cleaned_filterset.pop("ordered")
-            return cleaned_filterset
-        else:
-            return self.filterset.data
-
     def selected_filter_formatter(self) -> List[List[str]]:
         """
         A function that formats the selected filter choices into nicely written
@@ -242,6 +232,7 @@ def get_context_data(self, **kwargs):
                 ),
                 "selected_filter_lists": self.selected_filter_formatter(),
                 "workbasket": self.workbasket,
+                "sorting_urls": self.build_sorting_urls(),
             },
         )
         if context["has_previous_page"]:
@@ -253,10 +244,6 @@ def get_context_data(self, **kwargs):
             pk__in=self.measure_selections,
         ).values_list("sid", flat=True)
 
-        context["query_params"] = True
-        context["base_url"] = (
-            f'{reverse("measure-ui-list")}?{urlencode(self.cleaned_query_params())}'
-        )
         return context
 
     def get_initial(self):

quotas/jinja2/includes/quotas/tabs/measures.jinja

@@ -16,10 +16,8 @@
   ]) or "" }}
 {% endfor %}
 
-{% set base_url = url('quota-ui-detail', args=[object.sid]) %}
-
 {% set commodity_code %}
-  {{ create_sortable_anchor(request, "goods_nomenclature", "Commodity code", base_url, "#measures") }}
+  {{ create_sortable_anchor(request, "Commodity code", sorting_urls["goods_nomenclature"], "#measures") }}
 {% endset %}
 
 <div class="quota__measures govuk-grid-row">

quotas/jinja2/quotas/definitions.jinja

@@ -61,7 +61,6 @@
 
 {{ fake_tabs(links) }}
 <div class="quota-definitions">
-  {% set base_url = url('quota_definition-ui-list', args=[quota.sid] ) %}
   <div class="govuk-tabs__panel">
   {% if quota_type == "blocking_periods" %}
     {% include "quotas/tables/blocking_periods.jinja" %}

quotas/jinja2/quotas/tables/definitions.jinja

@@ -65,11 +65,11 @@
 {% endfor %}
 
 {% set sid %}
-  {{ create_sortable_anchor(request, "sid", "Quota definition SID", base_url) }}
+  {{ create_sortable_anchor(request, "Quota definition SID", sorting_urls["sid"]) }}
 {% endset %}
 
 {% set start_date %}
-  {{ create_sortable_anchor(request, "valid_between", "Start date", base_url) }}
+  {{ create_sortable_anchor(request, "Start date", sorting_urls["valid_between"]) }}
 {% endset %}
 
 {{ govukTable({

quotas/views/base.py

@@ -190,7 +190,7 @@ class QuotaList(QuotaOrderNumberMixin, TamatoListView):
     filterset_class = QuotaFilter
 
 
-class QuotaDetail(QuotaOrderNumberMixin, TrackedModelDetailView, SortingMixin):
+class QuotaDetail(QuotaOrderNumberMixin, SortingMixin, TrackedModelDetailView):
     template_name = "quotas/detail.jinja"
     sort_by_fields = ["goods_nomenclature"]
 

regulations/jinja2/includes/regulations/tabs/measures.jinja

@@ -45,18 +45,16 @@
     ]) or "" }}
   {% endfor %}
 
-  {% set base_url = url('regulation-ui-detail-measures', kwargs={"role_type": object.role_type, "regulation_id": object.regulation_id}) %}
-
   {% set commodity_code %}
-    {{ create_sortable_anchor(request, "goods_nomenclature", "Commodity code", base_url) }}
+    {{ create_sortable_anchor(request, "Commodity code", sorting_urls["goods_nomenclature"]) }}
   {% endset %}
 
   {% set geo_area %}
-    {{ create_sortable_anchor(request, "geo_area", "Geographical area", base_url) }}
+    {{ create_sortable_anchor(request, "Geographical area", sorting_urls["geo_area"]) }}
   {% endset %}
 
   {% set start_date %}
-    {{ create_sortable_anchor(request, "start_date", "Start date", base_url) }}
+    {{ create_sortable_anchor(request, "Start date", sorting_urls["start_date"]) }}
   {% endset %}
 
   {% if paginator.count > 0 %}

regulations/views.py

@@ -79,6 +79,7 @@ class RegulationDetailMeasures(SortingMixin, WithPaginationListMixin, ListView):
     paginate_by = 20
     sort_by_fields = ["goods_nomenclature", "geo_area", "start_date"]
     custom_sorting = {
+        "goods_nomenclature": "goods_nomenclature__item_id",
         "geo_area": "geographical_area__area_id",
         "start_date": "valid_between",
     }