Skip to content

Commit 313f301

Browse files
jonbulzjonbulz
committed
server side sorting
fix type annotations update translations styling in list view table header to prevent line break change search_input query from POST to GET switch search in POIs to GET sort by only a single column at a time restore translations revert changes to translations update translations remove default ordering from abstract base model fix unordered list object warning when no sort parameter is supplied add ADR for server-side sorting add release note for contact sorting
1 parent 6a042d5 commit 313f301

File tree

10 files changed

+188
-110
lines changed

10 files changed

+188
-110
lines changed

‎ADR/0002-server-side-sorting-for-list-views.md‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
# 2. Allow sorting in list views and handle sorting and pagination server-side
2+
3+
Date: 2026-01-28
4+
5+
## Status
6+
7+
Accepted
8+
9+
## Context
10+
11+
List views (for models such as Contacts, Events, Locations, ...) thus far did not
12+
support sorting by parameters other than the default.
13+
To develop a sort feature, a decision needed to be made whether the sorting should
14+
be implemented server-side or client-side.
15+
## Decision
16+
17+
Sorting will be handled server-side. This will allow an easy integration with Django's
18+
built in pagination, which minimizes maintenance and avoids longer loading times.
19+
20+
## Consequences
21+
22+
As the sorting is handled by the server, a new request is sent every time a user
23+
changes the sorting. However, this was deemed acceptable, since loading times
24+
for list views should be fast enough to not negatively affect user experience.
25+
Client-side sorting and pagination would require to send a complete list of records,
26+
which is less scalable and could drastically affect performance for large datasets.

‎integreat_cms/cms/templates/_sortable_table_header.html‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
{% load sort_tags %}
2+
<tr>
3+
<th class="py-3 pl-4 min">
4+
<input form="bulk-action-form" type="checkbox" id="bulk-select-all" />
5+
</th>
6+
{% for field, label in table_fields %}
7+
<th class="whitespace-nowrap text-left py-3 px-2">
8+
{% if field %}
9+
{# SORTABLE #}
10+
{% sort_link label field %}
11+
{% else %}
12+
{# NON-SORTABLE #}
13+
{{ label }}
14+
{% endif %}
15+
</th>
16+
{% endfor %}
17+
</tr>

‎integreat_cms/cms/templates/contacts/contact_list.html‎

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
{% extends "_base.html" %}
22
{% load i18n %}
33
{% load static %}
4+
{% load sort_tags %}
45
{% block content %}
56
{% get_current_language as LANGUAGE_CODE %}
67
<div class="table-header">
@@ -54,33 +55,7 @@ <h1 class="heading">
5455
<table class="w-full mt-4 rounded border border-solid border-gray-200 shadow bg-white">
5556
<thead>
5657
<tr class="border-b border-solid border-gray-200">
57-
<th class="py-3 pl-4 min">
58-
<input form="bulk-action-form" type="checkbox" id="bulk-select-all" />
59-
</th>
60-
<th class="text-sm text-left uppercase py-3 px-2">
61-
{% translate "Name of related location" %}
62-
</th>
63-
<th class="text-sm text-left uppercase py-3 px-2">
64-
{% translate "Area of responsibility" %}
65-
</th>
66-
<th class="text-sm text-left uppercase py-3 px-2">
67-
{% translate "Name" %}
68-
</th>
69-
<th class="text-sm text-left uppercase py-3 px-2">
70-
{% translate "E-Mail" %}
71-
</th>
72-
<th class="text-sm text-left uppercase py-3 px-2">
73-
{% translate "Phone number" %}
74-
</th>
75-
<th class="text-sm text-left uppercase py-3 px-2">
76-
{% translate "Mobile phone number" %}
77-
</th>
78-
<th class="text-sm text-left uppercase py-3 px-2">
79-
{% translate "Website" %}
80-
</th>
81-
<th class="text-sm text-left uppercase py-3 pr-4 min">
82-
{% translate "Options" %}
83-
</th>
58+
{% render_table_header view.table_fields %}
8459
</tr>
8560
</thead>
8661
<tbody>

‎integreat_cms/cms/templates/search_input_form.html‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
{% load static %}
2-
<form method="post" class="table-search relative w-auto">
3-
{% csrf_token %}
2+
<form method="get" class="table-search relative w-auto">
43
{% include "_search_input.html" %}
54
</form>

‎integreat_cms/cms/templatetags/sort_tags.py‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
from django import template
2+
from django.template.context import RequestContext
3+
from django.utils.http import urlencode
4+
from django.utils.safestring import mark_safe
5+
6+
register = template.Library()
7+
8+
9+
@register.simple_tag(takes_context=True)
10+
def sort_link(context: RequestContext, label: str, field: str) -> str:
11+
"""
12+
Sets the correct href for sortable table headers
13+
14+
Usage in template:
15+
{% sort_link "Name" "name" %}
16+
"""
17+
18+
request = context["request"]
19+
params = request.GET.copy()
20+
current = params.get("sort")
21+
22+
if field == current:
23+
params["sort"] = f"-{field}"
24+
arrow = " ▼"
25+
elif f"-{field}" == current:
26+
params.pop("sort", None)
27+
arrow = " ▲"
28+
else:
29+
params["sort"] = field
30+
arrow = ""
31+
32+
url = f"?{urlencode(params, doseq=True)}"
33+
34+
return mark_safe(f'<a href="{url}" class="hover:underline">{label}{arrow}</a>')
35+
36+
37+
@register.inclusion_tag("_sortable_table_header.html", takes_context=True)
38+
def render_table_header(context: RequestContext, table_fields: dict[str, str]) -> dict:
39+
return {
40+
"request": context["request"],
41+
"table_fields": table_fields,
42+
}

‎integreat_cms/cms/views/contacts/contact_list_view.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,16 @@ class ContactListView(
3131
template_name = "contacts/contact_list.html"
3232
archived = False
3333
filter_form_class = ContactSearchForm
34+
table_fields = [
35+
(None, _("Name of related location")),
36+
("area_of_responsibility", _("Area of responsibility")),
37+
("name", _("Name")),
38+
("email", _("E-Mail")),
39+
("phone_number", _("Phone number")),
40+
("mobile_phone_number", _("Mobile phone number")),
41+
("website", _("Website")),
42+
(None, _("Options")),
43+
]
3444

3545
def get(self, request: HttpRequest, *args: Any, **kwargs: Any) -> HttpResponse:
3646
r"""

‎integreat_cms/cms/views/mixins.py‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -191,20 +191,32 @@ class FilterSortMixin:
191191
Filtering logic is handled by the SearchForm. To add filtering to a view,
192192
set a ``filter_form_class` attribute (the ``filter_form_class`` should be a child of
193193
:class:`~integreat_cms.cms.forms.object_search_form.ObjectSearchForm`).
194-
To allow sorting, add a ``sort_fields`` list attribute to your view.
194+
To allow sorting, extend the ``table_fields`` list attribute in your view.
195195
Note that this mixin is intended for extending Django's View class (or child classes),
196196
and expects a ``self.request`` attribute. Django's generic View defines the request attribute
197197
in the dispatch phase.
198198
"""
199199

200200
request: Any
201201
filter_form_class: type[ObjectSearchForm] | None = None
202-
sort_fields: list[str] = []
202+
table_fields: list[tuple[str | None, str]] = []
203+
204+
@property
205+
def sort_fields(self) -> list[str]:
206+
"""
207+
Extract only sortable field names from table_fields.
208+
A field is sortable if its first element is not None.
209+
"""
210+
return [
211+
field
212+
for field, label in getattr(self, "table_fields", [])
213+
if field is not None
214+
]
203215

204216
def get_filter_form(self) -> ObjectSearchForm | None:
205217
if self.filter_form_class is None:
206218
return None
207-
return self.filter_form_class(self.request.POST or None)
219+
return self.filter_form_class(self.request.GET or None)
208220

209221
def get_filtered_sorted_queryset(self, queryset: QuerySet) -> QuerySet:
210222
form = self.get_filter_form()
@@ -213,7 +225,11 @@ def get_filtered_sorted_queryset(self, queryset: QuerySet) -> QuerySet:
213225

214226
order_by = [
215227
f
216-
for f in self.request.POST.get("sort", "").split(",")
228+
for f in self.request.GET.getlist("sort")
217229
if f.lstrip("-") in self.sort_fields
218230
]
219-
return queryset.order_by(*order_by)
231+
if order_by:
232+
# queryset.order_by([]) would override default ordering and result in an unordered queryset
233+
# so we only use order_by if "sort" is not empty
234+
return queryset.order_by(*order_by)
235+
return queryset

‎integreat_cms/cms/views/pois/poi_list_view.py‎

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@
33
import logging
44
from typing import TYPE_CHECKING
55

6-
from django.conf import settings
76
from django.contrib import messages
8-
from django.core.paginator import Paginator
97
from django.shortcuts import redirect, render
108
from django.utils.decorators import method_decorator
119
from django.utils.translation import gettext_lazy as _
@@ -14,7 +12,7 @@
1412
from ...decorators import permission_required
1513
from ...forms import ObjectSearchForm
1614
from ...models import POITranslation
17-
from ..mixins import MachineTranslationContextMixin
15+
from ..mixins import FilterSortMixin, MachineTranslationContextMixin, PaginationMixin
1816
from .poi_context_mixin import POIContextMixin
1917

2018
if TYPE_CHECKING:
@@ -26,7 +24,13 @@
2624

2725

2826
@method_decorator(permission_required("cms.view_poi"), name="dispatch")
29-
class POIListView(TemplateView, POIContextMixin, MachineTranslationContextMixin):
27+
class POIListView(
28+
TemplateView,
29+
POIContextMixin,
30+
MachineTranslationContextMixin,
31+
FilterSortMixin,
32+
PaginationMixin,
33+
):
3034
"""
3135
View for listing POIs (points of interests)
3236
"""
@@ -37,6 +41,7 @@ class POIListView(TemplateView, POIContextMixin, MachineTranslationContextMixin)
3741
archived = False
3842
#: The translation model of this list view (used to determine whether machine translations are permitted)
3943
translation_model = POITranslation
44+
filter_form_class = ObjectSearchForm
4045

4146
def get(self, request: HttpRequest, *args: Any, **kwargs: Any) -> HttpResponse:
4247
r"""
@@ -85,23 +90,20 @@ def get(self, request: HttpRequest, *args: Any, **kwargs: Any) -> HttpResponse:
8590
pois = region.pois.filter(archived=self.archived)
8691
query = None
8792

88-
search_data = kwargs.get("search_data")
89-
search_form = ObjectSearchForm(search_data)
93+
pois = self.get_filtered_sorted_queryset(pois)
94+
search_form = self.filter_form_class(request.GET)
9095
if search_form.is_valid():
91-
query = search_form.cleaned_data["query"]
92-
poi_keys = POITranslation.search(region, language_slug, query).values(
93-
"poi__pk",
94-
)
95-
pois = pois.filter(pk__in=poi_keys)
96-
97-
chunk_size = int(request.GET.get("size", settings.PER_PAGE))
98-
# for consistent pagination querysets should be ordered
99-
paginator = Paginator(
100-
pois.prefetch_translations().order_by("region__slug"),
101-
chunk_size,
102-
)
103-
chunk = request.GET.get("page")
104-
poi_chunk = paginator.get_page(chunk)
96+
# we have to include additional search results here
97+
# because we search the POITranslation model
98+
# this currently can't be handled by the FilterSortMixin
99+
query = search_form.cleaned_data.get("query")
100+
poi_keys = self.translation_model.search(
101+
region, language_slug, query
102+
).values("poi__pk")
103+
qs = region.pois.filter(pk__in=poi_keys)
104+
pois = pois.union(qs)
105+
106+
poi_chunk = self.paginate_queryset(pois)
105107

106108
return render(
107109
request,

0 commit comments

Comments
 (0)