Skip to content

Commit

Permalink
code duplication & tests
Browse files Browse the repository at this point in the history
- create a `BaseSearch` class which is shared by AdviserSearchForm & SingleCategorySearchForm, to stop code duplication
- add and update test suite
  • Loading branch information
MazOneTwoOne committed Dec 12, 2024
1 parent 9cd0380 commit 24e9265
Show file tree
Hide file tree
Showing 7 changed files with 135 additions and 80 deletions.
124 changes: 49 additions & 75 deletions fala/apps/adviser/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,25 +61,53 @@ class AdviserRootForm(forms.Form):
)


# Shared method to handle searches
def perform_search(form, postcode, categories, page):
try:
data = laalaa.find(
postcode=postcode,
categories=categories,
page=page,
organisation_types=form.cleaned_data.get("type"),
organisation_name=form.cleaned_data.get("name"),
)
if "error" in data:
form.add_error("postcode", data["error"])
return data
except laalaa.LaaLaaError:
form.add_error("postcode", "%s %s" % (_("Error looking up legal advisers."), _("Please try again later.")))
return {}
class BaseSearch:
def perform_search(self, form, postcode, categories, page):
try:
data = laalaa.find(
postcode=postcode,
categories=categories,
page=page,
organisation_types=form.cleaned_data.get("type"),
organisation_name=form.cleaned_data.get("name"),
)
if "error" in data:
form.add_error("postcode", data["error"])
return data
except laalaa.LaaLaaError:
form.add_error("postcode", "%s %s" % (_("Error looking up legal advisers."), _("Please try again later.")))
return {}

@property
def region(self):
# retrieve the api call variables
country_from_valid_postcode = getattr(self, "_country_from_valid_postcode", None)

# Return `Region.ENGLAND_OR_WALES` from `clean` if set
if not country_from_valid_postcode:
return getattr(self, "_region", None)

# for Guernsey & Jersey the country comes back as 'Channel Islands', we are using `nhs_ha` key, to distinguish between them
country, nhs_ha = country_from_valid_postcode

class AdviserSearchForm(AdviserRootForm):
if country == "Northern Ireland":
return Region.NI
elif country == "Isle of Man":
return Region.IOM
elif country == "Channel Islands" and nhs_ha == "Jersey Health Authority":
return Region.JERSEY
elif country == "Channel Islands" and nhs_ha == "Guernsey Health Authority":
return Region.GUERNSEY
elif country == "Scotland":
return Region.SCOTLAND
elif country == "England" or country == "Wales":
return Region.ENGLAND_OR_WALES
else:
self.add_error("postcode", _("This service is only available for England and Wales"))
return None


class AdviserSearchForm(AdviserRootForm, BaseSearch):
page = forms.IntegerField(required=False, widget=forms.HiddenInput())

def __init__(self, *args, **kwargs):
Expand Down Expand Up @@ -113,34 +141,6 @@ def clean(self):

return data

@property
def region(self):
# retrieve the api call variables
country_from_valid_postcode = getattr(self, "_country_from_valid_postcode", None)

# Return `Region.ENGLAND_OR_WALES` from `clean` if set
if not country_from_valid_postcode:
return getattr(self, "_region", None)

# for Guernsey & Jersey the country comes back as 'Channel Islands', we are using `nhs_ha` key, to distinguish between them
country, nhs_ha = country_from_valid_postcode

if country == "Northern Ireland":
return Region.NI
elif country == "Isle of Man":
return Region.IOM
elif country == "Channel Islands" and nhs_ha == "Jersey Health Authority":
return Region.JERSEY
elif country == "Channel Islands" and nhs_ha == "Guernsey Health Authority":
return Region.GUERNSEY
elif country == "Scotland":
return Region.SCOTLAND
elif country == "England" or country == "Wales":
return Region.ENGLAND_OR_WALES
else:
self.add_error("postcode", _("This service is only available for England and Wales"))
return None

@property
def current_page(self):
return self.cleaned_data.get("page", 1)
Expand All @@ -149,15 +149,15 @@ def validate_postcode_and_return_country(self, postcode):
return validate_postcode_and_return_country(postcode, form=self)

def search(self):
return perform_search(
return self.perform_search(
self,
self.cleaned_data.get("postcode"),
self.cleaned_data.get("categories"),
self.cleaned_data.get("page", 1),
)


class SingleCategorySearchForm(AdviserRootForm):
class SingleCategorySearchForm(AdviserRootForm, BaseSearch):
page = forms.IntegerField(required=False, widget=forms.HiddenInput())

def __init__(self, categories=None, *args, **kwargs):
Expand All @@ -174,13 +174,12 @@ def clean(self):
self.add_error("postcode", _("Enter a postcode"))
return data

# Validate postcode and set `_country_from_valid_postcode`
if postcode:
valid_postcode = self.validate_postcode_and_return_country(postcode)
if not valid_postcode:
self.add_error("postcode", _("Enter a valid postcode"))
else:
self._country_from_valid_postcode = valid_postcode # This is used by the `region` property
self._country_from_valid_postcode = valid_postcode

# Check if categories are provided
if not categories:
Expand All @@ -190,31 +189,6 @@ def clean(self):

return data

@property
def region(self):
country_from_valid_postcode = getattr(self, "_country_from_valid_postcode", None)

if not country_from_valid_postcode:
return getattr(self, "_region", None)

country, nhs_ha = country_from_valid_postcode

if country == "Northern Ireland":
return Region.NI
elif country == "Isle of Man":
return Region.IOM
elif country == "Channel Islands" and nhs_ha == "Jersey Health Authority":
return Region.JERSEY
elif country == "Channel Islands" and nhs_ha == "Guernsey Health Authority":
return Region.GUERNSEY
elif country == "Scotland":
return Region.SCOTLAND
elif country == "England" or country == "Wales":
return Region.ENGLAND_OR_WALES
else:
self.add_error("postcode", _("This service is only available for England and Wales"))
return None

@property
def current_page(self):
return self.cleaned_data.get("page", 1)
Expand All @@ -223,7 +197,7 @@ def validate_postcode_and_return_country(self, postcode):
return validate_postcode_and_return_country(postcode, form=self)

def search(self):
return perform_search(
return self.perform_search(
self,
self.cleaned_data.get("postcode"),
self.categories,
Expand Down
11 changes: 8 additions & 3 deletions fala/apps/adviser/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,12 +163,13 @@ def get_context_data(self):


class SingleSearchErrorState(object):
def __init__(self, form, category_display_name, category_message, category_code, search_url):
def __init__(self, form, category_display_name, category_message, category_code, search_url, category_slug):
self._form = form
self._category_display_name = category_display_name
self.category_display_name = category_display_name
self.category_message = category_message
self.category_code = category_code
self.search_url = search_url
self.category_slug = category_slug

@property
def template_name(self):
Expand All @@ -187,12 +188,16 @@ def get_context_data(self):
item = {"text": error[0], "href": f"#id_{field}"}
errorList.append(item)

if self.category_slug == "hlpas":
self.category_display_name = "Housing Loss Prevention Advice Service"

return {
"form": self._form,
"data": {},
"errorList": errorList,
"category_slug": self.category_slug,
"category_code": self.category_code,
"category_display_name": self._category_display_name,
"category_display_name": self.category_display_name,
"category_message": self.category_message,
"search_url": self.search_url,
}
14 changes: 14 additions & 0 deletions fala/apps/adviser/tests/page_objects.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,5 +75,19 @@ def search(self, postcode):
self._page.locator("#searchButton").click()


class SingleCategorySearchPage(FalaPage):
@property
def postcode_input_field(self):
return self._page.get_by_label("Postcode")

@property
def search_button(self):
return self.item_from_text("Search")

def search(self, postcode):
self._page.locator("#id_postcode").fill(postcode)
self._page.locator("#searchButton").click()


class CookiesPage(FalaPage):
pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
from playwright.sync_api import expect
from fala.playwright.setup import PlaywrightTestSetup


class SingleCategorySearchPageEndToEndJourneys(PlaywrightTestSetup):
hlpas_front_page_heading = "Find a legal aid adviser for Housing Loss Prevention Advice Service"
med_front_page_heading = "Find a legal aid adviser for clinical negligence"

def test_landing_page_with_hlpas(self):
page = self.visit_single_category_search_page("?categories=hlpas")
expect(page.h1).to_have_text(f"{self.hlpas_front_page_heading}")

def test_landing_page_with_med(self):
page = self.visit_single_category_search_page("?categories=med")
expect(page.h1).to_have_text(f"{self.med_front_page_heading}")

def test_postcode_search_journey(self):
test_cases = [
"SW1H 9AJ",
"SE11",
]
for postcode in test_cases:
with self.subTest(postcode=postcode):
page = self.visit_results_page(postcode=postcode)
expect(page.h1).to_have_text("Search results")

def test_invalid_postcode_journey(self):
test_cases = [
"ZZZ1",
"G12 OGJKLJGK",
"LS25 ghjkhjkh",
"IM4 TESTTTTTTTTTTTT",
]
for postcode in test_cases:
with self.subTest(postcode=postcode):
page = self.browser.new_page()
page.goto(f"{self.live_server_url}/single-category-search?categories=hlpas")
expect(page.locator("h1")).to_have_text(f"{self.hlpas_front_page_heading}")
page.get_by_label("Postcode").fill(f"{postcode}")
page.get_by_role("button", name="Search").click()
expect(page.locator("h1")).to_have_text(f"{self.hlpas_front_page_heading}")
expect(page.locator("css=.govuk-error-summary")).to_be_visible()
page.goto(f"{self.live_server_url}/single-category-search?categories=med")
expect(page.locator("h1")).to_have_text(f"{self.med_front_page_heading}")
page.get_by_label("Postcode").fill(f"{postcode}")
page.get_by_role("button", name="Search").click()
expect(page.locator("h1")).to_have_text(f"{self.med_front_page_heading}")
expect(page.locator("css=.govuk-error-summary")).to_be_visible()
5 changes: 4 additions & 1 deletion fala/apps/adviser/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,11 @@ def get(self, request, *args, **kwargs):
category_code = self.request.GET.get("categories", "")
search_url = reverse("search")

# this is so that we can get correct hlpas display name onto SingleSearchErrorState view
category_slug = self.request.GET.get("categories")

self.state = SingleSearchErrorState(
form, category_display_name, category_message, category_code, search_url
form, category_display_name, category_message, category_code, search_url, category_slug
)
else:
self.state = ErrorState(form)
Expand Down
13 changes: 12 additions & 1 deletion fala/playwright/setup.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import os
from django.contrib.staticfiles.testing import StaticLiveServerTestCase
from playwright.sync_api import sync_playwright
from fala.apps.adviser.tests.page_objects import ResultsPage, OtherRegionPage, SearchPage, CookiesPage
from fala.apps.adviser.tests.page_objects import (
ResultsPage,
OtherRegionPage,
SearchPage,
CookiesPage,
SingleCategorySearchPage,
)


class PlaywrightTestSetup(StaticLiveServerTestCase):
Expand Down Expand Up @@ -69,6 +75,11 @@ def visit_search_page(self):
page.goto(f"{self.live_server_url}")
return SearchPage(page)

def visit_single_category_search_page(self, url_params):
page = self.browser.new_page()
page.goto(f"{self.live_server_url}/single-category-search{url_params}")
return SingleCategorySearchPage(page)

def visit_cookies_page_from_footer(self):
page = self.browser.new_page()
page.goto(f"{self.live_server_url}")
Expand Down

0 comments on commit 24e9265

Please sign in to comment.