diff --git a/domain/abri/models.py b/domain/abri/models.py index e160c3e3b..0444ec1cc 100644 --- a/domain/abri/models.py +++ b/domain/abri/models.py @@ -1,6 +1,6 @@ from dataclasses import dataclass from datetime import date -from typing import Literal, NewType, Union +from typing import Literal, NewType, Optional, Union PlaceRef = NewType("PlaceRef", str) @@ -30,3 +30,21 @@ class AbriRequestRejected: LogJobResult = Union[JobLogged, AbriRequestRejected] + + +@dataclass(frozen=True) +class AmendJobRequest: + job_no: str + appointment_date: date + appointment_time: SlotCode + resource: Optional[str] = None + + +@dataclass(frozen=True) +class AppointmentAmended: + job_no: str + appointment_date: str + appointment_time: str + + +AmendJobResult = Union[AppointmentAmended, AbriRequestRejected] diff --git a/etl/hubspot/hubspot_deal_differ.py b/etl/hubspot/hubspot_deal_differ.py index b693cf7a4..977dc3e4d 100644 --- a/etl/hubspot/hubspot_deal_differ.py +++ b/etl/hubspot/hubspot_deal_differ.py @@ -197,7 +197,7 @@ class HubspotDealDiffer: return parse_hs_date(new_deal.get("confirmed_survey_date")) is not None @staticmethod - def check_for_abri_survey_amendment( + def check_for_abri_job_amendment( new_deal: Dict[str, str], new_project: Optional[ProjectData], old_deal: HubspotDealData, @@ -209,6 +209,10 @@ class HubspotDealDiffer: if old_deal.confirmed_survey_date is None: return False + # TODO: a cleared survey date (was set, now empty) still fires this + # check, but the amendoptiappt route cannot express a cancellation. + # Cancellation semantics (the "delete" action / canceljob route) need + # a deliberate decision before this trigger is wired to a consumer. new_survey_date = parse_hs_date(new_deal.get("confirmed_survey_date")) if old_deal.confirmed_survey_date != new_survey_date: return True diff --git a/etl/hubspot/tests/test_hubspot_deal_differ.py b/etl/hubspot/tests/test_hubspot_deal_differ.py index 08e6c78a1..c3e24e08f 100644 --- a/etl/hubspot/tests/test_hubspot_deal_differ.py +++ b/etl/hubspot/tests/test_hubspot_deal_differ.py @@ -548,12 +548,12 @@ def test_abri_job_logging__non_abri_project_id_with_abri_code__returns_false() - assert result is False -# =================================== -# ABRI SURVEY AMENDMENT TRIGGER TESTS -# =================================== +# ================================ +# ABRI JOB AMENDMENT TRIGGER TESTS +# ================================ -def test_abri_survey_amendment__date_changed_on_abri_deal__returns_true() -> None: +def test_abri_job_amendment__date_changed_on_abri_deal__returns_true() -> None: deal_id = uuid.uuid4() # Arrange @@ -569,7 +569,7 @@ def test_abri_survey_amendment__date_changed_on_abri_deal__returns_true() -> Non ) # Act - result = HubspotDealDiffer.check_for_abri_survey_amendment( + result = HubspotDealDiffer.check_for_abri_job_amendment( new_deal=new_deal, new_project=None, old_deal=old_deal, @@ -579,7 +579,7 @@ def test_abri_survey_amendment__date_changed_on_abri_deal__returns_true() -> Non assert result is True -def test_abri_survey_amendment__non_abri_deal__returns_false() -> None: +def test_abri_job_amendment__non_abri_deal__returns_false() -> None: deal_id = uuid.uuid4() # Arrange @@ -595,7 +595,7 @@ def test_abri_survey_amendment__non_abri_deal__returns_false() -> None: ) # Act - result = HubspotDealDiffer.check_for_abri_survey_amendment( + result = HubspotDealDiffer.check_for_abri_job_amendment( new_deal=new_deal, new_project=None, old_deal=old_deal, @@ -605,7 +605,7 @@ def test_abri_survey_amendment__non_abri_deal__returns_false() -> None: assert result is False -def test_abri_survey_amendment__date_and_time_unchanged__returns_false() -> None: +def test_abri_job_amendment__date_and_time_unchanged__returns_false() -> None: deal_id = uuid.uuid4() # Arrange @@ -623,7 +623,7 @@ def test_abri_survey_amendment__date_and_time_unchanged__returns_false() -> None ) # Act - result = HubspotDealDiffer.check_for_abri_survey_amendment( + result = HubspotDealDiffer.check_for_abri_job_amendment( new_deal=new_deal, new_project=None, old_deal=old_deal, @@ -633,7 +633,7 @@ def test_abri_survey_amendment__date_and_time_unchanged__returns_false() -> None assert result is False -def test_abri_survey_amendment__time_changed_on_abri_deal__returns_true() -> None: +def test_abri_job_amendment__time_changed_on_abri_deal__returns_true() -> None: deal_id = uuid.uuid4() # Arrange @@ -651,7 +651,7 @@ def test_abri_survey_amendment__time_changed_on_abri_deal__returns_true() -> Non ) # Act - result = HubspotDealDiffer.check_for_abri_survey_amendment( + result = HubspotDealDiffer.check_for_abri_job_amendment( new_deal=new_deal, new_project=None, old_deal=old_deal, @@ -661,7 +661,7 @@ def test_abri_survey_amendment__time_changed_on_abri_deal__returns_true() -> Non assert result is True -def test_abri_survey_amendment__date_first_set_on_abri_deal__returns_false() -> None: +def test_abri_job_amendment__date_first_set_on_abri_deal__returns_false() -> None: deal_id = uuid.uuid4() # Arrange @@ -677,7 +677,7 @@ def test_abri_survey_amendment__date_first_set_on_abri_deal__returns_false() -> ) # Act - result = HubspotDealDiffer.check_for_abri_survey_amendment( + result = HubspotDealDiffer.check_for_abri_job_amendment( new_deal=new_deal, new_project=None, old_deal=old_deal, diff --git a/infrastructure/abri/abri_client.py b/infrastructure/abri/abri_client.py index 72a3c4767..3c95c2766 100644 --- a/infrastructure/abri/abri_client.py +++ b/infrastructure/abri/abri_client.py @@ -1,10 +1,14 @@ import xml.etree.ElementTree as ET +from datetime import date from typing import List, Tuple, Union import requests from domain.abri.models import ( AbriRequestRejected, + AmendJobRequest, + AmendJobResult, + AppointmentAmended, JobLogged, LogJobRequest, LogJobResult, @@ -18,6 +22,10 @@ CLIENT_CODE = "HSG" RESOURCE_GROUP = "Surveyors" +def _format_appointment_date(appointment_date: date) -> str: + return appointment_date.strftime("%d/%m/%Y") + + class AbriClient: def __init__(self, config: AbriConfig) -> None: self._config = config @@ -33,7 +41,7 @@ class AbriClient: ("short_description", request.short_description), ("long_description", request.long_description), ("client_ref", request.client_ref), - ("appointment_date", request.appointment_date.strftime("%d/%m/%Y")), + ("appointment_date", _format_appointment_date(request.appointment_date)), ("appointment_time", request.appointment_time), ("resource", self._config.default_resource), ("resource_group", RESOURCE_GROUP), @@ -45,6 +53,26 @@ class AbriClient: return self._parse_job_logged(outcome) + def amend_job(self, request: AmendJobRequest) -> AmendJobResult: + parameters = [ + ("job_no", request.job_no), + ("action", "amend"), + ("appointment_date", _format_appointment_date(request.appointment_date)), + ("appointment_time", request.appointment_time), + ] + if request.resource is not None: + parameters.append(("resource", request.resource)) + + outcome = self._exchange( + request_type="amendoptiappt", + parameters=parameters, + ) + + if isinstance(outcome, AbriRequestRejected): + return outcome + + return self._parse_appointment_amended(outcome) + def _exchange( self, request_type: str, parameters: List[Tuple[str, str]] ) -> Union[ET.Element, AbriRequestRejected]: @@ -101,6 +129,36 @@ class AbriClient: return JobLogged(job_no=job_no, logged_info=logged_info) + @staticmethod + def _parse_appointment_amended(root: ET.Element) -> AppointmentAmended: + amended = root.find("AmendOptiAppt") + + if amended is None: + raise AbriResponseParseError( + "AmendOptiAppt element missing from relay response" + ) + + if amended.get("success") != "Y": + raise AbriResponseParseError( + "AmendOptiAppt element did not confirm success" + ) + + job_no = amended.get("job_no") + appointment_date = amended.get("appointment_date") + appointment_time = amended.get("appointment_time") + + if job_no is None or appointment_date is None or appointment_time is None: + raise AbriResponseParseError( + "AmendOptiAppt element missing job_no, appointment_date " + "or appointment_time" + ) + + return AppointmentAmended( + job_no=job_no, + appointment_date=appointment_date, + appointment_time=appointment_time, + ) + @staticmethod def _parse_rejection(root: ET.Element) -> AbriRequestRejected: success = root.findtext("success") diff --git a/tests/abri/abri_relay_amendoptiappt_relayerror_response.xml b/tests/abri/abri_relay_amendoptiappt_relayerror_response.xml new file mode 100644 index 000000000..225563172 --- /dev/null +++ b/tests/abri/abri_relay_amendoptiappt_relayerror_response.xml @@ -0,0 +1,6 @@ + + + false + RelayError + Job_no 1019905 is invalid. Job not found. + diff --git a/tests/abri/abri_relay_amendoptiappt_request_example.xml b/tests/abri/abri_relay_amendoptiappt_request_example.xml new file mode 100644 index 000000000..10c9b8941 --- /dev/null +++ b/tests/abri/abri_relay_amendoptiappt_request_example.xml @@ -0,0 +1,15 @@ + + +
+ +
+ + + + + + + + + + diff --git a/tests/abri/abri_relay_amendoptiappt_success_response.xml b/tests/abri/abri_relay_amendoptiappt_success_response.xml new file mode 100644 index 000000000..def7734c3 --- /dev/null +++ b/tests/abri/abri_relay_amendoptiappt_success_response.xml @@ -0,0 +1,3 @@ + + + diff --git a/tests/infrastructure/abri/test_abri_client.py b/tests/infrastructure/abri/test_abri_client.py index 133c74d8f..6ab6c7698 100644 --- a/tests/infrastructure/abri/test_abri_client.py +++ b/tests/infrastructure/abri/test_abri_client.py @@ -8,6 +8,8 @@ import requests from domain.abri.models import ( AbriRequestRejected, + AmendJobRequest, + AppointmentAmended, JobLogged, LogJobRequest, PlaceRef, @@ -50,6 +52,14 @@ def _spec_example_request() -> LogJobRequest: ) +def _spec_example_amend_request() -> AmendJobRequest: + return AmendJobRequest( + job_no="AC0439951", + appointment_date=date(2026, 6, 19), + appointment_time="PM", + ) + + @pytest.fixture() def mock_session() -> MagicMock: return MagicMock() @@ -192,3 +202,149 @@ def test_log_job_raises_retriable_parse_error_on_an_unexpectedly_shaped_response # Act / Assert with pytest.raises(AbriResponseParseError): client.log_job(_spec_example_request()) + + +# --- amend_job: success --- + + +def test_amend_job_returns_appointment_amended_echoing_the_response_attributes( + client: AbriClient, mock_session: MagicMock +) -> None: + # Arrange + mock_session.post.return_value.content = _load_fixture( + "abri_relay_amendoptiappt_success_response.xml" + ) + + # Act + result = client.amend_job(_spec_example_amend_request()) + + # Assert + assert result == AppointmentAmended( + job_no="AC0439951", + appointment_date="24/06/2026", + appointment_time="PM", + ) + + +def test_amend_job_sends_the_recorded_amendoptiappt_envelope_to_the_relay_endpoint( + client: AbriClient, mock_session: MagicMock +) -> None: + # Arrange + mock_session.post.return_value.content = _load_fixture( + "abri_relay_amendoptiappt_success_response.xml" + ) + + # Act + client.amend_job( + AmendJobRequest( + job_no="AC0439951", + appointment_date=date(2026, 6, 19), + appointment_time="PM", + resource="NAULKH", + ) + ) + + # Assert + (url,) = mock_session.post.call_args.args + sent_body: bytes = mock_session.post.call_args.kwargs["data"] + expected_body = _load_fixture("abri_relay_amendoptiappt_request_example.xml") + assert url == ENDPOINT_URL + assert ET.canonicalize(xml_data=sent_body, strip_text=True) == ET.canonicalize( + xml_data=expected_body, strip_text=True + ) + + +def test_amend_job_omits_the_resource_parameter_when_no_surveyor_is_provided( + client: AbriClient, mock_session: MagicMock +) -> None: + # Arrange + mock_session.post.return_value.content = _load_fixture( + "abri_relay_amendoptiappt_success_response.xml" + ) + + # Act + client.amend_job(_spec_example_amend_request()) + + # Assert + sent_body: bytes = mock_session.post.call_args.kwargs["data"] + sent_attributes = [ + parameter.get("attribute") + for parameter in ET.fromstring(sent_body).findall(".//Parameters") + ] + assert "resource" not in sent_attributes + + +# --- amend_job: rejection --- + + +def test_amend_job_returns_rejection_with_openhousing_error_code_and_message_verbatim( + client: AbriClient, mock_session: MagicMock +) -> None: + # Arrange + mock_session.post.return_value.content = _load_fixture( + "abri_relay_amendoptiappt_relayerror_response.xml" + ) + + # Act + result = client.amend_job(_spec_example_amend_request()) + + # Assert + assert result == AbriRequestRejected( + code="RelayError", + message="Job_no 1019905 is invalid. Job not found.", + ) + + +# --- amend_job: ambiguous responses are retriable, never success or rejection --- + + +@pytest.mark.parametrize( + "body", + [ + b'', + b'', + b'', + b"", + b"", + ], +) +def test_amend_job_raises_retriable_parse_error_when_the_amendment_is_unconfirmed( + client: AbriClient, mock_session: MagicMock, body: bytes +) -> None: + # Arrange + mock_session.post.return_value.content = body + + # Act / Assert + with pytest.raises(AbriResponseParseError): + client.amend_job(_spec_example_amend_request()) + + +# --- amend_job: transport failures --- + + +def test_amend_job_raises_transport_error_on_http_error_status( + client: AbriClient, mock_session: MagicMock +) -> None: + # Arrange + mock_session.post.return_value.content = b"Server Error" + mock_session.post.return_value.raise_for_status.side_effect = requests.HTTPError( + "500 Server Error" + ) + + # Act / Assert + with pytest.raises(AbriTransportError): + client.amend_job(_spec_example_amend_request()) + + +def test_amend_job_raises_transport_error_when_the_relay_is_unreachable( + client: AbriClient, mock_session: MagicMock +) -> None: + # Arrange + mock_session.post.side_effect = requests.ConnectionError("connection refused") + + # Act / Assert + with pytest.raises(AbriTransportError): + client.amend_job(_spec_example_amend_request())