From 78fcba45e5a3c5ed57a34152927e129322a4c5da Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 13:22:52 +0000 Subject: [PATCH 01/25] =?UTF-8?q?Abri=20tenancy=20signatory=20becomes=20an?= =?UTF-8?q?=20associated=20HubSpot=20deal=20contact=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 --- domain/abri/models.py | 30 ++++- etl/hubspot/deal_contacts_client.py | 25 ++++ infrastructure/abri/abri_client.py | 5 + .../tenant_data_sync_orchestrator.py | 37 ++++++ .../test_tenant_data_sync_orchestrator.py | 109 ++++++++++++++++++ 5 files changed, 205 insertions(+), 1 deletion(-) create mode 100644 etl/hubspot/deal_contacts_client.py create mode 100644 orchestration/tenant_data_sync_orchestrator.py create mode 100644 tests/orchestration/test_tenant_data_sync_orchestrator.py diff --git a/domain/abri/models.py b/domain/abri/models.py index 0444ec1cc..ceb03c34f 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, Optional, Union +from typing import Literal, NewType, Optional, Tuple, Union PlaceRef = NewType("PlaceRef", str) @@ -48,3 +48,31 @@ class AppointmentAmended: AmendJobResult = Union[AppointmentAmended, AbriRequestRejected] + + +@dataclass(frozen=True) +class Tenant: + """A tenancy signatory, minimised to the fields the HubSpot sync needs. + + Person title, per-person references, the main-contact flag and tenancy + dates are deliberately dropped at parse time (data minimisation); mobile + and telephone are the single best number of each kind, pre-selected by + priority. + """ + + forenames: str + surname: str + mobile: Optional[str] + telephone: Optional[str] + vulnerabilities: Tuple[str, ...] + + +@dataclass(frozen=True) +class TenancyData: + """Tenancy signatories for a place; the reference is PII-safe.""" + + tenancy_reference: str + tenants: Tuple[Tenant, ...] + + +GetTenantDataResult = Union[TenancyData, AbriRequestRejected] diff --git a/etl/hubspot/deal_contacts_client.py b/etl/hubspot/deal_contacts_client.py new file mode 100644 index 000000000..df8a8f0ec --- /dev/null +++ b/etl/hubspot/deal_contacts_client.py @@ -0,0 +1,25 @@ +from typing import Dict + +from hubspot.client import Client # type: ignore[reportMissingTypeStubs] + +# HubSpot-defined deal -> contact association ("deal_to_contact" in v3 terms). +DEAL_TO_CONTACT_ASSOCIATION_TYPE_ID = 3 + + +class HubspotDealContactsClient: + """Focused HubSpot client for the tenant-data sync flow. + + Deliberately separate from the legacy HubspotClient: the SDK client is + constructor-injected, and SDK errors are re-raised scrubbed of response + bodies (HubSpot validation errors echo submitted property values, which + here are tenant PII). + """ + + def __init__(self, sdk_client: Client) -> None: + self._client: Client = sdk_client + + def create_contact(self, properties: Dict[str, str]) -> str: + raise NotImplementedError + + def associate_contact_to_deal(self, deal_id: str, contact_id: str) -> None: + raise NotImplementedError diff --git a/infrastructure/abri/abri_client.py b/infrastructure/abri/abri_client.py index 3c95c2766..6c9cc883d 100644 --- a/infrastructure/abri/abri_client.py +++ b/infrastructure/abri/abri_client.py @@ -9,9 +9,11 @@ from domain.abri.models import ( AmendJobRequest, AmendJobResult, AppointmentAmended, + GetTenantDataResult, JobLogged, LogJobRequest, LogJobResult, + PlaceRef, ) from infrastructure.abri.config import AbriConfig from infrastructure.abri.envelope import serialise_relay_request @@ -73,6 +75,9 @@ class AbriClient: return self._parse_appointment_amended(outcome) + def get_tenant_data(self, place_ref: PlaceRef) -> GetTenantDataResult: + raise NotImplementedError + def _exchange( self, request_type: str, parameters: List[Tuple[str, str]] ) -> Union[ET.Element, AbriRequestRejected]: diff --git a/orchestration/tenant_data_sync_orchestrator.py b/orchestration/tenant_data_sync_orchestrator.py new file mode 100644 index 000000000..18790f9d1 --- /dev/null +++ b/orchestration/tenant_data_sync_orchestrator.py @@ -0,0 +1,37 @@ +from dataclasses import dataclass +from typing import Tuple, Union + +from domain.abri.models import AbriRequestRejected, PlaceRef +from etl.hubspot.deal_contacts_client import HubspotDealContactsClient +from infrastructure.abri.abri_client import AbriClient + + +@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 TenantDataSyncOrchestrator: + """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: + raise NotImplementedError diff --git a/tests/orchestration/test_tenant_data_sync_orchestrator.py b/tests/orchestration/test_tenant_data_sync_orchestrator.py new file mode 100644 index 000000000..89e188b35 --- /dev/null +++ b/tests/orchestration/test_tenant_data_sync_orchestrator.py @@ -0,0 +1,109 @@ +from unittest.mock import MagicMock, patch + +import pytest +from hubspot.crm.associations.v4 import AssociationSpec # type: ignore[reportMissingTypeStubs] +from hubspot.crm.contacts import SimplePublicObjectInputForCreate # type: ignore[reportMissingTypeStubs] + +from domain.abri.models import PlaceRef +from etl.hubspot.deal_contacts_client import HubspotDealContactsClient +from infrastructure.abri.abri_client import AbriClient +from infrastructure.abri.config import AbriConfig +from orchestration.tenant_data_sync_orchestrator import ( + TenantDataSyncOrchestrator, + TenantDataSyncSummary, +) + +ENDPOINT_URL = "https://relay.example.test/api/DomnaRelay?code=test-function-key" +PLACE_REF = PlaceRef("1004202A") +DEAL_ID = "9876543210" + +CONFIG = AbriConfig( + endpoint_url=ENDPOINT_URL, + username="DomnaWeb", + password="", + default_resource="NAULKH", +) + + +def _tenancy_response( + tenants_xml: str, tenancy_reference: str = "10042020051017" +) -> bytes: + return ( + f'' + f"{tenancy_reference}{tenants_xml}" + ).encode() + + +@pytest.fixture() +def mock_session() -> MagicMock: + return MagicMock() + + +@pytest.fixture() +def sdk_client() -> MagicMock: + sdk = MagicMock() + sdk.crm.contacts.basic_api.create.return_value.id = "60123" + return sdk + + +@pytest.fixture() +def orchestrator( + mock_session: MagicMock, sdk_client: MagicMock +) -> TenantDataSyncOrchestrator: + with patch( + "infrastructure.abri.abri_client.requests.Session", + return_value=mock_session, + ): + abri_client = AbriClient(config=CONFIG) + return TenantDataSyncOrchestrator( + abri_client=abri_client, + hubspot=HubspotDealContactsClient(sdk_client=sdk_client), + ) + + +# --- happy path: signatories become associated deal contacts --- + + +def test_run_creates_an_associated_deal_contact_per_signatory_and_returns_a_summary( + orchestrator: TenantDataSyncOrchestrator, + mock_session: MagicMock, + sdk_client: MagicMock, +) -> None: + # Arrange + mock_session.post.return_value.content = _tenancy_response( + '136847309853460810' + '02473757484' + ) + + # Act + result = orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + + # Assert + assert result == TenantDataSyncSummary( + tenancy_reference="10042020051017", + contact_ids=("60123",), + vulnerable_contact_count=0, + ) + sdk_client.crm.contacts.basic_api.create.assert_called_once_with( + simple_public_object_input_for_create=SimplePublicObjectInputForCreate( + properties={ + "firstname": "Amanjeet", + "lastname": "Okello", + "mobilephone": "09853460810", + "phone": "02473757484", + "is_vulnerable": "false", + } + ) + ) + sdk_client.crm.associations.v4.basic_api.create.assert_called_once_with( + object_type="deals", + object_id=DEAL_ID, + to_object_type="contacts", + to_object_id="60123", + association_spec=[ + AssociationSpec( + association_category="HUBSPOT_DEFINED", association_type_id=3 + ) + ], + ) From 76d0b4bac999bb0f76bebd8bd424ef8612c38592 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 13:24:47 +0000 Subject: [PATCH 02/25] =?UTF-8?q?Abri=20tenancy=20signatory=20becomes=20an?= =?UTF-8?q?=20associated=20HubSpot=20deal=20contact=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/deal_contacts_client.py | 22 +++++++- infrastructure/abri/abri_client.py | 53 ++++++++++++++++++- .../tenant_data_sync_orchestrator.py | 38 +++++++++++-- 3 files changed, 106 insertions(+), 7 deletions(-) diff --git a/etl/hubspot/deal_contacts_client.py b/etl/hubspot/deal_contacts_client.py index df8a8f0ec..6765b5da5 100644 --- a/etl/hubspot/deal_contacts_client.py +++ b/etl/hubspot/deal_contacts_client.py @@ -1,6 +1,8 @@ from typing import 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] # HubSpot-defined deal -> contact association ("deal_to_contact" in v3 terms). DEAL_TO_CONTACT_ASSOCIATION_TYPE_ID = 3 @@ -19,7 +21,23 @@ class HubspotDealContactsClient: self._client: Client = sdk_client def create_contact(self, properties: Dict[str, str]) -> str: - raise NotImplementedError + contact = self._client.crm.contacts.basic_api.create( # type: ignore[reportUnknownMemberType] + simple_public_object_input_for_create=SimplePublicObjectInputForCreate( + properties=properties + ) + ) + return str(contact.id) # type: ignore[reportUnknownMemberType] def associate_contact_to_deal(self, deal_id: str, contact_id: str) -> None: - raise NotImplementedError + self._client.crm.associations.v4.basic_api.create( # type: ignore[reportUnknownMemberType] + object_type="deals", + object_id=deal_id, + to_object_type="contacts", + to_object_id=contact_id, + association_spec=[ + AssociationSpec( + association_category="HUBSPOT_DEFINED", + association_type_id=DEAL_TO_CONTACT_ASSOCIATION_TYPE_ID, + ) + ], + ) diff --git a/infrastructure/abri/abri_client.py b/infrastructure/abri/abri_client.py index 6c9cc883d..16f74e17b 100644 --- a/infrastructure/abri/abri_client.py +++ b/infrastructure/abri/abri_client.py @@ -1,6 +1,6 @@ import xml.etree.ElementTree as ET from datetime import date -from typing import List, Tuple, Union +from typing import List, Optional, Tuple, Union import requests @@ -14,6 +14,8 @@ from domain.abri.models import ( LogJobRequest, LogJobResult, PlaceRef, + TenancyData, + Tenant, ) from infrastructure.abri.config import AbriConfig from infrastructure.abri.envelope import serialise_relay_request @@ -28,6 +30,14 @@ def _format_appointment_date(appointment_date: date) -> str: return appointment_date.strftime("%d/%m/%Y") +def _select_phone_number(entries: List[ET.Element]) -> Optional[str]: + for entry in entries: + number = (entry.text or "").strip() + if number: + return number + return None + + class AbriClient: def __init__(self, config: AbriConfig) -> None: self._config = config @@ -76,7 +86,15 @@ class AbriClient: return self._parse_appointment_amended(outcome) def get_tenant_data(self, place_ref: PlaceRef) -> GetTenantDataResult: - raise NotImplementedError + outcome = self._exchange( + request_type="getSCSTenantData", + parameters=[("place_ref", place_ref)], + ) + + if isinstance(outcome, AbriRequestRejected): + return outcome + + return self._parse_tenancy_data(outcome) def _exchange( self, request_type: str, parameters: List[Tuple[str, str]] @@ -164,6 +182,37 @@ class AbriClient: appointment_time=appointment_time, ) + @staticmethod + def _parse_tenancy_data(root: ET.Element) -> TenancyData: + tenancy = root.find("Tenancy") + + if tenancy is None: + raise AbriResponseParseError( + "Tenancy element missing from relay response" + ) + + tenancy_reference = (tenancy.text or "").strip() + + return TenancyData( + tenancy_reference=tenancy_reference, + tenants=tuple( + AbriClient._parse_tenant(tenant) + for tenant in tenancy.findall("Tenant") + ), + ) + + @staticmethod + def _parse_tenant(element: ET.Element) -> Tenant: + # Data minimisation: person_title, main-contact and the per-person + # reference (the element text) are deliberately not read. + return Tenant( + forenames=(element.get("forenames") or "").strip(), + surname=(element.get("surname") or "").strip(), + mobile=_select_phone_number(element.findall("Mobile")), + telephone=_select_phone_number(element.findall("Telephone")), + vulnerabilities=(), + ) + @staticmethod def _parse_rejection(root: ET.Element) -> AbriRequestRejected: success = root.findtext("success") diff --git a/orchestration/tenant_data_sync_orchestrator.py b/orchestration/tenant_data_sync_orchestrator.py index 18790f9d1..038489ea7 100644 --- a/orchestration/tenant_data_sync_orchestrator.py +++ b/orchestration/tenant_data_sync_orchestrator.py @@ -1,11 +1,24 @@ from dataclasses import dataclass -from typing import Tuple, Union +from typing import Dict, List, Optional, Tuple, Union -from domain.abri.models import AbriRequestRejected, PlaceRef +from domain.abri.models import AbriRequestRejected, PlaceRef, Tenant from etl.hubspot.deal_contacts_client import HubspotDealContactsClient from infrastructure.abri.abri_client import AbriClient +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, + } + 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.""" @@ -34,4 +47,23 @@ class TenantDataSyncOrchestrator: self._hubspot = hubspot def run(self, place_ref: PlaceRef, deal_id: str) -> TenantDataSyncResult: - raise NotImplementedError + tenancy = self._abri.get_tenant_data(place_ref) + + if isinstance(tenancy, AbriRequestRejected): + return tenancy + + contact_ids: List[str] = [] + for tenant in tenancy.tenants: + contact_id = self._hubspot.create_contact(_contact_properties(tenant)) + self._hubspot.associate_contact_to_deal( + deal_id=deal_id, contact_id=contact_id + ) + contact_ids.append(contact_id) + + 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 + ), + ) From 0ef1687bd7b70a4ed7e123b11ebb26566014cd05 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 13:26:07 +0000 Subject: [PATCH 03/25] =?UTF-8?q?Relay=20receives=20the=20spec's=20recorde?= =?UTF-8?q?d=20getSCSTenantData=20envelope=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 --- ...relay_getscstenantdata_request_example.xml | 11 +++++++ .../test_tenant_data_sync_orchestrator.py | 29 +++++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 tests/abri/abri_relay_getscstenantdata_request_example.xml diff --git a/tests/abri/abri_relay_getscstenantdata_request_example.xml b/tests/abri/abri_relay_getscstenantdata_request_example.xml new file mode 100644 index 000000000..785710678 --- /dev/null +++ b/tests/abri/abri_relay_getscstenantdata_request_example.xml @@ -0,0 +1,11 @@ + + +
+ +
+ + + + + + diff --git a/tests/orchestration/test_tenant_data_sync_orchestrator.py b/tests/orchestration/test_tenant_data_sync_orchestrator.py index 89e188b35..771abdb9d 100644 --- a/tests/orchestration/test_tenant_data_sync_orchestrator.py +++ b/tests/orchestration/test_tenant_data_sync_orchestrator.py @@ -1,3 +1,5 @@ +import xml.etree.ElementTree as ET +from pathlib import Path from unittest.mock import MagicMock, patch import pytest @@ -13,6 +15,7 @@ from orchestration.tenant_data_sync_orchestrator import ( TenantDataSyncSummary, ) +FIXTURE_DIR = Path(__file__).parents[1] / "abri" ENDPOINT_URL = "https://relay.example.test/api/DomnaRelay?code=test-function-key" PLACE_REF = PlaceRef("1004202A") DEAL_ID = "9876543210" @@ -25,6 +28,10 @@ CONFIG = AbriConfig( ) +def _load_fixture(name: str) -> bytes: + return (FIXTURE_DIR / name).read_bytes() + + def _tenancy_response( tenants_xml: str, tenancy_reference: str = "10042020051017" ) -> bytes: @@ -61,6 +68,28 @@ def orchestrator( ) +# --- outbound request: the spec's recorded envelope --- + + +def test_run_sends_the_recorded_getscstenantdata_envelope_to_the_relay_endpoint( + orchestrator: TenantDataSyncOrchestrator, mock_session: MagicMock +) -> None: + # Arrange + mock_session.post.return_value.content = _tenancy_response("") + + # Act + orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + + # Assert + (url,) = mock_session.post.call_args.args + sent_body: bytes = mock_session.post.call_args.kwargs["data"] + expected_body = _load_fixture("abri_relay_getscstenantdata_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 + ) + + # --- happy path: signatories become associated deal contacts --- From 01e61a62753fa2301e692c6821d19cb04efb1669 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 13:27:04 +0000 Subject: [PATCH 04/25] =?UTF-8?q?Each=20tenancy=20signatory=20gets=20its?= =?UTF-8?q?=20own=20deal=20contact,=20with=20blank=20fields=20omitted=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 --- ..._getscstenantdata_multitenant_response.xml | 8 ++ .../test_tenant_data_sync_orchestrator.py | 76 +++++++++++++++++++ 2 files changed, 84 insertions(+) create mode 100644 tests/abri/abri_relay_getscstenantdata_multitenant_response.xml diff --git a/tests/abri/abri_relay_getscstenantdata_multitenant_response.xml b/tests/abri/abri_relay_getscstenantdata_multitenant_response.xml new file mode 100644 index 000000000..d95e3ce81 --- /dev/null +++ b/tests/abri/abri_relay_getscstenantdata_multitenant_response.xml @@ -0,0 +1,8 @@ + + 10071650064015400721609261195827 + + 400721509286114923 + 02769674129 + + + diff --git a/tests/orchestration/test_tenant_data_sync_orchestrator.py b/tests/orchestration/test_tenant_data_sync_orchestrator.py index 771abdb9d..3ed1272aa 100644 --- a/tests/orchestration/test_tenant_data_sync_orchestrator.py +++ b/tests/orchestration/test_tenant_data_sync_orchestrator.py @@ -136,3 +136,79 @@ 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: TenantDataSyncOrchestrator, + mock_session: MagicMock, + sdk_client: MagicMock, +) -> None: + # Arrange + mock_session.post.return_value.content = _load_fixture( + "abri_relay_getscstenantdata_multitenant_response.xml" + ) + sdk_client.crm.contacts.basic_api.create.side_effect = [ + MagicMock(id="60123"), + MagicMock(id="60124"), + ] + + # Act + result = orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + + # Assert + assert result == TenantDataSyncSummary( + tenancy_reference="10071650064015", + contact_ids=("60123", "60124"), + vulnerable_contact_count=0, + ) + created_properties = [ + call.kwargs["simple_public_object_input_for_create"].properties + for call in sdk_client.crm.contacts.basic_api.create.call_args_list + ] + assert created_properties == [ + { + "firstname": "Harper", + "lastname": "Solecka", + "mobilephone": "09261195827", + "is_vulnerable": "false", + }, + { + "firstname": "Naoimh", + "lastname": "Malyar", + "mobilephone": "09286114923", + "phone": "02769674129", + "is_vulnerable": "false", + }, + ] + associated_contact_ids = [ + call.kwargs["to_object_id"] + for call in sdk_client.crm.associations.v4.basic_api.create.call_args_list + ] + assert associated_contact_ids == ["60123", "60124"] + + +def test_run_still_creates_a_contact_for_an_all_blank_signatory( + orchestrator: TenantDataSyncOrchestrator, + mock_session: MagicMock, + sdk_client: MagicMock, +) -> None: + # Arrange + mock_session.post.return_value.content = _tenancy_response( + '4007216' + '' + ) + + # Act + result = orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + + # Assert + assert result == TenantDataSyncSummary( + tenancy_reference="10042020051017", + contact_ids=("60123",), + vulnerable_contact_count=0, + ) + sdk_client.crm.contacts.basic_api.create.assert_called_once_with( + simple_public_object_input_for_create=SimplePublicObjectInputForCreate( + properties={"is_vulnerable": "false"} + ) + ) From 1d3219ca600e4657f59d5e13fa57faf80b513179 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 13:27:39 +0000 Subject: [PATCH 05/25] =?UTF-8?q?Contact=20carries=20the=20best-priority?= =?UTF-8?q?=20mobile=20and=20landline=20Abri=20lists=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_tenant_data_sync_orchestrator.py | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/orchestration/test_tenant_data_sync_orchestrator.py b/tests/orchestration/test_tenant_data_sync_orchestrator.py index 3ed1272aa..329f04a90 100644 --- a/tests/orchestration/test_tenant_data_sync_orchestrator.py +++ b/tests/orchestration/test_tenant_data_sync_orchestrator.py @@ -187,6 +187,38 @@ 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: TenantDataSyncOrchestrator, + mock_session: MagicMock, + sdk_client: MagicMock, +) -> None: + # Arrange + mock_session.post.return_value.content = _tenancy_response( + '1368473' + "07000000099" # no priority: sorts last + ' ' # blank number: ignored + '07000000020' + '07000000005' # lowest priority: wins + '01000000001' # unusable: sorts last + '01000000007' # tie, first in document + '01000000008' + "" + ) + + # Act + orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + + # Assert + created_properties = ( + sdk_client.crm.contacts.basic_api.create.call_args.kwargs[ + "simple_public_object_input_for_create" + ].properties + ) + assert created_properties["mobilephone"] == "07000000005" + assert created_properties["phone"] == "01000000007" + + def test_run_still_creates_a_contact_for_an_all_blank_signatory( orchestrator: TenantDataSyncOrchestrator, mock_session: MagicMock, From 1752a6cf1072164dec93948f9684ffac7358822f Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 13:27:59 +0000 Subject: [PATCH 06/25] =?UTF-8?q?Contact=20carries=20the=20best-priority?= =?UTF-8?q?=20mobile=20and=20landline=20Abri=20lists=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/abri/abri_client.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/infrastructure/abri/abri_client.py b/infrastructure/abri/abri_client.py index 16f74e17b..3d5a7f537 100644 --- a/infrastructure/abri/abri_client.py +++ b/infrastructure/abri/abri_client.py @@ -30,12 +30,18 @@ def _format_appointment_date(appointment_date: date) -> str: return appointment_date.strftime("%d/%m/%Y") +def _phone_priority(entry: ET.Element) -> Tuple[int, int]: + """Sort key: lowest numeric priority first; unusable priorities last.""" + try: + return (0, int((entry.get("priority") or "").strip())) + except ValueError: + return (1, 0) + + def _select_phone_number(entries: List[ET.Element]) -> Optional[str]: - for entry in entries: - number = (entry.text or "").strip() - if number: - return number - return None + usable = [entry for entry in entries if (entry.text or "").strip()] + ranked = sorted(usable, key=_phone_priority) # stable: ties keep document order + return (ranked[0].text or "").strip() if ranked else None class AbriClient: From 10636c17522e6414c6044231640ecf2932166a01 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 13:28:56 +0000 Subject: [PATCH 07/25] =?UTF-8?q?Runner-up=20number=20fills=20the=20contac?= =?UTF-8?q?t's=20secondary=20phone=20number=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 --- domain/abri/models.py | 4 ++- infrastructure/abri/abri_client.py | 1 + ...y_getscstenantdata_relayerror_response.xml | 10 ++++++ ...elay_getscstenantdata_success_response.xml | 7 ++++ .../test_tenant_data_sync_orchestrator.py | 34 +++++++++++++++++++ 5 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 tests/abri/abri_relay_getscstenantdata_relayerror_response.xml create mode 100644 tests/abri/abri_relay_getscstenantdata_success_response.xml diff --git a/domain/abri/models.py b/domain/abri/models.py index ceb03c34f..d96d27fda 100644 --- a/domain/abri/models.py +++ b/domain/abri/models.py @@ -57,13 +57,15 @@ class Tenant: Person title, per-person references, the main-contact flag and tenancy dates are deliberately dropped at parse time (data minimisation); mobile and telephone are the single best number of each kind, pre-selected by - priority. + priority, and secondary_number is the best of the numbers they left + unused. """ forenames: str surname: str mobile: Optional[str] telephone: Optional[str] + secondary_number: Optional[str] vulnerabilities: Tuple[str, ...] diff --git a/infrastructure/abri/abri_client.py b/infrastructure/abri/abri_client.py index 3d5a7f537..8594e2473 100644 --- a/infrastructure/abri/abri_client.py +++ b/infrastructure/abri/abri_client.py @@ -216,6 +216,7 @@ class AbriClient: surname=(element.get("surname") or "").strip(), mobile=_select_phone_number(element.findall("Mobile")), telephone=_select_phone_number(element.findall("Telephone")), + secondary_number=None, vulnerabilities=(), ) diff --git a/tests/abri/abri_relay_getscstenantdata_relayerror_response.xml b/tests/abri/abri_relay_getscstenantdata_relayerror_response.xml new file mode 100644 index 000000000..bc6da7b89 --- /dev/null +++ b/tests/abri/abri_relay_getscstenantdata_relayerror_response.xml @@ -0,0 +1,10 @@ + + + false + RelayError + No Tenancy Found for Query FOR EACH re-tncy-place WHERE re-tncy-place.org-code = '01' +and re-tncy-place.place-ref = '1004202A' +and re-tncy-place.start-date LE TODAY +and (re-tncy-place.end-date GE TODAY OR re-tncy-place.end-date = ?)and re-tncy-place.prime-place NO-LOCK, +EACH re-tenancy OF re-tncy-place NO-LOCK + diff --git a/tests/abri/abri_relay_getscstenantdata_success_response.xml b/tests/abri/abri_relay_getscstenantdata_success_response.xml new file mode 100644 index 000000000..8f3080898 --- /dev/null +++ b/tests/abri/abri_relay_getscstenantdata_success_response.xml @@ -0,0 +1,7 @@ + + 10042020051017136847309853460810 + 02473757484 + Blindness + + + diff --git a/tests/orchestration/test_tenant_data_sync_orchestrator.py b/tests/orchestration/test_tenant_data_sync_orchestrator.py index 329f04a90..946e34cdb 100644 --- a/tests/orchestration/test_tenant_data_sync_orchestrator.py +++ b/tests/orchestration/test_tenant_data_sync_orchestrator.py @@ -219,6 +219,40 @@ 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: TenantDataSyncOrchestrator, + mock_session: MagicMock, + sdk_client: MagicMock, +) -> None: + # Arrange + mock_session.post.return_value.content = _tenancy_response( + '1368473' + '07000000005' # best mobile + '07000000020' + '01000000007' # best leftover + '01000000003' # best landline + "" + ) + + # Act + orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + + # Assert + sdk_client.crm.contacts.basic_api.create.assert_called_once_with( + simple_public_object_input_for_create=SimplePublicObjectInputForCreate( + properties={ + "firstname": "Amanjeet", + "lastname": "Okello", + "mobilephone": "07000000005", + "phone": "01000000003", + "secondary_phone_number": "01000000007", + "is_vulnerable": "false", + } + ) + ) + + def test_run_still_creates_a_contact_for_an_all_blank_signatory( orchestrator: TenantDataSyncOrchestrator, mock_session: MagicMock, From 52f2286ee925e16518c6ba3065caaff6b524acd7 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 13:29:38 +0000 Subject: [PATCH 08/25] =?UTF-8?q?Runner-up=20number=20fills=20the=20contac?= =?UTF-8?q?t's=20secondary=20phone=20number=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/abri/abri_client.py | 51 ++++++++++++++++--- .../tenant_data_sync_orchestrator.py | 1 + 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/infrastructure/abri/abri_client.py b/infrastructure/abri/abri_client.py index 8594e2473..bdf246b8e 100644 --- a/infrastructure/abri/abri_client.py +++ b/infrastructure/abri/abri_client.py @@ -1,4 +1,5 @@ import xml.etree.ElementTree as ET +from dataclasses import dataclass from datetime import date from typing import List, Optional, Tuple, Union @@ -38,10 +39,45 @@ def _phone_priority(entry: ET.Element) -> Tuple[int, int]: return (1, 0) -def _select_phone_number(entries: List[ET.Element]) -> Optional[str]: - usable = [entry for entry in entries if (entry.text or "").strip()] - ranked = sorted(usable, key=_phone_priority) # stable: ties keep document order - return (ranked[0].text or "").strip() if ranked else None +def _best_phone_number(entries: List[ET.Element]) -> Optional[ET.Element]: + ranked = sorted(entries, key=_phone_priority) # stable: ties keep document order + return ranked[0] if ranked else None + + +@dataclass(frozen=True) +class _SelectedPhoneNumbers: + mobile: Optional[str] + telephone: Optional[str] + secondary: Optional[str] + + +def _select_phone_numbers(tenant_element: ET.Element) -> _SelectedPhoneNumbers: + """Best mobile, best landline, then the best of the leftovers of both kinds. + + All three picks use the same rule: lowest numeric priority wins, entries + without a usable priority sort last, ties break by document order, blank + numbers are ignored. + """ + usable = [ + entry + for entry in tenant_element + if entry.tag in ("Mobile", "Telephone") and (entry.text or "").strip() + ] + + best_mobile = _best_phone_number([e for e in usable if e.tag == "Mobile"]) + best_telephone = _best_phone_number([e for e in usable if e.tag == "Telephone"]) + runner_up = _best_phone_number( + [e for e in usable if e is not best_mobile and e is not best_telephone] + ) + + def _text(entry: Optional[ET.Element]) -> Optional[str]: + return (entry.text or "").strip() if entry is not None else None + + return _SelectedPhoneNumbers( + mobile=_text(best_mobile), + telephone=_text(best_telephone), + secondary=_text(runner_up), + ) class AbriClient: @@ -211,12 +247,13 @@ class AbriClient: def _parse_tenant(element: ET.Element) -> Tenant: # Data minimisation: person_title, main-contact and the per-person # reference (the element text) are deliberately not read. + numbers = _select_phone_numbers(element) return Tenant( forenames=(element.get("forenames") or "").strip(), surname=(element.get("surname") or "").strip(), - mobile=_select_phone_number(element.findall("Mobile")), - telephone=_select_phone_number(element.findall("Telephone")), - secondary_number=None, + mobile=numbers.mobile, + telephone=numbers.telephone, + secondary_number=numbers.secondary, vulnerabilities=(), ) diff --git a/orchestration/tenant_data_sync_orchestrator.py b/orchestration/tenant_data_sync_orchestrator.py index 038489ea7..c5a5ad557 100644 --- a/orchestration/tenant_data_sync_orchestrator.py +++ b/orchestration/tenant_data_sync_orchestrator.py @@ -12,6 +12,7 @@ def _contact_properties(tenant: Tenant) -> Dict[str, str]: "lastname": tenant.surname, "mobilephone": tenant.mobile, "phone": tenant.telephone, + "secondary_phone_number": tenant.secondary_number, } properties = {name: value for name, value in candidates.items() if value} # Always written, so "not vulnerable" is distinguishable from "not synced". From d4bdbf2e2f48ba99e51a69095771560b23da56dc Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 13:30:22 +0000 Subject: [PATCH 09/25] =?UTF-8?q?Tenant=20vulnerabilities=20are=20recorded?= =?UTF-8?q?=20on=20the=20tenant's=20own=20contact=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_tenant_data_sync_orchestrator.py | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tests/orchestration/test_tenant_data_sync_orchestrator.py b/tests/orchestration/test_tenant_data_sync_orchestrator.py index 946e34cdb..82aeac847 100644 --- a/tests/orchestration/test_tenant_data_sync_orchestrator.py +++ b/tests/orchestration/test_tenant_data_sync_orchestrator.py @@ -253,6 +253,58 @@ 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: TenantDataSyncOrchestrator, + mock_session: MagicMock, + sdk_client: MagicMock, +) -> None: + # Arrange + mock_session.post.return_value.content = _tenancy_response( + '4007216' + "Blindness" + "Hard of hearing" + "BLINDNESS" # within-tenant duplicate + "" + '4007215' + ) + sdk_client.crm.contacts.basic_api.create.side_effect = [ + MagicMock(id="60123"), + MagicMock(id="60124"), + ] + + # Act + result = orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + + # Assert + assert result == TenantDataSyncSummary( + tenancy_reference="10042020051017", + contact_ids=("60123", "60124"), + vulnerable_contact_count=1, + ) + created_properties = [ + call.kwargs["simple_public_object_input_for_create"].properties + for call in sdk_client.crm.contacts.basic_api.create.call_args_list + ] + assert created_properties == [ + { + "firstname": "Harper", + "lastname": "Solecka", + "is_vulnerable": "true", + "vulnerability_description": "Blindness\nHard of hearing", + }, + { + "firstname": "Naoimh", + "lastname": "Malyar", + "is_vulnerable": "false", + }, + ] + + def test_run_still_creates_a_contact_for_an_all_blank_signatory( orchestrator: TenantDataSyncOrchestrator, mock_session: MagicMock, From 64d397e6b4cfef23b246a6729cd0c7377a28ca7c Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 13:31:00 +0000 Subject: [PATCH 10/25] =?UTF-8?q?Tenant=20vulnerabilities=20are=20recorded?= =?UTF-8?q?=20on=20the=20tenant's=20own=20contact=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/abri/abri_client.py | 6 +++++- orchestration/tenant_data_sync_orchestrator.py | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/infrastructure/abri/abri_client.py b/infrastructure/abri/abri_client.py index bdf246b8e..ab59a6fb2 100644 --- a/infrastructure/abri/abri_client.py +++ b/infrastructure/abri/abri_client.py @@ -254,7 +254,11 @@ class AbriClient: mobile=numbers.mobile, telephone=numbers.telephone, secondary_number=numbers.secondary, - vulnerabilities=(), + vulnerabilities=tuple( + text + for vulnerability in element.findall("Vulnerability") + if (text := (vulnerability.text or "").strip()) + ), ) @staticmethod diff --git a/orchestration/tenant_data_sync_orchestrator.py b/orchestration/tenant_data_sync_orchestrator.py index c5a5ad557..6edff7156 100644 --- a/orchestration/tenant_data_sync_orchestrator.py +++ b/orchestration/tenant_data_sync_orchestrator.py @@ -6,6 +6,22 @@ from etl.hubspot.deal_contacts_client import HubspotDealContactsClient 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, @@ -13,6 +29,7 @@ def _contact_properties(tenant: Tenant) -> Dict[str, str]: "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". From 203ffac01f8fbe7aa661d7f313c3d859626d97b2 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 13:31:55 +0000 Subject: [PATCH 11/25] =?UTF-8?q?Empty=20tenancies,=20Abri=20rejections=20?= =?UTF-8?q?and=20ambiguous=20responses=20resolve=20like=20the=20other=20re?= =?UTF-8?q?lay=20operations=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_tenant_data_sync_orchestrator.py | 92 ++++++++++++++++++- 1 file changed, 91 insertions(+), 1 deletion(-) diff --git a/tests/orchestration/test_tenant_data_sync_orchestrator.py b/tests/orchestration/test_tenant_data_sync_orchestrator.py index 82aeac847..798b82d74 100644 --- a/tests/orchestration/test_tenant_data_sync_orchestrator.py +++ b/tests/orchestration/test_tenant_data_sync_orchestrator.py @@ -3,13 +3,15 @@ from pathlib import Path from unittest.mock import MagicMock, patch import pytest +import requests from hubspot.crm.associations.v4 import AssociationSpec # type: ignore[reportMissingTypeStubs] from hubspot.crm.contacts import SimplePublicObjectInputForCreate # type: ignore[reportMissingTypeStubs] -from domain.abri.models import PlaceRef +from domain.abri.models import AbriRequestRejected, PlaceRef from etl.hubspot.deal_contacts_client import HubspotDealContactsClient from infrastructure.abri.abri_client import AbriClient from infrastructure.abri.config import AbriConfig +from infrastructure.abri.errors import AbriResponseParseError, AbriTransportError from orchestration.tenant_data_sync_orchestrator import ( TenantDataSyncOrchestrator, TenantDataSyncSummary, @@ -330,3 +332,91 @@ def test_run_still_creates_a_contact_for_an_all_blank_signatory( properties={"is_vulnerable": "false"} ) ) + + +# --- unusual but valid outcomes --- + + +def test_run_treats_a_tenancy_with_no_signatories_as_a_valid_zero_contact_outcome( + orchestrator: TenantDataSyncOrchestrator, + mock_session: MagicMock, + sdk_client: MagicMock, +) -> None: + # Arrange + mock_session.post.return_value.content = _tenancy_response("") + + # Act + result = orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + + # Assert + assert result == TenantDataSyncSummary( + tenancy_reference="10042020051017", + contact_ids=(), + vulnerable_contact_count=0, + ) + assert sdk_client.mock_calls == [] + + +# --- rejection passthrough --- + + +def test_run_returns_the_abri_rejection_verbatim_and_makes_no_hubspot_calls( + orchestrator: TenantDataSyncOrchestrator, + mock_session: MagicMock, + sdk_client: MagicMock, +) -> None: + # Arrange + mock_session.post.return_value.content = _load_fixture( + "abri_relay_getscstenantdata_relayerror_response.xml" + ) + + # Act + result = orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + + # Assert + assert isinstance(result, AbriRequestRejected) + assert result.code == "RelayError" + assert result.message.startswith("No Tenancy Found for Query") + assert sdk_client.mock_calls == [] + + +# --- ambiguous responses are retriable, never success or rejection --- + + +@pytest.mark.parametrize( + "body", + [ + b"not xml at all <<<", + b"", # no Tenancy element + b"A1B2", # multiple + b'', + b"", + ], +) +def test_run_raises_a_retriable_parse_error_on_an_unexpectedly_shaped_response( + orchestrator: TenantDataSyncOrchestrator, + mock_session: MagicMock, + sdk_client: MagicMock, + body: bytes, +) -> None: + # Arrange + mock_session.post.return_value.content = body + + # Act / Assert + with pytest.raises(AbriResponseParseError): + orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + assert sdk_client.mock_calls == [] + + +def test_run_raises_a_transport_error_when_the_relay_is_unreachable( + orchestrator: TenantDataSyncOrchestrator, + mock_session: MagicMock, + sdk_client: MagicMock, +) -> None: + # Arrange + mock_session.post.side_effect = requests.ConnectionError("connection refused") + + # Act / Assert + with pytest.raises(AbriTransportError): + orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + assert sdk_client.mock_calls == [] From 3dd2dfe92c8354ad9c59cfbd12050276e8255115 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 13:32:13 +0000 Subject: [PATCH 12/25] =?UTF-8?q?Empty=20tenancies,=20Abri=20rejections=20?= =?UTF-8?q?and=20ambiguous=20responses=20resolve=20like=20the=20other=20re?= =?UTF-8?q?lay=20operations=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/abri/abri_client.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/infrastructure/abri/abri_client.py b/infrastructure/abri/abri_client.py index ab59a6fb2..a07da387f 100644 --- a/infrastructure/abri/abri_client.py +++ b/infrastructure/abri/abri_client.py @@ -226,15 +226,19 @@ class AbriClient: @staticmethod def _parse_tenancy_data(root: ET.Element) -> TenancyData: - tenancy = root.find("Tenancy") + tenancies = root.findall("Tenancy") - if tenancy is None: + if len(tenancies) != 1: raise AbriResponseParseError( - "Tenancy element missing from relay response" + f"expected exactly one Tenancy element, found {len(tenancies)}" ) + tenancy = tenancies[0] tenancy_reference = (tenancy.text or "").strip() + if not tenancy_reference: + raise AbriResponseParseError("Tenancy element missing its reference") + return TenancyData( tenancy_reference=tenancy_reference, tenants=tuple( From 2e19085875f2cb9c9bcbe678acaba76a2c623c45 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 13:33:42 +0000 Subject: [PATCH 13/25] =?UTF-8?q?HubSpot=20failures=20surface=20without=20?= =?UTF-8?q?the=20response=20body=20that=20echoes=20tenant=20PII=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_tenant_data_sync_orchestrator.py | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/orchestration/test_tenant_data_sync_orchestrator.py b/tests/orchestration/test_tenant_data_sync_orchestrator.py index 798b82d74..40b4c34da 100644 --- a/tests/orchestration/test_tenant_data_sync_orchestrator.py +++ b/tests/orchestration/test_tenant_data_sync_orchestrator.py @@ -1,3 +1,5 @@ +import json +import traceback import xml.etree.ElementTree as ET from pathlib import Path from unittest.mock import MagicMock, patch @@ -5,6 +7,7 @@ from unittest.mock import MagicMock, patch import pytest import requests from hubspot.crm.associations.v4 import AssociationSpec # type: ignore[reportMissingTypeStubs] +from hubspot.crm.contacts import ApiException as ContactsApiException # type: ignore[reportMissingTypeStubs] from hubspot.crm.contacts import SimplePublicObjectInputForCreate # type: ignore[reportMissingTypeStubs] from domain.abri.models import AbriRequestRejected, PlaceRef @@ -420,3 +423,47 @@ def test_run_raises_a_transport_error_when_the_relay_is_unreachable( with pytest.raises(AbriTransportError): orchestrator.run(PLACE_REF, deal_id=DEAL_ID) assert sdk_client.mock_calls == [] + + +# --- HubSpot failures: scrubbed of response bodies, which can echo PII --- + + +def _pii_echoing_api_error() -> ContactsApiException: + """A HubSpot validation error whose body echoes the submitted PII.""" + api_error = ContactsApiException(status=400, reason="Bad Request") + api_error.body = json.dumps( + { + "status": "error", + "message": 'Invalid value "09853460810" for mobilephone (Amanjeet Okello)', + "correlationId": "corr-abc-123", + "category": "VALIDATION_ERROR", + } + ) + api_error.headers = {} + return api_error + + +def test_run_raises_an_error_scrubbed_of_the_hubspot_response_body( + orchestrator: TenantDataSyncOrchestrator, + mock_session: MagicMock, + sdk_client: MagicMock, +) -> None: + # Arrange + mock_session.post.return_value.content = _tenancy_response( + '136847309853460810' + ) + sdk_client.crm.contacts.basic_api.create.side_effect = _pii_echoing_api_error() + + # Act + with pytest.raises(Exception) as exc_info: + orchestrator.run(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)) + 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 "Amanjeet" not in rendered_traceback + assert "Okello" not in rendered_traceback + assert "09853460810" not in rendered_traceback From e3894b0309e95440648ea459dc79f374233a94c9 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 13:34:33 +0000 Subject: [PATCH 14/25] =?UTF-8?q?HubSpot=20failures=20surface=20without=20?= =?UTF-8?q?the=20response=20body=20that=20echoes=20tenant=20PII=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 --- etl/hubspot/deal_contacts_client.py | 68 +++++++++++++++++++++-------- etl/hubspot/errors.py | 25 +++++++++++ 2 files changed, 76 insertions(+), 17 deletions(-) create mode 100644 etl/hubspot/errors.py diff --git a/etl/hubspot/deal_contacts_client.py b/etl/hubspot/deal_contacts_client.py index 6765b5da5..fe2516b84 100644 --- a/etl/hubspot/deal_contacts_client.py +++ b/etl/hubspot/deal_contacts_client.py @@ -1,13 +1,40 @@ -from typing import Dict +import json +from typing import Dict, Optional 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 etl.hubspot.errors import HubspotRequestError + # 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: + details = json.loads(getattr(error, "body", None) or "") + if isinstance(details, dict): + category = details.get("category") # type: ignore[reportUnknownMemberType] + correlation_id = details.get("correlationId") # type: ignore[reportUnknownMemberType] + 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. @@ -21,23 +48,30 @@ class HubspotDealContactsClient: self._client: Client = sdk_client def create_contact(self, properties: Dict[str, str]) -> str: - contact = self._client.crm.contacts.basic_api.create( # type: ignore[reportUnknownMemberType] - simple_public_object_input_for_create=SimplePublicObjectInputForCreate( - properties=properties + try: + contact = self._client.crm.contacts.basic_api.create( # type: ignore[reportUnknownMemberType] + simple_public_object_input_for_create=SimplePublicObjectInputForCreate( + properties=properties + ) ) - ) + except Exception as error: + # `from None` keeps the body-carrying SDK error out of tracebacks. + raise _scrubbed(error) from None return str(contact.id) # type: ignore[reportUnknownMemberType] def associate_contact_to_deal(self, deal_id: str, contact_id: str) -> None: - self._client.crm.associations.v4.basic_api.create( # type: ignore[reportUnknownMemberType] - object_type="deals", - object_id=deal_id, - to_object_type="contacts", - to_object_id=contact_id, - association_spec=[ - AssociationSpec( - association_category="HUBSPOT_DEFINED", - association_type_id=DEAL_TO_CONTACT_ASSOCIATION_TYPE_ID, - ) - ], - ) + try: + self._client.crm.associations.v4.basic_api.create( # type: ignore[reportUnknownMemberType] + object_type="deals", + object_id=deal_id, + to_object_type="contacts", + to_object_id=contact_id, + association_spec=[ + AssociationSpec( + association_category="HUBSPOT_DEFINED", + association_type_id=DEAL_TO_CONTACT_ASSOCIATION_TYPE_ID, + ) + ], + ) + except Exception as error: + raise _scrubbed(error) from None diff --git a/etl/hubspot/errors.py b/etl/hubspot/errors.py new file mode 100644 index 000000000..057e4d968 --- /dev/null +++ b/etl/hubspot/errors.py @@ -0,0 +1,25 @@ +from typing import Optional + + +class HubspotRequestError(Exception): + """A HubSpot API failure, scrubbed of the response body. + + HubSpot validation errors echo submitted property values back in the + body, so carrying (or exception-chaining) the SDK error would leak the + submitted PII into tracebacks and logs. Only non-PII diagnostics are + kept: status code, HubSpot error category and correlation id. + """ + + def __init__( + self, + status_code: Optional[int], + category: Optional[str], + correlation_id: Optional[str], + ) -> None: + self.status_code = status_code + self.category = category + self.correlation_id = correlation_id + super().__init__( + f"HubSpot request failed (status={status_code}, " + f"category={category}, correlation_id={correlation_id})" + ) From 065217f8af6b73bae5f2b8fe0ada4427e96003ce Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 13:35:20 +0000 Subject: [PATCH 15/25] =?UTF-8?q?A=20failed=20HubSpot=20write=20stops=20th?= =?UTF-8?q?e=20sync=20and=20reports=20the=20orphaned=20contacts=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 --- .../tenant_data_sync_orchestrator.py | 35 +++++++++++++++++++ .../test_tenant_data_sync_orchestrator.py | 32 +++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/orchestration/tenant_data_sync_orchestrator.py b/orchestration/tenant_data_sync_orchestrator.py index 6edff7156..aa8005bd8 100644 --- a/orchestration/tenant_data_sync_orchestrator.py +++ b/orchestration/tenant_data_sync_orchestrator.py @@ -49,6 +49,41 @@ class TenantDataSyncSummary: 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 TenantDataSyncOrchestrator: """Syncs an Abri tenancy's signatories to a HubSpot deal. diff --git a/tests/orchestration/test_tenant_data_sync_orchestrator.py b/tests/orchestration/test_tenant_data_sync_orchestrator.py index 40b4c34da..92dd4a885 100644 --- a/tests/orchestration/test_tenant_data_sync_orchestrator.py +++ b/tests/orchestration/test_tenant_data_sync_orchestrator.py @@ -16,6 +16,7 @@ from infrastructure.abri.abri_client import AbriClient from infrastructure.abri.config import AbriConfig from infrastructure.abri.errors import AbriResponseParseError, AbriTransportError from orchestration.tenant_data_sync_orchestrator import ( + TenantDataSyncError, TenantDataSyncOrchestrator, TenantDataSyncSummary, ) @@ -467,3 +468,34 @@ def test_run_raises_an_error_scrubbed_of_the_hubspot_response_body( assert "Amanjeet" not in rendered_traceback assert "Okello" not in rendered_traceback assert "09853460810" not in rendered_traceback + + +def test_run_fails_fast_when_association_fails_and_reports_the_orphaned_contact( + orchestrator: TenantDataSyncOrchestrator, + mock_session: MagicMock, + sdk_client: MagicMock, +) -> None: + # Arrange + mock_session.post.return_value.content = _load_fixture( + "abri_relay_getscstenantdata_multitenant_response.xml" + ) + sdk_client.crm.associations.v4.basic_api.create.side_effect = ( + _pii_echoing_api_error() + ) + + # Act + with pytest.raises(TenantDataSyncError) as exc_info: + orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + + # Assert: the flow stopped at the first failed write, with progress reported + error = exc_info.value + assert error.deal_id == DEAL_ID + assert error.tenant_index == 0 + assert error.contact_ids_created == ("60123",) + assert error.contact_ids_associated == () + assert error.orphaned_contact_ids == ("60123",) + assert sdk_client.crm.contacts.basic_api.create.call_count == 1 # fail fast + rendered_traceback = "".join(traceback.format_exception(error)) + assert "corr-abc-123" in str(error) # scrubbed diagnostics carried through + assert "Solecka" not in rendered_traceback + assert "09261195827" not in rendered_traceback From 81d25caeadf05d28ae406bd6c325241b3e466cf4 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 13:35:47 +0000 Subject: [PATCH 16/25] =?UTF-8?q?A=20failed=20HubSpot=20write=20stops=20th?= =?UTF-8?q?e=20sync=20and=20reports=20the=20orphaned=20contacts=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 --- .../tenant_data_sync_orchestrator.py | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/orchestration/tenant_data_sync_orchestrator.py b/orchestration/tenant_data_sync_orchestrator.py index aa8005bd8..383ca2a98 100644 --- a/orchestration/tenant_data_sync_orchestrator.py +++ b/orchestration/tenant_data_sync_orchestrator.py @@ -3,6 +3,7 @@ from typing import Dict, List, Optional, Tuple, Union from domain.abri.models import AbriRequestRejected, PlaceRef, Tenant from etl.hubspot.deal_contacts_client import HubspotDealContactsClient +from etl.hubspot.errors import HubspotRequestError from infrastructure.abri.abri_client import AbriClient @@ -106,12 +107,27 @@ class TenantDataSyncOrchestrator: return tenancy contact_ids: List[str] = [] - for tenant in tenancy.tenants: - contact_id = self._hubspot.create_contact(_contact_properties(tenant)) - self._hubspot.associate_contact_to_deal( - deal_id=deal_id, contact_id=contact_id - ) - contact_ids.append(contact_id) + 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, From 8d684c57c7aa8ccb387ed3e043e86676b39c8840 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 13:36:43 +0000 Subject: [PATCH 17/25] =?UTF-8?q?Transient=20HubSpot=20failures=20are=20re?= =?UTF-8?q?tried=20under=20the=20shared=20rate-limit=20policy=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_tenant_data_sync_orchestrator.py | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/orchestration/test_tenant_data_sync_orchestrator.py b/tests/orchestration/test_tenant_data_sync_orchestrator.py index 92dd4a885..6fd48e8ea 100644 --- a/tests/orchestration/test_tenant_data_sync_orchestrator.py +++ b/tests/orchestration/test_tenant_data_sync_orchestrator.py @@ -499,3 +499,32 @@ def test_run_fails_fast_when_association_fails_and_reports_the_orphaned_contact( assert "corr-abc-123" in str(error) # scrubbed diagnostics carried through assert "Solecka" not in rendered_traceback assert "09261195827" not in rendered_traceback + + +def test_run_retries_a_rate_limited_hubspot_call_before_succeeding( + orchestrator: TenantDataSyncOrchestrator, + mock_session: MagicMock, + sdk_client: MagicMock, +) -> None: + # Arrange + mock_session.post.return_value.content = _tenancy_response( + '1368473' + ) + rate_limited = ContactsApiException(status=429, reason="Too Many Requests") + rate_limited.headers = {"x-hubspot-ratelimit-interval-milliseconds": "0"} + sdk_client.crm.contacts.basic_api.create.side_effect = [ + rate_limited, + MagicMock(id="60123"), + ] + + # Act + result = orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + + # Assert + assert result == TenantDataSyncSummary( + tenancy_reference="10042020051017", + contact_ids=("60123",), + vulnerable_contact_count=0, + ) + assert sdk_client.crm.contacts.basic_api.create.call_count == 2 From 8d9fb0ade4915a0cd43dffa5bf4ddeb093d94f39 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 13:38:20 +0000 Subject: [PATCH 18/25] =?UTF-8?q?Transient=20HubSpot=20failures=20are=20re?= =?UTF-8?q?tried=20under=20the=20shared=20rate-limit=20policy=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 --- etl/hubspot/deal_contacts_client.py | 33 +++++++++++-------- etl/hubspot/hubspotClient.py | 39 ++-------------------- etl/hubspot/retry.py | 51 +++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 50 deletions(-) create mode 100644 etl/hubspot/retry.py diff --git a/etl/hubspot/deal_contacts_client.py b/etl/hubspot/deal_contacts_client.py index fe2516b84..c218b6e05 100644 --- a/etl/hubspot/deal_contacts_client.py +++ b/etl/hubspot/deal_contacts_client.py @@ -6,6 +6,7 @@ from hubspot.crm.associations.v4 import AssociationSpec # type: ignore[reportMi from hubspot.crm.contacts import SimplePublicObjectInputForCreate # type: ignore[reportMissingTypeStubs] from etl.hubspot.errors import HubspotRequestError +from etl.hubspot.retry import call_with_retry # HubSpot-defined deal -> contact association ("deal_to_contact" in v3 terms). DEAL_TO_CONTACT_ASSOCIATION_TYPE_ID = 3 @@ -49,9 +50,11 @@ class HubspotDealContactsClient: def create_contact(self, properties: Dict[str, str]) -> str: try: - contact = self._client.crm.contacts.basic_api.create( # type: ignore[reportUnknownMemberType] - simple_public_object_input_for_create=SimplePublicObjectInputForCreate( - properties=properties + contact = call_with_retry( + lambda: self._client.crm.contacts.basic_api.create( # type: ignore[reportUnknownMemberType] + simple_public_object_input_for_create=SimplePublicObjectInputForCreate( + properties=properties + ) ) ) except Exception as error: @@ -61,17 +64,19 @@ class HubspotDealContactsClient: def associate_contact_to_deal(self, deal_id: str, contact_id: str) -> None: try: - self._client.crm.associations.v4.basic_api.create( # type: ignore[reportUnknownMemberType] - object_type="deals", - object_id=deal_id, - to_object_type="contacts", - to_object_id=contact_id, - association_spec=[ - AssociationSpec( - association_category="HUBSPOT_DEFINED", - association_type_id=DEAL_TO_CONTACT_ASSOCIATION_TYPE_ID, - ) - ], + call_with_retry( + lambda: self._client.crm.associations.v4.basic_api.create( # type: ignore[reportUnknownMemberType] + object_type="deals", + object_id=deal_id, + to_object_type="contacts", + to_object_id=contact_id, + association_spec=[ + AssociationSpec( + association_category="HUBSPOT_DEFINED", + association_type_id=DEAL_TO_CONTACT_ASSOCIATION_TYPE_ID, + ) + ], + ) ) except Exception as error: raise _scrubbed(error) from None diff --git a/etl/hubspot/hubspotClient.py b/etl/hubspot/hubspotClient.py index 918701cac..6ee71a381 100644 --- a/etl/hubspot/hubspotClient.py +++ b/etl/hubspot/hubspotClient.py @@ -1,7 +1,5 @@ import os -import time from enum import Enum -from http import HTTPStatus from typing import Optional, cast, Callable, Any from hubspot.client import Client # type: ignore[reportMissingTypeStubs] @@ -32,6 +30,7 @@ from hubspot.crm.associations.v4.models import ( # type: ignore[reportMissingTy from backend.app.config import get_settings from etl.hubspot.company_data import CompanyData from etl.hubspot.project_data import ProjectData +from etl.hubspot.retry import call_with_retry from utils.logger import setup_logger import mimetypes @@ -87,40 +86,8 @@ class HubspotClient: # self.client def _call_with_retry(self, fn: Callable[[], Any], max_retries: int = 2) -> Any: - """ - Call fn(), retrying up to max_retries times on 429 rate-limit errors - or transient 5xx server errors. - Waits the minimal amount: the remaining interval window reported by HubSpot headers. - Falls back to the full interval (10s) if headers are absent. - - Note: each HubSpot sub-module (deals, companies, etc.) ships its own ApiException - class with no shared base beyond Exception, so we detect retryable statuses via duck-typing. - """ - retryable_statuses = { - HTTPStatus.TOO_MANY_REQUESTS, - HTTPStatus.INTERNAL_SERVER_ERROR, - HTTPStatus.BAD_GATEWAY, - HTTPStatus.SERVICE_UNAVAILABLE, - HTTPStatus.GATEWAY_TIMEOUT, - } - for attempt in range(max_retries + 1): - try: - return fn() - except Exception as e: - status = getattr(e, "status", None) - if status not in retryable_statuses or attempt == max_retries: - raise - headers = getattr(e, "headers", None) or {} - interval_ms = int( - headers.get("x-hubspot-ratelimit-interval-milliseconds", 10000) - ) - wait_s = interval_ms / 1000.0 - self.logger.warning( - f"HubSpot {status} (attempt {attempt + 1}/{max_retries}), " - f"waiting {wait_s:.1f}s before retry." - ) - time.sleep(wait_s) - raise RuntimeError("Unreachable") # pragma: no cover + """Delegates to the shared etl.hubspot.retry policy.""" + return call_with_retry(fn, max_retries=max_retries, logger=self.logger) def get_deal_ids_from_company(self, company_id: str) -> list[str]: associations_api: AssociationsBasicApi = ( # type: ignore[reportUnknownMemberType] diff --git a/etl/hubspot/retry.py b/etl/hubspot/retry.py new file mode 100644 index 000000000..d1500600a --- /dev/null +++ b/etl/hubspot/retry.py @@ -0,0 +1,51 @@ +import time +from http import HTTPStatus +from logging import Logger +from typing import Callable, Optional, TypeVar + +from utils.logger import setup_logger + +T = TypeVar("T") + +RETRYABLE_STATUSES = { + HTTPStatus.TOO_MANY_REQUESTS, + HTTPStatus.INTERNAL_SERVER_ERROR, + HTTPStatus.BAD_GATEWAY, + HTTPStatus.SERVICE_UNAVAILABLE, + HTTPStatus.GATEWAY_TIMEOUT, +} + + +def call_with_retry( + fn: Callable[[], T], + max_retries: int = 2, + logger: Optional[Logger] = None, +) -> T: + """ + Call fn(), retrying up to max_retries times on 429 rate-limit errors + or transient 5xx server errors. + Waits the minimal amount: the remaining interval window reported by HubSpot headers. + Falls back to the full interval (10s) if headers are absent. + + Note: each HubSpot sub-module (deals, companies, etc.) ships its own ApiException + class with no shared base beyond Exception, so we detect retryable statuses via duck-typing. + """ + logger = logger or setup_logger() + for attempt in range(max_retries + 1): + try: + return fn() + except Exception as e: + status = getattr(e, "status", None) + if status not in RETRYABLE_STATUSES or attempt == max_retries: + raise + headers = getattr(e, "headers", None) or {} + interval_ms = int( + headers.get("x-hubspot-ratelimit-interval-milliseconds", 10000) + ) + wait_s = interval_ms / 1000.0 + logger.warning( + f"HubSpot {status} (attempt {attempt + 1}/{max_retries}), " + f"waiting {wait_s:.1f}s before retry." + ) + time.sleep(wait_s) + raise RuntimeError("Unreachable") # pragma: no cover From b96aceaf7bf4161b5fcde9e0128317bef041494b Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 13:42:58 +0000 Subject: [PATCH 19/25] =?UTF-8?q?HubSpot=20calls=20share=20one=20retry=20p?= =?UTF-8?q?olicy=20behind=20a=20single=20guarded=20entry=20point=20?= =?UTF-8?q?=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 --- etl/hubspot/deal_contacts_client.py | 63 ++++++++++++++++------------- etl/hubspot/retry.py | 4 +- 2 files changed, 37 insertions(+), 30 deletions(-) diff --git a/etl/hubspot/deal_contacts_client.py b/etl/hubspot/deal_contacts_client.py index c218b6e05..9609b1be8 100644 --- a/etl/hubspot/deal_contacts_client.py +++ b/etl/hubspot/deal_contacts_client.py @@ -1,5 +1,5 @@ import json -from typing import Dict, Optional +from typing import Any, Callable, Dict, Optional, cast from hubspot.client import Client # type: ignore[reportMissingTypeStubs] from hubspot.crm.associations.v4 import AssociationSpec # type: ignore[reportMissingTypeStubs] @@ -24,10 +24,15 @@ def _scrubbed(error: Exception) -> HubspotRequestError: category: Optional[str] = None correlation_id: Optional[str] = None try: - details = json.loads(getattr(error, "body", None) or "") - if isinstance(details, dict): - category = details.get("category") # type: ignore[reportUnknownMemberType] - correlation_id = details.get("correlationId") # type: ignore[reportUnknownMemberType] + 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 @@ -49,34 +54,36 @@ class HubspotDealContactsClient: self._client: Client = sdk_client def create_contact(self, properties: Dict[str, str]) -> str: - try: - contact = call_with_retry( - lambda: self._client.crm.contacts.basic_api.create( # type: ignore[reportUnknownMemberType] - simple_public_object_input_for_create=SimplePublicObjectInputForCreate( - properties=properties - ) + contact = self._call( + lambda: self._client.crm.contacts.basic_api.create( # type: ignore[reportUnknownMemberType] + simple_public_object_input_for_create=SimplePublicObjectInputForCreate( + properties=properties ) ) - except Exception as error: - # `from None` keeps the body-carrying SDK error out of tracebacks. - raise _scrubbed(error) from None + ) return str(contact.id) # type: ignore[reportUnknownMemberType] def associate_contact_to_deal(self, deal_id: str, contact_id: str) -> None: - try: - call_with_retry( - lambda: self._client.crm.associations.v4.basic_api.create( # type: ignore[reportUnknownMemberType] - object_type="deals", - object_id=deal_id, - to_object_type="contacts", - to_object_id=contact_id, - association_spec=[ - AssociationSpec( - association_category="HUBSPOT_DEFINED", - association_type_id=DEAL_TO_CONTACT_ASSOCIATION_TYPE_ID, - ) - ], - ) + self._call( + lambda: self._client.crm.associations.v4.basic_api.create( # type: ignore[reportUnknownMemberType] + object_type="deals", + object_id=deal_id, + to_object_type="contacts", + to_object_id=contact_id, + association_spec=[ + AssociationSpec( + association_category="HUBSPOT_DEFINED", + association_type_id=DEAL_TO_CONTACT_ASSOCIATION_TYPE_ID, + ) + ], ) + ) + + @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 diff --git a/etl/hubspot/retry.py b/etl/hubspot/retry.py index d1500600a..7bd50c8a1 100644 --- a/etl/hubspot/retry.py +++ b/etl/hubspot/retry.py @@ -1,7 +1,7 @@ import time from http import HTTPStatus from logging import Logger -from typing import Callable, Optional, TypeVar +from typing import Callable, Dict, Optional, TypeVar, cast from utils.logger import setup_logger @@ -38,7 +38,7 @@ def call_with_retry( status = getattr(e, "status", None) if status not in RETRYABLE_STATUSES or attempt == max_retries: raise - headers = getattr(e, "headers", None) or {} + headers = cast(Dict[str, str], getattr(e, "headers", None) or {}) interval_ms = int( headers.get("x-hubspot-ratelimit-interval-milliseconds", 10000) ) From 8fd808ffd94dcbba33f7495ef011dc48353b7a63 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 14:01:47 +0000 Subject: [PATCH 20/25] =?UTF-8?q?HubSpot=20deal-contacts=20transport=20liv?= =?UTF-8?q?es=20under=20infrastructure=20like=20the=20other=20API=20client?= =?UTF-8?q?s=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 --- etl/hubspot/hubspotClient.py | 4 ++-- infrastructure/hubspot/__init__.py | 0 {etl => infrastructure}/hubspot/deal_contacts_client.py | 4 ++-- {etl => infrastructure}/hubspot/errors.py | 0 {etl => infrastructure}/hubspot/retry.py | 0 orchestration/tenant_data_sync_orchestrator.py | 4 ++-- tests/orchestration/test_tenant_data_sync_orchestrator.py | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) create mode 100644 infrastructure/hubspot/__init__.py rename {etl => infrastructure}/hubspot/deal_contacts_client.py (96%) rename {etl => infrastructure}/hubspot/errors.py (100%) rename {etl => infrastructure}/hubspot/retry.py (100%) diff --git a/etl/hubspot/hubspotClient.py b/etl/hubspot/hubspotClient.py index 6ee71a381..641695e27 100644 --- a/etl/hubspot/hubspotClient.py +++ b/etl/hubspot/hubspotClient.py @@ -30,7 +30,7 @@ from hubspot.crm.associations.v4.models import ( # type: ignore[reportMissingTy from backend.app.config import get_settings from etl.hubspot.company_data import CompanyData from etl.hubspot.project_data import ProjectData -from etl.hubspot.retry import call_with_retry +from infrastructure.hubspot.retry import call_with_retry from utils.logger import setup_logger import mimetypes @@ -86,7 +86,7 @@ class HubspotClient: # self.client def _call_with_retry(self, fn: Callable[[], Any], max_retries: int = 2) -> Any: - """Delegates to the shared etl.hubspot.retry policy.""" + """Delegates to the shared infrastructure.hubspot.retry policy.""" return call_with_retry(fn, max_retries=max_retries, logger=self.logger) def get_deal_ids_from_company(self, company_id: str) -> list[str]: diff --git a/infrastructure/hubspot/__init__.py b/infrastructure/hubspot/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/etl/hubspot/deal_contacts_client.py b/infrastructure/hubspot/deal_contacts_client.py similarity index 96% rename from etl/hubspot/deal_contacts_client.py rename to infrastructure/hubspot/deal_contacts_client.py index 9609b1be8..2c0eb0627 100644 --- a/etl/hubspot/deal_contacts_client.py +++ b/infrastructure/hubspot/deal_contacts_client.py @@ -5,8 +5,8 @@ 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 etl.hubspot.errors import HubspotRequestError -from etl.hubspot.retry import call_with_retry +from infrastructure.hubspot.errors import HubspotRequestError +from infrastructure.hubspot.retry import call_with_retry # HubSpot-defined deal -> contact association ("deal_to_contact" in v3 terms). DEAL_TO_CONTACT_ASSOCIATION_TYPE_ID = 3 diff --git a/etl/hubspot/errors.py b/infrastructure/hubspot/errors.py similarity index 100% rename from etl/hubspot/errors.py rename to infrastructure/hubspot/errors.py diff --git a/etl/hubspot/retry.py b/infrastructure/hubspot/retry.py similarity index 100% rename from etl/hubspot/retry.py rename to infrastructure/hubspot/retry.py diff --git a/orchestration/tenant_data_sync_orchestrator.py b/orchestration/tenant_data_sync_orchestrator.py index 383ca2a98..f1e5bb3e0 100644 --- a/orchestration/tenant_data_sync_orchestrator.py +++ b/orchestration/tenant_data_sync_orchestrator.py @@ -2,8 +2,8 @@ from dataclasses import dataclass from typing import Dict, List, Optional, Tuple, Union from domain.abri.models import AbriRequestRejected, PlaceRef, Tenant -from etl.hubspot.deal_contacts_client import HubspotDealContactsClient -from etl.hubspot.errors import HubspotRequestError +from infrastructure.hubspot.deal_contacts_client import HubspotDealContactsClient +from infrastructure.hubspot.errors import HubspotRequestError from infrastructure.abri.abri_client import AbriClient diff --git a/tests/orchestration/test_tenant_data_sync_orchestrator.py b/tests/orchestration/test_tenant_data_sync_orchestrator.py index 6fd48e8ea..1980a45c9 100644 --- a/tests/orchestration/test_tenant_data_sync_orchestrator.py +++ b/tests/orchestration/test_tenant_data_sync_orchestrator.py @@ -11,7 +11,7 @@ from hubspot.crm.contacts import ApiException as ContactsApiException # type: i from hubspot.crm.contacts import SimplePublicObjectInputForCreate # type: ignore[reportMissingTypeStubs] from domain.abri.models import AbriRequestRejected, PlaceRef -from etl.hubspot.deal_contacts_client import HubspotDealContactsClient +from infrastructure.hubspot.deal_contacts_client import HubspotDealContactsClient from infrastructure.abri.abri_client import AbriClient from infrastructure.abri.config import AbriConfig from infrastructure.abri.errors import AbriResponseParseError, AbriTransportError From 578de8845af3b6836274390c7cbdb4455ffd18df Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 14:02:38 +0000 Subject: [PATCH 21/25] =?UTF-8?q?Tenant-data=20fixtures=20carry=20fictiona?= =?UTF-8?q?l=20names=20and=20Ofcom-reserved=20phone=20numbers=20?= =?UTF-8?q?=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 --- ..._getscstenantdata_multitenant_response.xml | 6 +- ...elay_getscstenantdata_success_response.xml | 4 +- .../test_tenant_data_sync_orchestrator.py | 76 +++++++++---------- 3 files changed, 43 insertions(+), 43 deletions(-) diff --git a/tests/abri/abri_relay_getscstenantdata_multitenant_response.xml b/tests/abri/abri_relay_getscstenantdata_multitenant_response.xml index d95e3ce81..7a3b26efa 100644 --- a/tests/abri/abri_relay_getscstenantdata_multitenant_response.xml +++ b/tests/abri/abri_relay_getscstenantdata_multitenant_response.xml @@ -1,8 +1,8 @@ - 10071650064015400721609261195827 + 10071650064015400721607700900456 - 400721509286114923 - 02769674129 + 400721507700900789 + 01632960456 diff --git a/tests/abri/abri_relay_getscstenantdata_success_response.xml b/tests/abri/abri_relay_getscstenantdata_success_response.xml index 8f3080898..9056a8c81 100644 --- a/tests/abri/abri_relay_getscstenantdata_success_response.xml +++ b/tests/abri/abri_relay_getscstenantdata_success_response.xml @@ -1,6 +1,6 @@ - 10042020051017136847309853460810 - 02473757484 + 10042020051017136847307700900123 + 01632960123 Blindness diff --git a/tests/orchestration/test_tenant_data_sync_orchestrator.py b/tests/orchestration/test_tenant_data_sync_orchestrator.py index 1980a45c9..3bbfbe439 100644 --- a/tests/orchestration/test_tenant_data_sync_orchestrator.py +++ b/tests/orchestration/test_tenant_data_sync_orchestrator.py @@ -106,9 +106,9 @@ def test_run_creates_an_associated_deal_contact_per_signatory_and_returns_a_summ ) -> None: # Arrange mock_session.post.return_value.content = _tenancy_response( - '136847309853460810' - '02473757484' + '136847307700900123' + '01632960123' ) # Act @@ -123,10 +123,10 @@ def test_run_creates_an_associated_deal_contact_per_signatory_and_returns_a_summ sdk_client.crm.contacts.basic_api.create.assert_called_once_with( simple_public_object_input_for_create=SimplePublicObjectInputForCreate( properties={ - "firstname": "Amanjeet", - "lastname": "Okello", - "mobilephone": "09853460810", - "phone": "02473757484", + "firstname": "Jane", + "lastname": "Doe", + "mobilephone": "07700900123", + "phone": "01632960123", "is_vulnerable": "false", } ) @@ -173,16 +173,16 @@ def test_run_creates_a_separate_contact_for_each_signatory( ] assert created_properties == [ { - "firstname": "Harper", - "lastname": "Solecka", - "mobilephone": "09261195827", + "firstname": "Ann", + "lastname": "Bloggs", + "mobilephone": "07700900456", "is_vulnerable": "false", }, { - "firstname": "Naoimh", - "lastname": "Malyar", - "mobilephone": "09286114923", - "phone": "02769674129", + "firstname": "Beth", + "lastname": "Sample", + "mobilephone": "07700900789", + "phone": "01632960456", "is_vulnerable": "false", }, ] @@ -200,8 +200,8 @@ def test_run_picks_the_best_priority_number_of_each_kind_for_a_contact( ) -> None: # Arrange mock_session.post.return_value.content = _tenancy_response( - '1368473' + '1368473' "07000000099" # no priority: sorts last ' ' # blank number: ignored '07000000020' @@ -232,8 +232,8 @@ def test_run_fills_secondary_phone_number_with_the_best_leftover_number( ) -> None: # Arrange mock_session.post.return_value.content = _tenancy_response( - '1368473' + '1368473' '07000000005' # best mobile '07000000020' '01000000007' # best leftover @@ -248,8 +248,8 @@ def test_run_fills_secondary_phone_number_with_the_best_leftover_number( sdk_client.crm.contacts.basic_api.create.assert_called_once_with( simple_public_object_input_for_create=SimplePublicObjectInputForCreate( properties={ - "firstname": "Amanjeet", - "lastname": "Okello", + "firstname": "Jane", + "lastname": "Doe", "mobilephone": "07000000005", "phone": "01000000003", "secondary_phone_number": "01000000007", @@ -269,14 +269,14 @@ def test_run_records_vulnerabilities_on_the_tenants_own_contact( ) -> None: # Arrange mock_session.post.return_value.content = _tenancy_response( - '4007216' + '4007216' "Blindness" "Hard of hearing" "BLINDNESS" # within-tenant duplicate "" - '4007215' + '4007215' ) sdk_client.crm.contacts.basic_api.create.side_effect = [ MagicMock(id="60123"), @@ -298,14 +298,14 @@ def test_run_records_vulnerabilities_on_the_tenants_own_contact( ] assert created_properties == [ { - "firstname": "Harper", - "lastname": "Solecka", + "firstname": "Ann", + "lastname": "Bloggs", "is_vulnerable": "true", "vulnerability_description": "Blindness\nHard of hearing", }, { - "firstname": "Naoimh", - "lastname": "Malyar", + "firstname": "Beth", + "lastname": "Sample", "is_vulnerable": "false", }, ] @@ -435,7 +435,7 @@ def _pii_echoing_api_error() -> ContactsApiException: api_error.body = json.dumps( { "status": "error", - "message": 'Invalid value "09853460810" for mobilephone (Amanjeet Okello)', + "message": 'Invalid value "07700900123" for mobilephone (Jane Doe)', "correlationId": "corr-abc-123", "category": "VALIDATION_ERROR", } @@ -451,8 +451,8 @@ def test_run_raises_an_error_scrubbed_of_the_hubspot_response_body( ) -> None: # Arrange mock_session.post.return_value.content = _tenancy_response( - '136847309853460810' + '136847307700900123' ) sdk_client.crm.contacts.basic_api.create.side_effect = _pii_echoing_api_error() @@ -465,9 +465,9 @@ def test_run_raises_an_error_scrubbed_of_the_hubspot_response_body( 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 "Amanjeet" not in rendered_traceback - assert "Okello" not in rendered_traceback - assert "09853460810" not in rendered_traceback + assert "Jane" not in rendered_traceback + assert "Doe" not in rendered_traceback + assert "07700900123" not in rendered_traceback def test_run_fails_fast_when_association_fails_and_reports_the_orphaned_contact( @@ -497,8 +497,8 @@ def test_run_fails_fast_when_association_fails_and_reports_the_orphaned_contact( assert sdk_client.crm.contacts.basic_api.create.call_count == 1 # fail fast rendered_traceback = "".join(traceback.format_exception(error)) assert "corr-abc-123" in str(error) # scrubbed diagnostics carried through - assert "Solecka" not in rendered_traceback - assert "09261195827" not in rendered_traceback + assert "Bloggs" not in rendered_traceback + assert "07700900456" not in rendered_traceback def test_run_retries_a_rate_limited_hubspot_call_before_succeeding( @@ -508,8 +508,8 @@ def test_run_retries_a_rate_limited_hubspot_call_before_succeeding( ) -> None: # Arrange mock_session.post.return_value.content = _tenancy_response( - '1368473' + '1368473' ) rate_limited = ContactsApiException(status=429, reason="Too Many Requests") rate_limited.headers = {"x-hubspot-ratelimit-interval-milliseconds": "0"} From 76896f9a8edd40b84fc23f426eeca4a6021c2a99 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 14:05:42 +0000 Subject: [PATCH 22/25] rename tenant data sync to abri tenant data sync --- ... => abri_tenant_data_sync_orchestrator.py} | 6 +-- ...est_abri_tenant_data_sync_orchestrator.py} | 44 +++++++++---------- 2 files changed, 23 insertions(+), 27 deletions(-) rename orchestration/{tenant_data_sync_orchestrator.py => abri_tenant_data_sync_orchestrator.py} (96%) rename tests/orchestration/{test_tenant_data_sync_orchestrator.py => test_abri_tenant_data_sync_orchestrator.py} (93%) diff --git a/orchestration/tenant_data_sync_orchestrator.py b/orchestration/abri_tenant_data_sync_orchestrator.py similarity index 96% rename from orchestration/tenant_data_sync_orchestrator.py rename to orchestration/abri_tenant_data_sync_orchestrator.py index f1e5bb3e0..14ab68e2f 100644 --- a/orchestration/tenant_data_sync_orchestrator.py +++ b/orchestration/abri_tenant_data_sync_orchestrator.py @@ -85,7 +85,7 @@ class TenantDataSyncError(Exception): ) -class TenantDataSyncOrchestrator: +class AbriTenantDataSyncOrchestrator: """Syncs an Abri tenancy's signatories to a HubSpot deal. Tenant data flows straight from Abri to HubSpot; nothing is persisted @@ -110,9 +110,7 @@ class TenantDataSyncOrchestrator: associated_ids: List[str] = [] for tenant_index, tenant in enumerate(tenancy.tenants): try: - contact_id = self._hubspot.create_contact( - _contact_properties(tenant) - ) + 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 diff --git a/tests/orchestration/test_tenant_data_sync_orchestrator.py b/tests/orchestration/test_abri_tenant_data_sync_orchestrator.py similarity index 93% rename from tests/orchestration/test_tenant_data_sync_orchestrator.py rename to tests/orchestration/test_abri_tenant_data_sync_orchestrator.py index 3bbfbe439..ad7a738b5 100644 --- a/tests/orchestration/test_tenant_data_sync_orchestrator.py +++ b/tests/orchestration/test_abri_tenant_data_sync_orchestrator.py @@ -15,9 +15,9 @@ from infrastructure.hubspot.deal_contacts_client import HubspotDealContactsClien from infrastructure.abri.abri_client import AbriClient from infrastructure.abri.config import AbriConfig from infrastructure.abri.errors import AbriResponseParseError, AbriTransportError -from orchestration.tenant_data_sync_orchestrator import ( +from orchestration.abri_tenant_data_sync_orchestrator import ( TenantDataSyncError, - TenantDataSyncOrchestrator, + AbriTenantDataSyncOrchestrator, TenantDataSyncSummary, ) @@ -62,13 +62,13 @@ def sdk_client() -> MagicMock: @pytest.fixture() def orchestrator( mock_session: MagicMock, sdk_client: MagicMock -) -> TenantDataSyncOrchestrator: +) -> AbriTenantDataSyncOrchestrator: with patch( "infrastructure.abri.abri_client.requests.Session", return_value=mock_session, ): abri_client = AbriClient(config=CONFIG) - return TenantDataSyncOrchestrator( + return AbriTenantDataSyncOrchestrator( abri_client=abri_client, hubspot=HubspotDealContactsClient(sdk_client=sdk_client), ) @@ -78,7 +78,7 @@ def orchestrator( def test_run_sends_the_recorded_getscstenantdata_envelope_to_the_relay_endpoint( - orchestrator: TenantDataSyncOrchestrator, mock_session: MagicMock + orchestrator: AbriTenantDataSyncOrchestrator, mock_session: MagicMock ) -> None: # Arrange mock_session.post.return_value.content = _tenancy_response("") @@ -100,7 +100,7 @@ def test_run_sends_the_recorded_getscstenantdata_envelope_to_the_relay_endpoint( def test_run_creates_an_associated_deal_contact_per_signatory_and_returns_a_summary( - orchestrator: TenantDataSyncOrchestrator, + orchestrator: AbriTenantDataSyncOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: @@ -145,7 +145,7 @@ 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: TenantDataSyncOrchestrator, + orchestrator: AbriTenantDataSyncOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: @@ -194,7 +194,7 @@ def test_run_creates_a_separate_contact_for_each_signatory( def test_run_picks_the_best_priority_number_of_each_kind_for_a_contact( - orchestrator: TenantDataSyncOrchestrator, + orchestrator: AbriTenantDataSyncOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: @@ -216,17 +216,15 @@ def test_run_picks_the_best_priority_number_of_each_kind_for_a_contact( orchestrator.run(PLACE_REF, deal_id=DEAL_ID) # Assert - created_properties = ( - sdk_client.crm.contacts.basic_api.create.call_args.kwargs[ - "simple_public_object_input_for_create" - ].properties - ) + created_properties = sdk_client.crm.contacts.basic_api.create.call_args.kwargs[ + "simple_public_object_input_for_create" + ].properties assert created_properties["mobilephone"] == "07000000005" assert created_properties["phone"] == "01000000007" def test_run_fills_secondary_phone_number_with_the_best_leftover_number( - orchestrator: TenantDataSyncOrchestrator, + orchestrator: AbriTenantDataSyncOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: @@ -263,7 +261,7 @@ def test_run_fills_secondary_phone_number_with_the_best_leftover_number( def test_run_records_vulnerabilities_on_the_tenants_own_contact( - orchestrator: TenantDataSyncOrchestrator, + orchestrator: AbriTenantDataSyncOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: @@ -312,7 +310,7 @@ def test_run_records_vulnerabilities_on_the_tenants_own_contact( def test_run_still_creates_a_contact_for_an_all_blank_signatory( - orchestrator: TenantDataSyncOrchestrator, + orchestrator: AbriTenantDataSyncOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: @@ -342,7 +340,7 @@ def test_run_still_creates_a_contact_for_an_all_blank_signatory( def test_run_treats_a_tenancy_with_no_signatories_as_a_valid_zero_contact_outcome( - orchestrator: TenantDataSyncOrchestrator, + orchestrator: AbriTenantDataSyncOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: @@ -365,7 +363,7 @@ def test_run_treats_a_tenancy_with_no_signatories_as_a_valid_zero_contact_outcom def test_run_returns_the_abri_rejection_verbatim_and_makes_no_hubspot_calls( - orchestrator: TenantDataSyncOrchestrator, + orchestrator: AbriTenantDataSyncOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: @@ -398,7 +396,7 @@ def test_run_returns_the_abri_rejection_verbatim_and_makes_no_hubspot_calls( ], ) def test_run_raises_a_retriable_parse_error_on_an_unexpectedly_shaped_response( - orchestrator: TenantDataSyncOrchestrator, + orchestrator: AbriTenantDataSyncOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, body: bytes, @@ -413,7 +411,7 @@ def test_run_raises_a_retriable_parse_error_on_an_unexpectedly_shaped_response( def test_run_raises_a_transport_error_when_the_relay_is_unreachable( - orchestrator: TenantDataSyncOrchestrator, + orchestrator: AbriTenantDataSyncOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: @@ -445,7 +443,7 @@ def _pii_echoing_api_error() -> ContactsApiException: def test_run_raises_an_error_scrubbed_of_the_hubspot_response_body( - orchestrator: TenantDataSyncOrchestrator, + orchestrator: AbriTenantDataSyncOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: @@ -471,7 +469,7 @@ def test_run_raises_an_error_scrubbed_of_the_hubspot_response_body( def test_run_fails_fast_when_association_fails_and_reports_the_orphaned_contact( - orchestrator: TenantDataSyncOrchestrator, + orchestrator: AbriTenantDataSyncOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: @@ -502,7 +500,7 @@ def test_run_fails_fast_when_association_fails_and_reports_the_orphaned_contact( def test_run_retries_a_rate_limited_hubspot_call_before_succeeding( - orchestrator: TenantDataSyncOrchestrator, + orchestrator: AbriTenantDataSyncOrchestrator, mock_session: MagicMock, sdk_client: MagicMock, ) -> None: From de2a6d1d7913f278014ddde948ef0d2d3f9296d1 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 14:19:05 +0000 Subject: [PATCH 23/25] Add manual smoke script for the Abri tenant-contact HubSpot writes Co-Authored-By: Claude Fable 5 --- scripts/smoke_test_tenant_contacts.py | 140 ++++++++++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 scripts/smoke_test_tenant_contacts.py diff --git a/scripts/smoke_test_tenant_contacts.py b/scripts/smoke_test_tenant_contacts.py new file mode 100644 index 000000000..0b7073bd9 --- /dev/null +++ b/scripts/smoke_test_tenant_contacts.py @@ -0,0 +1,140 @@ +"""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 +through the real parse, phone-priority selection and vulnerability mapping, +and the resulting contact writes hit the real HubSpot portal. + +Usage: + 1. Ensure HUBSPOT_API_KEY is set in backend/.env. + 2. Create a throwaway test deal in the HubSpot UI and paste its id into + DEAL_ID below. + 3. Run: python scripts/smoke_test_tenant_contacts.py + 4. Check the deal in the UI: two contacts (Jane Doe, Joe Bloggs), with + Jane carrying mobilephone, phone, secondary_phone_number, + is_vulnerable=true and a two-line vulnerability_description, and Joe + carrying only a name and is_vulnerable=false. + 5. Clean up: paste the printed contact ids into CONTACT_IDS_TO_ARCHIVE, + set ARCHIVE = True and re-run, then delete the test deal in the UI. + +All tenant details below are fictional (Ofcom-reserved number ranges). +""" + +import os +from pathlib import Path +from typing import Any, List, cast + +from dotenv import dotenv_values +from hubspot.client import Client # 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.hubspot.deal_contacts_client import HubspotDealContactsClient +from orchestration.abri_tenant_data_sync_orchestrator import ( + AbriTenantDataSyncOrchestrator, +) + +DEAL_ID = "EDIT-ME" + +# Cleanup mode: set ARCHIVE = True and list the contact ids printed by a +# previous run to archive them instead of creating new ones. +ARCHIVE = False +CONTACT_IDS_TO_ARCHIVE: List[str] = [] + +PLACE_REF = PlaceRef("SMOKE1") + +# Jane Doe exercises best-of-kind phone selection, the runner-up pick into +# secondary_phone_number, and the vulnerability fields; Joe Bloggs exercises +# the always-written is_vulnerable=false with every other property omitted. +SMOKE_TENANCY_XML = b""" + SMOKETEST0001900000107700900123 + 07700900456 + 01632960123 + 01632960789 + Blindness + Hard of hearing + + 9000002 + +""" + + +class _CannedResponse: + content = SMOKE_TENANCY_XML + + def raise_for_status(self) -> None: + pass + + +class _StubAbriSession: + """Stands in for requests.Session; returns the canned tenancy.""" + + def post(self, url: str, data: bytes) -> _CannedResponse: + return _CannedResponse() + + +def _hubspot_sdk_client() -> Client: + # Read the key directly rather than via backend.app.config.get_settings, + # whose strict Settings model rejects unrelated extra keys in backend/.env. + env_file = Path(__file__).parents[1] / "backend" / ".env" + access_token = os.getenv("HUBSPOT_API_KEY") or dotenv_values(env_file).get( + "HUBSPOT_API_KEY" + ) + if not access_token: + raise SystemExit("Missing HUBSPOT_API_KEY (env var or backend/.env)") + return Client.create(access_token=access_token) # type: ignore[reportUnknownMemberType] + + +def _stubbed_abri_client() -> AbriClient: + client = AbriClient( + config=AbriConfig( + endpoint_url="https://stubbed.invalid/relay", + username="smoke-test", + password="", + default_resource="", + ) + ) + client._session = cast(Any, _StubAbriSession()) # pyright: ignore[reportPrivateUsage] + return client + + +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") + for contact_id in CONTACT_IDS_TO_ARCHIVE: + sdk_client.crm.contacts.basic_api.archive(contact_id=contact_id) # type: ignore[reportUnknownMemberType] + print(f"Archived contact {contact_id}") + + +def main() -> None: + sdk_client = _hubspot_sdk_client() + + if ARCHIVE: + _archive_contacts(sdk_client) + return + + if DEAL_ID == "EDIT-ME": + raise SystemExit("Set DEAL_ID to a throwaway test deal id first") + + orchestrator = AbriTenantDataSyncOrchestrator( + abri_client=_stubbed_abri_client(), + hubspot=HubspotDealContactsClient(sdk_client=sdk_client), + ) + + result = orchestrator.run(PLACE_REF, deal_id=DEAL_ID) + + if isinstance(result, AbriRequestRejected): + raise SystemExit(f"Unexpected Abri rejection from stub: {result}") + + print(f"Tenancy reference: {result.tenancy_reference}") + print(f"Contacts created: {list(result.contact_ids)}") + print(f"Vulnerable contact count: {result.vulnerable_contact_count}") + print( + "\nTo clean up: paste the ids above into CONTACT_IDS_TO_ARCHIVE, " + "set ARCHIVE = True and re-run." + ) + + +if __name__ == "__main__": + main() From 859a7ec49a978efe7cdfc9932d0322886a556d79 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 14:34:39 +0000 Subject: [PATCH 24/25] change fake contact details --- scripts/smoke_test_tenant_contacts.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/scripts/smoke_test_tenant_contacts.py b/scripts/smoke_test_tenant_contacts.py index 0b7073bd9..c211c076e 100644 --- a/scripts/smoke_test_tenant_contacts.py +++ b/scripts/smoke_test_tenant_contacts.py @@ -10,8 +10,8 @@ Usage: 2. Create a throwaway test deal in the HubSpot UI and paste its id into DEAL_ID below. 3. Run: python scripts/smoke_test_tenant_contacts.py - 4. Check the deal in the UI: two contacts (Jane Doe, Joe Bloggs), with - Jane carrying mobilephone, phone, secondary_phone_number, + 4. Check the deal in the UI: two contacts (Daniel Roth, Joe Bloggs), with + Daniel carrying mobilephone, phone, secondary_phone_number, is_vulnerable=true and a two-line vulnerability_description, and Joe carrying only a name and is_vulnerable=false. 5. Clean up: paste the printed contact ids into CONTACT_IDS_TO_ARCHIVE, @@ -35,7 +35,7 @@ from orchestration.abri_tenant_data_sync_orchestrator import ( AbriTenantDataSyncOrchestrator, ) -DEAL_ID = "EDIT-ME" +DEAL_ID = "485125892321" # Cleanup mode: set ARCHIVE = True and list the contact ids printed by a # previous run to archive them instead of creating new ones. @@ -44,11 +44,11 @@ CONTACT_IDS_TO_ARCHIVE: List[str] = [] PLACE_REF = PlaceRef("SMOKE1") -# Jane Doe exercises best-of-kind phone selection, the runner-up pick into +# Daniel Roth exercises best-of-kind phone selection, the runner-up pick into # secondary_phone_number, and the vulnerability fields; Joe Bloggs exercises # the always-written is_vulnerable=false with every other property omitted. SMOKE_TENANCY_XML = b""" - SMOKETEST0001900000107700900123 + SMOKETEST0001900000107700900123 07700900456 01632960123 01632960789 @@ -95,7 +95,9 @@ def _stubbed_abri_client() -> AbriClient: default_resource="", ) ) - client._session = cast(Any, _StubAbriSession()) # pyright: ignore[reportPrivateUsage] + client._session = cast( + Any, _StubAbriSession() + ) # pyright: ignore[reportPrivateUsage] return client From 06354a166e52b1712e7812cd192e0fcbb2585089 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 6 Jul 2026 14:47:37 +0000 Subject: [PATCH 25/25] =?UTF-8?q?Phone-number=20selection=20policy=20lives?= =?UTF-8?q?=20in=20the=20domain=20layer=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 --- domain/abri/phone_selection.py | 64 ++++++++++ infrastructure/abri/abri_client.py | 61 +++------- tests/domain/abri/test_phone_selection.py | 140 ++++++++++++++++++++++ 3 files changed, 218 insertions(+), 47 deletions(-) create mode 100644 domain/abri/phone_selection.py create mode 100644 tests/domain/abri/test_phone_selection.py diff --git a/domain/abri/phone_selection.py b/domain/abri/phone_selection.py new file mode 100644 index 000000000..fe3588d5f --- /dev/null +++ b/domain/abri/phone_selection.py @@ -0,0 +1,64 @@ +from dataclasses import dataclass +from typing import List, Literal, Optional, Sequence, Tuple + +PhoneKind = Literal["mobile", "telephone"] + + +@dataclass(frozen=True) +class PhoneEntry: + """One phone number as Abri lists it for a tenant, in document order.""" + + kind: PhoneKind + number: str + priority: Optional[str] = None + + +@dataclass(frozen=True) +class SelectedPhoneNumbers: + """The numbers a contact carries: best of each kind plus the runner-up.""" + + mobile: Optional[str] + telephone: Optional[str] + secondary: Optional[str] + + +# TODO(#1474): confirm the priority ordering with Abri — this assumes the +# lowest numeric priority marks the preferred number, and that entries with +# a missing or non-numeric priority rank last (ties keep document order). +def _priority_sort_key(entry: PhoneEntry) -> Tuple[int, int]: + """Sort key: lowest numeric priority first; unusable priorities last.""" + try: + return (0, int((entry.priority or "").strip())) + except ValueError: + return (1, 0) + + +def _best(entries: List[PhoneEntry]) -> Optional[PhoneEntry]: + ranked = sorted(entries, key=_priority_sort_key) # stable: ties keep document order + return ranked[0] if ranked else None + + +def _number(entry: Optional[PhoneEntry]) -> Optional[str]: + return entry.number.strip() if entry is not None else None + + +def select_phone_numbers(entries: Sequence[PhoneEntry]) -> SelectedPhoneNumbers: + """Best mobile, best landline, then the best of the leftovers of both kinds. + + All three picks use the same rule: lowest numeric priority wins, entries + without a usable priority sort last, ties break by document order, blank + numbers are ignored. + """ + usable = [entry for entry in entries if entry.number.strip()] + + best_mobile = _best([e for e in usable if e.kind == "mobile"]) + best_telephone = _best([e for e in usable if e.kind == "telephone"]) + runner_up = _best( + [e for e in usable if e is not best_mobile and e is not best_telephone] + ) + + return SelectedPhoneNumbers( + mobile=_number(best_mobile), + telephone=_number(best_telephone), + secondary=_number(runner_up), + ) diff --git a/infrastructure/abri/abri_client.py b/infrastructure/abri/abri_client.py index a07da387f..80f9cde77 100644 --- a/infrastructure/abri/abri_client.py +++ b/infrastructure/abri/abri_client.py @@ -1,7 +1,6 @@ import xml.etree.ElementTree as ET -from dataclasses import dataclass from datetime import date -from typing import List, Optional, Tuple, Union +from typing import Dict, List, Tuple, Union import requests @@ -18,6 +17,7 @@ from domain.abri.models import ( TenancyData, Tenant, ) +from domain.abri.phone_selection import PhoneEntry, PhoneKind, select_phone_numbers from infrastructure.abri.config import AbriConfig from infrastructure.abri.envelope import serialise_relay_request from infrastructure.abri.errors import AbriResponseParseError, AbriTransportError @@ -31,54 +31,21 @@ def _format_appointment_date(appointment_date: date) -> str: return appointment_date.strftime("%d/%m/%Y") -def _phone_priority(entry: ET.Element) -> Tuple[int, int]: - """Sort key: lowest numeric priority first; unusable priorities last.""" - try: - return (0, int((entry.get("priority") or "").strip())) - except ValueError: - return (1, 0) +# Wire tags -> the domain's phone kinds; selection policy lives in the domain. +_PHONE_KINDS: Dict[str, PhoneKind] = {"Mobile": "mobile", "Telephone": "telephone"} -def _best_phone_number(entries: List[ET.Element]) -> Optional[ET.Element]: - ranked = sorted(entries, key=_phone_priority) # stable: ties keep document order - return ranked[0] if ranked else None - - -@dataclass(frozen=True) -class _SelectedPhoneNumbers: - mobile: Optional[str] - telephone: Optional[str] - secondary: Optional[str] - - -def _select_phone_numbers(tenant_element: ET.Element) -> _SelectedPhoneNumbers: - """Best mobile, best landline, then the best of the leftovers of both kinds. - - All three picks use the same rule: lowest numeric priority wins, entries - without a usable priority sort last, ties break by document order, blank - numbers are ignored. - """ - usable = [ - entry - for entry in tenant_element - if entry.tag in ("Mobile", "Telephone") and (entry.text or "").strip() +def _phone_entries(tenant_element: ET.Element) -> List[PhoneEntry]: + return [ + PhoneEntry( + kind=_PHONE_KINDS[child.tag], + number=child.text or "", + priority=child.get("priority"), + ) + for child in tenant_element + if child.tag in _PHONE_KINDS ] - best_mobile = _best_phone_number([e for e in usable if e.tag == "Mobile"]) - best_telephone = _best_phone_number([e for e in usable if e.tag == "Telephone"]) - runner_up = _best_phone_number( - [e for e in usable if e is not best_mobile and e is not best_telephone] - ) - - def _text(entry: Optional[ET.Element]) -> Optional[str]: - return (entry.text or "").strip() if entry is not None else None - - return _SelectedPhoneNumbers( - mobile=_text(best_mobile), - telephone=_text(best_telephone), - secondary=_text(runner_up), - ) - class AbriClient: def __init__(self, config: AbriConfig) -> None: @@ -251,7 +218,7 @@ class AbriClient: def _parse_tenant(element: ET.Element) -> Tenant: # Data minimisation: person_title, main-contact and the per-person # reference (the element text) are deliberately not read. - numbers = _select_phone_numbers(element) + numbers = select_phone_numbers(_phone_entries(element)) return Tenant( forenames=(element.get("forenames") or "").strip(), surname=(element.get("surname") or "").strip(), diff --git a/tests/domain/abri/test_phone_selection.py b/tests/domain/abri/test_phone_selection.py new file mode 100644 index 000000000..70e2ec5a3 --- /dev/null +++ b/tests/domain/abri/test_phone_selection.py @@ -0,0 +1,140 @@ +from typing import Optional + +from domain.abri.phone_selection import ( + PhoneEntry, + SelectedPhoneNumbers, + select_phone_numbers, +) + + +def _mobile(number: str, priority: Optional[str] = None) -> PhoneEntry: + return PhoneEntry(kind="mobile", number=number, priority=priority) + + +def _telephone(number: str, priority: Optional[str] = None) -> PhoneEntry: + return PhoneEntry(kind="telephone", number=number, priority=priority) + + +def test_no_entries_selects_nothing() -> None: + assert select_phone_numbers([]) == SelectedPhoneNumbers( + mobile=None, telephone=None, secondary=None + ) + + +def test_lowest_numeric_priority_wins_within_a_kind() -> None: + # Arrange + entries = [ + _mobile("07700900020", priority="20"), + _mobile("07700900005", priority="5"), + _mobile("07700900010", priority="10"), + ] + + # Act + selected = select_phone_numbers(entries) + + # Assert + assert selected.mobile == "07700900005" + + +def test_entries_without_a_usable_priority_sort_last() -> None: + # Arrange + entries = [ + _mobile("07700900001"), # missing priority + _mobile("07700900002", priority="abc"), # non-numeric priority + _mobile("07700900003", priority="99"), + ] + + # Act + selected = select_phone_numbers(entries) + + # Assert + assert selected.mobile == "07700900003" + + +def test_priority_ties_break_by_document_order() -> None: + # Arrange + entries = [ + _telephone("01632960001", priority="7"), + _telephone("01632960002", priority="7"), + ] + + # Act + selected = select_phone_numbers(entries) + + # Assert + assert selected.telephone == "01632960001" + + +def test_blank_numbers_are_ignored_even_at_best_priority() -> None: + # Arrange + entries = [ + _mobile(" ", priority="1"), + _mobile("07700900002", priority="2"), + ] + + # Act + selected = select_phone_numbers(entries) + + # Assert + assert selected == SelectedPhoneNumbers( + mobile="07700900002", telephone=None, secondary=None + ) + + +def test_secondary_is_the_best_of_the_pooled_leftovers_of_both_kinds() -> None: + # Arrange + entries = [ + _mobile("07700900005", priority="5"), # best mobile + _mobile("07700900020", priority="20"), # leftover + _telephone("01632960003", priority="3"), # best landline + _telephone("01632960007", priority="7"), # best leftover: wins secondary + ] + + # Act + selected = select_phone_numbers(entries) + + # Assert + assert selected == SelectedPhoneNumbers( + mobile="07700900005", telephone="01632960003", secondary="01632960007" + ) + + +def test_no_secondary_when_the_best_picks_use_every_number() -> None: + # Arrange + entries = [ + _mobile("07700900005", priority="5"), + _telephone("01632960003", priority="3"), + ] + + # Act + selected = select_phone_numbers(entries) + + # Assert + assert selected.secondary is None + + +def test_a_single_kind_still_yields_a_secondary_from_its_own_leftovers() -> None: + # Arrange + entries = [ + _telephone("01632960001", priority="1"), + _telephone("01632960002", priority="2"), + ] + + # Act + selected = select_phone_numbers(entries) + + # Assert + assert selected == SelectedPhoneNumbers( + mobile=None, telephone="01632960001", secondary="01632960002" + ) + + +def test_selected_numbers_are_stripped_of_surrounding_whitespace() -> None: + # Arrange + entries = [_mobile(" 07700900123 ", priority="1")] + + # Act + selected = select_phone_numbers(entries) + + # Assert + assert selected.mobile == "07700900123"