Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
188 changes: 123 additions & 65 deletions postcode_lookup/app.py
Original file line number Diff line number Diff line change
@@ -1,19 +1,9 @@
import os
from pathlib import Path

from endpoints import (
design_system_view,
failover,
live_postcode_form,
live_postcode_view,
live_uprn_view,
mock_postcode_view,
redirect_root_to_postcode_form,
sandbox_postcode_form,
sandbox_postcode_view,
sandbox_uprn_view,
section_tester,
)
import endpoints.election_information
import endpoints.electoral_services_team
import endpoints.utils
from mangum import Mangum
from starlette.applications import Starlette
from starlette.middleware import Middleware
Expand All @@ -39,109 +29,177 @@
],
)

routes = [
Route("/", endpoint=redirect_root_to_postcode_form),
Route("/sections/{section}/", endpoint=section_tester),
Route("/failover", endpoint=failover, name="failover"),
util_routes = [
Route("/", endpoint=endpoints.utils.redirect_root_to_postcode_form),
Route("/sections/{section}/", endpoint=endpoints.utils.section_tester),
Route("/failover", endpoint=endpoints.utils.failover, name="failover"),
Route(
"/design-system",
endpoint=endpoints.utils.design_system_view,
name="design_system_view",
),
Mount(
"/themes/",
app=StaticFiles(directory=Path(__file__).parent / "themes"),
name="themes",
),
Mount(
"/static/",
app=StaticFiles(directory=Path(__file__).parent / "static"),
name="static",
),
]

election_information_routes = [
# Live, EN
Route(
"/i-am-a/voter/your-election-information",
endpoint=live_postcode_form,
endpoint=endpoints.election_information.live_postcode_form,
name="live_postcode_form_en",
),
Route(
"/polling-stations/address/{postcode}/{uprn}",
endpoint=live_uprn_view,
endpoint=endpoints.election_information.live_uprn_view,
name="live_uprn_en",
),
Route(
"/polling-stations",
endpoint=live_postcode_view,
endpoint=endpoints.election_information.live_postcode_view,
name="live_postcode_en",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every route needs to have a unique name.
I think if I was starting from scratch today, I would include election_information or something in the route names we already have to differentiate the two flavours of postcode form, postcode page, uprn page etc we now have.
For now, I've kept the existing names for the "election information" routes. If you want, I can go through and rename all the existing ones. The fiddly bit of this is making sure we've accounted for the renames everywhere we concatenate some string together to make a url_for()

),
# Live, CY
Route(
"/cy/rwyf-yneg-pleidleisiwr/pleidleisiwr/gwybodaeth-etholiad",
endpoint=live_postcode_form,
endpoint=endpoints.election_information.live_postcode_form,
name="live_postcode_form_cy",
),
Route(
"/cy/polling-stations/{postcode}/{uprn}",
endpoint=live_uprn_view,
"/cy/polling-stations/address/{postcode}/{uprn}",
endpoint=endpoints.election_information.live_uprn_view,
name="live_uprn_cy",
),
Route(
"/cy/polling-stations",
endpoint=live_postcode_view,
endpoint=endpoints.election_information.live_postcode_view,
name="live_postcode_cy",
),
# Sandbox
Route(
"/sandbox/polling-stations",
endpoint=sandbox_postcode_view,
name="sandbox_postcode_en",
),
# Sandbox, EN
Route(
"/sandbox/i-am-a/voter/your-election-information",
endpoint=sandbox_postcode_form,
endpoint=endpoints.election_information.sandbox_postcode_form,
name="sandbox_postcode_form_en",
),
Route(
"/sandbox/cy/i-am-a/voter/your-election-information",
endpoint=sandbox_postcode_form,
name="sandbox_postcode_form_cy",
"/sandbox/polling-stations/address/{postcode}/{uprn}",
endpoint=endpoints.election_information.sandbox_uprn_view,
name="sandbox_uprn_en",
),
Route(
"/cy/sandbox/polling-stations",
endpoint=sandbox_postcode_view,
name="sandbox_postcode_cy",
"/sandbox/polling-stations",
endpoint=endpoints.election_information.sandbox_postcode_view,
name="sandbox_postcode_en",
),
# Sandbox, CY
Route(
"/sandbox/polling-stations/{postcode}/{uprn}",
endpoint=sandbox_uprn_view,
name="sandbox_uprn_en",
"/cy/sandbox/i-am-a/voter/your-election-information",
endpoint=endpoints.election_information.sandbox_postcode_form,
name="sandbox_postcode_form_cy",
),
Route(
"/cy/sandbox/polling-stations/{postcode}/{uprn}",
endpoint=sandbox_uprn_view,
"/cy/sandbox/polling-stations/address/{postcode}/{uprn}",
endpoint=endpoints.election_information.sandbox_uprn_view,
name="sandbox_uprn_cy",
),
Route(
"/cy/sandbox/polling-stations",
endpoint=endpoints.election_information.sandbox_postcode_view,
name="sandbox_postcode_cy",
),
# Mock
Route(
"/mock/polling-stations",
endpoint=mock_postcode_view,
endpoint=endpoints.election_information.mock_postcode_view,
name="mock_postcode_en",
),
Route(
"/cy/mock/polling-stations",
endpoint=mock_postcode_view,
endpoint=endpoints.election_information.mock_postcode_view,
name="mock_postcode_cy",
),
]

electoral_services_team_routes = [
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note this PR introduces a bunch more URLs at the root of https://www.electoralcommission.org.uk (which could potentially conflict with URLs in their CMS, or might need CloudFlare changes to make sure the right requests get routed to us).
Obviously it is a breaking change to public URLs, but I wonder if we should just put all our stuff under a prefix? 🤔 Might make it easier to share a domain. Obviously that would require changing existing public URLs

# Live, EN
Route(
"/i-am-a/voter/electoral-services",
endpoint=endpoints.electoral_services_team.live_postcode_form,
name="electoral_services_live_postcode_form_en",
),
Route(
"/design-system",
endpoint=design_system_view,
name="design_system_view",
"/electoral-services/address/{postcode}/{uprn}",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing site has this surprising behaviour where the UPRN has a {postcode} param in the route that basically functions as an ignored slug - it doesn't actually do anything.
So you can do stuff like:

I'd actually argue that this is essentially a bug and we should either

  • Bin the {postcode} route param or
  • Return a 400 Bad Request if the {postcode} and {uprn} don't match

However I have decided to leave the worms inside that can in this PR.

What I have actually done in this PR is:

  • Added the new UPRN route(s) with a {postcode} URL param so the URLs under /polling-stations/ and /electoral-services/ follow the same format
  • Also completely ignored the {postcode} URL param so the /electoral-services/ routes are "bug for bug" compatible with the /polling-stations/ routes

endpoint=endpoints.electoral_services_team.live_uprn_view,
name="electoral_services_live_uprn_en",
),
# Route(
# "/mock/polling-stations/{postcode}/{uprn}",
# endpoint=sandbox_uprn_view,
# name="sandbox_uprn_en",
# ),
# Route(
# "/cy/sandbox/polling-stations/{postcode}/{uprn}",
# endpoint=sandbox_uprn_view,
# name="sandbox_uprn_cy",
# ),
Mount(
"/themes/",
app=StaticFiles(directory=Path(__file__).parent / "themes"),
name="themes",
Route(
"/electoral-services",
endpoint=endpoints.electoral_services_team.live_postcode_view,
name="electoral_services_live_postcode_en",
),
Mount(
"/static/",
app=StaticFiles(directory=Path(__file__).parent / "static"),
name="static",
# Live, CY
Route(
"/cy/rwyf-yneg-pleidleisiwr/pleidleisiwr/electoral-services",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: need a translated slug for /electoral-services before this goes live

endpoint=endpoints.electoral_services_team.live_postcode_form,
name="electoral_services_live_postcode_form_cy",
),
Route(
"/cy/electoral-services/address/{postcode}/{uprn}",
endpoint=endpoints.electoral_services_team.live_uprn_view,
name="electoral_services_live_uprn_cy",
),
Route(
"/cy/electoral-services",
endpoint=endpoints.electoral_services_team.live_postcode_view,
name="electoral_services_live_postcode_cy",
),
# Sandbox, EN
Route(
"/sandbox/electoral-services/address/{postcode}/{uprn}",
endpoint=endpoints.electoral_services_team.sandbox_uprn_view,
name="electoral_services_sandbox_uprn_en",
),
Route(
"/sandbox/electoral-services",
endpoint=endpoints.electoral_services_team.sandbox_postcode_view,
name="electoral_services_sandbox_postcode_en",
),
# Sandbox, CY
Route(
"/cy/sandbox/electoral-services/address/{postcode}/{uprn}",
endpoint=endpoints.electoral_services_team.sandbox_uprn_view,
name="electoral_services_sandbox_uprn_cy",
),
Route(
"/cy/sandbox/electoral-services",
endpoint=endpoints.electoral_services_team.sandbox_postcode_view,
name="electoral_services_sandbox_postcode_cy",
),
# Mock
Route(
"/mock/electoral-services",
endpoint=endpoints.electoral_services_team.mock_postcode_view,
name="electoral_services_mock_postcode_en",
),
Route(
"/cy/mock/electoral-services",
endpoint=endpoints.electoral_services_team.mock_postcode_view,
name="electoral_services_mock_postcode_cy",
),
]

routes = (
util_routes + election_information_routes + electoral_services_team_routes
)

shared_translator = get_translator() # process global instance
shared_translator.load_from_directories([Path(__file__).parent / "locale"])

Expand Down
22 changes: 16 additions & 6 deletions postcode_lookup/dc_api_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,27 @@
from starlette.requests import Request


class InvalidPostcodeException(Exception): ...
class InvalidPostcodeException(Exception):
def __init__(self, message="invalid postcode", status_code=400):
super().__init__(message)
self.status_code = status_code


class InvalidUPRNException(Exception): ...
class InvalidUPRNException(Exception):
def __init__(self, message="uprn not found", status_code=404):
super().__init__(message)
self.status_code = status_code


class ApiError(Exception):
"""
Custom exception class to represent errors encountered while interacting with the API.
"""

def __init__(self, message="internal server error", status_code=500):
super().__init__(message)
self.status_code = status_code


def valid_postcode(postcode: str):
postcode = str(postcode)[:10]
Expand Down Expand Up @@ -76,9 +86,9 @@ def _get(self, endpoint, params=None):
url = urljoin(self.get_base_url, endpoint)
req = httpx.get(url, params=params)
if endpoint.startswith("postcode/") and req.status_code == 400:
raise InvalidPostcodeException
raise InvalidPostcodeException()
if endpoint.startswith("address/") and req.status_code == 404:
raise InvalidUPRNException
raise InvalidUPRNException()
req.raise_for_status()
return req

Expand All @@ -100,7 +110,7 @@ def get_postcode(self, postcode: str) -> dict:
return self._get(endpoint=f"postcode/{postcode}/").json()
except httpx.HTTPError:
raise ApiError
raise InvalidPostcodeException
raise InvalidPostcodeException()

def get_uprn(self, uprn: str) -> dict:
try:
Expand All @@ -116,7 +126,7 @@ class SandboxAPIBackend(BaseAPIClient):

def get_postcode(self, postcode: str) -> dict:
if postcode not in self.POSTCODES:
raise InvalidPostcodeException
raise InvalidPostcodeException()

response_dict = self._get(endpoint=f"sandbox/postcode/{postcode}/")
return RootModel.parse_obj(response_dict.json()).dict()
Expand Down
Empty file.
Loading
Loading