From 98674ae895fc356d2a621c94f141146ac86c34bf Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 15:50:07 +0000 Subject: [PATCH 01/17] =?UTF-8?q?Log=20an=20OpenHousing=20job=20from=20a?= =?UTF-8?q?=20confirmed=20survey=20booking's=20deal=20fields=20?= =?UTF-8?q?=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../hubspot/deal_properties_client.py | 18 +++ orchestration/abri_log_job_orchestrator.py | 82 ++++++++++++ .../test_abri_log_job_orchestrator.py | 121 ++++++++++++++++++ 3 files changed, 221 insertions(+) create mode 100644 infrastructure/hubspot/deal_properties_client.py create mode 100644 orchestration/abri_log_job_orchestrator.py create mode 100644 tests/orchestration/test_abri_log_job_orchestrator.py diff --git a/infrastructure/hubspot/deal_properties_client.py b/infrastructure/hubspot/deal_properties_client.py new file mode 100644 index 000000000..eb2e132d4 --- /dev/null +++ b/infrastructure/hubspot/deal_properties_client.py @@ -0,0 +1,18 @@ +from hubspot.client import Client # type: ignore[reportMissingTypeStubs] + + +class HubspotDealPropertiesClient: + """Focused HubSpot client for writing single deal properties. + + Deliberately separate from the legacy HubspotClient (frozen for the + refactor tracked in issue #1473): the SDK client is constructor-injected, + and SDK errors are re-raised scrubbed of response bodies. + """ + + def __init__(self, sdk_client: Client) -> None: + self._client: Client = sdk_client + + def write_deal_property( + self, deal_id: str, property_name: str, value: str + ) -> None: + raise NotImplementedError diff --git a/orchestration/abri_log_job_orchestrator.py b/orchestration/abri_log_job_orchestrator.py new file mode 100644 index 000000000..b31cfe1c3 --- /dev/null +++ b/orchestration/abri_log_job_orchestrator.py @@ -0,0 +1,82 @@ +from dataclasses import dataclass +from datetime import date +from typing import Optional, Protocol, Union + +from domain.abri.models import AbriRequestRejected, PlaceRef +from infrastructure.abri.abri_client import AbriClient +from infrastructure.hubspot.deal_properties_client import HubspotDealPropertiesClient + +# The HubSpot deal property that carries the OpenHousing job number; the +# property ID is fixed externally, the domain and database call it job_no. +JOB_NO_DEAL_PROPERTY = "client_booking_reference" + + +@dataclass(frozen=True) +class ConfirmedSurveyBooking: + """The deal fields a confirmed survey booking contributes to a LogJob.""" + + deal_id: str + place_ref: PlaceRef + deal_name: str + confirmed_survey_date: date + confirmed_survey_time: Optional[str] + + +@dataclass(frozen=True) +class LogJobSummary: + """PII-free record of a logged job: safe to log and persist.""" + + deal_id: str + job_no: str + + +LogJobOrchestrationResult = Union[LogJobSummary, AbriRequestRejected] + + +class DealDatabaseGateway(Protocol): + def record_job_no(self, deal_id: str, job_no: str) -> None: ... + + +class LogJobWriteBackError(Exception): + """A job logged in OpenHousing whose job_no could not be written back. + + Carries the orphaned job_no so an operator can paste it into HubSpot + manually; the run must never retry log_job, which would log a duplicate + job in OpenHousing. + """ + + def __init__(self, message: str, deal_id: str, job_no: str) -> None: + self.deal_id = deal_id + self.job_no = job_no + super().__init__(f"{message} (deal: {deal_id}, orphaned job_no: {job_no})") + + +def client_ref_for_deal(deal_id: str) -> str: + """The client_ref sent to OpenHousing: the HubSpot deal ID, for now. + + UNCONFIRMED with Abri: whether client_ref must be unique per job (a deal + re-logging after a cancellation would repeat the bare deal ID) is an open + question on issue #1478. Swap the derivation here once Abri answers. + """ + raise NotImplementedError + + +class AbriLogJobOrchestrator: + """Logs an OpenHousing job for a deal and records the returned job_no. + + HubSpot is written first (it is the source of truth); the database + second, so a failed database write self-heals on the next scrape. + """ + + def __init__( + self, + abri_client: AbriClient, + hubspot: HubspotDealPropertiesClient, + deal_database: DealDatabaseGateway, + ) -> None: + self._abri = abri_client + self._hubspot = hubspot + self._deal_database = deal_database + + def run(self, booking: ConfirmedSurveyBooking) -> LogJobOrchestrationResult: + raise NotImplementedError diff --git a/tests/orchestration/test_abri_log_job_orchestrator.py b/tests/orchestration/test_abri_log_job_orchestrator.py new file mode 100644 index 000000000..afdc02f39 --- /dev/null +++ b/tests/orchestration/test_abri_log_job_orchestrator.py @@ -0,0 +1,121 @@ +import xml.etree.ElementTree as ET +from datetime import date +from pathlib import Path +from typing import List, Tuple +from unittest.mock import MagicMock, patch + +import pytest + +from domain.abri.models import PlaceRef +from infrastructure.abri.abri_client import AbriClient +from infrastructure.abri.config import AbriConfig +from infrastructure.hubspot.deal_properties_client import HubspotDealPropertiesClient +from orchestration.abri_log_job_orchestrator import ( + AbriLogJobOrchestrator, + ConfirmedSurveyBooking, +) + +FIXTURE_DIR = Path(__file__).parents[1] / "abri" +ENDPOINT_URL = "https://relay.example.test/api/DomnaRelay?code=test-function-key" +DEAL_ID = "9876543210" + +CONFIG = AbriConfig( + endpoint_url=ENDPOINT_URL, + username="DomnaWeb", + password="", + default_resource="NAULKH", +) + +BOOKING = ConfirmedSurveyBooking( + deal_id=DEAL_ID, + place_ref=PlaceRef("1007165"), + deal_name="49 Admers Crescent", + confirmed_survey_date=date(2026, 6, 18), + confirmed_survey_time="14:30", +) + + +def _load_fixture(name: str) -> bytes: + return (FIXTURE_DIR / name).read_bytes() + + +class FakeDealDatabase: + """In-memory stand-in for the deal-database gateway.""" + + def __init__(self) -> None: + self.recorded_job_nos: List[Tuple[str, str]] = [] + + def record_job_no(self, deal_id: str, job_no: str) -> None: + self.recorded_job_nos.append((deal_id, job_no)) + + +@pytest.fixture() +def mock_session() -> MagicMock: + return MagicMock() + + +@pytest.fixture() +def sdk_client() -> MagicMock: + return MagicMock() + + +@pytest.fixture() +def deal_database() -> FakeDealDatabase: + return FakeDealDatabase() + + +@pytest.fixture() +def orchestrator( + mock_session: MagicMock, + sdk_client: MagicMock, + deal_database: FakeDealDatabase, +) -> AbriLogJobOrchestrator: + with patch( + "infrastructure.abri.abri_client.requests.Session", + return_value=mock_session, + ): + abri_client = AbriClient(config=CONFIG) + return AbriLogJobOrchestrator( + abri_client=abri_client, + hubspot=HubspotDealPropertiesClient(sdk_client=sdk_client), + deal_database=deal_database, + ) + + +# --- outbound request: the LogJob envelope is built from the deal's fields --- + + +def test_run_sends_a_logjob_envelope_built_from_the_deals_fields( + orchestrator: AbriLogJobOrchestrator, mock_session: MagicMock +) -> None: + # Arrange + mock_session.post.return_value.content = _load_fixture( + "abri_relay_logjob_success_response.xml" + ) + + # Act + orchestrator.run(BOOKING) + + # Assert + (url,) = mock_session.post.call_args.args + sent_body: bytes = mock_session.post.call_args.kwargs["data"] + sent_parameters = { + parameter.get("attribute"): parameter.get("attribute_value") + for parameter in ET.fromstring(sent_body).findall("Body/Request/Parameters") + } + assert url == ENDPOINT_URL + assert sent_parameters == { + "place_ref": "1007165", + "std_job_code": "SCSEXT", + "client": "HSG", + "short_description": "Domna condition survey - 49 Admers Crescent", + "long_description": ( + "Domna condition survey visit at 49 Admers Crescent, " + "booked by Domna operations." + ), + "client_ref": DEAL_ID, + "appointment_date": "18/06/2026", + "appointment_time": "PM", + "resource": "NAULKH", + "resource_group": "Surveyors", + } From ce7b302c1790ce69f19ea99bb444e6a2268c7523 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 15:51:21 +0000 Subject: [PATCH 02/17] =?UTF-8?q?Log=20an=20OpenHousing=20job=20from=20a?= =?UTF-8?q?=20confirmed=20survey=20booking's=20deal=20fields=20?= =?UTF-8?q?=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- orchestration/abri_log_job_orchestrator.py | 25 +++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/orchestration/abri_log_job_orchestrator.py b/orchestration/abri_log_job_orchestrator.py index b31cfe1c3..b80d39bf5 100644 --- a/orchestration/abri_log_job_orchestrator.py +++ b/orchestration/abri_log_job_orchestrator.py @@ -2,7 +2,9 @@ from dataclasses import dataclass from datetime import date from typing import Optional, Protocol, Union -from domain.abri.models import AbriRequestRejected, PlaceRef +from domain.abri.descriptions import build_job_descriptions +from domain.abri.models import AbriRequestRejected, LogJobRequest, PlaceRef +from domain.abri.slots import slot_for_confirmed_time from infrastructure.abri.abri_client import AbriClient from infrastructure.hubspot.deal_properties_client import HubspotDealPropertiesClient @@ -58,7 +60,7 @@ def client_ref_for_deal(deal_id: str) -> str: re-logging after a cancellation would repeat the bare deal ID) is an open question on issue #1478. Swap the derivation here once Abri answers. """ - raise NotImplementedError + return deal_id class AbriLogJobOrchestrator: @@ -79,4 +81,21 @@ class AbriLogJobOrchestrator: self._deal_database = deal_database def run(self, booking: ConfirmedSurveyBooking) -> LogJobOrchestrationResult: - raise NotImplementedError + descriptions = build_job_descriptions(booking.deal_name) + outcome = self._abri.log_job( + LogJobRequest( + client_ref=client_ref_for_deal(booking.deal_id), + place_ref=booking.place_ref, + appointment_date=booking.confirmed_survey_date, + appointment_time=slot_for_confirmed_time( + booking.confirmed_survey_time + ), + short_description=descriptions.short_description, + long_description=descriptions.long_description, + ) + ) + + if isinstance(outcome, AbriRequestRejected): + return outcome + + return LogJobSummary(deal_id=booking.deal_id, job_no=outcome.job_no) From 6912dabf2211a95dab657cd90b5dc2329c3dedf9 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 15:52:35 +0000 Subject: [PATCH 03/17] =?UTF-8?q?A=20logged=20job's=20job=5Fno=20is=20writ?= =?UTF-8?q?ten=20to=20HubSpot=20and=20then=20the=20deal=20database=20?= =?UTF-8?q?=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../test_abri_log_job_orchestrator.py | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/orchestration/test_abri_log_job_orchestrator.py b/tests/orchestration/test_abri_log_job_orchestrator.py index afdc02f39..2cfb265f8 100644 --- a/tests/orchestration/test_abri_log_job_orchestrator.py +++ b/tests/orchestration/test_abri_log_job_orchestrator.py @@ -5,6 +5,7 @@ from typing import List, Tuple from unittest.mock import MagicMock, patch import pytest +from hubspot.crm.deals import SimplePublicObjectInput # type: ignore[reportMissingTypeStubs] from domain.abri.models import PlaceRef from infrastructure.abri.abri_client import AbriClient @@ -13,6 +14,7 @@ from infrastructure.hubspot.deal_properties_client import HubspotDealPropertiesC from orchestration.abri_log_job_orchestrator import ( AbriLogJobOrchestrator, ConfirmedSurveyBooking, + LogJobSummary, ) FIXTURE_DIR = Path(__file__).parents[1] / "abri" @@ -119,3 +121,31 @@ def test_run_sends_a_logjob_envelope_built_from_the_deals_fields( "resource": "NAULKH", "resource_group": "Surveyors", } + + +# --- happy path: the returned job_no lands in HubSpot, then the database --- + + +def test_run_writes_the_job_no_to_the_hubspot_deal_and_then_the_database( + orchestrator: AbriLogJobOrchestrator, + mock_session: MagicMock, + sdk_client: MagicMock, + deal_database: FakeDealDatabase, +) -> None: + # Arrange + mock_session.post.return_value.content = _load_fixture( + "abri_relay_logjob_success_response.xml" + ) + + # Act + result = orchestrator.run(BOOKING) + + # Assert + assert result == LogJobSummary(deal_id=DEAL_ID, job_no="AD0226519") + sdk_client.crm.deals.basic_api.update.assert_called_once_with( + DEAL_ID, + simple_public_object_input=SimplePublicObjectInput( + properties={"client_booking_reference": "AD0226519"} + ), + ) + assert deal_database.recorded_job_nos == [(DEAL_ID, "AD0226519")] From 96f86aacac05c7a6f0aa4ad1d1c53dc54e3e26d6 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 15:54:05 +0000 Subject: [PATCH 04/17] =?UTF-8?q?A=20logged=20job's=20job=5Fno=20is=20writ?= =?UTF-8?q?ten=20to=20HubSpot=20and=20then=20the=20deal=20database=20?= =?UTF-8?q?=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- infrastructure/hubspot/deal_properties_client.py | 8 +++++++- orchestration/abri_log_job_orchestrator.py | 9 +++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/infrastructure/hubspot/deal_properties_client.py b/infrastructure/hubspot/deal_properties_client.py index eb2e132d4..24cf5fca8 100644 --- a/infrastructure/hubspot/deal_properties_client.py +++ b/infrastructure/hubspot/deal_properties_client.py @@ -1,4 +1,5 @@ from hubspot.client import Client # type: ignore[reportMissingTypeStubs] +from hubspot.crm.deals import SimplePublicObjectInput # type: ignore[reportMissingTypeStubs] class HubspotDealPropertiesClient: @@ -15,4 +16,9 @@ class HubspotDealPropertiesClient: def write_deal_property( self, deal_id: str, property_name: str, value: str ) -> None: - raise NotImplementedError + self._client.crm.deals.basic_api.update( # type: ignore[reportUnknownMemberType] + deal_id, + simple_public_object_input=SimplePublicObjectInput( + properties={property_name: value} + ), + ) diff --git a/orchestration/abri_log_job_orchestrator.py b/orchestration/abri_log_job_orchestrator.py index b80d39bf5..811d008af 100644 --- a/orchestration/abri_log_job_orchestrator.py +++ b/orchestration/abri_log_job_orchestrator.py @@ -98,4 +98,13 @@ class AbriLogJobOrchestrator: if isinstance(outcome, AbriRequestRejected): return outcome + self._hubspot.write_deal_property( + deal_id=booking.deal_id, + property_name=JOB_NO_DEAL_PROPERTY, + value=outcome.job_no, + ) + self._deal_database.record_job_no( + deal_id=booking.deal_id, job_no=outcome.job_no + ) + return LogJobSummary(deal_id=booking.deal_id, job_no=outcome.job_no) From ddfbbd0052f26b2851aeaf936858ffb4c9ef2710 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 15:54:58 +0000 Subject: [PATCH 05/17] =?UTF-8?q?An=20OpenHousing=20rejection=20is=20surfa?= =?UTF-8?q?ced=20verbatim=20with=20nothing=20recorded=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../test_abri_log_job_orchestrator.py | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/tests/orchestration/test_abri_log_job_orchestrator.py b/tests/orchestration/test_abri_log_job_orchestrator.py index 2cfb265f8..6bab8181a 100644 --- a/tests/orchestration/test_abri_log_job_orchestrator.py +++ b/tests/orchestration/test_abri_log_job_orchestrator.py @@ -7,7 +7,7 @@ from unittest.mock import MagicMock, patch import pytest from hubspot.crm.deals import SimplePublicObjectInput # type: ignore[reportMissingTypeStubs] -from domain.abri.models import PlaceRef +from domain.abri.models import AbriRequestRejected, PlaceRef from infrastructure.abri.abri_client import AbriClient from infrastructure.abri.config import AbriConfig from infrastructure.hubspot.deal_properties_client import HubspotDealPropertiesClient @@ -149,3 +149,28 @@ def test_run_writes_the_job_no_to_the_hubspot_deal_and_then_the_database( ), ) assert deal_database.recorded_job_nos == [(DEAL_ID, "AD0226519")] + + +# --- rejection passthrough: OpenHousing's actual reason, nothing recorded --- + + +def test_run_returns_the_openhousing_rejection_verbatim_and_records_nothing( + orchestrator: AbriLogJobOrchestrator, + mock_session: MagicMock, + sdk_client: MagicMock, + deal_database: FakeDealDatabase, +) -> None: + # Arrange + mock_session.post.return_value.content = _load_fixture( + "abri_relay_logjob_relayerror_response.xml" + ) + + # Act + result = orchestrator.run(BOOKING) + + # Assert + assert isinstance(result, AbriRequestRejected) + assert result.code == "RelayError" + assert result.message.startswith("No property was found with UPRN/place_ref") + assert sdk_client.mock_calls == [] + assert deal_database.recorded_job_nos == [] From b847fa2f4c5fb5ef28aa8e4d89b702f0f06e8148 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 15:55:38 +0000 Subject: [PATCH 06/17] =?UTF-8?q?Transport=20and=20parse=20failures=20prop?= =?UTF-8?q?agate=20with=20nothing=20recorded=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../test_abri_log_job_orchestrator.py | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/orchestration/test_abri_log_job_orchestrator.py b/tests/orchestration/test_abri_log_job_orchestrator.py index 6bab8181a..689dc99d9 100644 --- a/tests/orchestration/test_abri_log_job_orchestrator.py +++ b/tests/orchestration/test_abri_log_job_orchestrator.py @@ -5,11 +5,13 @@ from typing import List, Tuple from unittest.mock import MagicMock, patch import pytest +import requests from hubspot.crm.deals import SimplePublicObjectInput # type: ignore[reportMissingTypeStubs] from domain.abri.models import AbriRequestRejected, PlaceRef from infrastructure.abri.abri_client import AbriClient from infrastructure.abri.config import AbriConfig +from infrastructure.abri.errors import AbriResponseParseError, AbriTransportError from infrastructure.hubspot.deal_properties_client import HubspotDealPropertiesClient from orchestration.abri_log_job_orchestrator import ( AbriLogJobOrchestrator, @@ -174,3 +176,48 @@ def test_run_returns_the_openhousing_rejection_verbatim_and_records_nothing( assert result.message.startswith("No property was found with UPRN/place_ref") assert sdk_client.mock_calls == [] assert deal_database.recorded_job_nos == [] + + +# --- ambiguous responses are retriable failures, never success or rejection --- + + +@pytest.mark.parametrize( + "body", + [ + b"not xml at all <<<", + b"", # no Jobs/Job_logged element + b"AD0226519", # no attrs + b"", + ], +) +def test_run_raises_a_retriable_parse_error_on_an_unexpectedly_shaped_response( + orchestrator: AbriLogJobOrchestrator, + mock_session: MagicMock, + sdk_client: MagicMock, + deal_database: FakeDealDatabase, + body: bytes, +) -> None: + # Arrange + mock_session.post.return_value.content = body + + # Act / Assert + with pytest.raises(AbriResponseParseError): + orchestrator.run(BOOKING) + assert sdk_client.mock_calls == [] + assert deal_database.recorded_job_nos == [] + + +def test_run_raises_a_transport_error_when_the_relay_is_unreachable( + orchestrator: AbriLogJobOrchestrator, + mock_session: MagicMock, + sdk_client: MagicMock, + deal_database: FakeDealDatabase, +) -> None: + # Arrange + mock_session.post.side_effect = requests.ConnectionError("connection refused") + + # Act / Assert + with pytest.raises(AbriTransportError): + orchestrator.run(BOOKING) + assert sdk_client.mock_calls == [] + assert deal_database.recorded_job_nos == [] From 350a76f9424a4934c87c092730b8e13dc6fdf198 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 15:57:16 +0000 Subject: [PATCH 07/17] =?UTF-8?q?A=20failed=20HubSpot=20write=20raises=20a?= =?UTF-8?q?=20scrubbed=20error=20carrying=20the=20orphaned=20job=5Fno=20?= =?UTF-8?q?=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../test_abri_log_job_orchestrator.py | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/tests/orchestration/test_abri_log_job_orchestrator.py b/tests/orchestration/test_abri_log_job_orchestrator.py index 689dc99d9..4673726d9 100644 --- a/tests/orchestration/test_abri_log_job_orchestrator.py +++ b/tests/orchestration/test_abri_log_job_orchestrator.py @@ -1,3 +1,5 @@ +import json +import traceback import xml.etree.ElementTree as ET from datetime import date from pathlib import Path @@ -6,6 +8,7 @@ from unittest.mock import MagicMock, patch import pytest import requests +from hubspot.crm.deals import ApiException as DealsApiException # type: ignore[reportMissingTypeStubs] from hubspot.crm.deals import SimplePublicObjectInput # type: ignore[reportMissingTypeStubs] from domain.abri.models import AbriRequestRejected, PlaceRef @@ -17,6 +20,7 @@ from orchestration.abri_log_job_orchestrator import ( AbriLogJobOrchestrator, ConfirmedSurveyBooking, LogJobSummary, + LogJobWriteBackError, ) FIXTURE_DIR = Path(__file__).parents[1] / "abri" @@ -221,3 +225,70 @@ def test_run_raises_a_transport_error_when_the_relay_is_unreachable( orchestrator.run(BOOKING) assert sdk_client.mock_calls == [] assert deal_database.recorded_job_nos == [] + + +# --- partial failure: job logged in OpenHousing, HubSpot write failed --- + + +def _body_echoing_api_error() -> DealsApiException: + """A HubSpot validation error whose body echoes the submitted values.""" + api_error = DealsApiException(status=400, reason="Bad Request") + api_error.body = json.dumps( + { + "status": "error", + "message": 'Invalid value "AD0226519" for client_booking_reference', + "correlationId": "corr-abc-123", + "category": "VALIDATION_ERROR", + } + ) + api_error.headers = {} + return api_error + + +def test_run_raises_an_error_carrying_the_orphaned_job_no_when_the_hubspot_write_fails( + orchestrator: AbriLogJobOrchestrator, + mock_session: MagicMock, + sdk_client: MagicMock, + deal_database: FakeDealDatabase, +) -> None: + # Arrange + mock_session.post.return_value.content = _load_fixture( + "abri_relay_logjob_success_response.xml" + ) + sdk_client.crm.deals.basic_api.update.side_effect = _body_echoing_api_error() + + # Act + with pytest.raises(LogJobWriteBackError) as exc_info: + orchestrator.run(BOOKING) + + # Assert: the orphaned job_no is reported for manual paste-in, the + # database write is skipped, and log_job is never retried. + error = exc_info.value + assert error.deal_id == DEAL_ID + assert error.job_no == "AD0226519" + assert "AD0226519" in str(error) + assert deal_database.recorded_job_nos == [] + assert mock_session.post.call_count == 1 + + +def test_run_raises_a_write_back_error_scrubbed_of_the_hubspot_response_body( + orchestrator: AbriLogJobOrchestrator, + mock_session: MagicMock, + sdk_client: MagicMock, +) -> None: + # Arrange + mock_session.post.return_value.content = _load_fixture( + "abri_relay_logjob_success_response.xml" + ) + sdk_client.crm.deals.basic_api.update.side_effect = _body_echoing_api_error() + + # Act + with pytest.raises(LogJobWriteBackError) as exc_info: + orchestrator.run(BOOKING) + + # Assert: non-PII diagnostics survive; the echoed body does not + rendered_traceback = "".join(traceback.format_exception(exc_info.value)) + assert "400" in str(exc_info.value) + assert "VALIDATION_ERROR" in str(exc_info.value) + assert "corr-abc-123" in str(exc_info.value) + assert "Invalid value" not in rendered_traceback From 049475ec96ffcd127c65b4577f874b4cc4178d1d Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 15:58:36 +0000 Subject: [PATCH 08/17] =?UTF-8?q?A=20failed=20HubSpot=20write=20raises=20a?= =?UTF-8?q?=20scrubbed=20error=20carrying=20the=20orphaned=20job=5Fno=20?= =?UTF-8?q?=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../hubspot/deal_properties_client.py | 50 ++++++++++++++++--- orchestration/abri_log_job_orchestrator.py | 21 ++++++-- 2 files changed, 60 insertions(+), 11 deletions(-) diff --git a/infrastructure/hubspot/deal_properties_client.py b/infrastructure/hubspot/deal_properties_client.py index 24cf5fca8..1564b22de 100644 --- a/infrastructure/hubspot/deal_properties_client.py +++ b/infrastructure/hubspot/deal_properties_client.py @@ -1,6 +1,40 @@ +import json +from typing import Any, Dict, Optional, cast + from hubspot.client import Client # type: ignore[reportMissingTypeStubs] from hubspot.crm.deals import SimplePublicObjectInput # type: ignore[reportMissingTypeStubs] +from infrastructure.hubspot.errors import HubspotRequestError + + +def _scrubbed(error: Exception) -> HubspotRequestError: + """Reduce an SDK error to non-PII diagnostics, dropping the body. + + Each HubSpot sub-module ships its own ApiException class with no shared + base beyond Exception, so the fields are read by duck-typing. + """ + status = getattr(error, "status", None) + status_code = status if isinstance(status, int) else None + + category: Optional[str] = None + correlation_id: Optional[str] = None + try: + parsed: Any = json.loads(getattr(error, "body", None) or "") + if isinstance(parsed, dict): + details = cast(Dict[str, Any], parsed) + raw_category = details.get("category") + raw_correlation_id = details.get("correlationId") + category = raw_category if isinstance(raw_category, str) else None + correlation_id = ( + raw_correlation_id if isinstance(raw_correlation_id, str) else None + ) + except ValueError: + pass + + return HubspotRequestError( + status_code=status_code, category=category, correlation_id=correlation_id + ) + class HubspotDealPropertiesClient: """Focused HubSpot client for writing single deal properties. @@ -16,9 +50,13 @@ class HubspotDealPropertiesClient: def write_deal_property( self, deal_id: str, property_name: str, value: str ) -> None: - self._client.crm.deals.basic_api.update( # type: ignore[reportUnknownMemberType] - deal_id, - simple_public_object_input=SimplePublicObjectInput( - properties={property_name: value} - ), - ) + try: + self._client.crm.deals.basic_api.update( # type: ignore[reportUnknownMemberType] + deal_id, + simple_public_object_input=SimplePublicObjectInput( + properties={property_name: value} + ), + ) + except Exception as error: + # `from None` keeps the body-carrying SDK error out of tracebacks. + raise _scrubbed(error) from None diff --git a/orchestration/abri_log_job_orchestrator.py b/orchestration/abri_log_job_orchestrator.py index 811d008af..325d63be6 100644 --- a/orchestration/abri_log_job_orchestrator.py +++ b/orchestration/abri_log_job_orchestrator.py @@ -7,6 +7,7 @@ from domain.abri.models import AbriRequestRejected, LogJobRequest, PlaceRef from domain.abri.slots import slot_for_confirmed_time from infrastructure.abri.abri_client import AbriClient from infrastructure.hubspot.deal_properties_client import HubspotDealPropertiesClient +from infrastructure.hubspot.errors import HubspotRequestError # The HubSpot deal property that carries the OpenHousing job number; the # property ID is fixed externally, the domain and database call it job_no. @@ -98,11 +99,21 @@ class AbriLogJobOrchestrator: if isinstance(outcome, AbriRequestRejected): return outcome - self._hubspot.write_deal_property( - deal_id=booking.deal_id, - property_name=JOB_NO_DEAL_PROPERTY, - value=outcome.job_no, - ) + try: + self._hubspot.write_deal_property( + deal_id=booking.deal_id, + property_name=JOB_NO_DEAL_PROPERTY, + value=outcome.job_no, + ) + except HubspotRequestError as error: + # The job now exists in OpenHousing; never re-run log_job (it + # would log a duplicate). `error` is already scrubbed, so + # chaining it keeps tracebacks free of the response body. + raise LogJobWriteBackError( + message=str(error), + deal_id=booking.deal_id, + job_no=outcome.job_no, + ) from error self._deal_database.record_job_no( deal_id=booking.deal_id, job_no=outcome.job_no ) From 7e5dc1722e7246a8eb4459d687e79ffa597795b0 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 15:59:27 +0000 Subject: [PATCH 09/17] =?UTF-8?q?A=20rate-limited=20HubSpot=20write=20is?= =?UTF-8?q?=20retried=20before=20succeeding=20=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../test_abri_log_job_orchestrator.py | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/orchestration/test_abri_log_job_orchestrator.py b/tests/orchestration/test_abri_log_job_orchestrator.py index 4673726d9..19c112280 100644 --- a/tests/orchestration/test_abri_log_job_orchestrator.py +++ b/tests/orchestration/test_abri_log_job_orchestrator.py @@ -292,3 +292,26 @@ def test_run_raises_a_write_back_error_scrubbed_of_the_hubspot_response_body( assert "VALIDATION_ERROR" in str(exc_info.value) assert "corr-abc-123" in str(exc_info.value) assert "Invalid value" not in rendered_traceback + + +def test_run_retries_a_rate_limited_hubspot_write_before_succeeding( + orchestrator: AbriLogJobOrchestrator, + mock_session: MagicMock, + sdk_client: MagicMock, + deal_database: FakeDealDatabase, +) -> None: + # Arrange + mock_session.post.return_value.content = _load_fixture( + "abri_relay_logjob_success_response.xml" + ) + rate_limited = DealsApiException(status=429, reason="Too Many Requests") + rate_limited.headers = {"x-hubspot-ratelimit-interval-milliseconds": "0"} + sdk_client.crm.deals.basic_api.update.side_effect = [rate_limited, MagicMock()] + + # Act + result = orchestrator.run(BOOKING) + + # Assert + assert result == LogJobSummary(deal_id=DEAL_ID, job_no="AD0226519") + assert sdk_client.crm.deals.basic_api.update.call_count == 2 + assert deal_database.recorded_job_nos == [(DEAL_ID, "AD0226519")] From 2ea92570362742f86a536012f59847786e035ddb Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 15:59:53 +0000 Subject: [PATCH 10/17] =?UTF-8?q?A=20rate-limited=20HubSpot=20write=20is?= =?UTF-8?q?=20retried=20before=20succeeding=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- infrastructure/hubspot/deal_properties_client.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/infrastructure/hubspot/deal_properties_client.py b/infrastructure/hubspot/deal_properties_client.py index 1564b22de..3893c795b 100644 --- a/infrastructure/hubspot/deal_properties_client.py +++ b/infrastructure/hubspot/deal_properties_client.py @@ -5,6 +5,7 @@ from hubspot.client import Client # type: ignore[reportMissingTypeStubs] from hubspot.crm.deals import SimplePublicObjectInput # type: ignore[reportMissingTypeStubs] from infrastructure.hubspot.errors import HubspotRequestError +from infrastructure.hubspot.retry import call_with_retry def _scrubbed(error: Exception) -> HubspotRequestError: @@ -51,11 +52,13 @@ class HubspotDealPropertiesClient: self, deal_id: str, property_name: str, value: str ) -> None: try: - self._client.crm.deals.basic_api.update( # type: ignore[reportUnknownMemberType] - deal_id, - simple_public_object_input=SimplePublicObjectInput( - properties={property_name: value} - ), + call_with_retry( + lambda: self._client.crm.deals.basic_api.update( # type: ignore[reportUnknownMemberType] + deal_id, + simple_public_object_input=SimplePublicObjectInput( + properties={property_name: value} + ), + ) ) except Exception as error: # `from None` keeps the body-carrying SDK error out of tracebacks. From ed08a6adfe1843f41287c4fbda340a31e6906b12 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 16:02:22 +0000 Subject: [PATCH 11/17] =?UTF-8?q?HubSpot=20clients=20share=20one=20scrubbe?= =?UTF-8?q?d-retry=20call=20policy=20=F0=9F=9F=AA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../hubspot/deal_contacts_client.py | 42 ++------------- .../hubspot/deal_properties_client.py | 53 +++---------------- infrastructure/hubspot/scrubbed_calls.py | 45 ++++++++++++++++ 3 files changed, 56 insertions(+), 84 deletions(-) create mode 100644 infrastructure/hubspot/scrubbed_calls.py diff --git a/infrastructure/hubspot/deal_contacts_client.py b/infrastructure/hubspot/deal_contacts_client.py index 2c0eb0627..948a2d953 100644 --- a/infrastructure/hubspot/deal_contacts_client.py +++ b/infrastructure/hubspot/deal_contacts_client.py @@ -1,46 +1,15 @@ -import json -from typing import Any, Callable, Dict, Optional, cast +from typing import Any, Callable, Dict from hubspot.client import Client # type: ignore[reportMissingTypeStubs] from hubspot.crm.associations.v4 import AssociationSpec # type: ignore[reportMissingTypeStubs] from hubspot.crm.contacts import SimplePublicObjectInputForCreate # type: ignore[reportMissingTypeStubs] -from infrastructure.hubspot.errors import HubspotRequestError -from infrastructure.hubspot.retry import call_with_retry +from infrastructure.hubspot.scrubbed_calls import scrubbed_call_with_retry # HubSpot-defined deal -> contact association ("deal_to_contact" in v3 terms). DEAL_TO_CONTACT_ASSOCIATION_TYPE_ID = 3 -def _scrubbed(error: Exception) -> HubspotRequestError: - """Reduce an SDK error to non-PII diagnostics, dropping the body. - - Each HubSpot sub-module ships its own ApiException class with no shared - base beyond Exception, so the fields are read by duck-typing. - """ - status = getattr(error, "status", None) - status_code = status if isinstance(status, int) else None - - category: Optional[str] = None - correlation_id: Optional[str] = None - try: - parsed: Any = json.loads(getattr(error, "body", None) or "") - if isinstance(parsed, dict): - details = cast(Dict[str, Any], parsed) - raw_category = details.get("category") - raw_correlation_id = details.get("correlationId") - category = raw_category if isinstance(raw_category, str) else None - correlation_id = ( - raw_correlation_id if isinstance(raw_correlation_id, str) else None - ) - except ValueError: - pass - - return HubspotRequestError( - status_code=status_code, category=category, correlation_id=correlation_id - ) - - class HubspotDealContactsClient: """Focused HubSpot client for the tenant-data sync flow. @@ -81,9 +50,4 @@ class HubspotDealContactsClient: @staticmethod def _call(fn: Callable[[], Any]) -> Any: - """Run one SDK call under the shared retry policy, scrubbed on failure.""" - try: - return call_with_retry(fn) - except Exception as error: - # `from None` keeps the body-carrying SDK error out of tracebacks. - raise _scrubbed(error) from None + return scrubbed_call_with_retry(fn) diff --git a/infrastructure/hubspot/deal_properties_client.py b/infrastructure/hubspot/deal_properties_client.py index 3893c795b..dda4813a4 100644 --- a/infrastructure/hubspot/deal_properties_client.py +++ b/infrastructure/hubspot/deal_properties_client.py @@ -1,40 +1,7 @@ -import json -from typing import Any, Dict, Optional, cast - from hubspot.client import Client # type: ignore[reportMissingTypeStubs] from hubspot.crm.deals import SimplePublicObjectInput # type: ignore[reportMissingTypeStubs] -from infrastructure.hubspot.errors import HubspotRequestError -from infrastructure.hubspot.retry import call_with_retry - - -def _scrubbed(error: Exception) -> HubspotRequestError: - """Reduce an SDK error to non-PII diagnostics, dropping the body. - - Each HubSpot sub-module ships its own ApiException class with no shared - base beyond Exception, so the fields are read by duck-typing. - """ - status = getattr(error, "status", None) - status_code = status if isinstance(status, int) else None - - category: Optional[str] = None - correlation_id: Optional[str] = None - try: - parsed: Any = json.loads(getattr(error, "body", None) or "") - if isinstance(parsed, dict): - details = cast(Dict[str, Any], parsed) - raw_category = details.get("category") - raw_correlation_id = details.get("correlationId") - category = raw_category if isinstance(raw_category, str) else None - correlation_id = ( - raw_correlation_id if isinstance(raw_correlation_id, str) else None - ) - except ValueError: - pass - - return HubspotRequestError( - status_code=status_code, category=category, correlation_id=correlation_id - ) +from infrastructure.hubspot.scrubbed_calls import scrubbed_call_with_retry class HubspotDealPropertiesClient: @@ -51,15 +18,11 @@ class HubspotDealPropertiesClient: def write_deal_property( self, deal_id: str, property_name: str, value: str ) -> None: - try: - call_with_retry( - lambda: self._client.crm.deals.basic_api.update( # type: ignore[reportUnknownMemberType] - deal_id, - simple_public_object_input=SimplePublicObjectInput( - properties={property_name: value} - ), - ) + scrubbed_call_with_retry( + lambda: self._client.crm.deals.basic_api.update( # type: ignore[reportUnknownMemberType] + deal_id, + simple_public_object_input=SimplePublicObjectInput( + properties={property_name: value} + ), ) - except Exception as error: - # `from None` keeps the body-carrying SDK error out of tracebacks. - raise _scrubbed(error) from None + ) diff --git a/infrastructure/hubspot/scrubbed_calls.py b/infrastructure/hubspot/scrubbed_calls.py new file mode 100644 index 000000000..886358f01 --- /dev/null +++ b/infrastructure/hubspot/scrubbed_calls.py @@ -0,0 +1,45 @@ +import json +from typing import Any, Callable, Dict, Optional, TypeVar, cast + +from infrastructure.hubspot.errors import HubspotRequestError +from infrastructure.hubspot.retry import call_with_retry + +T = TypeVar("T") + + +def scrubbed(error: Exception) -> HubspotRequestError: + """Reduce an SDK error to non-PII diagnostics, dropping the body. + + Each HubSpot sub-module ships its own ApiException class with no shared + base beyond Exception, so the fields are read by duck-typing. + """ + status = getattr(error, "status", None) + status_code = status if isinstance(status, int) else None + + category: Optional[str] = None + correlation_id: Optional[str] = None + try: + parsed: Any = json.loads(getattr(error, "body", None) or "") + if isinstance(parsed, dict): + details = cast(Dict[str, Any], parsed) + raw_category = details.get("category") + raw_correlation_id = details.get("correlationId") + category = raw_category if isinstance(raw_category, str) else None + correlation_id = ( + raw_correlation_id if isinstance(raw_correlation_id, str) else None + ) + except ValueError: + pass + + return HubspotRequestError( + status_code=status_code, category=category, correlation_id=correlation_id + ) + + +def scrubbed_call_with_retry(fn: Callable[[], T]) -> T: + """Run one SDK call under the shared retry policy, scrubbed on failure.""" + try: + return call_with_retry(fn) + except Exception as error: + # `from None` keeps the body-carrying SDK error out of tracebacks. + raise scrubbed(error) from None From faac50eca4c190b891785b47f57dc988601436f5 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 16:04:54 +0000 Subject: [PATCH 12/17] =?UTF-8?q?A=20client=5Fbooking=5Freference=20change?= =?UTF-8?q?=20triggers=20a=20deal=20database=20update=20=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- backend/app/db/models/hubspot_deal_data.py | 4 ++ etl/hubspot/tests/test_hubspot_deal_differ.py | 52 +++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/backend/app/db/models/hubspot_deal_data.py b/backend/app/db/models/hubspot_deal_data.py index 48a6f1ba0..036610340 100644 --- a/backend/app/db/models/hubspot_deal_data.py +++ b/backend/app/db/models/hubspot_deal_data.py @@ -71,6 +71,10 @@ class HubspotDealData(SQLModel, table=True): confirmed_survey_time: Optional[str] = Field(default=None) surveyed_date: Optional[datetime] = Field(default=None) design_type: Optional[str] = Field(default=None) + # OpenHousing job number; HubSpot property ID is client_booking_reference. + # Column is created by a drizzle migration in the TS frontend repo, which + # must deploy before this model change (issue #1478). + job_no: Optional[str] = Field(default=None) survey_type: Optional[str] = Field(default=None) measures_for_pibi_ordered: Optional[str] = Field(default=None) diff --git a/etl/hubspot/tests/test_hubspot_deal_differ.py b/etl/hubspot/tests/test_hubspot_deal_differ.py index c3e24e08f..5e9bcfdfa 100644 --- a/etl/hubspot/tests/test_hubspot_deal_differ.py +++ b/etl/hubspot/tests/test_hubspot_deal_differ.py @@ -1226,3 +1226,55 @@ def test_db_update_trigger__listing_changed__returns_true() -> None: ) assert result is True + + +def test_db_update_trigger__client_booking_reference_changed__returns_true() -> None: + deal_id = uuid.uuid4() + + # Arrange + old_deal = make_old_deal( + id=deal_id, + job_no=None, + ) + new_deal = make_new_deal( + deal_id, + hs_object_id="1", + client_booking_reference="AD0226519", + ) + + # Act + result = HubspotDealDiffer.check_for_db_update_trigger( + new_deal=new_deal, + new_company=None, + new_listing=None, + old_deal=old_deal, + ) + + # Assert + assert result is True + + +def test_db_update_trigger__client_booking_reference_unchanged__returns_false() -> None: + deal_id = uuid.uuid4() + + # Arrange + old_deal = make_old_deal( + id=deal_id, + job_no="AD0226519", + ) + new_deal = make_new_deal( + deal_id, + hs_object_id="1", + client_booking_reference="AD0226519", + ) + + # Act + result = HubspotDealDiffer.check_for_db_update_trigger( + new_deal=new_deal, + new_company=None, + new_listing=None, + old_deal=old_deal, + ) + + # Assert + assert result is False From 0cd4efbf83da6d7821e6d160be1a3a8fa99ea5d4 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 16:05:20 +0000 Subject: [PATCH 13/17] =?UTF-8?q?A=20client=5Fbooking=5Freference=20change?= =?UTF-8?q?=20triggers=20a=20deal=20database=20update=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- etl/hubspot/hubspot_deal_differ.py | 1 + 1 file changed, 1 insertion(+) diff --git a/etl/hubspot/hubspot_deal_differ.py b/etl/hubspot/hubspot_deal_differ.py index 977dc3e4d..39a1956bb 100644 --- a/etl/hubspot/hubspot_deal_differ.py +++ b/etl/hubspot/hubspot_deal_differ.py @@ -97,6 +97,7 @@ class HubspotDealDiffer: "design_type": "design_type", "surveyor": "surveyor", "confirmed_survey_time": "confirmed_survey_time", + "client_booking_reference": "job_no", "survey_type": "survey_type", "measures_for_pibi_ordered": "measures_for_pibi_ordered", "property_halted_reason": "property_halted_reason", From 62d9bbf1475af6bc5565badbdba6815da67b9418 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 16:08:17 +0000 Subject: [PATCH 14/17] =?UTF-8?q?The=20deal=20upsert=20lands=20client=5Fbo?= =?UTF-8?q?oking=5Freference=20in=20the=20job=5Fno=20column=20=F0=9F=9F=A5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- etl/hubspot/tests/test_hubspot_data_to_db.py | 40 ++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/etl/hubspot/tests/test_hubspot_data_to_db.py b/etl/hubspot/tests/test_hubspot_data_to_db.py index ff4b82940..60c937460 100644 --- a/etl/hubspot/tests/test_hubspot_data_to_db.py +++ b/etl/hubspot/tests/test_hubspot_data_to_db.py @@ -85,3 +85,43 @@ def test_update_existing_deal__no_project__clears_project_id() -> None: ) assert existing.project_id is None + + +def test_update_existing_deal__client_booking_reference_maps_to_job_no() -> None: + existing = HubspotDealData(deal_id="MOCK_DEAL_ID", job_no=None) + deal_data = {"client_booking_reference": "AD0226519"} + + _make_instance()._update_existing_deal( + existing=existing, + deal_data=deal_data, + listing=None, + company=None, + ) + + assert existing.job_no == "AD0226519" + + +def test_update_existing_deal__client_booking_reference_cleared__nulls_job_no() -> None: + existing = HubspotDealData(deal_id="MOCK_DEAL_ID", job_no="AD0226519") + deal_data = {} + + _make_instance()._update_existing_deal( + existing=existing, + deal_data=deal_data, + listing=None, + company=None, + ) + + assert existing.job_no is None + + +def test_build_new_deal__client_booking_reference_maps_to_job_no() -> None: + new_deal = _make_instance()._build_new_deal( + deal_id="MOCK_DEAL_ID", + deal_data={"client_booking_reference": "AD0226519"}, + listing=None, + company=None, + project=None, + ) + + assert new_deal.job_no == "AD0226519" From 359516fa5f8697f137acd77acb5b73b20cfffd0f Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 16:10:29 +0000 Subject: [PATCH 15/17] =?UTF-8?q?The=20deal=20upsert=20lands=20client=5Fbo?= =?UTF-8?q?oking=5Freference=20in=20the=20job=5Fno=20column=20=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- etl/hubspot/hubspotDataTodB.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/etl/hubspot/hubspotDataTodB.py b/etl/hubspot/hubspotDataTodB.py index f72564c69..49719da3b 100644 --- a/etl/hubspot/hubspotDataTodB.py +++ b/etl/hubspot/hubspotDataTodB.py @@ -274,6 +274,7 @@ class HubspotDataToDb: deal_data.get("confirmed_survey_date") ), "confirmed_survey_time": deal_data.get("confirmed_survey_time"), + "job_no": deal_data.get("client_booking_reference"), "surveyed_date": parse_hs_date(deal_data.get("surveyed_date")), "design_type": deal_data.get("design_type"), "survey_type": deal_data.get("survey_type"), @@ -382,6 +383,7 @@ class HubspotDataToDb: surveyor=deal_data.get("surveyor"), confirmed_survey_date=parse_hs_date(deal_data.get("confirmed_survey_date")), confirmed_survey_time=deal_data.get("confirmed_survey_time"), + job_no=deal_data.get("client_booking_reference"), surveyed_date=parse_hs_date(deal_data.get("surveyed_date")), design_type=deal_data.get("design_type"), survey_type=deal_data.get("survey_type"), From 01f0392ce23db53aa6259a4007e13f2acca29870 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 16:14:25 +0000 Subject: [PATCH 16/17] remove overly-specific comment --- backend/app/db/models/hubspot_deal_data.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/backend/app/db/models/hubspot_deal_data.py b/backend/app/db/models/hubspot_deal_data.py index 036610340..c221cbcdc 100644 --- a/backend/app/db/models/hubspot_deal_data.py +++ b/backend/app/db/models/hubspot_deal_data.py @@ -72,8 +72,6 @@ class HubspotDealData(SQLModel, table=True): surveyed_date: Optional[datetime] = Field(default=None) design_type: Optional[str] = Field(default=None) # OpenHousing job number; HubSpot property ID is client_booking_reference. - # Column is created by a drizzle migration in the TS frontend repo, which - # must deploy before this model change (issue #1478). job_no: Optional[str] = Field(default=None) survey_type: Optional[str] = Field(default=None) From a5868cc2e01d4f27b9177833ad2f4d8e99443563 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 16:26:48 +0000 Subject: [PATCH 17/17] =?UTF-8?q?One=20AbriOrchestrator=20runs=20every=20A?= =?UTF-8?q?bri=20flow=20for=20a=20deal=20=F0=9F=9F=AA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- orchestration/abri_log_job_orchestrator.py | 121 --------- orchestration/abri_orchestrator.py | 248 ++++++++++++++++++ .../abri_tenant_data_sync_orchestrator.py | 136 ---------- scripts/smoke_test_tenant_contacts.py | 24 +- ...r.py => test_abri_orchestrator_log_job.py} | 60 +++-- ...est_abri_orchestrator_tenant_data_sync.py} | 104 ++++---- 6 files changed, 352 insertions(+), 341 deletions(-) delete mode 100644 orchestration/abri_log_job_orchestrator.py create mode 100644 orchestration/abri_orchestrator.py delete mode 100644 orchestration/abri_tenant_data_sync_orchestrator.py rename tests/orchestration/{test_abri_log_job_orchestrator.py => test_abri_orchestrator_log_job.py} (84%) rename tests/orchestration/{test_abri_tenant_data_sync_orchestrator.py => test_abri_orchestrator_tenant_data_sync.py} (81%) diff --git a/orchestration/abri_log_job_orchestrator.py b/orchestration/abri_log_job_orchestrator.py deleted file mode 100644 index 325d63be6..000000000 --- a/orchestration/abri_log_job_orchestrator.py +++ /dev/null @@ -1,121 +0,0 @@ -from dataclasses import dataclass -from datetime import date -from typing import Optional, Protocol, Union - -from domain.abri.descriptions import build_job_descriptions -from domain.abri.models import AbriRequestRejected, LogJobRequest, PlaceRef -from domain.abri.slots import slot_for_confirmed_time -from infrastructure.abri.abri_client import AbriClient -from infrastructure.hubspot.deal_properties_client import HubspotDealPropertiesClient -from infrastructure.hubspot.errors import HubspotRequestError - -# The HubSpot deal property that carries the OpenHousing job number; the -# property ID is fixed externally, the domain and database call it job_no. -JOB_NO_DEAL_PROPERTY = "client_booking_reference" - - -@dataclass(frozen=True) -class ConfirmedSurveyBooking: - """The deal fields a confirmed survey booking contributes to a LogJob.""" - - deal_id: str - place_ref: PlaceRef - deal_name: str - confirmed_survey_date: date - confirmed_survey_time: Optional[str] - - -@dataclass(frozen=True) -class LogJobSummary: - """PII-free record of a logged job: safe to log and persist.""" - - deal_id: str - job_no: str - - -LogJobOrchestrationResult = Union[LogJobSummary, AbriRequestRejected] - - -class DealDatabaseGateway(Protocol): - def record_job_no(self, deal_id: str, job_no: str) -> None: ... - - -class LogJobWriteBackError(Exception): - """A job logged in OpenHousing whose job_no could not be written back. - - Carries the orphaned job_no so an operator can paste it into HubSpot - manually; the run must never retry log_job, which would log a duplicate - job in OpenHousing. - """ - - def __init__(self, message: str, deal_id: str, job_no: str) -> None: - self.deal_id = deal_id - self.job_no = job_no - super().__init__(f"{message} (deal: {deal_id}, orphaned job_no: {job_no})") - - -def client_ref_for_deal(deal_id: str) -> str: - """The client_ref sent to OpenHousing: the HubSpot deal ID, for now. - - UNCONFIRMED with Abri: whether client_ref must be unique per job (a deal - re-logging after a cancellation would repeat the bare deal ID) is an open - question on issue #1478. Swap the derivation here once Abri answers. - """ - return deal_id - - -class AbriLogJobOrchestrator: - """Logs an OpenHousing job for a deal and records the returned job_no. - - HubSpot is written first (it is the source of truth); the database - second, so a failed database write self-heals on the next scrape. - """ - - def __init__( - self, - abri_client: AbriClient, - hubspot: HubspotDealPropertiesClient, - deal_database: DealDatabaseGateway, - ) -> None: - self._abri = abri_client - self._hubspot = hubspot - self._deal_database = deal_database - - def run(self, booking: ConfirmedSurveyBooking) -> LogJobOrchestrationResult: - descriptions = build_job_descriptions(booking.deal_name) - outcome = self._abri.log_job( - LogJobRequest( - client_ref=client_ref_for_deal(booking.deal_id), - place_ref=booking.place_ref, - appointment_date=booking.confirmed_survey_date, - appointment_time=slot_for_confirmed_time( - booking.confirmed_survey_time - ), - short_description=descriptions.short_description, - long_description=descriptions.long_description, - ) - ) - - if isinstance(outcome, AbriRequestRejected): - return outcome - - try: - self._hubspot.write_deal_property( - deal_id=booking.deal_id, - property_name=JOB_NO_DEAL_PROPERTY, - value=outcome.job_no, - ) - except HubspotRequestError as error: - # The job now exists in OpenHousing; never re-run log_job (it - # would log a duplicate). `error` is already scrubbed, so - # chaining it keeps tracebacks free of the response body. - raise LogJobWriteBackError( - message=str(error), - deal_id=booking.deal_id, - job_no=outcome.job_no, - ) from error - self._deal_database.record_job_no( - deal_id=booking.deal_id, job_no=outcome.job_no - ) - - return LogJobSummary(deal_id=booking.deal_id, job_no=outcome.job_no) diff --git a/orchestration/abri_orchestrator.py b/orchestration/abri_orchestrator.py new file mode 100644 index 000000000..920eaffcf --- /dev/null +++ b/orchestration/abri_orchestrator.py @@ -0,0 +1,248 @@ +from dataclasses import dataclass +from datetime import date +from typing import Dict, List, Optional, Protocol, Tuple, Union + +from domain.abri.descriptions import build_job_descriptions +from domain.abri.models import ( + AbriRequestRejected, + LogJobRequest, + PlaceRef, + Tenant, +) +from domain.abri.slots import slot_for_confirmed_time +from infrastructure.abri.abri_client import AbriClient +from infrastructure.hubspot.deal_contacts_client import HubspotDealContactsClient +from infrastructure.hubspot.deal_properties_client import HubspotDealPropertiesClient +from infrastructure.hubspot.errors import HubspotRequestError + +# The HubSpot deal property that carries the OpenHousing job number; the +# property ID is fixed externally, the domain and database call it job_no. +JOB_NO_DEAL_PROPERTY = "client_booking_reference" + + +def _vulnerability_description(tenant: Tenant) -> Optional[str]: + """The tenant's distinct vulnerabilities, verbatim and newline-joined. + + Case-insensitively distinct within the tenant, keeping the first-seen + casing and document order. + """ + distinct: List[str] = [] + seen: set[str] = set() + for vulnerability in tenant.vulnerabilities: + if vulnerability.lower() in seen: + continue + seen.add(vulnerability.lower()) + distinct.append(vulnerability) + return "\n".join(distinct) if distinct else None + + +def _contact_properties(tenant: Tenant) -> Dict[str, str]: + candidates: Dict[str, Optional[str]] = { + "firstname": tenant.forenames, + "lastname": tenant.surname, + "mobilephone": tenant.mobile, + "phone": tenant.telephone, + "secondary_phone_number": tenant.secondary_number, + "vulnerability_description": _vulnerability_description(tenant), + } + properties = {name: value for name, value in candidates.items() if value} + # Always written, so "not vulnerable" is distinguishable from "not synced". + properties["is_vulnerable"] = "true" if tenant.vulnerabilities else "false" + return properties + + +@dataclass(frozen=True) +class TenantDataSyncSummary: + """PII-free record of what a sync run did: safe to log and persist.""" + + tenancy_reference: str + contact_ids: Tuple[str, ...] + vulnerable_contact_count: int + + +TenantDataSyncResult = Union[TenantDataSyncSummary, AbriRequestRejected] + + +class TenantDataSyncError(Exception): + """A sync that stopped at the first failed HubSpot write. + + Carries a PII-free progress report so the operator knows the deal's + half-state before cleaning up and re-running; contacts created but not + associated are orphaned in HubSpot and need manual cleanup. + """ + + def __init__( + self, + message: str, + deal_id: str, + tenant_index: int, + contact_ids_created: Tuple[str, ...], + contact_ids_associated: Tuple[str, ...], + ) -> None: + self.deal_id = deal_id + self.tenant_index = tenant_index + self.contact_ids_created = contact_ids_created + self.contact_ids_associated = contact_ids_associated + super().__init__( + f"{message} (deal: {deal_id}, tenant index: {tenant_index}, " + f"contacts created: {list(contact_ids_created)}, " + f"associated: {list(contact_ids_associated)})" + ) + + @property + def orphaned_contact_ids(self) -> Tuple[str, ...]: + return tuple( + contact_id + for contact_id in self.contact_ids_created + if contact_id not in self.contact_ids_associated + ) + + +@dataclass(frozen=True) +class ConfirmedSurveyBooking: + """The deal fields a confirmed survey booking contributes to a LogJob.""" + + deal_id: str + place_ref: PlaceRef + deal_name: str + confirmed_survey_date: date + confirmed_survey_time: Optional[str] + + +@dataclass(frozen=True) +class LogJobSummary: + """PII-free record of a logged job: safe to log and persist.""" + + deal_id: str + job_no: str + + +LogJobOrchestrationResult = Union[LogJobSummary, AbriRequestRejected] + + +class DealDatabaseGateway(Protocol): + def record_job_no(self, deal_id: str, job_no: str) -> None: ... + + +class LogJobWriteBackError(Exception): + """A job logged in OpenHousing whose job_no could not be written back. + + Carries the orphaned job_no so an operator can paste it into HubSpot + manually; the run must never retry log_job, which would log a duplicate + job in OpenHousing. + """ + + def __init__(self, message: str, deal_id: str, job_no: str) -> None: + self.deal_id = deal_id + self.job_no = job_no + super().__init__(f"{message} (deal: {deal_id}, orphaned job_no: {job_no})") + + +def client_ref_for_deal(deal_id: str) -> str: + """The client_ref sent to OpenHousing: the HubSpot deal ID, for now. + + UNCONFIRMED with Abri: whether client_ref must be unique per job (a deal + re-logging after a cancellation would repeat the bare deal ID) is an open + question on issue #1478. Swap the derivation here once Abri answers. + """ + return deal_id + + +class AbriOrchestrator: + """Runs the Abri OpenHousing flows for a HubSpot deal, one method each. + + Tenant data flows straight from Abri to HubSpot; nothing tenant-related + is persisted in Domna's database or logs. For job logging, HubSpot is + written first (it is the source of truth); the database second, so a + failed database write self-heals on the next scrape. + """ + + def __init__( + self, + abri_client: AbriClient, + deal_contacts: HubspotDealContactsClient, + deal_properties: HubspotDealPropertiesClient, + deal_database: DealDatabaseGateway, + ) -> None: + self._abri = abri_client + self._deal_contacts = deal_contacts + self._deal_properties = deal_properties + self._deal_database = deal_database + + def sync_tenant_data( + self, place_ref: PlaceRef, deal_id: str + ) -> TenantDataSyncResult: + tenancy = self._abri.get_tenant_data(place_ref) + + if isinstance(tenancy, AbriRequestRejected): + return tenancy + + contact_ids: List[str] = [] + associated_ids: List[str] = [] + for tenant_index, tenant in enumerate(tenancy.tenants): + try: + contact_id = self._deal_contacts.create_contact( + _contact_properties(tenant) + ) + contact_ids.append(contact_id) + self._deal_contacts.associate_contact_to_deal( + deal_id=deal_id, contact_id=contact_id + ) + associated_ids.append(contact_id) + except HubspotRequestError as error: + # Fail fast at the first failed write; `error` is already + # scrubbed, so chaining it keeps tracebacks PII-free. + raise TenantDataSyncError( + message=str(error), + deal_id=deal_id, + tenant_index=tenant_index, + contact_ids_created=tuple(contact_ids), + contact_ids_associated=tuple(associated_ids), + ) from error + + return TenantDataSyncSummary( + tenancy_reference=tenancy.tenancy_reference, + contact_ids=tuple(contact_ids), + vulnerable_contact_count=sum( + 1 for tenant in tenancy.tenants if tenant.vulnerabilities + ), + ) + + def log_job(self, booking: ConfirmedSurveyBooking) -> LogJobOrchestrationResult: + descriptions = build_job_descriptions(booking.deal_name) + outcome = self._abri.log_job( + LogJobRequest( + client_ref=client_ref_for_deal(booking.deal_id), + place_ref=booking.place_ref, + appointment_date=booking.confirmed_survey_date, + appointment_time=slot_for_confirmed_time( + booking.confirmed_survey_time + ), + short_description=descriptions.short_description, + long_description=descriptions.long_description, + ) + ) + + if isinstance(outcome, AbriRequestRejected): + return outcome + + try: + self._deal_properties.write_deal_property( + deal_id=booking.deal_id, + property_name=JOB_NO_DEAL_PROPERTY, + value=outcome.job_no, + ) + except HubspotRequestError as error: + # The job now exists in OpenHousing; never re-run log_job (it + # would log a duplicate). `error` is already scrubbed, so + # chaining it keeps tracebacks free of the response body. + raise LogJobWriteBackError( + message=str(error), + deal_id=booking.deal_id, + job_no=outcome.job_no, + ) from error + self._deal_database.record_job_no( + deal_id=booking.deal_id, job_no=outcome.job_no + ) + + return LogJobSummary(deal_id=booking.deal_id, job_no=outcome.job_no) diff --git a/orchestration/abri_tenant_data_sync_orchestrator.py b/orchestration/abri_tenant_data_sync_orchestrator.py deleted file mode 100644 index 14ab68e2f..000000000 --- a/orchestration/abri_tenant_data_sync_orchestrator.py +++ /dev/null @@ -1,136 +0,0 @@ -from dataclasses import dataclass -from typing import Dict, List, Optional, Tuple, Union - -from domain.abri.models import AbriRequestRejected, PlaceRef, Tenant -from infrastructure.hubspot.deal_contacts_client import HubspotDealContactsClient -from infrastructure.hubspot.errors import HubspotRequestError -from infrastructure.abri.abri_client import AbriClient - - -def _vulnerability_description(tenant: Tenant) -> Optional[str]: - """The tenant's distinct vulnerabilities, verbatim and newline-joined. - - Case-insensitively distinct within the tenant, keeping the first-seen - casing and document order. - """ - distinct: List[str] = [] - seen: set[str] = set() - for vulnerability in tenant.vulnerabilities: - if vulnerability.lower() in seen: - continue - seen.add(vulnerability.lower()) - distinct.append(vulnerability) - return "\n".join(distinct) if distinct else None - - -def _contact_properties(tenant: Tenant) -> Dict[str, str]: - candidates: Dict[str, Optional[str]] = { - "firstname": tenant.forenames, - "lastname": tenant.surname, - "mobilephone": tenant.mobile, - "phone": tenant.telephone, - "secondary_phone_number": tenant.secondary_number, - "vulnerability_description": _vulnerability_description(tenant), - } - properties = {name: value for name, value in candidates.items() if value} - # Always written, so "not vulnerable" is distinguishable from "not synced". - properties["is_vulnerable"] = "true" if tenant.vulnerabilities else "false" - return properties - - -@dataclass(frozen=True) -class TenantDataSyncSummary: - """PII-free record of what a sync run did: safe to log and persist.""" - - tenancy_reference: str - contact_ids: Tuple[str, ...] - vulnerable_contact_count: int - - -TenantDataSyncResult = Union[TenantDataSyncSummary, AbriRequestRejected] - - -class TenantDataSyncError(Exception): - """A sync that stopped at the first failed HubSpot write. - - Carries a PII-free progress report so the operator knows the deal's - half-state before cleaning up and re-running; contacts created but not - associated are orphaned in HubSpot and need manual cleanup. - """ - - def __init__( - self, - message: str, - deal_id: str, - tenant_index: int, - contact_ids_created: Tuple[str, ...], - contact_ids_associated: Tuple[str, ...], - ) -> None: - self.deal_id = deal_id - self.tenant_index = tenant_index - self.contact_ids_created = contact_ids_created - self.contact_ids_associated = contact_ids_associated - super().__init__( - f"{message} (deal: {deal_id}, tenant index: {tenant_index}, " - f"contacts created: {list(contact_ids_created)}, " - f"associated: {list(contact_ids_associated)})" - ) - - @property - def orphaned_contact_ids(self) -> Tuple[str, ...]: - return tuple( - contact_id - for contact_id in self.contact_ids_created - if contact_id not in self.contact_ids_associated - ) - - -class AbriTenantDataSyncOrchestrator: - """Syncs an Abri tenancy's signatories to a HubSpot deal. - - Tenant data flows straight from Abri to HubSpot; nothing is persisted - in Domna's database or logs. - """ - - def __init__( - self, - abri_client: AbriClient, - hubspot: HubspotDealContactsClient, - ) -> None: - self._abri = abri_client - self._hubspot = hubspot - - def run(self, place_ref: PlaceRef, deal_id: str) -> TenantDataSyncResult: - tenancy = self._abri.get_tenant_data(place_ref) - - if isinstance(tenancy, AbriRequestRejected): - return tenancy - - contact_ids: List[str] = [] - associated_ids: List[str] = [] - for tenant_index, tenant in enumerate(tenancy.tenants): - try: - contact_id = self._hubspot.create_contact(_contact_properties(tenant)) - contact_ids.append(contact_id) - self._hubspot.associate_contact_to_deal( - deal_id=deal_id, contact_id=contact_id - ) - associated_ids.append(contact_id) - except HubspotRequestError as error: - # Fail fast at the first failed write; `error` is already - # scrubbed, so chaining it keeps tracebacks PII-free. - raise TenantDataSyncError( - message=str(error), - deal_id=deal_id, - tenant_index=tenant_index, - contact_ids_created=tuple(contact_ids), - contact_ids_associated=tuple(associated_ids), - ) from error - - return TenantDataSyncSummary( - tenancy_reference=tenancy.tenancy_reference, - contact_ids=tuple(contact_ids), - vulnerable_contact_count=sum( - 1 for tenant in tenancy.tenants if tenant.vulnerabilities - ), - ) diff --git a/scripts/smoke_test_tenant_contacts.py b/scripts/smoke_test_tenant_contacts.py index c211c076e..f9b55dc3b 100644 --- a/scripts/smoke_test_tenant_contacts.py +++ b/scripts/smoke_test_tenant_contacts.py @@ -1,7 +1,7 @@ """Manual smoke test for the Abri tenant-data -> HubSpot sync (issue #1467). -Drives the real TenantDataSyncOrchestrator end-to-end with only the Abri HTTP -edge stubbed (no Abri API key needed): the canned tenancy XML below flows +Drives the real AbriOrchestrator tenant-data flow end-to-end with only the +Abri HTTP edge stubbed (no Abri API key needed): the canned tenancy XML below flows through the real parse, phone-priority selection and vulnerability mapping, and the resulting contact writes hit the real HubSpot portal. @@ -31,9 +31,8 @@ from domain.abri.models import AbriRequestRejected, PlaceRef from infrastructure.abri.abri_client import AbriClient from infrastructure.abri.config import AbriConfig from infrastructure.hubspot.deal_contacts_client import HubspotDealContactsClient -from orchestration.abri_tenant_data_sync_orchestrator import ( - AbriTenantDataSyncOrchestrator, -) +from infrastructure.hubspot.deal_properties_client import HubspotDealPropertiesClient +from orchestration.abri_orchestrator import AbriOrchestrator DEAL_ID = "485125892321" @@ -101,6 +100,13 @@ def _stubbed_abri_client() -> AbriClient: return client +class _UnusedDealDatabase: + """Satisfies the gateway dependency; the tenant-data flow never writes it.""" + + def record_job_no(self, deal_id: str, job_no: str) -> None: + raise AssertionError("tenant-data sync must not touch the deal database") + + def _archive_contacts(sdk_client: Client) -> None: if not CONTACT_IDS_TO_ARCHIVE: raise SystemExit("ARCHIVE is set but CONTACT_IDS_TO_ARCHIVE is empty") @@ -119,12 +125,14 @@ def main() -> None: if DEAL_ID == "EDIT-ME": raise SystemExit("Set DEAL_ID to a throwaway test deal id first") - orchestrator = AbriTenantDataSyncOrchestrator( + orchestrator = AbriOrchestrator( abri_client=_stubbed_abri_client(), - hubspot=HubspotDealContactsClient(sdk_client=sdk_client), + deal_contacts=HubspotDealContactsClient(sdk_client=sdk_client), + deal_properties=HubspotDealPropertiesClient(sdk_client=sdk_client), + deal_database=_UnusedDealDatabase(), ) - result = orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + result = orchestrator.sync_tenant_data(PLACE_REF, deal_id=DEAL_ID) if isinstance(result, AbriRequestRejected): raise SystemExit(f"Unexpected Abri rejection from stub: {result}") diff --git a/tests/orchestration/test_abri_log_job_orchestrator.py b/tests/orchestration/test_abri_orchestrator_log_job.py similarity index 84% rename from tests/orchestration/test_abri_log_job_orchestrator.py rename to tests/orchestration/test_abri_orchestrator_log_job.py index 19c112280..b91739c52 100644 --- a/tests/orchestration/test_abri_log_job_orchestrator.py +++ b/tests/orchestration/test_abri_orchestrator_log_job.py @@ -15,9 +15,10 @@ from domain.abri.models import AbriRequestRejected, PlaceRef from infrastructure.abri.abri_client import AbriClient from infrastructure.abri.config import AbriConfig from infrastructure.abri.errors import AbriResponseParseError, AbriTransportError +from infrastructure.hubspot.deal_contacts_client import HubspotDealContactsClient from infrastructure.hubspot.deal_properties_client import HubspotDealPropertiesClient -from orchestration.abri_log_job_orchestrator import ( - AbriLogJobOrchestrator, +from orchestration.abri_orchestrator import ( + AbriOrchestrator, ConfirmedSurveyBooking, LogJobSummary, LogJobWriteBackError, @@ -77,15 +78,16 @@ def orchestrator( mock_session: MagicMock, sdk_client: MagicMock, deal_database: FakeDealDatabase, -) -> AbriLogJobOrchestrator: +) -> AbriOrchestrator: with patch( "infrastructure.abri.abri_client.requests.Session", return_value=mock_session, ): abri_client = AbriClient(config=CONFIG) - return AbriLogJobOrchestrator( + return AbriOrchestrator( abri_client=abri_client, - hubspot=HubspotDealPropertiesClient(sdk_client=sdk_client), + deal_contacts=HubspotDealContactsClient(sdk_client=sdk_client), + deal_properties=HubspotDealPropertiesClient(sdk_client=sdk_client), deal_database=deal_database, ) @@ -93,8 +95,8 @@ def orchestrator( # --- outbound request: the LogJob envelope is built from the deal's fields --- -def test_run_sends_a_logjob_envelope_built_from_the_deals_fields( - orchestrator: AbriLogJobOrchestrator, mock_session: MagicMock +def test_log_job_sends_a_logjob_envelope_built_from_the_deals_fields( + orchestrator: AbriOrchestrator, mock_session: MagicMock ) -> None: # Arrange mock_session.post.return_value.content = _load_fixture( @@ -102,7 +104,7 @@ def test_run_sends_a_logjob_envelope_built_from_the_deals_fields( ) # Act - orchestrator.run(BOOKING) + orchestrator.log_job(BOOKING) # Assert (url,) = mock_session.post.call_args.args @@ -132,8 +134,8 @@ def test_run_sends_a_logjob_envelope_built_from_the_deals_fields( # --- happy path: the returned job_no lands in HubSpot, then the database --- -def test_run_writes_the_job_no_to_the_hubspot_deal_and_then_the_database( - orchestrator: AbriLogJobOrchestrator, +def test_log_job_writes_the_job_no_to_the_hubspot_deal_and_then_the_database( + orchestrator: AbriOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, deal_database: FakeDealDatabase, @@ -144,7 +146,7 @@ def test_run_writes_the_job_no_to_the_hubspot_deal_and_then_the_database( ) # Act - result = orchestrator.run(BOOKING) + result = orchestrator.log_job(BOOKING) # Assert assert result == LogJobSummary(deal_id=DEAL_ID, job_no="AD0226519") @@ -160,8 +162,8 @@ def test_run_writes_the_job_no_to_the_hubspot_deal_and_then_the_database( # --- rejection passthrough: OpenHousing's actual reason, nothing recorded --- -def test_run_returns_the_openhousing_rejection_verbatim_and_records_nothing( - orchestrator: AbriLogJobOrchestrator, +def test_log_job_returns_the_openhousing_rejection_verbatim_and_records_nothing( + orchestrator: AbriOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, deal_database: FakeDealDatabase, @@ -172,7 +174,7 @@ def test_run_returns_the_openhousing_rejection_verbatim_and_records_nothing( ) # Act - result = orchestrator.run(BOOKING) + result = orchestrator.log_job(BOOKING) # Assert assert isinstance(result, AbriRequestRejected) @@ -194,8 +196,8 @@ def test_run_returns_the_openhousing_rejection_verbatim_and_records_nothing( b"", ], ) -def test_run_raises_a_retriable_parse_error_on_an_unexpectedly_shaped_response( - orchestrator: AbriLogJobOrchestrator, +def test_log_job_raises_a_retriable_parse_error_on_an_unexpectedly_shaped_response( + orchestrator: AbriOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, deal_database: FakeDealDatabase, @@ -206,13 +208,13 @@ def test_run_raises_a_retriable_parse_error_on_an_unexpectedly_shaped_response( # Act / Assert with pytest.raises(AbriResponseParseError): - orchestrator.run(BOOKING) + orchestrator.log_job(BOOKING) assert sdk_client.mock_calls == [] assert deal_database.recorded_job_nos == [] -def test_run_raises_a_transport_error_when_the_relay_is_unreachable( - orchestrator: AbriLogJobOrchestrator, +def test_log_job_raises_a_transport_error_when_the_relay_is_unreachable( + orchestrator: AbriOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, deal_database: FakeDealDatabase, @@ -222,7 +224,7 @@ def test_run_raises_a_transport_error_when_the_relay_is_unreachable( # Act / Assert with pytest.raises(AbriTransportError): - orchestrator.run(BOOKING) + orchestrator.log_job(BOOKING) assert sdk_client.mock_calls == [] assert deal_database.recorded_job_nos == [] @@ -245,8 +247,8 @@ def _body_echoing_api_error() -> DealsApiException: return api_error -def test_run_raises_an_error_carrying_the_orphaned_job_no_when_the_hubspot_write_fails( - orchestrator: AbriLogJobOrchestrator, +def test_log_job_raises_an_error_carrying_the_orphaned_job_no_when_the_hubspot_write_fails( + orchestrator: AbriOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, deal_database: FakeDealDatabase, @@ -259,7 +261,7 @@ def test_run_raises_an_error_carrying_the_orphaned_job_no_when_the_hubspot_write # Act with pytest.raises(LogJobWriteBackError) as exc_info: - orchestrator.run(BOOKING) + orchestrator.log_job(BOOKING) # Assert: the orphaned job_no is reported for manual paste-in, the # database write is skipped, and log_job is never retried. @@ -271,8 +273,8 @@ def test_run_raises_an_error_carrying_the_orphaned_job_no_when_the_hubspot_write assert mock_session.post.call_count == 1 -def test_run_raises_a_write_back_error_scrubbed_of_the_hubspot_response_body( - orchestrator: AbriLogJobOrchestrator, +def test_log_job_raises_a_write_back_error_scrubbed_of_the_hubspot_response_body( + orchestrator: AbriOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: @@ -284,7 +286,7 @@ def test_run_raises_a_write_back_error_scrubbed_of_the_hubspot_response_body( # Act with pytest.raises(LogJobWriteBackError) as exc_info: - orchestrator.run(BOOKING) + orchestrator.log_job(BOOKING) # Assert: non-PII diagnostics survive; the echoed body does not rendered_traceback = "".join(traceback.format_exception(exc_info.value)) @@ -294,8 +296,8 @@ def test_run_raises_a_write_back_error_scrubbed_of_the_hubspot_response_body( assert "Invalid value" not in rendered_traceback -def test_run_retries_a_rate_limited_hubspot_write_before_succeeding( - orchestrator: AbriLogJobOrchestrator, +def test_log_job_retries_a_rate_limited_hubspot_write_before_succeeding( + orchestrator: AbriOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, deal_database: FakeDealDatabase, @@ -309,7 +311,7 @@ def test_run_retries_a_rate_limited_hubspot_write_before_succeeding( sdk_client.crm.deals.basic_api.update.side_effect = [rate_limited, MagicMock()] # Act - result = orchestrator.run(BOOKING) + result = orchestrator.log_job(BOOKING) # Assert assert result == LogJobSummary(deal_id=DEAL_ID, job_no="AD0226519") diff --git a/tests/orchestration/test_abri_tenant_data_sync_orchestrator.py b/tests/orchestration/test_abri_orchestrator_tenant_data_sync.py similarity index 81% rename from tests/orchestration/test_abri_tenant_data_sync_orchestrator.py rename to tests/orchestration/test_abri_orchestrator_tenant_data_sync.py index ad7a738b5..e45e48686 100644 --- a/tests/orchestration/test_abri_tenant_data_sync_orchestrator.py +++ b/tests/orchestration/test_abri_orchestrator_tenant_data_sync.py @@ -12,12 +12,13 @@ from hubspot.crm.contacts import SimplePublicObjectInputForCreate # type: ignor from domain.abri.models import AbriRequestRejected, PlaceRef from infrastructure.hubspot.deal_contacts_client import HubspotDealContactsClient +from infrastructure.hubspot.deal_properties_client import HubspotDealPropertiesClient from infrastructure.abri.abri_client import AbriClient from infrastructure.abri.config import AbriConfig from infrastructure.abri.errors import AbriResponseParseError, AbriTransportError -from orchestration.abri_tenant_data_sync_orchestrator import ( +from orchestration.abri_orchestrator import ( TenantDataSyncError, - AbriTenantDataSyncOrchestrator, + AbriOrchestrator, TenantDataSyncSummary, ) @@ -59,32 +60,41 @@ def sdk_client() -> MagicMock: return sdk +class FakeDealDatabase: + """In-memory stand-in for the deal-database gateway; unused by this flow.""" + + def record_job_no(self, deal_id: str, job_no: str) -> None: + raise AssertionError("tenant-data sync must not touch the deal database") + + @pytest.fixture() def orchestrator( mock_session: MagicMock, sdk_client: MagicMock -) -> AbriTenantDataSyncOrchestrator: +) -> AbriOrchestrator: with patch( "infrastructure.abri.abri_client.requests.Session", return_value=mock_session, ): abri_client = AbriClient(config=CONFIG) - return AbriTenantDataSyncOrchestrator( + return AbriOrchestrator( abri_client=abri_client, - hubspot=HubspotDealContactsClient(sdk_client=sdk_client), + deal_contacts=HubspotDealContactsClient(sdk_client=sdk_client), + deal_properties=HubspotDealPropertiesClient(sdk_client=sdk_client), + deal_database=FakeDealDatabase(), ) # --- outbound request: the spec's recorded envelope --- -def test_run_sends_the_recorded_getscstenantdata_envelope_to_the_relay_endpoint( - orchestrator: AbriTenantDataSyncOrchestrator, mock_session: MagicMock +def test_sync_tenant_data_sends_the_recorded_getscstenantdata_envelope_to_the_relay_endpoint( + orchestrator: AbriOrchestrator, mock_session: MagicMock ) -> None: # Arrange mock_session.post.return_value.content = _tenancy_response("") # Act - orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + orchestrator.sync_tenant_data(PLACE_REF, deal_id=DEAL_ID) # Assert (url,) = mock_session.post.call_args.args @@ -99,8 +109,8 @@ def test_run_sends_the_recorded_getscstenantdata_envelope_to_the_relay_endpoint( # --- happy path: signatories become associated deal contacts --- -def test_run_creates_an_associated_deal_contact_per_signatory_and_returns_a_summary( - orchestrator: AbriTenantDataSyncOrchestrator, +def test_sync_tenant_data_creates_an_associated_deal_contact_per_signatory_and_returns_a_summary( + orchestrator: AbriOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: @@ -112,7 +122,7 @@ def test_run_creates_an_associated_deal_contact_per_signatory_and_returns_a_summ ) # Act - result = orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + result = orchestrator.sync_tenant_data(PLACE_REF, deal_id=DEAL_ID) # Assert assert result == TenantDataSyncSummary( @@ -144,8 +154,8 @@ def test_run_creates_an_associated_deal_contact_per_signatory_and_returns_a_summ ) -def test_run_creates_a_separate_contact_for_each_signatory( - orchestrator: AbriTenantDataSyncOrchestrator, +def test_sync_tenant_data_creates_a_separate_contact_for_each_signatory( + orchestrator: AbriOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: @@ -159,7 +169,7 @@ def test_run_creates_a_separate_contact_for_each_signatory( ] # Act - result = orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + result = orchestrator.sync_tenant_data(PLACE_REF, deal_id=DEAL_ID) # Assert assert result == TenantDataSyncSummary( @@ -193,8 +203,8 @@ def test_run_creates_a_separate_contact_for_each_signatory( assert associated_contact_ids == ["60123", "60124"] -def test_run_picks_the_best_priority_number_of_each_kind_for_a_contact( - orchestrator: AbriTenantDataSyncOrchestrator, +def test_sync_tenant_data_picks_the_best_priority_number_of_each_kind_for_a_contact( + orchestrator: AbriOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: @@ -213,7 +223,7 @@ def test_run_picks_the_best_priority_number_of_each_kind_for_a_contact( ) # Act - orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + orchestrator.sync_tenant_data(PLACE_REF, deal_id=DEAL_ID) # Assert created_properties = sdk_client.crm.contacts.basic_api.create.call_args.kwargs[ @@ -223,8 +233,8 @@ def test_run_picks_the_best_priority_number_of_each_kind_for_a_contact( assert created_properties["phone"] == "01000000007" -def test_run_fills_secondary_phone_number_with_the_best_leftover_number( - orchestrator: AbriTenantDataSyncOrchestrator, +def test_sync_tenant_data_fills_secondary_phone_number_with_the_best_leftover_number( + orchestrator: AbriOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: @@ -240,7 +250,7 @@ def test_run_fills_secondary_phone_number_with_the_best_leftover_number( ) # Act - orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + orchestrator.sync_tenant_data(PLACE_REF, deal_id=DEAL_ID) # Assert sdk_client.crm.contacts.basic_api.create.assert_called_once_with( @@ -260,8 +270,8 @@ def test_run_fills_secondary_phone_number_with_the_best_leftover_number( # --- vulnerabilities: recorded on the tenant's own contact --- -def test_run_records_vulnerabilities_on_the_tenants_own_contact( - orchestrator: AbriTenantDataSyncOrchestrator, +def test_sync_tenant_data_records_vulnerabilities_on_the_tenants_own_contact( + orchestrator: AbriOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: @@ -282,7 +292,7 @@ def test_run_records_vulnerabilities_on_the_tenants_own_contact( ] # Act - result = orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + result = orchestrator.sync_tenant_data(PLACE_REF, deal_id=DEAL_ID) # Assert assert result == TenantDataSyncSummary( @@ -309,8 +319,8 @@ def test_run_records_vulnerabilities_on_the_tenants_own_contact( ] -def test_run_still_creates_a_contact_for_an_all_blank_signatory( - orchestrator: AbriTenantDataSyncOrchestrator, +def test_sync_tenant_data_still_creates_a_contact_for_an_all_blank_signatory( + orchestrator: AbriOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: @@ -321,7 +331,7 @@ def test_run_still_creates_a_contact_for_an_all_blank_signatory( ) # Act - result = orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + result = orchestrator.sync_tenant_data(PLACE_REF, deal_id=DEAL_ID) # Assert assert result == TenantDataSyncSummary( @@ -339,8 +349,8 @@ def test_run_still_creates_a_contact_for_an_all_blank_signatory( # --- unusual but valid outcomes --- -def test_run_treats_a_tenancy_with_no_signatories_as_a_valid_zero_contact_outcome( - orchestrator: AbriTenantDataSyncOrchestrator, +def test_sync_tenant_data_treats_a_tenancy_with_no_signatories_as_a_valid_zero_contact_outcome( + orchestrator: AbriOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: @@ -348,7 +358,7 @@ def test_run_treats_a_tenancy_with_no_signatories_as_a_valid_zero_contact_outcom mock_session.post.return_value.content = _tenancy_response("") # Act - result = orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + result = orchestrator.sync_tenant_data(PLACE_REF, deal_id=DEAL_ID) # Assert assert result == TenantDataSyncSummary( @@ -362,8 +372,8 @@ def test_run_treats_a_tenancy_with_no_signatories_as_a_valid_zero_contact_outcom # --- rejection passthrough --- -def test_run_returns_the_abri_rejection_verbatim_and_makes_no_hubspot_calls( - orchestrator: AbriTenantDataSyncOrchestrator, +def test_sync_tenant_data_returns_the_abri_rejection_verbatim_and_makes_no_hubspot_calls( + orchestrator: AbriOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: @@ -373,7 +383,7 @@ def test_run_returns_the_abri_rejection_verbatim_and_makes_no_hubspot_calls( ) # Act - result = orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + result = orchestrator.sync_tenant_data(PLACE_REF, deal_id=DEAL_ID) # Assert assert isinstance(result, AbriRequestRejected) @@ -395,8 +405,8 @@ def test_run_returns_the_abri_rejection_verbatim_and_makes_no_hubspot_calls( b"", ], ) -def test_run_raises_a_retriable_parse_error_on_an_unexpectedly_shaped_response( - orchestrator: AbriTenantDataSyncOrchestrator, +def test_sync_tenant_data_raises_a_retriable_parse_error_on_an_unexpectedly_shaped_response( + orchestrator: AbriOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, body: bytes, @@ -406,12 +416,12 @@ def test_run_raises_a_retriable_parse_error_on_an_unexpectedly_shaped_response( # Act / Assert with pytest.raises(AbriResponseParseError): - orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + orchestrator.sync_tenant_data(PLACE_REF, deal_id=DEAL_ID) assert sdk_client.mock_calls == [] -def test_run_raises_a_transport_error_when_the_relay_is_unreachable( - orchestrator: AbriTenantDataSyncOrchestrator, +def test_sync_tenant_data_raises_a_transport_error_when_the_relay_is_unreachable( + orchestrator: AbriOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: @@ -420,7 +430,7 @@ def test_run_raises_a_transport_error_when_the_relay_is_unreachable( # Act / Assert with pytest.raises(AbriTransportError): - orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + orchestrator.sync_tenant_data(PLACE_REF, deal_id=DEAL_ID) assert sdk_client.mock_calls == [] @@ -442,8 +452,8 @@ def _pii_echoing_api_error() -> ContactsApiException: return api_error -def test_run_raises_an_error_scrubbed_of_the_hubspot_response_body( - orchestrator: AbriTenantDataSyncOrchestrator, +def test_sync_tenant_data_raises_an_error_scrubbed_of_the_hubspot_response_body( + orchestrator: AbriOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: @@ -456,7 +466,7 @@ def test_run_raises_an_error_scrubbed_of_the_hubspot_response_body( # Act with pytest.raises(Exception) as exc_info: - orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + orchestrator.sync_tenant_data(PLACE_REF, deal_id=DEAL_ID) # Assert: non-PII diagnostics survive; the echoed body does not rendered_traceback = "".join(traceback.format_exception(exc_info.value)) @@ -468,8 +478,8 @@ def test_run_raises_an_error_scrubbed_of_the_hubspot_response_body( assert "07700900123" not in rendered_traceback -def test_run_fails_fast_when_association_fails_and_reports_the_orphaned_contact( - orchestrator: AbriTenantDataSyncOrchestrator, +def test_sync_tenant_data_fails_fast_when_association_fails_and_reports_the_orphaned_contact( + orchestrator: AbriOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: @@ -483,7 +493,7 @@ def test_run_fails_fast_when_association_fails_and_reports_the_orphaned_contact( # Act with pytest.raises(TenantDataSyncError) as exc_info: - orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + orchestrator.sync_tenant_data(PLACE_REF, deal_id=DEAL_ID) # Assert: the flow stopped at the first failed write, with progress reported error = exc_info.value @@ -499,8 +509,8 @@ def test_run_fails_fast_when_association_fails_and_reports_the_orphaned_contact( assert "07700900456" not in rendered_traceback -def test_run_retries_a_rate_limited_hubspot_call_before_succeeding( - orchestrator: AbriTenantDataSyncOrchestrator, +def test_sync_tenant_data_retries_a_rate_limited_hubspot_call_before_succeeding( + orchestrator: AbriOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: @@ -517,7 +527,7 @@ def test_run_retries_a_rate_limited_hubspot_call_before_succeeding( ] # Act - result = orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + result = orchestrator.sync_tenant_data(PLACE_REF, deal_id=DEAL_ID) # Assert assert result == TenantDataSyncSummary(