From 35d9d7195c25f6d684cbfa0ec582c912b05ea675 Mon Sep 17 00:00:00 2001 From: Samuel Young Date: Mon, 25 Nov 2024 14:14:31 +0000 Subject: [PATCH 1/7] PC-1429: add logic for handling response codes on recommendations endpoint on 404: this means no results on 500: could be due to a timeout. try 3 more times, if fails after 3rd raise error and return no results --- help_to_heat/frontdoor/interface.py | 46 +++++++++++++++++++++++++++-- help_to_heat/frontdoor/views.py | 4 +-- tests/test_interface.py | 2 +- 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/help_to_heat/frontdoor/interface.py b/help_to_heat/frontdoor/interface.py index bb55828c..4404dfdc 100644 --- a/help_to_heat/frontdoor/interface.py +++ b/help_to_heat/frontdoor/interface.py @@ -1,11 +1,13 @@ import ast import logging +import time from http import HTTPStatus import marshmallow import osdatahub import requests from django.conf import settings +from requests import RequestException from help_to_heat import portal from help_to_heat.utils import Entity, Interface, register_event, with_schema @@ -37,6 +39,8 @@ from .os_api import OSApi, ThrottledApiException from .routing import calculate_journey +logger = logging.getLogger(__name__) + class SaveAnswerSchema(marshmallow.Schema): session_id = marshmallow.fields.UUID() @@ -613,9 +617,45 @@ def get_address_and_epc_lmk(self, building_name_or_number, postcode): def get_epc(self, lmk): epc_api = EPCApi() - epc_details = epc_api.get_epc_details(lmk) - epc_recommendations = epc_api.get_epc_recommendations(lmk) - return epc_details, epc_recommendations + epc_details_response = self._get_epc_details(epc_api, lmk) + epc_recommendations = self._get_epc_recommendations(epc_api, lmk) + return ( + epc_details_response, + epc_recommendations, + ) + + def _get_epc_details(self, epc_api, lmk): + return epc_api.get_epc_details(lmk)["rows"][0] + + RECOMMENDATION_RETRIES_COUNT = 5 + + def _get_epc_recommendations(self, epc_api, lmk): + delay = 0.5 + for i in range(self.RECOMMENDATION_RETRIES_COUNT): + try: + return epc_api.get_epc_recommendations(lmk)["rows"] + except RequestException as requestException: + match requestException.response.status_code: + case 500: + # if on the last try + if i == self.RECOMMENDATION_RETRIES_COUNT - 1: + # raise this to logs, though let user continue with no recommendations + logger.exception(requestException) + return [] + else: + # exponential backoff + time.sleep(delay) + delay = delay * 1.5 + + # else if not on last try, try again + continue + + # 404 is confirmed to mean no recommendations, so this is e.b. and so can return an empty list + case 404: + return [] + + # re-raise any other status codes + raise requestException class Feedback(Entity): diff --git a/help_to_heat/frontdoor/views.py b/help_to_heat/frontdoor/views.py index 228b057c..736210da 100644 --- a/help_to_heat/frontdoor/views.py +++ b/help_to_heat/frontdoor/views.py @@ -683,9 +683,7 @@ def save_post_data(self, data, session_id, page_name): return data try: - epc_details_response, epc_recommendations_response = interface.api.epc.get_epc(lmk) - epc_details = epc_details_response["rows"][0] - recommendations = epc_recommendations_response["rows"] + epc_details, recommendations = interface.api.epc.get_epc(lmk) except Exception as e: # noqa: B902 logger.exception(f"An error occurred: {e}") reset_epc_details(session_id) diff --git a/tests/test_interface.py b/tests/test_interface.py index 99895ff3..e4003488 100644 --- a/tests/test_interface.py +++ b/tests/test_interface.py @@ -68,7 +68,7 @@ def test_get_address(): def test_get_epc(): assert interface.api.epc.get_address_and_epc_lmk("22", "FL23 4JA") found_epc, _ = interface.api.epc.get_epc("1111111111111111111111111111111111") - assert found_epc["rows"][0].get("current-energy-rating").upper() == "G" + assert found_epc.get("current-energy-rating").upper() == "G" @unittest.mock.patch("help_to_heat.frontdoor.interface.EPCApi", MockNotFoundEPCApi) From 5ea6d68beeda97566f9f2c39d2071f3804681d39 Mon Sep 17 00:00:00 2001 From: Samuel Young Date: Mon, 25 Nov 2024 15:18:09 +0000 Subject: [PATCH 2/7] PC-1429: add tests for when such responses are thrown --- help_to_heat/frontdoor/mock_epc_api.py | 27 +++++++ tests/test_frontdoor.py | 100 +++++++++++++++++++++++++ 2 files changed, 127 insertions(+) diff --git a/help_to_heat/frontdoor/mock_epc_api.py b/help_to_heat/frontdoor/mock_epc_api.py index 0513aee2..caa54e15 100644 --- a/help_to_heat/frontdoor/mock_epc_api.py +++ b/help_to_heat/frontdoor/mock_epc_api.py @@ -56,6 +56,27 @@ def get_epc_recommendations(self, lmk_key): raise NotFoundRequestException() +class MockRecommendationsNotFoundEPCApi(MockEPCApi): + def get_epc_recommendations(self, lmk_key): + raise NotFoundRequestException() + + +class MockRecommendationsInternalServerErrorEPCApi(MockEPCApi): + def get_epc_recommendations(self, lmk_key): + raise InternalServerErrorRequestException() + + +class MockRecommendationsTransientInternalServerErrorEPCApi(MockEPCApi): + number_of_calls = 0 + + def get_epc_recommendations(self, lmk_key): + self.number_of_calls += 1 + if self.number_of_calls <= 2: + raise InternalServerErrorRequestException() + + return super().get_epc_recommendations(lmk_key) + + class UnauthorizedRequestException(requests.exceptions.RequestException): def __init__(self): self.response = requests.Response() @@ -66,3 +87,9 @@ class NotFoundRequestException(requests.exceptions.RequestException): def __init__(self): self.response = requests.Response() self.response.status_code = HTTPStatus.NOT_FOUND + + +class InternalServerErrorRequestException(requests.exceptions.RequestException): + def __init__(self): + self.response = requests.Response() + self.response.status_code = HTTPStatus.INTERNAL_SERVER_ERROR diff --git a/tests/test_frontdoor.py b/tests/test_frontdoor.py index 51739c06..cbc388f8 100644 --- a/tests/test_frontdoor.py +++ b/tests/test_frontdoor.py @@ -8,13 +8,22 @@ from help_to_heat.frontdoor import interface from help_to_heat.frontdoor.consts import ( address_all_address_and_lmk_details_field, + address_building_name_or_number_field, address_page, + address_postcode_field, + country_field, country_page, + lmk_field, + recommendations_field, + supplier_field, ) from help_to_heat.frontdoor.mock_epc_api import ( MockEPCApi, MockEPCApiWithEPCC, MockNotFoundEPCApi, + MockRecommendationsInternalServerErrorEPCApi, + MockRecommendationsNotFoundEPCApi, + MockRecommendationsTransientInternalServerErrorEPCApi, ) from help_to_heat.frontdoor.mock_os_api import EmptyOSApi, MockOSApi from help_to_heat.portal import models @@ -2157,6 +2166,97 @@ def test_epc_select_only_shows_most_recent_epc_per_uprn(): assert data[address_all_address_and_lmk_details_field][1]["lmk-key"] != "3333333333333333333333333333333333" +@unittest.mock.patch("help_to_heat.frontdoor.interface.EPCApi", MockRecommendationsNotFoundEPCApi) +def test_on_recommendations_not_found_response_recommendations_are_empty_list(): + _get_empty_recommendations_journey() + + +@unittest.mock.patch("help_to_heat.frontdoor.interface.EPCApi", MockRecommendationsInternalServerErrorEPCApi) +def test_on_recommendations_internal_server_error_response_recommendations_are_empty_list(): + _get_empty_recommendations_journey() + + +def _get_empty_recommendations_journey(): + client = utils.get_client() + page = client.get("/start") + assert page.status_code == 302 + page = page.follow() + + assert page.status_code == 200 + session_id = page.path.split("/")[1] + assert uuid.UUID(session_id) + _check_page = _make_check_page(session_id) + + form = page.get_form() + form[country_field] = "England" + page = form.submit().follow() + + form = page.get_form() + form[supplier_field] = "Utilita" + page = form.submit().follow() + + assert page.has_text("Do you own the property?") + page = _check_page(page, "own-property", "own_property", "Yes, I own my property and live in it") + + assert page.has_text("Do you live in a park home") + page = _check_page(page, "park-home", "park_home", "No") + + form = page.get_form() + form[address_building_name_or_number_field] = "22" + form[address_postcode_field] = "FL23 4JA" + page = form.submit().follow() + + form = page.get_form() + form[lmk_field] = "222222222222222222222222222222222" + page = form.submit().follow() + + data = interface.api.session.get_answer(session_id, page_name="epc-select") + assert data[recommendations_field] == [] + + assert page.has_one("h1:contains('What is the council tax band of your property?')") + + +@unittest.mock.patch("help_to_heat.frontdoor.interface.EPCApi", MockRecommendationsTransientInternalServerErrorEPCApi) +def test_on_recommendations_transient_internal_server_error_response_recommendations_are_empty_list(): + client = utils.get_client() + page = client.get("/start") + assert page.status_code == 302 + page = page.follow() + + assert page.status_code == 200 + session_id = page.path.split("/")[1] + assert uuid.UUID(session_id) + _check_page = _make_check_page(session_id) + + form = page.get_form() + form[country_field] = "England" + page = form.submit().follow() + + form = page.get_form() + form[supplier_field] = "Utilita" + page = form.submit().follow() + + assert page.has_text("Do you own the property?") + page = _check_page(page, "own-property", "own_property", "Yes, I own my property and live in it") + + assert page.has_text("Do you live in a park home") + page = _check_page(page, "park-home", "park_home", "No") + + form = page.get_form() + form[address_building_name_or_number_field] = "22" + form[address_postcode_field] = "FL23 4JA" + page = form.submit().follow() + + form = page.get_form() + form[lmk_field] = "222222222222222222222222222222222" + page = form.submit().follow() + + assert page.has_one("h1:contains('What is the council tax band of your property?')") + page = _check_page(page, "council-tax-band", "council_tax_band", "B") + + assert page.has_one("h1:contains('We found an Energy Performance Certificate that might be yours')") + + @unittest.mock.patch("help_to_heat.frontdoor.interface.EPCApi", MockEPCApi) def test_success_page_still_shows_if_journey_cannot_reach_it(): supplier = "Utilita" From f0d93f5da68c7c79e4ed4d6d447aec22b4590189 Mon Sep 17 00:00:00 2001 From: Samuel Young Date: Mon, 25 Nov 2024 15:18:31 +0000 Subject: [PATCH 3/7] PC-1429: bump black targetted python version to 3.11 this allows it to parse match syntax --- README.md | 2 +- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 99f10766..9d5c21e4 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ Formerly known as "Help to Heat". ### (Optional) Poetry Setup If you want to be able to make changes to the dependency list using Poetry, or install the dependencies locally (so you can click into them in your IDE, or use autocomplete) perform these steps: 1. [Install Poetry](https://python-poetry.org/docs/) on your machine -2. [Install Python 3.9.13](https://www.python.org/downloads/release/python-3913/) +2. [Install Python 3.11.9](https://www.python.org/downloads/release/python-3119/) 3. Set up a virtual environment within the project for Poetry to manage your dependencies. A guide for Pycharm can be found [here](https://www.jetbrains.com/help/pycharm/poetry.html). ## Using Docker diff --git a/pyproject.toml b/pyproject.toml index 1257e1f9..0dd9cd50 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -63,7 +63,7 @@ phonenumbers = "^8.13.38" [tool.black] line-length = 120 -target-version = ['py38'] +target-version = ['py311'] [tool.isort] multi_line_output = 3 From 68c0c2b3694a3839a6cacbc88b71da2bd1f1ac85 Mon Sep 17 00:00:00 2001 From: Samuel Young Date: Mon, 25 Nov 2024 15:25:57 +0000 Subject: [PATCH 4/7] PC-1429: extract translations --- help_to_heat/locale/cy/LC_MESSAGES/django.po | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/help_to_heat/locale/cy/LC_MESSAGES/django.po b/help_to_heat/locale/cy/LC_MESSAGES/django.po index 87cee2c6..64b76806 100644 --- a/help_to_heat/locale/cy/LC_MESSAGES/django.po +++ b/help_to_heat/locale/cy/LC_MESSAGES/django.po @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: PACKAGE VERSION\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2024-11-25 11:59+0000\n" +"POT-Creation-Date: 2024-11-25 15:19+0000\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME \n" "Language-Team: LANGUAGE \n" @@ -671,7 +671,7 @@ msgstr "Fflat" msgid "Maisonette" msgstr "Fflat deulawr" -#: help_to_heat/frontdoor/views.py:667 help_to_heat/frontdoor/views.py:737 +#: help_to_heat/frontdoor/views.py:667 help_to_heat/frontdoor/views.py:735 msgid "I cannot find my address, I want to enter it manually" msgstr "Dwi'n methu gweld fy nghyfeiriad i. Hoffwn ei roi fy hunan." From f2b827dcfa45a3362138d45f010c51a9689b0c81 Mon Sep 17 00:00:00 2001 From: Samuel Young Date: Tue, 26 Nov 2024 11:10:36 +0000 Subject: [PATCH 5/7] PC-1425: strip building name & postcode before saving to session --- help_to_heat/frontdoor/views.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/help_to_heat/frontdoor/views.py b/help_to_heat/frontdoor/views.py index 736210da..8fe89f3b 100644 --- a/help_to_heat/frontdoor/views.py +++ b/help_to_heat/frontdoor/views.py @@ -619,8 +619,13 @@ def save_post_data(self, data, session_id, page_name): reset_epc_details(session_id) session_data = interface.api.session.get_session(session_id) country = session_data.get(country_field) - building_name_or_number = data.get(address_building_name_or_number_field) - postcode = data.get(address_postcode_field) + building_name_or_number = data.get(address_building_name_or_number_field).strip() + postcode = data.get(address_postcode_field).strip() + + # overwrite the answers with stripped ones + data[address_building_name_or_number_field] = building_name_or_number + data[address_postcode_field] = postcode + try: if country != country_field_scotland: address_and_lmk_details = interface.api.epc.get_address_and_epc_lmk(building_name_or_number, postcode) @@ -716,9 +721,9 @@ def save_post_data(self, data, session_id, page_name): @register_page(address_select_page) class AddressSelectView(PageView): def build_extra_context(self, request, session_id, page_name, data, is_change_page): - data = interface.api.session.get_answer(session_id, address_page) - building_name_or_number = data[address_building_name_or_number_field] - postcode = data[address_postcode_field] + address_data = interface.api.session.get_answer(session_id, address_page) + building_name_or_number = address_data[address_building_name_or_number_field] + postcode = address_data[address_postcode_field] addresses = interface.api.address.find_addresses(building_name_or_number, postcode) uprn_options = tuple( From 83dfbc620035e380c25f397ee12651914f3eb23d Mon Sep 17 00:00:00 2001 From: Samuel Young Date: Wed, 27 Nov 2024 14:04:21 +0000 Subject: [PATCH 6/7] PC-1425: add verifying test --- help_to_heat/frontdoor/mock_epc_api.py | 11 ++++++++ tests/test_frontdoor.py | 38 ++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/help_to_heat/frontdoor/mock_epc_api.py b/help_to_heat/frontdoor/mock_epc_api.py index caa54e15..746e64d0 100644 --- a/help_to_heat/frontdoor/mock_epc_api.py +++ b/help_to_heat/frontdoor/mock_epc_api.py @@ -77,6 +77,17 @@ def get_epc_recommendations(self, lmk_key): return super().get_epc_recommendations(lmk_key) +def get_mock_epc_api_expecting_address_and_postcode(check_address, check_postcode): + class MockEpcApiExpectingAddressAndPostcode(MockEPCApi): + def search_epc_details(self, address, postcode): + if address != check_address or postcode != check_postcode: + raise NotFoundRequestException() + else: + return super().search_epc_details(address, postcode) + + return MockEpcApiExpectingAddressAndPostcode + + class UnauthorizedRequestException(requests.exceptions.RequestException): def __init__(self): self.response = requests.Response() diff --git a/tests/test_frontdoor.py b/tests/test_frontdoor.py index cbc388f8..d346d5fe 100644 --- a/tests/test_frontdoor.py +++ b/tests/test_frontdoor.py @@ -24,6 +24,7 @@ MockRecommendationsInternalServerErrorEPCApi, MockRecommendationsNotFoundEPCApi, MockRecommendationsTransientInternalServerErrorEPCApi, + get_mock_epc_api_expecting_address_and_postcode, ) from help_to_heat.frontdoor.mock_os_api import EmptyOSApi, MockOSApi from help_to_heat.portal import models @@ -2271,6 +2272,43 @@ def test_success_page_still_shows_if_journey_cannot_reach_it(): assert page.has_one(f"h1:contains('Your details have been submitted to {supplier}')") +@unittest.mock.patch( + "help_to_heat.frontdoor.interface.EPCApi", get_mock_epc_api_expecting_address_and_postcode("22", "FL23 4JA") +) +def test_epc_api_is_called_with_trimmed_address_and_postcode(): + client = utils.get_client() + page = client.get("/start") + assert page.status_code == 302 + page = page.follow() + + assert page.status_code == 200 + session_id = page.path.split("/")[1] + assert uuid.UUID(session_id) + _check_page = _make_check_page(session_id) + + form = page.get_form() + form["country"] = "England" + page = form.submit().follow() + + form = page.get_form() + form["supplier"] = "Utilita" + page = form.submit().follow() + + assert page.has_text("Do you own the property?") + page = _check_page(page, "own-property", "own_property", "Yes, I own my property and live in it") + + assert page.has_text("Do you live in a park home") + page = _check_page(page, "park-home", "park_home", "No") + + form = page.get_form() + form["building_name_or_number"] = " 22 " + form["postcode"] = " FL23 4JA " + page = form.submit().follow() + + assert page.has_one("label:contains('22 Acacia Avenue, Upper Wellgood, Fulchester, FL23 4JA')") + assert page.has_one("label:contains('11 Acacia Avenue, Upper Wellgood, Fulchester, FL23 4JA')") + + def _setup_client_and_page(): client = utils.get_client() page = client.get("/start") From 6f12a02e437b4913ff54a78f35f056d599d77463 Mon Sep 17 00:00:00 2001 From: Samuel Young Date: Thu, 28 Nov 2024 09:48:34 +0000 Subject: [PATCH 7/7] PC-1425: extract translations --- help_to_heat/locale/cy/LC_MESSAGES/django.po | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/help_to_heat/locale/cy/LC_MESSAGES/django.po b/help_to_heat/locale/cy/LC_MESSAGES/django.po index 64b76806..1402ecdd 100644 --- a/help_to_heat/locale/cy/LC_MESSAGES/django.po +++ b/help_to_heat/locale/cy/LC_MESSAGES/django.po @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: PACKAGE VERSION\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2024-11-25 15:19+0000\n" +"POT-Creation-Date: 2024-11-28 09:50+0000\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME \n" "Language-Team: LANGUAGE \n" @@ -671,7 +671,7 @@ msgstr "Fflat" msgid "Maisonette" msgstr "Fflat deulawr" -#: help_to_heat/frontdoor/views.py:667 help_to_heat/frontdoor/views.py:735 +#: help_to_heat/frontdoor/views.py:672 help_to_heat/frontdoor/views.py:740 msgid "I cannot find my address, I want to enter it manually" msgstr "Dwi'n methu gweld fy nghyfeiriad i. Hoffwn ei roi fy hunan."