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