Merge pull request #1686 from Hestia-Homes/feature/abri-api-void-job

Abri API: Log empty properties in hubspot as Void
This commit is contained in:
Daniel Roth 2026-07-27 15:21:07 +01:00 committed by GitHub
commit 7680d9346e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 318 additions and 12 deletions

View file

@ -472,6 +472,10 @@ _Avoid_: new API, current API
The auth credential required by the New EPC API; stored in the `EPC_AUTH_TOKEN` environment variable.
_Avoid_: API key, auth token, secret
**Void Property**:
A place Abri reports as having no live tenancy, signalled by `getSCSTenantData` returning a `RelayError` whose message is exactly `No_tenancy_found`. A normal outcome of the tenant-data sync, not a failure: there are no signatories to create as deal contacts, so the flow completes instead of dead-lettering the message, prefixing the deal's **Extra booking information** (`notes_for_surveyor`) with the `Void.` marker. HubSpot has no append, so that property is read live and rewritten whole — never rewritten from the scraped snapshot, which lags by a scrape cycle and would discard a coordinator's unscraped edit. The marker doubles as the idempotency guard: a note already carrying it is left untouched, so redelivery never stacks markers. Distinct from a tenancy carrying no tenants (a tenancy exists; nobody is recorded against it), and from the same words arriving as a dumped 4GL query (`No Tenancy Found for Query FOR EACH ...`), which stays a rejection — Abri does not send that form to mean "known empty".
_Avoid_: empty property, no tenancy (names the message, not the state), untenanted
## Team
Who's who on the project, so commit authorship and review history read

View file

@ -23,6 +23,7 @@ from orchestration.abri_orchestrator import (
DealDatabaseGateway,
LogJobOrchestrationResult,
LogJobWriteBackError,
PropertyReportedVoid,
TenantDataSyncResult,
)
from utilities.aws_lambda.task_handler import NonRetriableTaskError
@ -198,6 +199,13 @@ def _run_tenant_sync(request: AbriTriggerRequest, flows: AbriFlows) -> str:
)
if isinstance(outcome, AbriRequestRejected):
raise AbriFlowRejectedError(flow="sync_tenant_data", rejection=outcome)
if isinstance(outcome, PropertyReportedVoid):
# Not a failure: an empty property has no signatories to sync, so
# redelivery could only park the message in the dead-letter queue.
return (
f"no live tenancy for place {outcome.place_ref}; "
"property noted as void"
)
return (
f"{len(outcome.contact_ids)} contacts created "
f"({outcome.vulnerable_contact_count} vulnerable)"

View file

@ -26,6 +26,10 @@ class HubspotDealData(SQLModel, table=True):
number_of_attempts: Optional[str] = Field(default=None)
outcome_notes: Optional[str] = Field(default=None)
booking_status: Optional[str] = Field(default=None)
# The HubSpot deal property behind the "Extra booking information"
# label; the property ID is fixed externally and is what the scrape,
# the differ and the Abri void flag all key on.
notes_for_surveyor: Optional[str] = Field(default=None)
major_condition_issue_description: Optional[str] = Field(default=None)
major_condition_issue_photos: Optional[str] = Field(default=None)

View file

@ -26,7 +26,7 @@ counts.
| When you… | Abri's system… | How you know it worked |
|---|---|---|
| Set **Expected commencement date** for the first time | Sends us the tenancy details for the property | Tenant contact records appear in HubSpot, linked to the deal |
| Set **Expected commencement date** for the first time | Sends us the tenancy details for the property | Tenant contact records appear in HubSpot, linked to the deal — unless Abri reports the property as empty (see below) |
| Set **Confirmed survey date** for the first time | Creates the survey booking (a "job") against the property, assigned to the surveyor on the deal | Abri's job number appears in **Client booking reference** on the deal |
| Change **Confirmed survey date**, **Confirmed survey time** or **Third-party surveyor identifier** on a deal that already has a booking | Updates the existing appointment — new date/time and/or reassigns it to the new surveyor | The deal keeps the same Client booking reference |
| Record a **3rd attempt** (Number of attempts reaches 3) **and** set **Outcome** to an unsuccessful value (see below) | Cancels the booking as abandoned | — (this only fires once per deal) |
@ -41,6 +41,16 @@ counts.
Any other outcome (or fewer than 3 attempts) does **not** cancel anything in Abri's
system.
**Empty properties.** If Abri has no live tenancy for the property, no tenant contacts
appear on the deal — there is nobody to add — and **Extra booking information** is
prefixed with `Void.` so the surveyor sees it first. Anything already in that field is
kept, after the prefix. This is expected, not a fault, and nothing needs re-triggering.
Note that Abri sends the same signal for a property reference it does not recognise, so
if you were expecting tenants, check the property is right before assuming it's empty.
Delete the `Void.` prefix if you establish the property is not empty after all — the
system only adds it once, so it will not come back on its own.
## The fields, and why they matter
| HubSpot deal field | What it feeds |

View file

@ -108,7 +108,20 @@ class TenancyData:
tenants: Tuple[Tenant, ...]
GetTenantDataResult = Union[TenancyData, AbriRequestRejected]
@dataclass(frozen=True)
class NoTenancyFound:
"""Abri reports no live tenancy for the place: the property is void.
Distinct from a TenancyData carrying no tenants (a tenancy exists, but
nobody is recorded against it) and from a rejection (the request itself
was refused). Only Abri's exact No_tenancy_found signal means this; the
query-dump variant of the same words is a genuine failure.
"""
place_ref: PlaceRef
GetTenantDataResult = Union[TenancyData, NoTenancyFound, AbriRequestRejected]
@dataclass(frozen=True)

View file

@ -264,6 +264,7 @@ class HubspotClient:
"number_of_attempts",
"outcome_notes",
"booking_status",
"notes_for_surveyor",
"project_code",
"major_condition_issue_description",
"major_condition_issue_photos",

View file

@ -214,6 +214,7 @@ class HubspotDataToDb:
"number_of_attempts": deal_data.get("number_of_attempts"),
"outcome_notes": deal_data.get("outcome_notes"),
"booking_status": deal_data.get("booking_status"),
"notes_for_surveyor": deal_data.get("notes_for_surveyor"),
"project_code": deal_data.get("project_code"),
"company_id": company,
"major_condition_issue_description": deal_data.get(
@ -340,6 +341,7 @@ class HubspotDataToDb:
number_of_attempts=deal_data.get("number_of_attempts"),
outcome_notes=deal_data.get("outcome_notes"),
booking_status=deal_data.get("booking_status"),
notes_for_surveyor=deal_data.get("notes_for_surveyor"),
project_code=deal_data.get("project_code"),
company_id=company,
major_condition_issue_description=deal_data.get(

View file

@ -70,6 +70,7 @@ class HubspotDealDiffer:
"project_code": "project_code",
"outcome_notes": "outcome_notes",
"booking_status": "booking_status",
"notes_for_surveyor": "notes_for_surveyor",
"major_condition_issue_description": "major_condition_issue_description",
"major_condition_issue_photos": "major_condition_issue_photos",
"coordination_status__stage_1_": "coordination_status",

View file

@ -16,6 +16,7 @@ from domain.abri.models import (
JobLogged,
LogJobRequest,
LogJobResult,
NoTenancyFound,
PlaceRef,
TenancyData,
Tenant,
@ -25,6 +26,12 @@ from infrastructure.abri.config import AbriConfig
from infrastructure.abri.envelope import serialise_relay_request
from infrastructure.abri.errors import AbriResponseParseError, AbriTransportError
# Abri's agreed signal, sent as an otherwise ordinary RelayError, that the
# place is known to have no live tenancy. Matched exactly: the same words
# also arrive as a dumped 4GL query ("No Tenancy Found for Query FOR EACH
# ..."), which is a genuine failure and must stay a rejection.
NO_TENANCY_FOUND_MESSAGE = "No_tenancy_found"
STD_JOB_CODE = "SCSEXT"
CLIENT_CODE = "HSG"
RESOURCE_GROUP = "Surveyors"
@ -118,6 +125,8 @@ class AbriClient:
)
if isinstance(outcome, AbriRequestRejected):
if outcome.message == NO_TENANCY_FOUND_MESSAGE:
return NoTenancyFound(place_ref=place_ref)
return outcome
return self._parse_tenancy_data(outcome)

View file

@ -1,5 +1,8 @@
from typing import Dict, Optional, cast
from hubspot.client import Client # type: ignore[reportMissingTypeStubs]
from hubspot.crm.deals import SimplePublicObjectInput # type: ignore[reportMissingTypeStubs]
from hubspot.crm.objects.models import SimplePublicObject as HubspotObject # type: ignore[reportMissingTypeStubs]
from infrastructure.hubspot.scrubbed_calls import scrubbed_call_with_retry
@ -15,6 +18,26 @@ class HubspotDealPropertiesClient:
def __init__(self, sdk_client: Client) -> None:
self._client: Client = sdk_client
def read_deal_property(self, deal_id: str, property_name: str) -> Optional[str]:
"""The property's current value, or None when it is unset.
Read live rather than from the scraped snapshot: callers that rewrite
a property whole (HubSpot has no append) would otherwise overwrite
edits made since the last scrape.
"""
deal: HubspotObject = scrubbed_call_with_retry(
lambda: cast(
HubspotObject,
self._client.crm.deals.basic_api.get_by_id( # type: ignore[reportUnknownMemberType]
deal_id, properties=[property_name]
),
)
)
properties: Dict[str, Optional[str]] = cast(
Dict[str, Optional[str]], deal.properties # type: ignore[reportUnknownMemberType]
)
return properties.get(property_name)
def write_deal_property(
self, deal_id: str, property_name: str, value: str
) -> None:

View file

@ -11,6 +11,7 @@ from domain.abri.models import (
AppointmentAmended,
JobAbandoned,
LogJobRequest,
NoTenancyFound,
PlaceRef,
Tenant,
)
@ -24,6 +25,17 @@ from infrastructure.hubspot.errors import HubspotRequestError
# property ID is fixed externally, the domain and database call it job_no.
JOB_NO_DEAL_PROPERTY = "client_booking_reference"
# The HubSpot deal property behind "Extra booking information"; the property
# ID is fixed externally, and the surveyor-facing label is what users see.
NOTES_FOR_SURVEYOR_DEAL_PROPERTY = "notes_for_surveyor"
# Prepended to that property when Abri reports the place as having no live
# tenancy, so a surveyor sees it first. Doubles as the idempotency marker: a
# note already carrying it is left alone, so redelivery never stacks markers.
# Carries no separator of its own — the write below spaces it off whatever was
# already in the note, and a note that is otherwise empty gets the bare marker.
VOID_MARKER = "Void."
def _vulnerability_description(tenant: Tenant) -> Optional[str]:
"""The tenant's distinct vulnerabilities, verbatim and newline-joined.
@ -65,7 +77,20 @@ class TenantDataSyncSummary:
vulnerable_contact_count: int
TenantDataSyncResult = Union[TenantDataSyncSummary, AbriRequestRejected]
@dataclass(frozen=True)
class PropertyReportedVoid:
"""Abri reported no live tenancy: no contacts to create for the deal.
A normal outcome, not a failure an empty property has no signatories to
sync, so the flow completes rather than failing the task.
"""
place_ref: PlaceRef
TenantDataSyncResult = Union[
TenantDataSyncSummary, PropertyReportedVoid, AbriRequestRejected
]
class TenantDataSyncError(Exception):
@ -246,6 +271,10 @@ class AbriOrchestrator:
if isinstance(tenancy, AbriRequestRejected):
return tenancy
if isinstance(tenancy, NoTenancyFound):
self._note_property_void(deal_id=deal_id)
return PropertyReportedVoid(place_ref=place_ref)
contact_ids: List[str] = []
associated_ids: List[str] = []
for tenant_index, tenant in enumerate(tenancy.tenants):
@ -277,6 +306,31 @@ class AbriOrchestrator:
),
)
def _note_property_void(self, deal_id: str) -> None:
"""Flag the void property at the front of the deal's booking information.
HubSpot has no append, so the property is read and rewritten whole.
The read is live rather than from the scraped snapshot: the snapshot
lags by a scrape cycle, and rewriting from it would discard whatever a
coordinator typed in the meantime. The write is still last-write-wins
HubSpot offers no compare-and-set so the race is narrowed, not
closed.
"""
existing = (
self._deal_properties.read_deal_property(
deal_id=deal_id, property_name=NOTES_FOR_SURVEYOR_DEAL_PROPERTY
)
or ""
)
if existing.startswith(VOID_MARKER):
return
self._deal_properties.write_deal_property(
deal_id=deal_id,
property_name=NOTES_FOR_SURVEYOR_DEAL_PROPERTY,
value=f"{VOID_MARKER} {existing}".rstrip(),
)
def amend_job(self, change: AppointmentChange) -> AmendJobOrchestrationResult:
job_no = self._deal_database.job_no_for_deal(change.deal_id)
if job_no is None:
@ -286,9 +340,7 @@ class AbriOrchestrator:
AmendJobRequest(
job_no=job_no,
appointment_date=change.confirmed_survey_date,
appointment_time=slot_for_confirmed_time(
change.confirmed_survey_time
),
appointment_time=slot_for_confirmed_time(change.confirmed_survey_time),
resource=change.third_party_surveyor_identifier,
)
)
@ -321,9 +373,7 @@ class AbriOrchestrator:
client_ref=client_ref_for_deal(booking.deal_id),
place_ref=booking.place_ref,
appointment_date=booking.confirmed_survey_date,
appointment_time=slot_for_confirmed_time(
booking.confirmed_survey_time
),
appointment_time=slot_for_confirmed_time(booking.confirmed_survey_time),
short_description=descriptions.short_description,
long_description=descriptions.long_description,
resource=booking.third_party_surveyor_identifier,

View file

@ -0,0 +1,6 @@
<?xml version="1.0" encoding="utf-8"?>
<response>
<success>false</success>
<code>RelayError</code>
<message>No_tenancy_found</message>
</response>

View file

@ -23,6 +23,7 @@ from orchestration.abri_orchestrator import (
DealAbandonment,
JobNoNotYetRecordedError,
LogJobOrchestrationResult,
PropertyReportedVoid,
LogJobSummary,
LogJobWriteBackError,
TenantDataSyncResult,
@ -426,3 +427,23 @@ def test_the_summary_records_an_abandoned_job(
# Assert
assert summary == {"abandon_job": "job AC0439951 abandoned"}
# --- a void property completes the flow rather than failing the task ---
def test_a_void_property_completes_the_tenant_sync_instead_of_failing_the_task(
orchestrator: FakeOrchestrator, deal_database: FakeDealDatabase
) -> None:
# Arrange: an empty property has no signatories, so redelivering the
# message could only park it in the dead-letter queue.
orchestrator.sync_outcome = PropertyReportedVoid(place_ref=PlaceRef("1007165"))
request = _request(["sync_tenant_data"])
# Act
summary = dispatch_abri_flows(request, orchestrator, deal_database)
# Assert
assert summary == {
"sync_tenant_data": "no live tenancy for place 1007165; property noted as void"
}

View file

@ -15,6 +15,7 @@ from domain.abri.models import (
JobAbandoned,
JobLogged,
LogJobRequest,
NoTenancyFound,
PlaceRef,
)
from infrastructure.abri.abri_client import AbriClient
@ -556,3 +557,38 @@ def test_abandon_job_sends_the_recorded_canceljob_envelope_to_the_relay_endpoint
assert ET.canonicalize(xml_data=sent_body, strip_text=True) == ET.canonicalize(
xml_data=expected_body, strip_text=True
)
# --- get_tenant_data: a void property is not a rejection ---
def test_get_tenant_data_reports_a_void_property_when_abri_finds_no_tenancy(
client: AbriClient, mock_session: MagicMock
) -> None:
# Arrange: Abri's agreed signal that the place is known to be empty.
mock_session.post.return_value.content = _load_fixture(
"abri_relay_getscstenantdata_notenancyfound_response.xml"
)
# Act
result = client.get_tenant_data(PlaceRef("1004202A"))
# Assert
assert result == NoTenancyFound(place_ref=PlaceRef("1004202A"))
def test_get_tenant_data_still_rejects_the_query_dump_form_of_no_tenancy_found(
client: AbriClient, mock_session: MagicMock
) -> None:
# Arrange: the same words, dumped as a 4GL query. Abri does not send this
# to mean "known empty", so it must not be read as a void property.
mock_session.post.return_value.content = _load_fixture(
"abri_relay_getscstenantdata_relayerror_response.xml"
)
# Act
result = client.get_tenant_data(PlaceRef("1004202A"))
# Assert
assert isinstance(result, AbriRequestRejected)
assert result.message.startswith("No Tenancy Found for Query")

View file

@ -10,9 +10,12 @@ 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 hubspot.crm.deals import ApiException as DealsApiException # type: ignore[reportMissingTypeStubs]
from hubspot.crm.deals import SimplePublicObjectInput as DealPropertyInput # type: ignore[reportMissingTypeStubs]
from domain.abri.models import AbriRequestRejected, PlaceRef
from infrastructure.hubspot.deal_contacts_client import HubspotDealContactsClient
from infrastructure.hubspot.errors import HubspotRequestError
from infrastructure.hubspot.deal_properties_client import HubspotDealPropertiesClient
from infrastructure.abri.abri_client import AbriClient
from infrastructure.abri.config import AbriConfig
@ -20,6 +23,7 @@ from infrastructure.abri.errors import AbriResponseParseError, AbriTransportErro
from orchestration.abri_orchestrator import (
TenantDataSyncError,
AbriOrchestrator,
PropertyReportedVoid,
TenantDataSyncSummary,
)
@ -71,9 +75,7 @@ class FakeDealDatabase:
@pytest.fixture()
def orchestrator(
mock_session: MagicMock, sdk_client: MagicMock
) -> AbriOrchestrator:
def orchestrator(mock_session: MagicMock, sdk_client: MagicMock) -> AbriOrchestrator:
with patch(
"infrastructure.abri.abri_client.requests.Session",
return_value=mock_session,
@ -539,3 +541,119 @@ def test_sync_tenant_data_retries_a_rate_limited_hubspot_call_before_succeeding(
vulnerable_contact_count=0,
)
assert sdk_client.crm.contacts.basic_api.create.call_count == 2
# --- void property: no tenancy to sync, and not a failure ---
def test_sync_tenant_data_reports_the_property_void_and_creates_no_contacts(
orchestrator: AbriOrchestrator,
mock_session: MagicMock,
sdk_client: MagicMock,
) -> None:
# Arrange: Abri's agreed signal that the place is known to be empty.
mock_session.post.return_value.content = _load_fixture(
"abri_relay_getscstenantdata_notenancyfound_response.xml"
)
# Act
result = orchestrator.sync_tenant_data(PLACE_REF, deal_id=DEAL_ID)
# Assert
assert result == PropertyReportedVoid(place_ref=PLACE_REF)
assert sdk_client.crm.contacts.mock_calls == []
def test_a_void_property_is_flagged_at_the_front_of_the_deals_booking_information(
orchestrator: AbriOrchestrator,
mock_session: MagicMock,
sdk_client: MagicMock,
) -> None:
# Arrange: a surveyor must see the property is empty before whatever the
# coordinator already wrote for them.
mock_session.post.return_value.content = _load_fixture(
"abri_relay_getscstenantdata_notenancyfound_response.xml"
)
sdk_client.crm.deals.basic_api.get_by_id.return_value.properties = {
"notes_for_surveyor": "Key safe code 1234"
}
# Act
orchestrator.sync_tenant_data(PLACE_REF, deal_id=DEAL_ID)
# Assert
sdk_client.crm.deals.basic_api.update.assert_called_once_with(
DEAL_ID,
simple_public_object_input=DealPropertyInput(
properties={"notes_for_surveyor": "Void. Key safe code 1234"}
),
)
def test_a_void_property_with_no_existing_booking_information_is_flagged_alone(
orchestrator: AbriOrchestrator,
mock_session: MagicMock,
sdk_client: MagicMock,
) -> None:
# Arrange: HubSpot sends "" for an unset property.
mock_session.post.return_value.content = _load_fixture(
"abri_relay_getscstenantdata_notenancyfound_response.xml"
)
sdk_client.crm.deals.basic_api.get_by_id.return_value.properties = {
"notes_for_surveyor": ""
}
# Act
orchestrator.sync_tenant_data(PLACE_REF, deal_id=DEAL_ID)
# Assert: no dangling separator on an otherwise empty note.
sdk_client.crm.deals.basic_api.update.assert_called_once_with(
DEAL_ID,
simple_public_object_input=DealPropertyInput(
properties={"notes_for_surveyor": "Void."}
),
)
def test_a_deal_already_flagged_void_is_left_alone_so_redelivery_never_stacks_flags(
orchestrator: AbriOrchestrator,
mock_session: MagicMock,
sdk_client: MagicMock,
) -> None:
# Arrange: the state a redelivered message finds.
mock_session.post.return_value.content = _load_fixture(
"abri_relay_getscstenantdata_notenancyfound_response.xml"
)
sdk_client.crm.deals.basic_api.get_by_id.return_value.properties = {
"notes_for_surveyor": "Void. Key safe code 1234"
}
# Act
result = orchestrator.sync_tenant_data(PLACE_REF, deal_id=DEAL_ID)
# Assert
assert result == PropertyReportedVoid(place_ref=PLACE_REF)
assert sdk_client.crm.deals.basic_api.update.mock_calls == []
def test_a_failed_void_flag_write_fails_the_sync_rather_than_reporting_success(
orchestrator: AbriOrchestrator,
mock_session: MagicMock,
sdk_client: MagicMock,
) -> None:
# Arrange: nothing was created for a void property, so the whole flow is
# safe to re-run — the failure must reach the caller and be redelivered.
mock_session.post.return_value.content = _load_fixture(
"abri_relay_getscstenantdata_notenancyfound_response.xml"
)
sdk_client.crm.deals.basic_api.get_by_id.return_value.properties = {
"notes_for_surveyor": ""
}
api_error = DealsApiException(status=400, reason="Bad Request")
api_error.body = json.dumps({"category": "VALIDATION_ERROR"})
api_error.headers = {}
sdk_client.crm.deals.basic_api.update.side_effect = api_error
# Act / Assert
with pytest.raises(HubspotRequestError):
orchestrator.sync_tenant_data(PLACE_REF, deal_id=DEAL_ID)