From 59912f6ddb6b28cd093a554fed4724ebc8fc80f9 Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Fri, 12 Jan 2024 15:16:31 +0000 Subject: [PATCH] Use short "Go" view for search shortcut --- website/common/utils.py | 11 ++++++ website/search/models.py | 9 +++-- website/search/serializers.py | 6 ++- website/search/tests.py | 73 +++++++++++++++++++++++++++++++++-- website/search/urls.py | 1 + website/search/views.py | 45 ++++++++++++++------- 6 files changed, 124 insertions(+), 21 deletions(-) diff --git a/website/common/utils.py b/website/common/utils.py index 484de8e..51c8ded 100644 --- a/website/common/utils.py +++ b/website/common/utils.py @@ -5,6 +5,7 @@ from typing import Iterable, Optional, Type import requests from bs4 import BeautifulSoup, SoupStrainer from django.conf import settings +from django.db import models from django.http.request import HttpRequest from django.utils.text import re_words, slugify from django_cache_decorator import django_cache_decorator @@ -118,3 +119,13 @@ def get_url_mime_type(url: str) -> Optional[str]: return requests_session.head(url).headers.get("Content-Type") except requests.exceptions.RequestException: return None + + +def get_or_none(queryset: models.QuerySet) -> models.Model: + """ + Helper method to get a single instance, or None if there is not exactly 1 matches + """ + try: + return queryset.get() + except (queryset.model.DoesNotExist, queryset.model.MultipleObjectsReturned): + return None diff --git a/website/search/models.py b/website/search/models.py index c140d6b..df40244 100644 --- a/website/search/models.py +++ b/website/search/models.py @@ -13,7 +13,7 @@ from wagtail.search.utils import parse_query_string from website.common.models import BaseContentPage, BaseListingPage from website.common.utils import get_page_models -from .serializers import MIN_SEARCH_LENGTH, SearchParamsSerializer +from .serializers import MIN_SEARCH_LENGTH, SearchPageParamsSerializer class SearchPage(RoutablePageMixin, BaseContentPage): @@ -44,11 +44,12 @@ class SearchPage(RoutablePageMixin, BaseContentPage): context["SEO_INDEX"] = False return context - def get_listing_pages(self) -> models.QuerySet: + @classmethod + def get_listing_pages(cls) -> models.QuerySet: return ( Page.objects.live() .public() - .not_type(self.__class__, *self.EXCLUDED_PAGE_TYPES) + .not_type(cls.__class__, *cls.EXCLUDED_PAGE_TYPES) ) @route(r"^results/$") @@ -57,7 +58,7 @@ class SearchPage(RoutablePageMixin, BaseContentPage): if not request.htmx: return HttpResponseBadRequest() - serializer = SearchParamsSerializer(data=request.GET) + serializer = SearchPageParamsSerializer(data=request.GET) if not serializer.is_valid(): return TemplateResponse( diff --git a/website/search/serializers.py b/website/search/serializers.py index 4935125..1dea73c 100644 --- a/website/search/serializers.py +++ b/website/search/serializers.py @@ -5,5 +5,9 @@ from website.common.serializers import PaginationSerializer MIN_SEARCH_LENGTH = 3 -class SearchParamsSerializer(PaginationSerializer): +class SearchParamSerializer(serializers.Serializer): q = serializers.CharField(min_length=MIN_SEARCH_LENGTH) + + +class SearchPageParamsSerializer(SearchParamSerializer, PaginationSerializer): + pass diff --git a/website/search/tests.py b/website/search/tests.py index c90d00c..f1ec2e4 100644 --- a/website/search/tests.py +++ b/website/search/tests.py @@ -140,18 +140,85 @@ class OpenSearchTestCase(TestCase): ContentPageFactory(parent=cls.home_page, title=f"Post {i}") def test_opensearch_description(self) -> None: - with self.assertNumQueries(8): + with self.assertNumQueries(6): response = self.client.get(reverse("opensearch")) self.assertEqual(response.status_code, 200) - self.assertContains(response, self.page.get_url()) + self.assertContains(response, reverse("go")) self.assertContains(response, reverse("opensearch-suggestions")) def test_opensearch_suggestions(self) -> None: - with self.assertNumQueries(4): + with self.assertNumQueries(3): response = self.client.get(reverse("opensearch-suggestions"), {"q": "post"}) self.assertEqual(response.status_code, 200) data = response.json() self.assertEqual(data[0], "post") self.assertEqual(data[1], [f"Post {i}" for i in range(5)]) + + +class GoViewTestCase(TestCase): + @classmethod + def setUpTestData(cls) -> None: + cls.home_page = HomePage.objects.get() + cls.search_page = SearchPageFactory(parent=cls.home_page) + + cls.post_1 = ContentPageFactory( + parent=cls.home_page, title="Post Title 1", slug="post-slug-1" + ) + cls.post_2 = ContentPageFactory( + parent=cls.home_page, title="Post Title 2", slug="post-slug-2" + ) + + def test_by_title(self) -> None: + with self.assertNumQueries(5): + response = self.client.get(reverse("go"), {"q": self.post_1.title}) + + self.assertRedirects( + response, self.post_1.get_url(), fetch_redirect_response=True + ) + + def test_by_slug(self) -> None: + with self.assertNumQueries(7): + response = self.client.get(reverse("go"), {"q": self.post_2.slug}) + + self.assertRedirects( + response, self.post_2.get_url(), fetch_redirect_response=True + ) + + def test_no_match(self) -> None: + with self.assertNumQueries(6): + response = self.client.get(reverse("go"), {"q": "Something else"}) + + self.assertRedirects( + response, + self.search_page.get_url() + "?q=Something+else", + fetch_redirect_response=True, + ) + + def test_no_query(self) -> None: + with self.assertNumQueries(3): + response = self.client.get(reverse("go")) + + self.assertRedirects( + response, self.search_page.get_url(), fetch_redirect_response=True + ) + + def test_multiple_matches(self) -> None: + ContentPageFactory(parent=self.home_page, title=self.post_1.title) + + with self.assertNumQueries(6): + response = self.client.get(reverse("go"), {"q": self.post_1.title}) + + self.assertRedirects( + response, + self.search_page.get_url() + f"?q={self.post_1.title}", + fetch_redirect_response=True, + ) + + def test_no_search_page(self) -> None: + self.search_page.delete() + + response = self.client.get(reverse("go")) + + self.assertEqual(response.status_code, 404) diff --git a/website/search/urls.py b/website/search/urls.py index 9b725c5..33eaf60 100644 --- a/website/search/urls.py +++ b/website/search/urls.py @@ -9,4 +9,5 @@ urlpatterns = [ views.OpenSearchSuggestionsView.as_view(), name="opensearch-suggestions", ), + path("go/", views.GoView.as_view(), name="go"), ] diff --git a/website/search/views.py b/website/search/views.py index 5631b5b..1cfa1da 100644 --- a/website/search/views.py +++ b/website/search/views.py @@ -1,18 +1,17 @@ -from django.http import HttpRequest, JsonResponse -from django.shortcuts import get_object_or_404 +from django.http import Http404, HttpRequest, JsonResponse from django.urls import reverse from django.utils.decorators import method_decorator -from django.views.decorators.cache import cache_control -from django.views.generic import TemplateView, View +from django.views.decorators.cache import cache_control, cache_page +from django.views.generic import RedirectView, TemplateView, View from wagtail.search.utils import parse_query_string from wagtail_favicon.models import FaviconSettings from wagtail_favicon.utils import get_rendition_url -from website.common.utils import get_site_title +from website.common.utils import get_or_none, get_site_title from website.contrib.singleton_page.utils import SingletonPageCache from .models import SearchPage -from .serializers import SearchParamsSerializer +from .serializers import SearchParamSerializer @method_decorator(cache_control(max_age=60 * 60), name="dispatch") @@ -32,9 +31,7 @@ class OpenSearchView(TemplateView): ) ) - context["search_page_url"] = self.request.build_absolute_uri( - SingletonPageCache.get_url(SearchPage, self.request) - ) + context["search_page_url"] = self.request.build_absolute_uri(reverse("go")) context["search_suggestions_url"] = self.request.build_absolute_uri( reverse("opensearch-suggestions") ) @@ -47,17 +44,15 @@ class OpenSearchView(TemplateView): @method_decorator(cache_control(max_age=60 * 60), name="dispatch") class OpenSearchSuggestionsView(View): def get(self, request: HttpRequest) -> JsonResponse: - serializer = SearchParamsSerializer(data=request.GET) + serializer = SearchParamSerializer(data=request.GET) if not serializer.is_valid(): return JsonResponse(serializer.errors, status=400) - search_page = get_object_or_404(SearchPage) - filters, query = parse_query_string(serializer.validated_data["q"]) results = ( - search_page.get_listing_pages() + SearchPage.get_listing_pages() .search(query, order_by_relevance=True)[:5] .get_queryset() ) @@ -69,3 +64,27 @@ class OpenSearchSuggestionsView(View): ], safe=False, ) + + +@method_decorator(cache_page(60 * 60), name="dispatch") +class GoView(RedirectView): + def get_redirect_url(self) -> str: + serializer = SearchParamSerializer(data=self.request.GET) + search_page_url = SingletonPageCache.get_url(SearchPage, self.request) + + if search_page_url is None: + raise Http404 + + if not serializer.is_valid(): + return search_page_url + + query = serializer.validated_data["q"] + pages = SearchPage.get_listing_pages() + + if title_match := get_or_none(pages.filter(title__iexact=query)): + return title_match.get_url(request=self.request) + + if slug_match := get_or_none(pages.filter(slug__iexact=query)): + return slug_match.get_url(request=self.request) + + return f"{search_page_url}?{self.request.GET.urlencode()}"