From c4109e42f1ec902935b70a3233809669e0246cd5 Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Fri, 29 Jul 2022 16:50:44 +0100 Subject: [PATCH] Use serializer to validate search params --- website/search/models.py | 58 +++++++++++-------- .../search/templates/search/search_page.html | 10 +++- 2 files changed, 43 insertions(+), 25 deletions(-) diff --git a/website/search/models.py b/website/search/models.py index d36a45c..5255cbc 100644 --- a/website/search/models.py +++ b/website/search/models.py @@ -1,6 +1,7 @@ -from django.core.paginator import EmptyPage, PageNotAnInteger, Paginator +from django.core.paginator import EmptyPage, Paginator from django.http.request import HttpRequest from django.utils.functional import cached_property +from rest_framework import serializers from wagtail.models import Page from wagtail.query import PageQuerySet from wagtail.search.models import Query @@ -19,6 +20,10 @@ class SearchPage(BaseContentMixin, BasePage): # type: ignore[misc] search_fields = BasePage.search_fields + BaseContentMixin.search_fields PAGE_SIZE = 15 + class SearchParamsSerializer(serializers.Serializer): + q = serializers.CharField() + page = serializers.IntegerField(min_value=1, default=1) + @cached_property def reading_time(self) -> int: """ @@ -35,29 +40,36 @@ class SearchPage(BaseContentMixin, BasePage): # type: ignore[misc] def get_context(self, request: HttpRequest) -> dict: context = super().get_context(request) - if query_string := request.GET.get("q", ""): - filters, query = parse_query_string(query_string) - Query.get(query_string).add_hit() + + serializer = self.SearchParamsSerializer(data=request.GET) + + if serializer.is_valid(): + search_query = serializer.validated_data["q"] + filters, query = parse_query_string(search_query) + Query.get(search_query).add_hit() pages = self.get_search_pages().search(query) + + paginator = Paginator(pages, self.PAGE_SIZE) + context["paginator"] = paginator + page_num = serializer.validated_data["page"] + context["page_num"] = page_num + try: + results = paginator.page(page_num) + + # HACK: Search results aren't a queryset, so we can't call `.specific` on it. This forces it to one as efficiently as possible + if not isinstance(results.object_list, PageQuerySet): + results.object_list = Page.objects.filter( + id__in=list( + results.object_list.get_queryset().values_list( + "id", flat=True + ) + ) + ).specific() + except EmptyPage: + results = [] + + context["results"] = results else: - pages = Page.objects.none() - - paginator = Paginator(pages, self.PAGE_SIZE) - page_num = request.GET.get("page", "1") - try: - results = paginator.page(page_num) - - # HACK: Search results aren't a queryset, so we can't call `.specific` on it. This forces it to one as efficiently as possible - if not isinstance(results.object_list, PageQuerySet): - results.object_list = Page.objects.filter( - id__in=list( - results.object_list.get_queryset().values_list("id", flat=True) - ) - ).specific() - except (PageNotAnInteger, EmptyPage): - results = None - context["invalid_page"] = True - - context["results"] = results + context["invalid_search"] = True return context diff --git a/website/search/templates/search/search_page.html b/website/search/templates/search/search_page.html index 9d99d78..44a3fd1 100644 --- a/website/search/templates/search/search_page.html +++ b/website/search/templates/search/search_page.html @@ -11,8 +11,14 @@ {% endif %}
- {% if invalid_page %} -

Invalid page

+ {% if invalid_search %} +

Invalid search

+ {% elif results|length == 0 %} + {% if page_num > paginator.num_pages %} +

There aren't {{ page_num }} page - only {{ paginator.num_pages }}.

+ {% else %} +

No results

+ {% endif %} {% else %} {% for page in results %} {% include "common/listing-item.html" %}