From 6e9f52a5406aca97f187ac703e207240c5e90029 Mon Sep 17 00:00:00 2001
From: Daniel Roth <daniel_roth@hotmail.co.uk>
Date: Fri, 24 Jul 2026 15:50:00 +0000
Subject: [PATCH] =?UTF-8?q?Log=20an=20Abri=20job=20only=20when=20both=20su?=
 =?UTF-8?q?rvey=20date=20and=20surveyor=20complete=20the=20pair=20?=
 =?UTF-8?q?=F0=9F=9F=A9?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 etl/hubspot/hubspot_deal_differ.py           | 49 +++++++++++++++-----
 etl/hubspot/tests/test_abri_flow_triggers.py | 15 ++++--
 2 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/etl/hubspot/hubspot_deal_differ.py b/etl/hubspot/hubspot_deal_differ.py
index bb1e58674..96311ae4a 100644
--- a/etl/hubspot/hubspot_deal_differ.py
+++ b/etl/hubspot/hubspot_deal_differ.py
@@ -295,19 +295,37 @@ class HubspotDealDiffer:
         if not HubspotDealDiffer._is_abri_condition_deal(new_deal, new_project):
             return False
 
-        new_survey_date = parse_hs_date(new_deal.get("confirmed_survey_date"))
-        logger.info(
-            "Abri job-logging check: old_confirmed_survey_date=%s "
-            "new_confirmed_survey_date=%s",
-            old_deal.confirmed_survey_date,
-            new_survey_date,
-        )
-
-        # Only a first-time survey date (previously empty) logs a new job.
-        if old_deal.confirmed_survey_date is not None:
+        # A deal that already carries a Job Number is logged; never log it
+        # again (that would duplicate the job in OpenHousing).
+        if HubspotDealDiffer._client_booking_reference(new_deal) is not None:
             return False
 
-        return new_survey_date is not None
+        new_survey_date = parse_hs_date(new_deal.get("confirmed_survey_date"))
+        new_surveyor = HubspotDealDiffer._normalised_surveyor(
+            new_deal.get("third_party_surveyor_identifier")
+        )
+        old_surveyor = HubspotDealDiffer._normalised_surveyor(
+            old_deal.third_party_surveyor_identifier
+        )
+        pair_complete_now = new_survey_date is not None and new_surveyor is not None
+        pair_complete_before = (
+            old_deal.confirmed_survey_date is not None and old_surveyor is not None
+        )
+        logger.info(
+            "Abri job-logging check: old_confirmed_survey_date=%s "
+            "new_confirmed_survey_date=%s old_surveyor=%s new_surveyor=%s "
+            "pair_complete_before=%s pair_complete_now=%s",
+            old_deal.confirmed_survey_date,
+            new_survey_date,
+            old_surveyor,
+            new_surveyor,
+            pair_complete_before,
+            pair_complete_now,
+        )
+
+        # Fire exactly once, on the scrape that completes the pair — whichever
+        # of the survey date or surveyor lands second.
+        return pair_complete_now and not pair_complete_before
 
     @staticmethod
     def check_for_abri_job_amendment(
@@ -428,6 +446,15 @@ class HubspotDealDiffer:
             return None
         return value
 
+    @staticmethod
+    def _client_booking_reference(new_deal: Dict[str, str]) -> Optional[str]:
+        # The Job Number OpenHousing returns on a successful log; a blank value
+        # carries no Job Number, so blank and missing both read as "no job".
+        value = new_deal.get("client_booking_reference")
+        if value is None or not value.strip():
+            return None
+        return value
+
     @staticmethod
     def _is_abandoned(
         number_of_attempts: Optional[str], outcome: Optional[str]
diff --git a/etl/hubspot/tests/test_abri_flow_triggers.py b/etl/hubspot/tests/test_abri_flow_triggers.py
index 72fe6a1b8..d7a3adfd2 100644
--- a/etl/hubspot/tests/test_abri_flow_triggers.py
+++ b/etl/hubspot/tests/test_abri_flow_triggers.py
@@ -64,9 +64,10 @@ def test_a_surveyor_landing_after_the_date_completes_the_pair_and_logs() -> None
         old_deal=old_deal,
     )
 
-    # Assert
+    # Assert: the completing scrape logs (amend suppression is asserted
+    # separately); the point here is that a late surveyor logs, not amends.
     assert message is not None
-    assert message["flows"] == ["log_job"]
+    assert "log_job" in message["flows"]
 
 
 def test_a_date_landing_after_the_surveyor_completes_the_pair_and_logs() -> None:
@@ -461,9 +462,15 @@ def test_the_test_override_env_var_fires_a_deal_on_its_own_project_code(
     # Arrange: a sandbox deal on its own project code, with the test override
     # pointing the gate at that code so it fires like a production Abri deal.
     monkeypatch.setenv("TEST_ABRI_PROJECT_CODE_OVERRIDE", "TEST-SANDBOX-CODE")
-    old_deal = make_old_deal(project_code="TEST-SANDBOX-CODE", confirmed_survey_date=None)
+    old_deal = make_old_deal(
+        project_code="TEST-SANDBOX-CODE",
+        confirmed_survey_date=None,
+        third_party_surveyor_identifier=None,
+    )
     new_deal = make_new_deal(
-        project_code="TEST-SANDBOX-CODE", confirmed_survey_date="2026-06-24"
+        project_code="TEST-SANDBOX-CODE",
+        confirmed_survey_date="2026-06-24",
+        third_party_surveyor_identifier="THIRDPARTY",
     )
 
     # Act