Merge branch 'main' into el-1982-error-validation-onto-wrong-form

fala/apps/adviser/forms.py

@@ -73,7 +73,6 @@ def perform_search(form, postcode, categories, page):
         )
         if "error" in data:
             form.add_error("postcode", data["error"])
-            return {}
         return data
     except laalaa.LaaLaaError:
         form.add_error("postcode", "%s %s" % (_("Error looking up legal advisers."), _("Please try again later.")))

fala/apps/adviser/laa_laa_paginator.py

@@ -1,11 +1,14 @@
+import math
+
+
 class LaaLaaPaginator(object):
     def __init__(self, count, page_size, window_size, current_page):
         self._window_size = window_size
         if current_page is None:
             self._current_page = 1
         else:
             self._current_page = current_page
-        self._very_last_page = count // page_size + 1
+        self._very_last_page = math.ceil(count / page_size)
 
     @property
     def page_range(self):

fala/apps/adviser/models.py

@@ -1,5 +1,6 @@
 from django.db import models
 from django.utils import timezone
+from django.http import Http404
 from .regions import Region
 from .laa_laa_paginator import LaaLaaPaginator
 import urllib
@@ -29,69 +30,72 @@ def get_queryset(self):
         return self._data.get("results", None)
 
     def get_context_data(self):
-        pages = LaaLaaPaginator(self._data["count"], 10, 3, self._form.current_page)
-        current_page = pages.current_page()
-        params = {
-            "postcode": self._form.cleaned_data["postcode"],
-            "name": self._form.cleaned_data["name"],
-        }
-        categories = self._form.cleaned_data["categories"]
-
-        # create list of tuples which can be passed to urlencode for pagination links
-        category_tuples = [("categories", c) for c in categories]
-
-        def item_for(page_num):
-            if len(categories) > 0:
-                page_params = {"page": page_num}
-                href = (
-                    "/search?"
-                    + urllib.parse.urlencode({**page_params, **params})
-                    + "&"
-                    + urllib.parse.urlencode(category_tuples)
-                )
-            else:
-                page_params = {"page": page_num}
-                href = "/search?" + urllib.parse.urlencode({**page_params, **params})
-
-            return {"number": page_num, "current": self._form.current_page == page_num, "href": href}
-
-        pagination = {"items": [item_for(page_num) for page_num in pages.page_range]}
-
-        if current_page.has_previous():
-            if len(categories) > 0:
-                page_params = {"page": current_page.previous_page_number()}
-                prev_link = (
-                    "/search?"
-                    + urllib.parse.urlencode({**page_params, **params})
-                    + "&"
-                    + urllib.parse.urlencode(category_tuples)
-                )
-            else:
-                page_params = {"page": current_page.previous_page_number()}
-                prev_link = "/search?" + urllib.parse.urlencode({**page_params, **params})
-            pagination["previous"] = {"href": prev_link}
-
-        if current_page.has_next():
-            if len(categories) > 0:
-                page_params = {"page": current_page.next_page_number()}
-                href = (
-                    "/search?"
-                    + urllib.parse.urlencode({**page_params, **params})
-                    + "&"
-                    + urllib.parse.urlencode(category_tuples)
-                )
-            else:
-                page_params = {"page": current_page.next_page_number()}
-                href = "/search?" + urllib.parse.urlencode({**page_params, **params})
-            pagination["next"] = {"href": href}
-
-        return {
-            "form": self._form,
-            "data": self._data,
-            "params": params,
-            "FEATURE_FLAG_SURVEY_MONKEY": settings.FEATURE_FLAG_SURVEY_MONKEY,
-            "pagination": pagination,
-        }
+        if "Invalid page" in self._data.get("error", []):
+            raise Http404("Page not found")
+        else:
+            pages = LaaLaaPaginator(self._data["count"], 10, 3, self._form.current_page)
+            current_page = pages.current_page()
+            params = {
+                "postcode": self._form.cleaned_data["postcode"],
+                "name": self._form.cleaned_data["name"],
+            }
+            categories = self._form.cleaned_data["categories"]
+
+            # create list of tuples which can be passed to urlencode for pagination links
+            category_tuples = [("categories", c) for c in categories]
+
+            def item_for(page_num):
+                if len(categories) > 0:
+                    page_params = {"page": page_num}
+                    href = (
+                        "/search?"
+                        + urllib.parse.urlencode({**page_params, **params})
+                        + "&"
+                        + urllib.parse.urlencode(category_tuples)
+                    )
+                else:
+                    page_params = {"page": page_num}
+                    href = "/search?" + urllib.parse.urlencode({**page_params, **params})
+
+                return {"number": page_num, "current": self._form.current_page == page_num, "href": href}
+
+            pagination = {"items": [item_for(page_num) for page_num in pages.page_range]}
+
+            if current_page.has_previous():
+                if len(categories) > 0:
+                    page_params = {"page": current_page.previous_page_number()}
+                    prev_link = (
+                        "/search?"
+                        + urllib.parse.urlencode({**page_params, **params})
+                        + "&"
+                        + urllib.parse.urlencode(category_tuples)
+                    )
+                else:
+                    page_params = {"page": current_page.previous_page_number()}
+                    prev_link = "/search?" + urllib.parse.urlencode({**page_params, **params})
+                pagination["previous"] = {"href": prev_link}
+
+            if current_page.has_next():
+                if len(categories) > 0:
+                    page_params = {"page": current_page.next_page_number()}
+                    href = (
+                        "/search?"
+                        + urllib.parse.urlencode({**page_params, **params})
+                        + "&"
+                        + urllib.parse.urlencode(category_tuples)
+                    )
+                else:
+                    page_params = {"page": current_page.next_page_number()}
+                    href = "/search?" + urllib.parse.urlencode({**page_params, **params})
+                pagination["next"] = {"href": href}
+
+            return {
+                "form": self._form,
+                "data": self._data,
+                "params": params,
+                "FEATURE_FLAG_SURVEY_MONKEY": settings.FEATURE_FLAG_SURVEY_MONKEY,
+                "pagination": pagination if len(pagination["items"]) > 1 else {},
+            }
 
 
 class OtherJurisdictionState(object):

fala/apps/adviser/tests/test_fala_views_and_functions.py

@@ -248,3 +248,14 @@ class AccessibilityViewTest(SimpleTestCase):
     def test_shows_new_accessibility_statement(self):
         response = self.client.get(self.url)
         self.assertContains(response, "Accessibility statement for Find a legal advisor")
+
+
+class ErrorPageTest(SimpleTestCase):
+    client = Client()
+    url = reverse("search")
+
+    def test_raises_404_when_page_number_does_not_exist(
+        self,
+    ):
+        response = self.client.get(self.url, {"postcode": "SE11", "page": 500})
+        self.assertEqual(response.status_code, 404)

fala/apps/adviser/tests/test_other_regions_playwright.py

@@ -4,7 +4,7 @@
 
 class OtherRegionsTest(PlaywrightTestSetup):
     def test_jersey(self):
-        results_page = self.visit_results_page("JE1")
+        results_page = self.visit_results_page(postcode="JE1")
         expect(results_page.h1).to_have_text("The postcode JE1 is in Jersey")
         search_page = results_page.change_search()
         # We don't preserve Jersey postcodes search term, by design, as we don't want residents from Jersey to use our service
@@ -13,7 +13,7 @@ def test_jersey(self):
     def test_scotland_with_persistant_search_and_categories(self):
         checkboxes = ["Family mediation", "Clinical Negligence"]
 
-        results_page = self.visit_results_page("TD13", checkboxes)
+        results_page = self.visit_results_page(postcode="TD13", checkbox_labels=checkboxes)
         expect(results_page.item_from_text("Legal Aid in Scotland")).to_be_visible()
         search_page = results_page.change_search()
 

fala/apps/adviser/tests/test_pagination_playwright.py

@@ -7,7 +7,7 @@ class PaginationResults(PlaywrightTestSetup):
     def test_results_pagination(self):
         checkboxes = ["Crime"]
 
-        page = self.visit_results_page("M25 3JF", checkboxes)
+        page = self.visit_results_page(postcode="M25 3JF", checkbox_labels=checkboxes)
         expect(page.item_from_text("results in order of closeness to M25 3JF.")).to_be_visible()
         first_page_result_count = page.result_count.inner_text()
         expect(page.pagination_link_title).to_be_visible()

fala/apps/adviser/tests/test_results_page_journeys_playwright.py

@@ -4,7 +4,15 @@
 
 class ResultPageEndToEndJourneys(PlaywrightTestSetup):
     def test_change_search(self):
-        results_page = self.visit_results_page("SE11")
+        results_page = self.visit_results_page(postcode="SE11")
         expect(results_page.change_search_grey_box).to_have_text("Postcode: SE11")
         search_page = results_page.change_search()
         expect(search_page.h1).to_have_text("Find a legal aid adviser or family mediator")
+
+    def test_pagination_does_not_appear_when_there_is_one_page(self):
+        results_page = self.visit_results_page(postcode="W1J5BF", organisation="charles")
+        expect(results_page.pagination_link_title).not_to_be_visible()
+
+    def test_pagination_does_appears_when_there_is_more_than_one_page(self):
+        results_page = self.visit_results_page(postcode="SE11")
+        expect(results_page.pagination_link_title).to_be_visible()

fala/apps/adviser/tests/test_search_page_journeys_playwright.py

@@ -36,7 +36,7 @@ def test_postcode_search_journey(self):
         ]
         for postcode in test_cases:
             with self.subTest(postcode=postcode):
-                page = self.visit_results_page(postcode)
+                page = self.visit_results_page(postcode=postcode)
                 expect(page.h1).to_have_text("Search results")
                 expect(page.change_search_grey_box.nth(0)).to_have_text(f"Postcode: {postcode}")
                 expect(page.item_from_text("in order of closeness")).to_be_visible()

fala/apps/laalaa/api.py

@@ -41,7 +41,12 @@ def laalaa_url(**kwargs):
 def laalaa_search(**kwargs):
     try:
         response = requests.get(laalaa_url(**kwargs))
+        response.raise_for_status()
         return response.json()
+    except requests.exceptions.HTTPError as e:
+        if response.status_code == 404:
+            return response.json()
+        raise LaaLaaError(e)
     except (requests.exceptions.RequestException, ValueError) as e:
         raise LaaLaaError(e)
 
@@ -65,6 +70,10 @@ def find(postcode=None, categories=None, page=1, organisation_types=None, organi
         organisation_name=organisation_name,
     )
 
-    data["results"] = list(map(decode_categories, data.get("results", [])))
-
-    return data
+    # because of the way the current error capture and forms are set up, it is
+    # easier to allow the 404 errors through as a valid response and treat them separately.
+    if "error" in data:
+        return data
+    else:
+        data["results"] = list(map(decode_categories, data.get("results", [])))
+        return data

fala/playwright/setup.py

@@ -26,12 +26,18 @@ def tearDown(self):
         self.browser.close()
         super().tearDown()
 
-    def visit_results_page(self, postcode, checkbox_labels=None):
+    def visit_results_page(self, **kwargs):
+        postcode = kwargs.get("postcode")
+        organisation = kwargs.get("organisation", None)
+        checkbox_labels = kwargs.get("checkbox_labels", None)
+
         if checkbox_labels is None:
             checkbox_labels = []
         page = self.browser.new_page()
         page.goto(f"{self.live_server_url}")
         page.get_by_label("Postcode").fill(postcode)
+        if organisation:
+            page.get_by_label("Name of organisation you are looking for (optional)").fill(organisation)
         for label in checkbox_labels:
             page.get_by_label(label).check()
         page.get_by_role("button", name="Search").click()