Merge pull request #1534 from Hestia-Homes/feature/abri-api-integration

Abri: always send a bookable resource on log/amend (+ survey-time labels)

domain/abri/models.py

@@ -49,7 +49,8 @@ class LogJobRequest:
 @dataclass(frozen=True)
 class JobLogged:
     job_no: str
-    logged_info: str
+    # Informational only (never read downstream); absent on some successes.
+    logged_info: Optional[str]
 
 
 @dataclass(frozen=True)

domain/abri/phone_selection.py

@@ -22,9 +22,9 @@ class SelectedPhoneNumbers:
     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).
+# Abri confirmed the lowest numeric priority marks the preferred number;
+# 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:

domain/abri/slots.py

@@ -5,9 +5,16 @@ from domain.abri.models import SlotCode
 
 _MIDDAY = time(12, 0)
 
+# HubSpot's confirmed-time field carries these half-day labels, not clock times.
+_LABEL_SLOTS: dict[str, SlotCode] = {"morning": "AM", "afternoon": "PM"}
+
 
 def slot_for_confirmed_time(confirmed_time: Optional[str]) -> SlotCode:
     if confirmed_time is None or confirmed_time.strip() == "":
         return "AD"
-    parsed = datetime.strptime(confirmed_time.strip(), "%H:%M").time()
+    normalised = confirmed_time.strip()
+    label_slot = _LABEL_SLOTS.get(normalised.lower())
+    if label_slot is not None:
+        return label_slot
+    parsed = datetime.strptime(normalised, "%H:%M").time()
     return "AM" if parsed < _MIDDAY else "PM"

infrastructure/abri/abri_client.py

@@ -78,14 +78,21 @@ 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),
         ]
-        if request.resource is not None:
-            parameters.append(("resource", request.resource))
 
         outcome = self._exchange(
             request_type="amendoptiappt",
@@ -174,14 +181,13 @@ class AbriClient:
             )
 
         job_no = job_logged.get("job_no")
-        logged_info = job_logged.get("logged_info")
 
-        if job_no is None or logged_info is None:
-            raise AbriResponseParseError(
-                "Job_logged element missing job_no or logged_info"
-            )
+        if job_no is None:
+            raise AbriResponseParseError("Job_logged element missing job_no")
 
-        return JobLogged(job_no=job_no, logged_info=logged_info)
+        # logged_info is purely informational and absent on some successes, so
+        # its presence is not required to accept the job as logged.
+        return JobLogged(job_no=job_no, logged_info=job_logged.get("logged_info"))
 
     @staticmethod
     def _parse_appointment_amended(root: ET.Element) -> AppointmentAmended:

infrastructure/abri/config.py

@@ -11,9 +11,16 @@ class AbriConfig:
 
     @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=env["ABRI_RELAY_DEFAULT_RESOURCE"],
+            default_resource=default_resource,
         )

tests/domain/abri/test_slots.py

@@ -38,6 +38,26 @@ def test_a_confirmed_time_at_or_after_midday_books_the_afternoon_slot(
     assert slot == "PM"
 
 
+@pytest.mark.parametrize("confirmed_time", ["morning", "Morning", "  MORNING  "])
+def test_hubspots_morning_label_books_the_morning_slot(confirmed_time: str) -> None:
+    # Act
+    slot = slot_for_confirmed_time(confirmed_time)
+
+    # Assert
+    assert slot == "AM"
+
+
+@pytest.mark.parametrize("confirmed_time", ["afternoon", "Afternoon", "  AFTERNOON  "])
+def test_hubspots_afternoon_label_books_the_afternoon_slot(
+    confirmed_time: str,
+) -> None:
+    # Act
+    slot = slot_for_confirmed_time(confirmed_time)
+
+    # Assert
+    assert slot == "PM"
+
+
 @pytest.mark.parametrize("confirmed_time", ["half nine", "25:00", "9.30am"])
 def test_an_unparseable_confirmed_time_fails_loudly_rather_than_booking_all_day(
     confirmed_time: str,

tests/infrastructure/abri/test_abri_client.py

@@ -106,6 +106,22 @@ def test_log_job_returns_job_logged_with_openhousing_job_number(
     )
 
 
+def test_log_job_accepts_a_success_without_logged_info(
+    client: AbriClient, mock_session: MagicMock
+) -> None:
+    # Arrange: logged_info is informational, so a success carrying only a
+    # job_no must still be accepted rather than fail a job that was logged.
+    mock_session.post.return_value.content = (
+        b'<Root><Jobs><Job_logged job_no="AD1" /></Jobs></Root>'
+    )
+
+    # Act
+    result = client.log_job(_spec_example_request())
+
+    # Assert
+    assert result == JobLogged(job_no="AD1", logged_info=None)
+
+
 def test_log_job_sends_the_recorded_logjob_envelope_to_the_relay_endpoint(
     client: AbriClient, mock_session: MagicMock
 ) -> None:
@@ -394,10 +410,12 @@ def test_amend_job_sends_the_recorded_amendoptiappt_envelope_to_the_relay_endpoi
     )
 
 
-def test_amend_job_omits_the_resource_parameter_when_no_surveyor_is_provided(
+def test_amend_job_defaults_the_resource_to_the_configured_surveyor(
     client: AbriClient, mock_session: MagicMock
 ) -> None:
-    # Arrange
+    # 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"
     )
@@ -407,11 +425,12 @@ def test_amend_job_omits_the_resource_parameter_when_no_surveyor_is_provided(
 
     # Assert
     sent_body: bytes = mock_session.post.call_args.kwargs["data"]
-    sent_attributes = [
-        parameter.get("attribute")
+    resources = [
+        parameter.get("attribute_value")
         for parameter in ET.fromstring(sent_body).findall(".//Parameters")
+        if parameter.get("attribute") == "resource"
     ]
-    assert "resource" not in sent_attributes
+    assert resources == [CONFIG.default_resource]
 
 
 # --- amend_job: rejection ---

tests/infrastructure/abri/test_abri_config.py

@@ -1,3 +1,5 @@
+import pytest
+
 from infrastructure.abri.config import AbriConfig
 
 
@@ -20,3 +22,18 @@ def test_config_hydrates_relay_credentials_and_defaults_from_environment() -> No
         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)

tests/orchestration/test_abri_orchestrator_amend_job.py

@@ -114,6 +114,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,
     }