From a6b6814599937798aba36961ac4935660579e578 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Mon, 11 Nov 2024 21:10:46 +0000 Subject: [PATCH 01/13] refactor: doc search via POST (WIP) Changes the search view to use a POST instead of a GET. Refactors cache key computation to use cleaned data. Still todo: * refactor frontpage view to match * refactor menubar search (?) * refactor stats view that uses SearchForm * revive or drop the "backwards compatibility" branch --- ietf/doc/utils.py | 10 +-- ietf/doc/views_search.py | 100 ++++++++++++++------- ietf/templates/doc/search/search_form.html | 5 +- 3 files changed, 75 insertions(+), 40 deletions(-) diff --git a/ietf/doc/utils.py b/ietf/doc/utils.py index 97243a20d6..319c889332 100644 --- a/ietf/doc/utils.py +++ b/ietf/doc/utils.py @@ -3,9 +3,7 @@ import datetime -import hashlib import io -import json import math import os import re @@ -1047,12 +1045,8 @@ def get_replaces_tree(doc): return sorted(history, key=lambda x: x['published']) -def get_search_cache_key(params): - from ietf.doc.views_search import SearchForm - fields = set(SearchForm.base_fields) - set(['sort',]) - kwargs = dict([ (k,v) for (k,v) in list(params.items()) if k in fields ]) - key = "doc:document:search:" + hashlib.sha512(json.dumps(kwargs, sort_keys=True).encode('utf-8')).hexdigest() - return key +def get_search_cache_key(key_fragment): + return f"doc:document:search:{key_fragment}" def build_file_urls(doc: Union[Document, DocHistory]): diff --git a/ietf/doc/views_search.py b/ietf/doc/views_search.py index 4fa3b2560c..559cc81dcb 100644 --- a/ietf/doc/views_search.py +++ b/ietf/doc/views_search.py @@ -37,6 +37,8 @@ import re import datetime import copy +import hashlib +import json import operator from collections import defaultdict @@ -46,7 +48,7 @@ from django.conf import settings from django.core.cache import cache, caches from django.urls import reverse as urlreverse -from django.db.models import Q +from django.db.models import Q, QuerySet from django.http import Http404, HttpResponseBadRequest, HttpResponse, HttpResponseRedirect, QueryDict from django.shortcuts import render from django.utils import timezone @@ -145,6 +147,28 @@ def clean(self): q['irtfstate'] = None return q + def cache_key_fragment(self): + """Hash a bound form to get a value for use in a cache key + + Raises a ValueError if the form is not valid. + """ + def _serialize_value(val): + if isinstance(val, QuerySet): + return [item.pk for item in val] + else: + return getattr(val, "pk", val) # use pk if present, else value + + if not self.is_valid(): + raise ValueError(f"SearchForm invalid: {self.errors}") + contents = { + field_name: _serialize_value(field_value) + for field_name, field_value in self.cleaned_data.items() + if field_name != "sort" and field_value is not None + } + contents_json = json.dumps(contents, sort_keys=True) + return hashlib.sha512(contents_json.encode("utf-8")).hexdigest() + + def retrieve_search_results(form, all_types=False): """Takes a validated SearchForm and return the results.""" @@ -257,42 +281,58 @@ def retrieve_search_results(form, all_types=False): return docs def search(request): - if request.GET: - # backwards compatibility - get_params = request.GET.copy() - if 'activeDrafts' in request.GET: - get_params['activedrafts'] = request.GET['activeDrafts'] - if 'oldDrafts' in request.GET: - get_params['olddrafts'] = request.GET['oldDrafts'] - if 'subState' in request.GET: - get_params['substate'] = request.GET['subState'] - - form = SearchForm(get_params) - if not form.is_valid(): - return HttpResponseBadRequest("form not valid: %s" % form.errors) - - cache_key = get_search_cache_key(get_params) - cached_val = cache.get(cache_key) - if cached_val: - [results, meta] = cached_val - else: - results = retrieve_search_results(form) - results, meta = prepare_document_table(request, results, get_params) - cache.set(cache_key, [results, meta]) # for settings.CACHE_MIDDLEWARE_SECONDS - log(f"Search results computed for {get_params}") - meta['searching'] = True + """Search for a draft""" + if request.method == "POST": + form = SearchForm(data=request.POST) + if form.is_valid(): + cache_key = get_search_cache_key( + form.cache_key_fragment() + ) + cached_val = cache.get(cache_key) + if cached_val: + [results, meta] = cached_val + else: + results = retrieve_search_results(form) + results, meta = prepare_document_table(request, results, form.cleaned_data) + cache.set(cache_key, [results, meta]) # for settings.CACHE_MIDDLEWARE_SECONDS + log(f"Search results computed for {form.cleaned_data}") + meta['searching'] = True else: + if request.GET: + # backwards compatibility + # todo - move this to the form? eliminate it? + get_params = request.GET.copy() + if "activeDrafts" in request.GET: + get_params["activedrafts"] = request.GET["activeDrafts"] + if "oldDrafts" in request.GET: + get_params["olddrafts"] = request.GET["oldDrafts"] + if "subState" in request.GET: + get_params["substate"] = request.GET["subState"] + # todo redirect to the search form = SearchForm() results = [] - meta = { 'by': None, 'searching': False } - get_params = QueryDict('') + meta = { + "by": None, + "searching": False, + } - return render(request, 'doc/search/search.html', { - 'form':form, 'docs':results, 'meta':meta, 'queryargs':get_params.urlencode() }, + return render( + request, + 'doc/search/search.html', + context={ + "form": form, + "docs": results, + "meta": meta + }, ) def frontpage(request): - form = SearchForm() + if request.method == "POST": + form = SearchForm(data=request.POST) + if form.is_valid(): + """do stuff""" + else: + form = SearchForm() return render(request, 'doc/frontpage.html', {'form':form}) def search_for_name(request, name): diff --git a/ietf/templates/doc/search/search_form.html b/ietf/templates/doc/search/search_form.html index d4f463ec66..d4b91b089c 100644 --- a/ietf/templates/doc/search/search_form.html +++ b/ietf/templates/doc/search/search_form.html @@ -4,8 +4,9 @@ {% load widget_tweaks %} {% load ietf_filters %}
+ method="post" + class="form-horizontal"> + {% csrf_token %}
{{ form.name|add_class:"form-control"|attr:"placeholder:Document name/title/RFC number"|attr:"aria-label:Document name/title/RFC number" }} From 19cc4cf15f225e936c6d7a257ce5b44555b26285 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 12 Nov 2024 01:59:22 +0000 Subject: [PATCH 02/13] feat: convert GET search to POST Still todo: * refactor frontpage view to match * refactor menubar search (?) * refactor stats view that uses SearchForm --- ietf/doc/views_search.py | 17 +++++++++++++---- ietf/templates/doc/search/search_form.html | 3 ++- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/ietf/doc/views_search.py b/ietf/doc/views_search.py index 559cc81dcb..4119b09cac 100644 --- a/ietf/doc/views_search.py +++ b/ietf/doc/views_search.py @@ -46,6 +46,7 @@ from django import forms from django.conf import settings +from django.contrib import messages from django.core.cache import cache, caches from django.urls import reverse as urlreverse from django.db.models import Q, QuerySet @@ -299,8 +300,7 @@ def search(request): meta['searching'] = True else: if request.GET: - # backwards compatibility - # todo - move this to the form? eliminate it? + # backwards compatibility - fill in the form get_params = request.GET.copy() if "activeDrafts" in request.GET: get_params["activedrafts"] = request.GET["activeDrafts"] @@ -308,8 +308,17 @@ def search(request): get_params["olddrafts"] = request.GET["oldDrafts"] if "subState" in request.GET: get_params["substate"] = request.GET["subState"] - # todo redirect to the search - form = SearchForm() + form = SearchForm(data=get_params) + messages.error( + request, + ( + 'Searching via the URL query string is deprecated.' + 'The form below has been filled in with your search parameters.' + 'To execute your search, please click "Search".' + ), + ) + else: + form = SearchForm() results = [] meta = { "by": None, diff --git a/ietf/templates/doc/search/search_form.html b/ietf/templates/doc/search/search_form.html index d4b91b089c..6c91894c8c 100644 --- a/ietf/templates/doc/search/search_form.html +++ b/ietf/templates/doc/search/search_form.html @@ -5,7 +5,8 @@ {% load ietf_filters %} + class="form-horizontal" + action="{% url 'ietf.doc.views_search.search' %}"> {% csrf_token %}
From fa48f0de33aa9f44d6719115b3c1ae4776f9d6fd Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 12 Nov 2024 16:19:42 -0400 Subject: [PATCH 03/13] fix: revert frontpage changes, search works Still todo: * refactor stats view that uses SearchForm --- ietf/doc/views_search.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/ietf/doc/views_search.py b/ietf/doc/views_search.py index 4119b09cac..7e50f46169 100644 --- a/ietf/doc/views_search.py +++ b/ietf/doc/views_search.py @@ -336,12 +336,7 @@ def search(request): ) def frontpage(request): - if request.method == "POST": - form = SearchForm(data=request.POST) - if form.is_valid(): - """do stuff""" - else: - form = SearchForm() + form = SearchForm() return render(request, 'doc/frontpage.html', {'form':form}) def search_for_name(request, name): From 1aaec401f153f76a8abdbe6e6ace6a709e01d3f4 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 12 Nov 2024 16:27:52 -0400 Subject: [PATCH 04/13] fix: define vars in all branches --- ietf/doc/views_search.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/ietf/doc/views_search.py b/ietf/doc/views_search.py index 7e50f46169..219e1d1776 100644 --- a/ietf/doc/views_search.py +++ b/ietf/doc/views_search.py @@ -283,6 +283,10 @@ def retrieve_search_results(form, all_types=False): def search(request): """Search for a draft""" + # defaults for results / meta + results = [] + meta = {"by": None, "searching": False} + if request.method == "POST": form = SearchForm(data=request.POST) if form.is_valid(): @@ -319,11 +323,6 @@ def search(request): ) else: form = SearchForm() - results = [] - meta = { - "by": None, - "searching": False, - } return render( request, From 1674f3463a2be0f9d5268eb19a341ae6e92f8f96 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 12 Nov 2024 16:35:13 -0400 Subject: [PATCH 05/13] refactor: update stats use of SearchForm --- ietf/doc/views_stats.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/ietf/doc/views_stats.py b/ietf/doc/views_stats.py index cefc7e152b..8f8867974a 100644 --- a/ietf/doc/views_stats.py +++ b/ietf/doc/views_stats.py @@ -124,14 +124,11 @@ def chart_newrevisiondocevent(request): #@cache_page(60*15) def chart_data_newrevisiondocevent(request): - queryargs = request.GET - if queryargs: - cache_key = get_search_cache_key(queryargs) + form = SearchForm(data=request.GET) + if form.is_valid(): + cache_key = get_search_cache_key(form.cache_key_fragment()) results = cache.get(cache_key) if not results: - form = SearchForm(queryargs) - if not form.is_valid(): - return HttpResponseBadRequest("form not valid: %s" % form.errors) results = retrieve_search_results(form) if results.exists(): cache.set(cache_key, results) @@ -140,7 +137,7 @@ def chart_data_newrevisiondocevent(request): else: data = [] else: - data = [] + return HttpResponseBadRequest("form not valid: %s" % form.errors) return JsonResponse(data, safe=False) From a91bd6def7820949fd3a7a82c741634aefe2b39a Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 13 Nov 2024 15:23:09 -0400 Subject: [PATCH 06/13] chore: improve message --- ietf/doc/views_search.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ietf/doc/views_search.py b/ietf/doc/views_search.py index 219e1d1776..4ae8511479 100644 --- a/ietf/doc/views_search.py +++ b/ietf/doc/views_search.py @@ -316,8 +316,8 @@ def search(request): messages.error( request, ( - 'Searching via the URL query string is deprecated.' - 'The form below has been filled in with your search parameters.' + 'Searching via the URL query string is no longer supported. ' + 'The form below has been filled in with the parameters from your request. ' 'To execute your search, please click "Search".' ), ) From 141cbbb758bf98d2f9cd8f253cbcb17f09df6708 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 13 Nov 2024 15:46:33 -0400 Subject: [PATCH 07/13] fix: remove lint --- ietf/doc/views_search.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ietf/doc/views_search.py b/ietf/doc/views_search.py index 4ae8511479..047ade0257 100644 --- a/ietf/doc/views_search.py +++ b/ietf/doc/views_search.py @@ -49,14 +49,14 @@ from django.contrib import messages from django.core.cache import cache, caches from django.urls import reverse as urlreverse -from django.db.models import Q, QuerySet -from django.http import Http404, HttpResponseBadRequest, HttpResponse, HttpResponseRedirect, QueryDict +from django.db.models import Q +from django.http import Http404, HttpResponseBadRequest, HttpResponse, HttpResponseRedirect from django.shortcuts import render from django.utils import timezone from django.utils.html import strip_tags from django.utils.cache import _generate_cache_key # type: ignore from django.utils.text import slugify - +from django_stubs_ext import QuerySetAny import debug # pyflakes:ignore @@ -154,7 +154,8 @@ def cache_key_fragment(self): Raises a ValueError if the form is not valid. """ def _serialize_value(val): - if isinstance(val, QuerySet): + # Need QuerySetAny instead of QuerySet until django-stubs 5.0.1 + if isinstance(val, QuerySetAny): return [item.pk for item in val] else: return getattr(val, "pk", val) # use pk if present, else value From 7c0b03073ccbe668f697f77f2628c98c24a614c3 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 13 Nov 2024 15:46:58 -0400 Subject: [PATCH 08/13] chore: comments re: QuerySetAny --- ietf/api/serializer.py | 1 + ietf/doc/utils.py | 1 + ietf/liaisons/forms.py | 1 + ietf/liaisons/widgets.py | 1 + 4 files changed, 4 insertions(+) diff --git a/ietf/api/serializer.py b/ietf/api/serializer.py index 27f194c5b5..ca34ea649e 100644 --- a/ietf/api/serializer.py +++ b/ietf/api/serializer.py @@ -146,6 +146,7 @@ def end_object(self, obj): field_value = None else: field_value = field + # Need QuerySetAny instead of QuerySet until django-stubs 5.0.1 if isinstance(field_value, QuerySetAny) or isinstance(field_value, list): self._current[name] = dict([ (rel.pk, self.expand_related(rel, name)) for rel in field_value ]) else: diff --git a/ietf/doc/utils.py b/ietf/doc/utils.py index 319c889332..9b2570d8ba 100644 --- a/ietf/doc/utils.py +++ b/ietf/doc/utils.py @@ -346,6 +346,7 @@ def augment_events_with_revision(doc, events): """Take a set of events for doc and add a .rev attribute with the revision they refer to by checking NewRevisionDocEvents.""" + # Need QuerySetAny instead of QuerySet until django-stubs 5.0.1 if isinstance(events, QuerySetAny): qs = events.filter(newrevisiondocevent__isnull=False) else: diff --git a/ietf/liaisons/forms.py b/ietf/liaisons/forms.py index 1d91041b25..a75028bf79 100644 --- a/ietf/liaisons/forms.py +++ b/ietf/liaisons/forms.py @@ -203,6 +203,7 @@ def get_results(self): class CustomModelMultipleChoiceField(ModelMultipleChoiceField): '''If value is a QuerySet, return it as is (for use in widget.render)''' def prepare_value(self, value): + # Need QuerySetAny instead of QuerySet until django-stubs 5.0.1 if isinstance(value, QuerySetAny): return value if (hasattr(value, '__iter__') and diff --git a/ietf/liaisons/widgets.py b/ietf/liaisons/widgets.py index 74368e83f2..3d4f2d13a5 100644 --- a/ietf/liaisons/widgets.py +++ b/ietf/liaisons/widgets.py @@ -35,6 +35,7 @@ def render(self, name, value, **kwargs): html = '
' % name html += 'No files attached' html += '
' + # Need QuerySetAny instead of QuerySet until django-stubs 5.0.1 if value and isinstance(value, QuerySetAny): for attachment in value: html += '%s ' % (conditional_escape(attachment.document.get_href()), conditional_escape(attachment.document.title)) From 523809a1669acc07444122e69de988cc21554db5 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 13 Nov 2024 16:22:03 -0400 Subject: [PATCH 09/13] test: test query string search params --- ietf/doc/tests.py | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/ietf/doc/tests.py b/ietf/doc/tests.py index e3534ba725..843ac6a05e 100644 --- a/ietf/doc/tests.py +++ b/ietf/doc/tests.py @@ -71,6 +71,38 @@ class SearchTests(TestCase): + def test_search_handles_querystring_parameters(self): + """Search parameters via querystring should not actually search""" + url = urlreverse("ietf.doc.views_search.search") + r = self.client.get(url + "?name=some-document-name&oldDrafts=on") + # Check that we got a valid response and that the warning about query string parameters is shown. + self.assertContains( + r, + "Searching via the URL query string is no longer supported.", + status_code=200, + ) + # Check that the form was filled in correctly (not an exhaustive check, but different from the + # form defaults) + pq = PyQuery(r.content) + self.assertEqual( + pq("form#search_form input#id_name").attr("value"), + "some-document-name", + "The name field should be set in the SearchForm", + ) + self.assertEqual( + pq("form#search_form input#id_olddrafts").attr("checked"), + "checked", + "The old drafts checkbox should be selected in the SearchForm", + ) + self.assertIsNone( + pq("form#search_form input#id_rfcs").attr("checked"), + "The RFCs checkbox should not be selected in the SearchForm", + ) + self.assertIsNone( + pq("form#search_form input#id_activedrafts").attr("checked"), + "The active drafts checkbox should not be selected in the SearchForm", + ) + def test_search(self): draft = WgDraftFactory(name='draft-ietf-mars-test',group=GroupFactory(acronym='mars',parent=Group.objects.get(acronym='farfut')),authors=[PersonFactory()],ad=PersonFactory()) From 5fc45374bf625da44db293f7491cc026f852adc3 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 13 Nov 2024 16:44:55 -0400 Subject: [PATCH 10/13] style: Black --- ietf/doc/tests.py | 55 ++++++++++++++++++++++++++++++---------- ietf/doc/views_search.py | 29 +++++++++++---------- 2 files changed, 57 insertions(+), 27 deletions(-) diff --git a/ietf/doc/tests.py b/ietf/doc/tests.py index 843ac6a05e..74ae9538a4 100644 --- a/ietf/doc/tests.py +++ b/ietf/doc/tests.py @@ -104,14 +104,24 @@ def test_search_handles_querystring_parameters(self): ) def test_search(self): - - draft = WgDraftFactory(name='draft-ietf-mars-test',group=GroupFactory(acronym='mars',parent=Group.objects.get(acronym='farfut')),authors=[PersonFactory()],ad=PersonFactory()) + draft = WgDraftFactory( + name="draft-ietf-mars-test", + group=GroupFactory( + acronym="mars", parent=Group.objects.get(acronym="farfut") + ), + authors=[PersonFactory()], + ad=PersonFactory(), + ) rfc = WgRfcFactory() draft.set_state(State.objects.get(used=True, type="draft-iesg", slug="pub-req")) - old_draft = IndividualDraftFactory(name='draft-foo-mars-test',authors=[PersonFactory()],title="Optimizing Martian Network Topologies") + old_draft = IndividualDraftFactory( + name="draft-foo-mars-test", + authors=[PersonFactory()], + title="Optimizing Martian Network Topologies", + ) old_draft.set_state(State.objects.get(used=True, type="draft", slug="expired")) - base_url = urlreverse('ietf.doc.views_search.search') + base_url = urlreverse("ietf.doc.views_search.search") # only show form, no search yet r = self.client.get(base_url) @@ -154,35 +164,50 @@ def test_search(self): r = self.client.get(base_url + "?olddrafts=on&name=%s" % draft.name) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) - + draft.set_state(State.objects.get(type="draft", slug="active")) # find by title - r = self.client.get(base_url + "?activedrafts=on&name=%s" % draft.title.split()[0]) + r = self.client.get( + base_url + "?activedrafts=on&name=%s" % draft.title.split()[0] + ) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) # find by author - r = self.client.get(base_url + "?activedrafts=on&by=author&author=%s" % draft.documentauthor_set.first().person.name_parts()[1]) + r = self.client.get( + base_url + + "?activedrafts=on&by=author&author=%s" + % draft.documentauthor_set.first().person.name_parts()[1] + ) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) # find by group - r = self.client.get(base_url + "?activedrafts=on&by=group&group=%s" % draft.group.acronym) + r = self.client.get( + base_url + "?activedrafts=on&by=group&group=%s" % draft.group.acronym + ) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) - r = self.client.get(base_url + "?activedrafts=on&by=group&group=%s" % draft.group.acronym.swapcase()) + r = self.client.get( + base_url + + "?activedrafts=on&by=group&group=%s" % draft.group.acronym.swapcase() + ) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) # find by area - r = self.client.get(base_url + "?activedrafts=on&by=area&area=%s" % draft.group.parent_id) + r = self.client.get( + base_url + "?activedrafts=on&by=area&area=%s" % draft.group.parent_id + ) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) # find by area - r = self.client.get(base_url + "?activedrafts=on&by=area&area=%s" % draft.group.parent_id) + r = self.client.get( + base_url + "?activedrafts=on&by=area&area=%s" % draft.group.parent_id + ) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) @@ -192,7 +217,11 @@ def test_search(self): self.assertContains(r, draft.title) # find by IESG state - r = self.client.get(base_url + "?activedrafts=on&by=state&state=%s&substate=" % draft.get_state("draft-iesg").pk) + r = self.client.get( + base_url + + "?activedrafts=on&by=state&state=%s&substate=" + % draft.get_state("draft-iesg").pk + ) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) @@ -201,7 +230,7 @@ def test_search_became_rfc(self): rfc = WgRfcFactory() draft.set_state(State.objects.get(type="draft", slug="rfc")) draft.relateddocument_set.create(relationship_id="became_rfc", target=rfc) - base_url = urlreverse('ietf.doc.views_search.search') + base_url = urlreverse("ietf.doc.views_search.search") # find by RFC r = self.client.get(base_url + f"?rfcs=on&name={rfc.name}") diff --git a/ietf/doc/views_search.py b/ietf/doc/views_search.py index 047ade0257..7b71dd77bd 100644 --- a/ietf/doc/views_search.py +++ b/ietf/doc/views_search.py @@ -282,6 +282,7 @@ def retrieve_search_results(form, all_types=False): return docs + def search(request): """Search for a draft""" # defaults for results / meta @@ -291,18 +292,20 @@ def search(request): if request.method == "POST": form = SearchForm(data=request.POST) if form.is_valid(): - cache_key = get_search_cache_key( - form.cache_key_fragment() - ) + cache_key = get_search_cache_key(form.cache_key_fragment()) cached_val = cache.get(cache_key) if cached_val: [results, meta] = cached_val else: results = retrieve_search_results(form) - results, meta = prepare_document_table(request, results, form.cleaned_data) - cache.set(cache_key, [results, meta]) # for settings.CACHE_MIDDLEWARE_SECONDS + results, meta = prepare_document_table( + request, results, form.cleaned_data + ) + cache.set( + cache_key, [results, meta] + ) # for settings.CACHE_MIDDLEWARE_SECONDS log(f"Search results computed for {form.cleaned_data}") - meta['searching'] = True + meta["searching"] = True else: if request.GET: # backwards compatibility - fill in the form @@ -317,8 +320,8 @@ def search(request): messages.error( request, ( - 'Searching via the URL query string is no longer supported. ' - 'The form below has been filled in with the parameters from your request. ' + "Searching via the URL query string is no longer supported. " + "The form below has been filled in with the parameters from your request. " 'To execute your search, please click "Search".' ), ) @@ -327,18 +330,16 @@ def search(request): return render( request, - 'doc/search/search.html', - context={ - "form": form, - "docs": results, - "meta": meta - }, + "doc/search/search.html", + context={"form": form, "docs": results, "meta": meta}, ) + def frontpage(request): form = SearchForm() return render(request, 'doc/frontpage.html', {'form':form}) + def search_for_name(request, name): def find_unique(n): exact = Document.objects.filter(name__iexact=n).first() From f6cf450f28ffff261c4062cadb59fb5f7d4d88ec Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 13 Nov 2024 17:12:52 -0400 Subject: [PATCH 11/13] test: refactor test_search() --- ietf/doc/tests.py | 114 ++++++++++++++++++++++++---------------------- 1 file changed, 60 insertions(+), 54 deletions(-) diff --git a/ietf/doc/tests.py b/ietf/doc/tests.py index 74ae9538a4..6b7142b66f 100644 --- a/ietf/doc/tests.py +++ b/ietf/doc/tests.py @@ -106,9 +106,7 @@ def test_search_handles_querystring_parameters(self): def test_search(self): draft = WgDraftFactory( name="draft-ietf-mars-test", - group=GroupFactory( - acronym="mars", parent=Group.objects.get(acronym="farfut") - ), + group=GroupFactory(acronym="mars", parent=Group.objects.get(acronym="farfut")), authors=[PersonFactory()], ad=PersonFactory(), ) @@ -120,107 +118,115 @@ def test_search(self): title="Optimizing Martian Network Topologies", ) old_draft.set_state(State.objects.get(used=True, type="draft", slug="expired")) - - base_url = urlreverse("ietf.doc.views_search.search") - + + url = urlreverse("ietf.doc.views_search.search") + # only show form, no search yet - r = self.client.get(base_url) + r = self.client.get(url) self.assertEqual(r.status_code, 200) - + # no match - r = self.client.get(base_url + "?activedrafts=on&name=thisisnotadocumentname") + r = self.client.post(url, {"activedrafts": "on", "name": "thisisnotadocumentname"}) self.assertEqual(r.status_code, 200) self.assertContains(r, "No documents match") - - r = self.client.get(base_url + "?rfcs=on&name=xyzzy") + + r = self.client.post(url, {"rfcs": "on", "name": "xyzzy"}) self.assertEqual(r.status_code, 200) self.assertContains(r, "No documents match") - - r = self.client.get(base_url + "?olddrafts=on&name=bar") + + r = self.client.post(url, {"olddrafts": "on", "name": "bar"}) self.assertEqual(r.status_code, 200) self.assertContains(r, "No documents match") - - r = self.client.get(base_url + "?olddrafts=on&name=foo") + + r = self.client.post(url, {"olddrafts": "on", "name": "foo"}) self.assertEqual(r.status_code, 200) self.assertContains(r, "draft-foo-mars-test") - - r = self.client.get(base_url + "?olddrafts=on&name=FoO") # mixed case + + r = self.client.post(url, {"olddrafts": "on", "name": "FoO"}) # mixed case self.assertEqual(r.status_code, 200) self.assertContains(r, "draft-foo-mars-test") - + # find by RFC - r = self.client.get(base_url + "?rfcs=on&name=%s" % rfc.name) + r = self.client.post(url, {"rfcs": "on", "name": rfc.name}) self.assertEqual(r.status_code, 200) self.assertContains(r, rfc.title) - + # find by active/inactive - + draft.set_state(State.objects.get(type="draft", slug="active")) - r = self.client.get(base_url + "?activedrafts=on&name=%s" % draft.name) + r = self.client.post(url, {"activedrafts": "on", "name": draft.name}) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) - + draft.set_state(State.objects.get(type="draft", slug="expired")) - r = self.client.get(base_url + "?olddrafts=on&name=%s" % draft.name) + r = self.client.post(url, {"olddrafts": "on", "name": draft.name}) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) - + draft.set_state(State.objects.get(type="draft", slug="active")) - + # find by title - r = self.client.get( - base_url + "?activedrafts=on&name=%s" % draft.title.split()[0] - ) + r = self.client.post(url, {"activedrafts": "on", "name": draft.title.split()[0]}) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) - + # find by author - r = self.client.get( - base_url - + "?activedrafts=on&by=author&author=%s" - % draft.documentauthor_set.first().person.name_parts()[1] + r = self.client.post( + url, + { + "activedrafts": "on", + "by": "author", + "author": draft.documentauthor_set.first().person.name_parts()[1], + }, ) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) - + # find by group - r = self.client.get( - base_url + "?activedrafts=on&by=group&group=%s" % draft.group.acronym + r = self.client.post( + url, + {"activedrafts": "on", "by": "group", "group": draft.group.acronym}, ) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) - - r = self.client.get( - base_url - + "?activedrafts=on&by=group&group=%s" % draft.group.acronym.swapcase() + + r = self.client.post( + url, + {"activedrafts": "on", "by": "group", "group": draft.group.acronym.swapcase()}, ) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) - + # find by area - r = self.client.get( - base_url + "?activedrafts=on&by=area&area=%s" % draft.group.parent_id + r = self.client.post( + url, + {"activedrafts": "on", "by": "area", "area": draft.group.parent_id}, ) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) - + # find by area - r = self.client.get( - base_url + "?activedrafts=on&by=area&area=%s" % draft.group.parent_id + r = self.client.post( + url, + {"activedrafts": "on", "by": "area", "area": draft.group.parent_id}, ) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) - + # find by AD - r = self.client.get(base_url + "?activedrafts=on&by=ad&ad=%s" % draft.ad_id) + r = self.client.post(url, {"activedrafts": "on", "by": "ad", "ad": draft.ad_id}) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) - + # find by IESG state - r = self.client.get( - base_url - + "?activedrafts=on&by=state&state=%s&substate=" - % draft.get_state("draft-iesg").pk + r = self.client.post( + url, + { + "activedrafts": "on", + "by": "state", + "state": draft.get_state("draft-iesg").pk, + "substate": "", + }, ) self.assertEqual(r.status_code, 200) self.assertContains(r, draft.title) From 1ef4e0f6fb361c2aba71a27d627b50d58b684e8c Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 13 Nov 2024 17:15:20 -0400 Subject: [PATCH 12/13] test: refactor test_search_became_rfc() --- ietf/doc/tests.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ietf/doc/tests.py b/ietf/doc/tests.py index 6b7142b66f..07165f8718 100644 --- a/ietf/doc/tests.py +++ b/ietf/doc/tests.py @@ -236,15 +236,15 @@ def test_search_became_rfc(self): rfc = WgRfcFactory() draft.set_state(State.objects.get(type="draft", slug="rfc")) draft.relateddocument_set.create(relationship_id="became_rfc", target=rfc) - base_url = urlreverse("ietf.doc.views_search.search") + url = urlreverse("ietf.doc.views_search.search") # find by RFC - r = self.client.get(base_url + f"?rfcs=on&name={rfc.name}") + r = self.client.post(url, {"rfcs": "on", "name": rfc.name}) self.assertEqual(r.status_code, 200) self.assertContains(r, rfc.title) # find by draft - r = self.client.get(base_url + f"?activedrafts=on&rfcs=on&name={draft.name}") + r = self.client.post(url, {"activedrafts": "on", "rfcs": "on", "name": draft.name}) self.assertEqual(r.status_code, 200) self.assertContains(r, rfc.title) From 7c831081ef98ef47b0fc5437a4876c80b5d0cb33 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 13 Nov 2024 18:03:30 -0400 Subject: [PATCH 13/13] test: use scroll_and_click helper --- ietf/doc/tests_js.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ietf/doc/tests_js.py b/ietf/doc/tests_js.py index acd74c4a0b..9a5aad13b9 100644 --- a/ietf/doc/tests_js.py +++ b/ietf/doc/tests_js.py @@ -92,10 +92,8 @@ def _read_author_form(form_elt): self.assertEqual(len(author_forms), 1) # get the "add author" button so we can add blank author forms - add_author_button = self.driver.find_element(By.ID, 'add-author-button') for index, auth in enumerate(authors): - self.scroll_to_element(add_author_button) # Can only click if it's in view! - add_author_button.click() # Create a new form. Automatically scrolls to it. + self.scroll_and_click((By.ID, 'add-author-button')) # Create new form. Automatically scrolls to it. author_forms = authors_list.find_elements(By.CLASS_NAME, 'author-panel') authors_added = index + 1 self.assertEqual(len(author_forms), authors_added + 1) # Started with 1 author, hence +1