TP2000-959 Add sorting to the MeasuresList filter results table (#1034)

* Add sorting to results table * Add sorting for end dates * Fix existing tests * Add tests for Sort by * Change query param arg to bool * Remove the string check for ordering in get queryset * Rename anchor to fragment * Rename sort_by to create-sortable_anchor * Fix broken tests
uktrade · Oct 3, 2023 · 949f547 · 949f547
1 parent 1695287
commit 949f547
Show file tree

Hide file tree

Showing 20 changed files with 231 additions and 75 deletions.
diff --git a/additional_codes/jinja2/includes/additional_codes/tabs/measures.jinja b/additional_codes/jinja2/includes/additional_codes/tabs/measures.jinja
@@ -1,4 +1,4 @@
-{% from "components/sort_by.jinja" import sort_by %}
+{% from "components/create_sortable_anchor.jinja" import create_sortable_anchor %}
 {% from "components/table/macro.njk" import govukTable %}
 
 {% set table_rows = [] %}
@@ -37,4 +37,4 @@
     {% endif %}
     </div>
     {% include "includes/common/actions.jinja"%}
-</div>
+</div>
diff --git a/commodities/jinja2/includes/commodities/tabs/measures-declarable.jinja b/commodities/jinja2/includes/commodities/tabs/measures-declarable.jinja
@@ -1,5 +1,5 @@
 {% extends "commodities/detail.jinja" %}
-{% from "components/sort_by.jinja" import sort_by %}
+{% from "components/create_sortable_anchor.jinja" import create_sortable_anchor %}
 
 {% set extra_tabs_links = [
     {
@@ -65,19 +65,19 @@
 {% set base_url = url('commodity-ui-detail-measures-declarable', args=[commodity.sid]) %}
 
 {% set comm_code %}
-  {{ sort_by(request, "commodity", "Commodity code", base_url) }}
+  {{ create_sortable_anchor(request, "commodity", "Commodity code", base_url) }}
 {% endset %}
 
 {% set measure_type %}
-  {{ sort_by(request, "measure_type", "Measure type", base_url) }}
+  {{ create_sortable_anchor(request, "measure_type", "Measure type", base_url) }}
 {% endset %}
 
 {% set geo_area %}
-  {{ sort_by(request, "geo_area", "Geographical area", base_url) }}
+  {{ create_sortable_anchor(request, "geo_area", "Geographical area", base_url) }}
 {% endset %}
 
 {% set start_date %}
-  {{ sort_by(request, "start_date", "Start date", base_url) }}
+  {{ create_sortable_anchor(request, "start_date", "Start date", base_url) }}
 {% endset %}
 
 {% if paginator.count > 0 %}
@@ -100,4 +100,4 @@
 {% endif %}
 {% include "includes/common/pagination.jinja" %}
 
-{% endblock %}
+{% endblock %}
diff --git a/commodities/jinja2/includes/commodities/tabs/measures-defined.jinja b/commodities/jinja2/includes/commodities/tabs/measures-defined.jinja
@@ -1,5 +1,5 @@
 {% extends "commodities/detail.jinja" %}
-{% from "components/sort_by.jinja" import sort_by %}
+{% from "components/create_sortable_anchor.jinja" import create_sortable_anchor %}
 
 {% set extra_tabs_links = [
     {
@@ -59,15 +59,15 @@
 {% set base_url = url('commodity-ui-detail-measures-as-defined', args=[commodity.sid]) %}
 
 {% set measure_type %}
-  {{ sort_by(request, "measure_type", "Measure type", base_url) }}
+  {{ create_sortable_anchor(request, "measure_type", "Measure type", base_url) }}
 {% endset %}
 
 {% set geo_area %}
-  {{ sort_by(request, "geo_area", "Geographical area", base_url) }}
+  {{ create_sortable_anchor(request, "geo_area", "Geographical area", base_url) }}
 {% endset %}
 
 {% set start_date %}
-  {{ sort_by(request, "start_date", "Start date", base_url) }}
+  {{ create_sortable_anchor(request, "start_date", "Start date", base_url) }}
 {% endset %}
 
 {% if paginator.count > 0 %}
@@ -89,4 +89,4 @@
   <p class="govuk-body">There are no measures for this commodity code.</p>
 {% endif %}
 {% include "includes/common/pagination.jinja" %}
-{% endblock %}
+{% endblock %}
diff --git a/commodities/jinja2/includes/commodities/tabs/measures-vat-excise.jinja b/commodities/jinja2/includes/commodities/tabs/measures-vat-excise.jinja
@@ -1,5 +1,5 @@
 {% extends "commodities/detail.jinja" %}
-{% from "components/sort_by.jinja" import sort_by %}
+{% from "components/create_sortable_anchor.jinja" import create_sortable_anchor %}
 
 {% set extra_tabs_links = [
     {
@@ -81,4 +81,4 @@
   <p class="govuk-body">There are no VAT or excise measures for this commodity code.</p>
 {% endif %}
 
-{% endblock %}
+{% endblock %}
diff --git a/commodities/tests/test_views.py b/commodities/tests/test_views.py
@@ -245,7 +245,7 @@ def test_commodity_measures_sorting_geo_area(
         url_name,
         kwargs={"sid": commodity.sid},
     )
-    response = valid_user_client.get(f"{url}?sort_by=geo_area&order=desc")
+    response = valid_user_client.get(f"{url}?sort_by=geo_area&ordered=desc")
     assert response.status_code == 200
 
     soup = BeautifulSoup(response.content.decode(response.charset), "html.parser")
@@ -259,7 +259,7 @@ def test_commodity_measures_sorting_geo_area(
         "commodity-ui-detail-measures-as-defined",
         kwargs={"sid": commodity.sid},
     )
-    response = valid_user_client.get(f"{url}?sort_by=geo_area&order=asc")
+    response = valid_user_client.get(f"{url}?sort_by=geo_area&ordered=asc")
     assert response.status_code == 200
 
     soup = BeautifulSoup(response.content.decode(response.charset), "html.parser")
@@ -298,7 +298,7 @@ def test_commodity_measures_sorting_start_date(
         url_name,
         kwargs={"sid": commodity.sid},
     )
-    response = valid_user_client.get(f"{url}?sort_by=start_date&order=desc")
+    response = valid_user_client.get(f"{url}?sort_by=start_date&ordered=desc")
     assert response.status_code == 200
 
     soup = BeautifulSoup(response.content.decode(response.charset), "html.parser")
@@ -311,7 +311,7 @@ def test_commodity_measures_sorting_start_date(
         "commodity-ui-detail-measures-as-defined",
         kwargs={"sid": commodity.sid},
     )
-    response = valid_user_client.get(f"{url}?sort_by=start_date&order=asc")
+    response = valid_user_client.get(f"{url}?sort_by=start_date&ordered=asc")
     assert response.status_code == 200
 
     soup = BeautifulSoup(response.content.decode(response.charset), "html.parser")
@@ -353,7 +353,7 @@ def test_commodity_measures_sorting_measure_type(
         url_name,
         kwargs={"sid": commodity.sid},
     )
-    response = valid_user_client.get(f"{url}?sort_by=measure_type&order=desc")
+    response = valid_user_client.get(f"{url}?sort_by=measure_type&ordered=desc")
     assert response.status_code == 200
 
     soup = BeautifulSoup(response.content.decode(response.charset), "html.parser")
@@ -366,7 +366,7 @@ def test_commodity_measures_sorting_measure_type(
         "commodity-ui-detail-measures-as-defined",
         kwargs={"sid": commodity.sid},
     )
-    response = valid_user_client.get(f"{url}?sort_by=measure_type&order=asc")
+    response = valid_user_client.get(f"{url}?sort_by=measure_type&ordered=asc")
     assert response.status_code == 200
 
     soup = BeautifulSoup(response.content.decode(response.charset), "html.parser")

diff --git a/common/jinja2/components/create_sortable_anchor.jinja b/common/jinja2/components/create_sortable_anchor.jinja
@@ -0,0 +1,21 @@
+
+{% 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>
+  {% 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 }}">
+      {{ title }} <img src="{{ static('common/images/sort_icon.svg') }}" alt="">
+    </a>
+  {% endif %}
+
+{% endmacro %}
diff --git a/common/jinja2/components/sort_by.jinja b/common/jinja2/components/sort_by.jinja
diff --git a/common/views.py b/common/views.py
@@ -435,7 +435,7 @@ def get_queryset(self):
 
     def get_ordering(self):
         sort_by = self.request.GET.get("sort_by")
-        order = self.request.GET.get("order")
+        ordered = self.request.GET.get("ordered")
         assert hasattr(
             self,
             "sort_by_fields",
@@ -446,7 +446,7 @@ def get_ordering(self):
             if hasattr(self, "custom_sorting") and self.custom_sorting.get(sort_by):
                 sort_by = self.custom_sorting.get(sort_by)
 
-            if order == "desc":
+            if ordered == "desc":
                 sort_by = f"-{sort_by}"
 
             return sort_by

diff --git a/geo_areas/jinja2/includes/geo_areas/tabs/measures.jinja b/geo_areas/jinja2/includes/geo_areas/tabs/measures.jinja
@@ -1,6 +1,6 @@
 {% extends "geo_areas/detail.jinja" %}
 
-{% from "components/sort_by.jinja" import sort_by %}
+{% from "components/create_sortable_anchor.jinja" import create_sortable_anchor %}
 {% from "components/table/macro.njk" import govukTable %}
 
 {% set actions_menu %}
@@ -43,11 +43,11 @@
   {% set base_url = url('geo_area-ui-detail-measures', kwargs={"sid": object.sid}) %}
 
   {% set commodity_code %}
-    {{ sort_by(request, "goods_nomenclature", "Commodity code", base_url) }}
+    {{ create_sortable_anchor(request, "goods_nomenclature", "Commodity code", base_url) }}
   {% endset %}
 
   {% set start_date %}
-    {{ sort_by(request, "start_date", "Start date", base_url) }}
+    {{ create_sortable_anchor(request, "start_date", "Start date", base_url) }}
   {% endset %}
 
   {% if paginator.count > 0 %}
@@ -68,4 +68,4 @@
     <p class="govuk-body">There are no measures for this regulation.</p>
   {% endif %}
   {% include "includes/common/pagination.jinja" %}
-{% endblock %}
+{% endblock %}
diff --git a/geo_areas/tests/test_views.py b/geo_areas/tests/test_views.py
@@ -585,7 +585,7 @@ def test_geo_area_detail_measures_view_sorting_commodity(valid_user_client):
             "sid": geo_area.sid,
         },
     )
-    response = valid_user_client.get(f"{url}?sort_by=goods_nomenclature&order=asc")
+    response = valid_user_client.get(f"{url}?sort_by=goods_nomenclature&ordered=asc")
     page = BeautifulSoup(
         response.content.decode(response.charset),
         "html.parser",
@@ -596,7 +596,7 @@ def test_geo_area_detail_measures_view_sorting_commodity(valid_user_client):
     ]
     assert table_commodity_codes == commodity_codes
 
-    response = valid_user_client.get(f"{url}?sort_by=goods_nomenclature&order=desc")
+    response = valid_user_client.get(f"{url}?sort_by=goods_nomenclature&ordered=desc")
     page = BeautifulSoup(
         response.content.decode(response.charset),
         "html.parser",
@@ -635,7 +635,7 @@ def test_geo_area_detail_measures_view_sorting_start_date(
             "sid": geo_area.sid,
         },
     )
-    response = valid_user_client.get(f"{url}?sort_by=start_date&order=asc")
+    response = valid_user_client.get(f"{url}?sort_by=start_date&ordered=asc")
     page = BeautifulSoup(
         response.content.decode(response.charset),
         "html.parser",
@@ -645,7 +645,7 @@ def test_geo_area_detail_measures_view_sorting_start_date(
     ]
     assert table_measure_sids == [measure1.sid, measure2.sid, measure3.sid]
 
-    response = valid_user_client.get(f"{url}?sort_by=start_date&order=desc")
+    response = valid_user_client.get(f"{url}?sort_by=start_date&ordered=desc")
     page = BeautifulSoup(
         response.content.decode(response.charset),
         "html.parser",

diff --git a/measures/jinja2/includes/measures/list.jinja b/measures/jinja2/includes/measures/list.jinja
@@ -2,8 +2,10 @@
 {% from 'macros/footnotes_display.jinja' import footnotes_display %}
 {% from "includes/measures/conditions.jinja" import conditions_list %}
 {% from "components/button/macro.njk" import govukButton %}
+{% from "components/create_sortable_anchor.jinja" import create_sortable_anchor %}
 {% from "components/warning-text/macro.njk" import govukWarningText %}
 
+
 {# Sets out checkbox #}
 {% macro checkbox_item(field) -%}
   {% set id = field.id_for_label %}
@@ -26,6 +28,26 @@
 <div id="check-all-checkbox"></div>
 {%- endset %}
 
+{% set sid %}
+  {{ create_sortable_anchor(request, "sid", "ID", base_url, query_params) }}
+{% endset %}
+
+{% set measure_type %}
+  {{ create_sortable_anchor(request, "measure_type", "Type", base_url, query_params) }}
+{% endset %}
+
+{% set geo_area %}
+  {{ create_sortable_anchor(request, "geo_area", "Geographical area", base_url, query_params) }}
+{% endset %}
+
+{% set start_date %}
+  {{ create_sortable_anchor(request, "start_date", "Start date", base_url, query_params) }}
+{% endset %}
+
+{% set end_date %}
+  {{ create_sortable_anchor(request, "end_date", "End date", base_url, query_params) }}
+{% endset %}
+
 {# sets out form #}
 <form method="post">
   <input type="hidden" name="csrfmiddlewaretoken" value="{{ csrf_token }}">
@@ -67,14 +89,14 @@
         {{ govukTable({
           "head": [
             {"html": checkbox_check_all},
-            {"text": "ID"},
-            {"text": "Type"},
-            {"text": "Commodity code"},
-            {"text": "Start date"},
-            {"text": "End date"},
+            {"text": sid},
+            {"text": measure_type},
+            {"text": "Commodity"},
+            {"text": start_date},
+            {"text": end_date},
             {"text": "Duties"},
             {"text": "Additional code"},
-            {"text": "Geographical area"},
+            {"text": geo_area},
             {"text": "Quota"},
             {"text": "Footnote"},
             {"text": "Conditions"},