feat: POST for document search requests (ietf-tools#8206)

* 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

* feat: convert GET search to POST

Still todo:
 * refactor frontpage view to match
 * refactor menubar search (?)
 * refactor stats view that uses SearchForm

* fix: revert frontpage changes, search works

Still todo:
 * refactor stats view that uses SearchForm

* fix: define vars in all branches

* refactor: update stats use of SearchForm

* chore: improve message

* fix: remove lint

* chore: comments re: QuerySetAny

* test: test query string search params

* style: Black

* test: refactor test_search()

* test: refactor test_search_became_rfc()

* test: use scroll_and_click helper

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:

ietf/doc/tests.py

@@ -71,96 +71,163 @@
 
 
 class SearchTests(TestCase):
-    def test_search(self):
+    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",
+        )
 
-        draft = WgDraftFactory(name='draft-ietf-mars-test',group=GroupFactory(acronym='mars',parent=Group.objects.get(acronym='farfut')),authors=[PersonFactory()],ad=PersonFactory())
+    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(),
+        )
         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')
-
+    
+        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)
 
@@ -169,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)
 

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

ietf/doc/utils.py

@@ -3,9 +3,7 @@
 
 
 import datetime
-import hashlib
 import io
-import json
 import math
 import os
 import re
@@ -348,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:
@@ -1047,12 +1046,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]):