diff --git a/domain/abri/models.py b/domain/abri/models.py index 0444ec1cc..d96d27fda 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,33 @@ 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, 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, ...] + + +@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/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/etl/hubspot/hubspotClient.py b/etl/hubspot/hubspotClient.py index 918701cac..641695e27 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 infrastructure.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 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]: associations_api: AssociationsBasicApi = ( # type: ignore[reportUnknownMemberType] diff --git a/infrastructure/abri/abri_client.py b/infrastructure/abri/abri_client.py index 3c95c2766..80f9cde77 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 Dict, List, Tuple, Union import requests @@ -9,10 +9,15 @@ from domain.abri.models import ( AmendJobRequest, AmendJobResult, AppointmentAmended, + GetTenantDataResult, JobLogged, LogJobRequest, LogJobResult, + PlaceRef, + 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 @@ -26,6 +31,22 @@ def _format_appointment_date(appointment_date: date) -> str: return appointment_date.strftime("%d/%m/%Y") +# Wire tags -> the domain's phone kinds; selection policy lives in the domain. +_PHONE_KINDS: Dict[str, PhoneKind] = {"Mobile": "mobile", "Telephone": "telephone"} + + +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 + ] + + class AbriClient: def __init__(self, config: AbriConfig) -> None: self._config = config @@ -73,6 +94,17 @@ class AbriClient: return self._parse_appointment_amended(outcome) + def get_tenant_data(self, place_ref: PlaceRef) -> GetTenantDataResult: + 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]] ) -> Union[ET.Element, AbriRequestRejected]: @@ -159,6 +191,47 @@ class AbriClient: appointment_time=appointment_time, ) + @staticmethod + def _parse_tenancy_data(root: ET.Element) -> TenancyData: + tenancies = root.findall("Tenancy") + + if len(tenancies) != 1: + raise AbriResponseParseError( + 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( + 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. + numbers = select_phone_numbers(_phone_entries(element)) + return Tenant( + forenames=(element.get("forenames") or "").strip(), + surname=(element.get("surname") or "").strip(), + mobile=numbers.mobile, + telephone=numbers.telephone, + secondary_number=numbers.secondary, + vulnerabilities=tuple( + text + for vulnerability in element.findall("Vulnerability") + if (text := (vulnerability.text or "").strip()) + ), + ) + @staticmethod def _parse_rejection(root: ET.Element) -> AbriRequestRejected: success = root.findtext("success") diff --git a/infrastructure/hubspot/__init__.py b/infrastructure/hubspot/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/infrastructure/hubspot/deal_contacts_client.py b/infrastructure/hubspot/deal_contacts_client.py new file mode 100644 index 000000000..2c0eb0627 --- /dev/null +++ b/infrastructure/hubspot/deal_contacts_client.py @@ -0,0 +1,89 @@ +import json +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] +from hubspot.crm.contacts import SimplePublicObjectInputForCreate # type: ignore[reportMissingTypeStubs] + +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 + + +def _scrubbed(error: Exception) -> HubspotRequestError: + """Reduce an SDK error to non-PII diagnostics, dropping the body. + + Each HubSpot sub-module ships its own ApiException class with no shared + base beyond Exception, so the fields are read by duck-typing. + """ + status = getattr(error, "status", None) + status_code = status if isinstance(status, int) else None + + category: Optional[str] = None + correlation_id: Optional[str] = None + try: + parsed: Any = json.loads(getattr(error, "body", None) or "") + if isinstance(parsed, dict): + details = cast(Dict[str, Any], parsed) + raw_category = details.get("category") + raw_correlation_id = details.get("correlationId") + category = raw_category if isinstance(raw_category, str) else None + correlation_id = ( + raw_correlation_id if isinstance(raw_correlation_id, str) else None + ) + except ValueError: + pass + + return HubspotRequestError( + status_code=status_code, category=category, correlation_id=correlation_id + ) + + +class HubspotDealContactsClient: + """Focused HubSpot client for the tenant-data sync flow. + + 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: + contact = self._call( + lambda: 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: + 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/infrastructure/hubspot/errors.py b/infrastructure/hubspot/errors.py new file mode 100644 index 000000000..057e4d968 --- /dev/null +++ b/infrastructure/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})" + ) diff --git a/infrastructure/hubspot/retry.py b/infrastructure/hubspot/retry.py new file mode 100644 index 000000000..7bd50c8a1 --- /dev/null +++ b/infrastructure/hubspot/retry.py @@ -0,0 +1,51 @@ +import time +from http import HTTPStatus +from logging import Logger +from typing import Callable, Dict, Optional, TypeVar, cast + +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 = cast(Dict[str, str], 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 diff --git a/orchestration/abri_tenant_data_sync_orchestrator.py b/orchestration/abri_tenant_data_sync_orchestrator.py new file mode 100644 index 000000000..14ab68e2f --- /dev/null +++ b/orchestration/abri_tenant_data_sync_orchestrator.py @@ -0,0 +1,136 @@ +from dataclasses import dataclass +from typing import Dict, List, Optional, Tuple, Union + +from domain.abri.models import AbriRequestRejected, PlaceRef, Tenant +from infrastructure.hubspot.deal_contacts_client import HubspotDealContactsClient +from infrastructure.hubspot.errors import HubspotRequestError +from infrastructure.abri.abri_client import AbriClient + + +def _vulnerability_description(tenant: Tenant) -> Optional[str]: + """The tenant's distinct vulnerabilities, verbatim and newline-joined. + + Case-insensitively distinct within the tenant, keeping the first-seen + casing and document order. + """ + distinct: List[str] = [] + seen: set[str] = set() + for vulnerability in tenant.vulnerabilities: + if vulnerability.lower() in seen: + continue + seen.add(vulnerability.lower()) + distinct.append(vulnerability) + return "\n".join(distinct) if distinct else None + + +def _contact_properties(tenant: Tenant) -> Dict[str, str]: + candidates: Dict[str, Optional[str]] = { + "firstname": tenant.forenames, + "lastname": tenant.surname, + "mobilephone": tenant.mobile, + "phone": tenant.telephone, + "secondary_phone_number": tenant.secondary_number, + "vulnerability_description": _vulnerability_description(tenant), + } + properties = {name: value for name, value in candidates.items() if value} + # Always written, so "not vulnerable" is distinguishable from "not synced". + properties["is_vulnerable"] = "true" if tenant.vulnerabilities else "false" + return properties + + +@dataclass(frozen=True) +class TenantDataSyncSummary: + """PII-free record of what a sync run did: safe to log and persist.""" + + tenancy_reference: str + contact_ids: Tuple[str, ...] + vulnerable_contact_count: int + + +TenantDataSyncResult = Union[TenantDataSyncSummary, AbriRequestRejected] + + +class TenantDataSyncError(Exception): + """A sync that stopped at the first failed HubSpot write. + + Carries a PII-free progress report so the operator knows the deal's + half-state before cleaning up and re-running; contacts created but not + associated are orphaned in HubSpot and need manual cleanup. + """ + + def __init__( + self, + message: str, + deal_id: str, + tenant_index: int, + contact_ids_created: Tuple[str, ...], + contact_ids_associated: Tuple[str, ...], + ) -> None: + self.deal_id = deal_id + self.tenant_index = tenant_index + self.contact_ids_created = contact_ids_created + self.contact_ids_associated = contact_ids_associated + super().__init__( + f"{message} (deal: {deal_id}, tenant index: {tenant_index}, " + f"contacts created: {list(contact_ids_created)}, " + f"associated: {list(contact_ids_associated)})" + ) + + @property + def orphaned_contact_ids(self) -> Tuple[str, ...]: + return tuple( + contact_id + for contact_id in self.contact_ids_created + if contact_id not in self.contact_ids_associated + ) + + +class AbriTenantDataSyncOrchestrator: + """Syncs an Abri tenancy's signatories to a HubSpot deal. + + Tenant data flows straight from Abri to HubSpot; nothing is persisted + in Domna's database or logs. + """ + + def __init__( + self, + abri_client: AbriClient, + hubspot: HubspotDealContactsClient, + ) -> None: + self._abri = abri_client + self._hubspot = hubspot + + def run(self, place_ref: PlaceRef, deal_id: str) -> TenantDataSyncResult: + tenancy = self._abri.get_tenant_data(place_ref) + + if isinstance(tenancy, AbriRequestRejected): + return tenancy + + contact_ids: List[str] = [] + associated_ids: List[str] = [] + for tenant_index, tenant in enumerate(tenancy.tenants): + try: + contact_id = self._hubspot.create_contact(_contact_properties(tenant)) + contact_ids.append(contact_id) + self._hubspot.associate_contact_to_deal( + deal_id=deal_id, contact_id=contact_id + ) + associated_ids.append(contact_id) + except HubspotRequestError as error: + # Fail fast at the first failed write; `error` is already + # scrubbed, so chaining it keeps tracebacks PII-free. + raise TenantDataSyncError( + message=str(error), + deal_id=deal_id, + tenant_index=tenant_index, + contact_ids_created=tuple(contact_ids), + contact_ids_associated=tuple(associated_ids), + ) from error + + return TenantDataSyncSummary( + tenancy_reference=tenancy.tenancy_reference, + contact_ids=tuple(contact_ids), + vulnerable_contact_count=sum( + 1 for tenant in tenancy.tenants if tenant.vulnerabilities + ), + ) diff --git a/scripts/smoke_test_tenant_contacts.py b/scripts/smoke_test_tenant_contacts.py new file mode 100644 index 000000000..c211c076e --- /dev/null +++ b/scripts/smoke_test_tenant_contacts.py @@ -0,0 +1,142 @@ +"""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 (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, + 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 = "485125892321" + +# 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") + +# 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 + 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() 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..7a3b26efa --- /dev/null +++ b/tests/abri/abri_relay_getscstenantdata_multitenant_response.xml @@ -0,0 +1,8 @@ + + 10071650064015400721607700900456 + + 400721507700900789 + 01632960456 + + + 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_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/abri/abri_relay_getscstenantdata_success_response.xml b/tests/abri/abri_relay_getscstenantdata_success_response.xml new file mode 100644 index 000000000..9056a8c81 --- /dev/null +++ b/tests/abri/abri_relay_getscstenantdata_success_response.xml @@ -0,0 +1,7 @@ + + 10042020051017136847307700900123 + 01632960123 + Blindness + + + 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" diff --git a/tests/orchestration/test_abri_tenant_data_sync_orchestrator.py b/tests/orchestration/test_abri_tenant_data_sync_orchestrator.py new file mode 100644 index 000000000..ad7a738b5 --- /dev/null +++ b/tests/orchestration/test_abri_tenant_data_sync_orchestrator.py @@ -0,0 +1,528 @@ +import json +import traceback +import xml.etree.ElementTree as ET +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 ApiException as ContactsApiException # type: ignore[reportMissingTypeStubs] +from hubspot.crm.contacts import SimplePublicObjectInputForCreate # type: ignore[reportMissingTypeStubs] + +from domain.abri.models import AbriRequestRejected, PlaceRef +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 orchestration.abri_tenant_data_sync_orchestrator import ( + TenantDataSyncError, + AbriTenantDataSyncOrchestrator, + 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" + +CONFIG = AbriConfig( + endpoint_url=ENDPOINT_URL, + username="DomnaWeb", + password="", + default_resource="NAULKH", +) + + +def _load_fixture(name: str) -> bytes: + return (FIXTURE_DIR / name).read_bytes() + + +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 +) -> AbriTenantDataSyncOrchestrator: + with patch( + "infrastructure.abri.abri_client.requests.Session", + return_value=mock_session, + ): + abri_client = AbriClient(config=CONFIG) + return AbriTenantDataSyncOrchestrator( + abri_client=abri_client, + hubspot=HubspotDealContactsClient(sdk_client=sdk_client), + ) + + +# --- outbound request: the spec's recorded envelope --- + + +def test_run_sends_the_recorded_getscstenantdata_envelope_to_the_relay_endpoint( + orchestrator: AbriTenantDataSyncOrchestrator, 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 --- + + +def test_run_creates_an_associated_deal_contact_per_signatory_and_returns_a_summary( + orchestrator: AbriTenantDataSyncOrchestrator, + mock_session: MagicMock, + sdk_client: MagicMock, +) -> None: + # Arrange + mock_session.post.return_value.content = _tenancy_response( + '136847307700900123' + '01632960123' + ) + + # 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": "Jane", + "lastname": "Doe", + "mobilephone": "07700900123", + "phone": "01632960123", + "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 + ) + ], + ) + + +def test_run_creates_a_separate_contact_for_each_signatory( + orchestrator: AbriTenantDataSyncOrchestrator, + 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": "Ann", + "lastname": "Bloggs", + "mobilephone": "07700900456", + "is_vulnerable": "false", + }, + { + "firstname": "Beth", + "lastname": "Sample", + "mobilephone": "07700900789", + "phone": "01632960456", + "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_picks_the_best_priority_number_of_each_kind_for_a_contact( + orchestrator: AbriTenantDataSyncOrchestrator, + 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_fills_secondary_phone_number_with_the_best_leftover_number( + orchestrator: AbriTenantDataSyncOrchestrator, + 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": "Jane", + "lastname": "Doe", + "mobilephone": "07000000005", + "phone": "01000000003", + "secondary_phone_number": "01000000007", + "is_vulnerable": "false", + } + ) + ) + + +# --- vulnerabilities: recorded on the tenant's own contact --- + + +def test_run_records_vulnerabilities_on_the_tenants_own_contact( + orchestrator: AbriTenantDataSyncOrchestrator, + 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": "Ann", + "lastname": "Bloggs", + "is_vulnerable": "true", + "vulnerability_description": "Blindness\nHard of hearing", + }, + { + "firstname": "Beth", + "lastname": "Sample", + "is_vulnerable": "false", + }, + ] + + +def test_run_still_creates_a_contact_for_an_all_blank_signatory( + orchestrator: AbriTenantDataSyncOrchestrator, + 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"} + ) + ) + + +# --- unusual but valid outcomes --- + + +def test_run_treats_a_tenancy_with_no_signatories_as_a_valid_zero_contact_outcome( + orchestrator: AbriTenantDataSyncOrchestrator, + 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: AbriTenantDataSyncOrchestrator, + 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: AbriTenantDataSyncOrchestrator, + 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: AbriTenantDataSyncOrchestrator, + 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 == [] + + +# --- 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 "07700900123" for mobilephone (Jane Doe)', + "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: AbriTenantDataSyncOrchestrator, + mock_session: MagicMock, + sdk_client: MagicMock, +) -> None: + # Arrange + mock_session.post.return_value.content = _tenancy_response( + '136847307700900123' + ) + 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 "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( + orchestrator: AbriTenantDataSyncOrchestrator, + 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 "Bloggs" not in rendered_traceback + assert "07700900456" not in rendered_traceback + + +def test_run_retries_a_rate_limited_hubspot_call_before_succeeding( + orchestrator: AbriTenantDataSyncOrchestrator, + 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