Make the OpenHousing resource mandatory, dropping the default fallback

The deal's third_party_surveyor_identifier is now required by the log
and amend flows: the trigger contract rejects messages without it
(blank treated as missing, since a blank resource makes OpenHousing
silently drop the appointment), and LogJobRequest/AmendJobRequest carry
it as a required field. The ABRI_RELAY_DEFAULT_RESOURCE fallback and
its config/terraform/workflow wiring are removed as dead code.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Daniel Roth 2026-07-17 13:13:28 +00:00
parent c8fe4585bf
commit edd4ec00de
20 changed files with 81 additions and 153 deletions

View file

@ -107,8 +107,6 @@ on:
required: false
TF_VAR_abri_relay_password:
required: false
TF_VAR_abri_relay_default_resource:
required: false
TF_VAR_test_abri_project_code_override:
required: false
jobs:
@ -189,7 +187,6 @@ jobs:
TF_VAR_abri_relay_url: ${{ secrets.TF_VAR_abri_relay_url }}
TF_VAR_abri_relay_username: ${{ secrets.TF_VAR_abri_relay_username }}
TF_VAR_abri_relay_password: ${{ secrets.TF_VAR_abri_relay_password }}
TF_VAR_abri_relay_default_resource: ${{ secrets.TF_VAR_abri_relay_default_resource }}
TF_VAR_test_abri_project_code_override: ${{ secrets.TF_VAR_test_abri_project_code_override }}
run: |
ECR_REPO_URL_VAR=""
@ -247,7 +244,6 @@ jobs:
TF_VAR_abri_relay_url: ${{ secrets.TF_VAR_abri_relay_url }}
TF_VAR_abri_relay_username: ${{ secrets.TF_VAR_abri_relay_username }}
TF_VAR_abri_relay_password: ${{ secrets.TF_VAR_abri_relay_password }}
TF_VAR_abri_relay_default_resource: ${{ secrets.TF_VAR_abri_relay_default_resource }}
run: |
EXTRA_VARS=""
if [[ -n "${{ inputs.ecr_repo }}" ]]; then

View file

@ -740,7 +740,6 @@ jobs:
TF_VAR_abri_relay_url: ${{ secrets.ABRI_RELAY_URL }}
TF_VAR_abri_relay_username: ${{ secrets.ABRI_RELAY_USERNAME }}
TF_VAR_abri_relay_password: ${{ secrets.ABRI_RELAY_PASSWORD }}
TF_VAR_abri_relay_default_resource: ${{ secrets.ABRI_RELAY_DEFAULT_RESOURCE }}
# ============================================================
# Build Audit Generator image

View file

@ -9,7 +9,7 @@ visibility surface.
from datetime import date
from typing import Dict, List, Literal, Optional, Tuple
from pydantic import BaseModel, ConfigDict, Field, model_validator
from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator
AbriFlow = Literal["amend_job", "log_job", "sync_tenant_data", "abandon_job"]
@ -19,8 +19,13 @@ AbriFlow = Literal["amend_job", "log_job", "sync_tenant_data", "abandon_job"]
# alone would reject valid messages; the orchestrator fails the flow if a deal
# carries neither. Its outcome is optional too, only feeding the reason seam.
_FIELDS_BY_FLOW: Dict[AbriFlow, Tuple[str, ...]] = {
"amend_job": ("confirmed_survey_date",),
"log_job": ("place_ref", "deal_name", "confirmed_survey_date"),
"amend_job": ("confirmed_survey_date", "third_party_surveyor_identifier"),
"log_job": (
"place_ref",
"deal_name",
"confirmed_survey_date",
"third_party_surveyor_identifier",
),
"sync_tenant_data": ("place_ref",),
"abandon_job": (),
}
@ -37,10 +42,21 @@ class AbriTriggerRequest(BaseModel):
confirmed_survey_time: Optional[str] = None
last_submission_date: Optional[date] = None
outcome: Optional[str] = None
# OpenHousing's bookable resource for the survey; optional because the
# Abri client falls back to the configured default surveyor when absent.
# OpenHousing's bookable resource for the survey; required by the log
# and amend flows. A blank resource does not error at the relay —
# OpenHousing silently drops the appointment — so blank is treated as
# missing and fails the per-flow validation instead.
third_party_surveyor_identifier: Optional[str] = None
@field_validator("third_party_surveyor_identifier")
@classmethod
def _blank_surveyor_identifier_is_missing(
cls, value: Optional[str]
) -> Optional[str]:
if value is not None and not value.strip():
return None
return value
@model_validator(mode="after")
def _each_fired_flow_has_its_fields(self) -> "AbriTriggerRequest":
missing = [

View file

@ -81,8 +81,13 @@ def dispatch_abri_flows(
def _run_amend(request: AbriTriggerRequest, flows: AbriFlows) -> str:
if request.confirmed_survey_date is None:
raise ValueError("amend_job fired without a confirmed_survey_date")
if (
request.confirmed_survey_date is None
or request.third_party_surveyor_identifier is None
):
raise ValueError(
"amend_job fired without a confirmed_survey_date/surveyor identifier"
)
outcome = flows.amend_job(
AppointmentChange(
deal_id=request.hubspot_deal_id,
@ -112,8 +117,12 @@ def _run_log(
request.place_ref is None
or request.deal_name is None
or request.confirmed_survey_date is None
or request.third_party_surveyor_identifier is None
):
raise ValueError("log_job fired without place_ref/deal_name/survey date")
raise ValueError(
"log_job fired without place_ref/deal_name/survey date/"
"surveyor identifier"
)
try:
outcome = flows.log_job(
ConfirmedSurveyBooking(

View file

@ -24,10 +24,9 @@ module "lambda" {
# Abri relay credentials are per-stage: the dev/staging stage points at
# Abri's UAT relay endpoint; production values are pending from Abri.
ABRI_RELAY_URL = var.abri_relay_url
ABRI_RELAY_USERNAME = var.abri_relay_username
ABRI_RELAY_PASSWORD = var.abri_relay_password
ABRI_RELAY_DEFAULT_RESOURCE = var.abri_relay_default_resource
ABRI_RELAY_URL = var.abri_relay_url
ABRI_RELAY_USERNAME = var.abri_relay_username
ABRI_RELAY_PASSWORD = var.abri_relay_password
HUBSPOT_API_KEY = var.hubspot_api_key

View file

@ -58,11 +58,6 @@ variable "abri_relay_password" {
sensitive = true
}
variable "abri_relay_default_resource" {
type = string
sensitive = true
}
variable "hubspot_api_key" {
type = string
sensitive = true

View file

@ -44,7 +44,10 @@ class LogJobRequest:
appointment_time: SlotCode
short_description: str
long_description: str
resource: Optional[str] = None
# The bookable OpenHousing resource. Mandatory: a missing or blank
# resource does not error at the relay — OpenHousing logs the job but
# silently drops the appointment.
resource: str
@dataclass(frozen=True)
@ -68,7 +71,10 @@ class AmendJobRequest:
job_no: str
appointment_date: date
appointment_time: SlotCode
resource: Optional[str] = None
# The bookable OpenHousing resource. Mandatory: a missing or blank
# resource does not error at the relay — OpenHousing silently drops
# the appointment.
resource: str
@dataclass(frozen=True)

View file

@ -56,14 +56,6 @@ class AbriClient:
self._session = requests.Session()
def log_job(self, request: LogJobRequest) -> LogJobResult:
# OpenHousing needs a bookable resource, so fall back to the
# configured surveyor when the request carries no explicit override
# (mirrors amend_job).
resource = (
request.resource
if request.resource is not None
else self._config.default_resource
)
outcome = self._exchange(
request_type="logjob",
parameters=[
@ -75,7 +67,7 @@ class AbriClient:
("client_ref", request.client_ref),
("appointment_date", _format_appointment_date(request.appointment_date)),
("appointment_time", request.appointment_time),
("resource", resource),
("resource", request.resource),
("resource_group", RESOURCE_GROUP),
],
)
@ -86,20 +78,12 @@ class AbriClient:
return self._parse_job_logged(outcome)
def amend_job(self, request: AmendJobRequest) -> AmendJobResult:
# OpenHousing silently drops the appointment unless a bookable resource
# is sent, so fall back to the configured surveyor when the request
# carries no explicit override (mirrors log_job).
resource = (
request.resource
if request.resource is not None
else self._config.default_resource
)
parameters = [
("job_no", request.job_no),
("action", "amend"),
("appointment_date", _format_appointment_date(request.appointment_date)),
("appointment_time", request.appointment_time),
("resource", resource),
("resource", request.resource),
]
outcome = self._exchange(

View file

@ -7,20 +7,11 @@ class AbriConfig:
endpoint_url: str
username: str
password: str
default_resource: str
@classmethod
def from_env(cls, env: Mapping[str, str]) -> "AbriConfig":
# A blank default_resource does not error at the relay — OpenHousing
# logs the job but silently drops the appointment — so reject it here
# rather than let the misconfiguration ship an empty resource.
default_resource = env["ABRI_RELAY_DEFAULT_RESOURCE"]
if not default_resource.strip():
raise ValueError("ABRI_RELAY_DEFAULT_RESOURCE must not be blank")
return cls(
endpoint_url=env["ABRI_RELAY_URL"],
username=env["ABRI_RELAY_USERNAME"],
password=env["ABRI_RELAY_PASSWORD"],
default_resource=default_resource,
)

View file

@ -114,7 +114,7 @@ class ConfirmedSurveyBooking:
confirmed_survey_time: Optional[str]
# OpenHousing calls it the resource; the HubSpot deal property (and the
# database column) call it the third-party surveyor identifier.
third_party_surveyor_identifier: Optional[str] = None
third_party_surveyor_identifier: str
@dataclass(frozen=True)
@ -143,7 +143,7 @@ class AppointmentChange:
confirmed_survey_time: Optional[str]
# OpenHousing calls it the resource; the HubSpot deal property (and the
# database column) call it the third-party surveyor identifier.
third_party_surveyor_identifier: Optional[str] = None
third_party_surveyor_identifier: str
AmendJobOrchestrationResult = Union[AppointmentAmended, AbriRequestRejected]

View file

@ -13,7 +13,7 @@ production database.
Usage:
1. Ensure these are set in the environment or backend/.env:
HUBSPOT_API_KEY, ABRI_RELAY_URL (the UAT endpoint),
ABRI_RELAY_USERNAME, ABRI_RELAY_PASSWORD, ABRI_RELAY_DEFAULT_RESOURCE.
ABRI_RELAY_USERNAME, ABRI_RELAY_PASSWORD.
2. Create a throwaway test deal in the HubSpot UI and paste its id into
DEAL_ID below; paste Abri's agreed UAT test place reference into
PLACE_REF.
@ -46,6 +46,9 @@ from orchestration.abri_orchestrator import (
DEAL_ID = "EDIT-ME"
PLACE_REF = PlaceRef("EDIT-ME")
# The bookable OpenHousing resource to book the UAT job against (mandatory:
# in production it comes from the deal's third_party_surveyor_identifier).
SURVEYOR = "EDIT-ME"
# Log for the first weekday at least a week out (a slot Abri UAT will take),
# then amend to the following day's opposite slot.
@ -73,7 +76,6 @@ def _uat_abri_client() -> AbriClient:
endpoint_url=_env("ABRI_RELAY_URL"),
username=_env("ABRI_RELAY_USERNAME"),
password=_env("ABRI_RELAY_PASSWORD"),
default_resource=_env("ABRI_RELAY_DEFAULT_RESOURCE"),
)
)
@ -94,8 +96,8 @@ class InMemoryDealDatabase:
def main() -> None:
if DEAL_ID == "EDIT-ME" or PLACE_REF == PlaceRef("EDIT-ME"):
raise SystemExit("Edit DEAL_ID and PLACE_REF before running")
if "EDIT-ME" in (DEAL_ID, str(PLACE_REF), SURVEYOR):
raise SystemExit("Edit DEAL_ID, PLACE_REF and SURVEYOR before running")
sdk_client = _hubspot_sdk_client()
deal_database = InMemoryDealDatabase()
@ -114,6 +116,7 @@ def main() -> None:
deal_name=f"Domna UAT smoke test {date.today().isoformat()}",
confirmed_survey_date=LOG_DATE,
confirmed_survey_time=LOG_TIME,
third_party_surveyor_identifier=SURVEYOR,
)
)
if isinstance(log_result, AbriRequestRejected):
@ -128,6 +131,7 @@ def main() -> None:
deal_id=DEAL_ID,
confirmed_survey_date=AMEND_DATE,
confirmed_survey_time=AMEND_TIME,
third_party_surveyor_identifier=SURVEYOR,
)
)
if isinstance(amend_result, AbriRequestRejected):

View file

@ -229,7 +229,6 @@ def main() -> None:
endpoint_url="https://stubbed.invalid/relay",
username="smoke-test",
password="",
default_resource="SMOKE",
)
)
abri_client._session = cast( # pyright: ignore[reportPrivateUsage]

View file

@ -91,7 +91,6 @@ def _stubbed_abri_client() -> AbriClient:
endpoint_url="https://stubbed.invalid/relay",
username="smoke-test",
password="",
default_resource="",
)
)
client._session = cast(

View file

@ -15,6 +15,7 @@ def _full_message() -> Dict[str, Any]:
"deal_name": "49 Admers Crescent",
"confirmed_survey_date": "2026-06-24",
"confirmed_survey_time": "14:30",
"third_party_surveyor_identifier": "THIRDPARTY",
}
@ -29,6 +30,7 @@ def test_a_full_message_parses_into_a_typed_request() -> None:
assert request.deal_name == "49 Admers Crescent"
assert request.confirmed_survey_date == date(2026, 6, 24)
assert request.confirmed_survey_time == "14:30"
assert request.third_party_surveyor_identifier == "THIRDPARTY"
@pytest.mark.parametrize(
@ -37,7 +39,9 @@ def test_a_full_message_parses_into_a_typed_request() -> None:
("log_job", "place_ref"),
("log_job", "deal_name"),
("log_job", "confirmed_survey_date"),
("log_job", "third_party_surveyor_identifier"),
("amend_job", "confirmed_survey_date"),
("amend_job", "third_party_surveyor_identifier"),
("sync_tenant_data", "place_ref"),
],
)
@ -97,25 +101,18 @@ def test_a_message_naming_abandon_job_validates_with_only_the_deal_id() -> None:
assert request.flows == ["abandon_job"]
def test_a_message_carries_the_third_party_surveyor_identifier() -> None:
# Arrange
@pytest.mark.parametrize("blank", ["", " "])
def test_a_blank_surveyor_identifier_is_treated_as_missing(blank: str) -> None:
# A blank resource does not error at the relay: OpenHousing logs the job
# but silently drops the appointment. Treat blank as missing so the
# per-flow validation fails loudly instead.
message = _full_message()
message["third_party_surveyor_identifier"] = "THIRDPARTY"
message["flows"] = ["log_job"]
message["third_party_surveyor_identifier"] = blank
# Act
request = AbriTriggerRequest.model_validate(message)
# Assert
assert request.third_party_surveyor_identifier == "THIRDPARTY"
def test_a_message_without_a_surveyor_identifier_parses_with_none() -> None:
# Arrange: optional because the Abri client falls back to the configured
# default surveyor when the deal names no third-party resource.
request = AbriTriggerRequest.model_validate(_full_message())
# Assert
assert request.third_party_surveyor_identifier is None
# Act / Assert
with pytest.raises(ValidationError, match="third_party_surveyor_identifier"):
AbriTriggerRequest.model_validate(message)
def test_a_message_carries_the_outcome_for_the_abandonment_reason_mapping() -> None:

View file

@ -28,7 +28,6 @@ CONFIG = AbriConfig(
endpoint_url=ENDPOINT_URL,
username="DomnaWeb",
password="",
default_resource="NAULKH",
)
@ -52,6 +51,7 @@ def _spec_example_request() -> LogJobRequest:
appointment_time="PM",
short_description="Christian's SCS External Test.",
long_description="Christian's SCS External Test.",
resource="NAULKH",
)
@ -60,6 +60,7 @@ def _spec_example_amend_request() -> AmendJobRequest:
job_no="AC0439951",
appointment_date=date(2026, 6, 19),
appointment_time="PM",
resource="NAULKH",
)
@ -145,11 +146,11 @@ def test_log_job_sends_the_recorded_logjob_envelope_to_the_relay_endpoint(
)
def test_log_job_sends_the_requested_resource_instead_of_the_configured_default(
def test_log_job_books_the_job_against_the_requests_resource(
client: AbriClient, mock_session: MagicMock
) -> None:
# Arrange: a deal carrying a third-party surveyor identifier books the
# job against that resource, not the configured default surveyor.
# Arrange: the resource is mandatory on the request — the deal's
# third-party surveyor identifier — and is sent verbatim.
mock_session.post.return_value.content = _load_fixture(
"abri_relay_logjob_success_response.xml"
)
@ -177,29 +178,6 @@ def test_log_job_sends_the_requested_resource_instead_of_the_configured_default(
assert resources == ["THIRDPARTY"]
def test_log_job_defaults_the_resource_to_the_configured_surveyor(
client: AbriClient, mock_session: MagicMock
) -> None:
# Arrange: OpenHousing needs a bookable resource, so a log without an
# explicit surveyor falls back to the configured default rather than
# omitting the parameter.
mock_session.post.return_value.content = _load_fixture(
"abri_relay_logjob_success_response.xml"
)
# Act
client.log_job(_spec_example_request())
# Assert
sent_body: bytes = mock_session.post.call_args.kwargs["data"]
resources = [
parameter.get("attribute_value")
for parameter in ET.fromstring(sent_body).findall(".//Parameters")
if parameter.get("attribute") == "resource"
]
assert resources == [CONFIG.default_resource]
def test_log_job_declares_the_xml_body_content_type_the_relay_requires(
client: AbriClient, mock_session: MagicMock
) -> None:
@ -465,29 +443,6 @@ def test_amend_job_sends_the_recorded_amendoptiappt_envelope_to_the_relay_endpoi
)
def test_amend_job_defaults_the_resource_to_the_configured_surveyor(
client: AbriClient, mock_session: MagicMock
) -> None:
# Arrange: OpenHousing silently drops the appointment unless a bookable
# resource is sent, so an amend without an explicit surveyor falls back to
# the configured default rather than omitting the parameter.
mock_session.post.return_value.content = _load_fixture(
"abri_relay_amendoptiappt_success_response.xml"
)
# Act
client.amend_job(_spec_example_amend_request())
# Assert
sent_body: bytes = mock_session.post.call_args.kwargs["data"]
resources = [
parameter.get("attribute_value")
for parameter in ET.fromstring(sent_body).findall(".//Parameters")
if parameter.get("attribute") == "resource"
]
assert resources == [CONFIG.default_resource]
# --- amend_job: rejection ---

View file

@ -1,15 +1,12 @@
import pytest
from infrastructure.abri.config import AbriConfig
def test_config_hydrates_relay_credentials_and_defaults_from_environment() -> None:
def test_config_hydrates_relay_credentials_from_environment() -> None:
# Arrange
env = {
"ABRI_RELAY_URL": "https://relay.example.test/api/DomnaRelay?code=key",
"ABRI_RELAY_USERNAME": "DomnaWeb",
"ABRI_RELAY_PASSWORD": "secret",
"ABRI_RELAY_DEFAULT_RESOURCE": "CBRYAN",
}
# Act
@ -20,20 +17,4 @@ def test_config_hydrates_relay_credentials_and_defaults_from_environment() -> No
endpoint_url="https://relay.example.test/api/DomnaRelay?code=key",
username="DomnaWeb",
password="secret",
default_resource="CBRYAN",
)
@pytest.mark.parametrize("blank", ["", " "])
def test_config_rejects_a_blank_default_resource(blank: str) -> None:
# A blank resource does not error at the relay: OpenHousing logs the job
# but silently drops the appointment. Fail loudly at config load instead.
env = {
"ABRI_RELAY_URL": "https://relay.example.test/api/DomnaRelay?code=key",
"ABRI_RELAY_USERNAME": "DomnaWeb",
"ABRI_RELAY_PASSWORD": "secret",
"ABRI_RELAY_DEFAULT_RESOURCE": blank,
}
with pytest.raises(ValueError, match="ABRI_RELAY_DEFAULT_RESOURCE"):
AbriConfig.from_env(env)

View file

@ -29,7 +29,6 @@ CONFIG = AbriConfig(
endpoint_url=ENDPOINT_URL,
username="DomnaWeb",
password="",
default_resource="NAULKH",
)
ABANDONMENT = DealAbandonment(

View file

@ -27,13 +27,13 @@ CONFIG = AbriConfig(
endpoint_url=ENDPOINT_URL,
username="DomnaWeb",
password="",
default_resource="NAULKH",
)
CHANGE = AppointmentChange(
deal_id=DEAL_ID,
confirmed_survey_date=date(2026, 6, 24),
confirmed_survey_time="14:30",
third_party_surveyor_identifier="NAULKH",
)
@ -115,9 +115,9 @@ def test_amend_job_sends_an_amendoptiappt_envelope_for_the_deals_recorded_job(
"action": "amend",
"appointment_date": "24/06/2026",
"appointment_time": "PM",
# OpenHousing drops the appointment without a bookable resource, so the
# amend defaults to the configured surveyor when none is overridden.
"resource": CONFIG.default_resource,
# OpenHousing drops the appointment without a bookable resource, so
# the deal's third-party surveyor identifier is always sent.
"resource": "NAULKH",
}

View file

@ -33,7 +33,6 @@ CONFIG = AbriConfig(
endpoint_url=ENDPOINT_URL,
username="DomnaWeb",
password="",
default_resource="NAULKH",
)
BOOKING = ConfirmedSurveyBooking(
@ -42,6 +41,7 @@ BOOKING = ConfirmedSurveyBooking(
deal_name="49 Admers Crescent",
confirmed_survey_date=date(2026, 6, 18),
confirmed_survey_time="14:30",
third_party_surveyor_identifier="NAULKH",
)

View file

@ -32,7 +32,6 @@ CONFIG = AbriConfig(
endpoint_url=ENDPOINT_URL,
username="DomnaWeb",
password="",
default_resource="NAULKH",
)