diff --git a/etl/hubspot/hubspot_deal_differ.py b/etl/hubspot/hubspot_deal_differ.py index 957955031..e50e07dc8 100644 --- a/etl/hubspot/hubspot_deal_differ.py +++ b/etl/hubspot/hubspot_deal_differ.py @@ -455,21 +455,24 @@ class HubspotDealDiffer: return new_outcome == "surveyed" and old_outcome != "surveyed" @staticmethod - def _normalised_surveyor(value: Optional[str]) -> Optional[str]: - # HubSpot sends "" for an unset property and the DB stores whatever - # the scrape wrote, so blank and missing must compare as equal. + def _blank_to_none(value: Optional[str]) -> Optional[str]: + # HubSpot sends "" for an unset property and the DB stores whatever the + # scrape wrote, so blank and missing must always compare as equal. if value is None or not value.strip(): return None return value + @staticmethod + def _normalised_surveyor(value: Optional[str]) -> Optional[str]: + return HubspotDealDiffer._blank_to_none(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 + return HubspotDealDiffer._blank_to_none( + new_deal.get("client_booking_reference") + ) @staticmethod def _is_abandoned( diff --git a/etl/hubspot/tests/test_hubspot_deal_differ.py b/etl/hubspot/tests/test_hubspot_deal_differ.py index 9a9e8f05e..e851b206b 100644 --- a/etl/hubspot/tests/test_hubspot_deal_differ.py +++ b/etl/hubspot/tests/test_hubspot_deal_differ.py @@ -353,19 +353,21 @@ def test_magicplan_trigger__outcome_surveyed_uppercase__returns_true() -> None: # ============================== -def test_abri_job_logging__date_first_set_on_abri_deal__returns_true() -> None: +def test_abri_job_logging__pair_completed_on_abri_deal__returns_true() -> None: deal_id = uuid.uuid4() - # Arrange + # Arrange: the surveyor lands to complete the date+surveyor pair. old_deal = make_old_deal( id=deal_id, project_code=ABRI_PROJECT_CODE, confirmed_survey_date=None, + third_party_surveyor_identifier=None, ) new_deal = make_new_deal( deal_id, project_code=ABRI_PROJECT_CODE, confirmed_survey_date="2026-07-15", + third_party_surveyor_identifier="THIRDPARTY", ) # Act @@ -439,11 +441,13 @@ def test_abri_job_logging__project_code_cased_differently__returns_true() -> Non id=deal_id, project_code=ABRI_PROJECT_CODE.upper(), confirmed_survey_date=None, + third_party_surveyor_identifier=None, ) new_deal = make_new_deal( deal_id, project_code=ABRI_PROJECT_CODE.upper(), confirmed_survey_date="2026-07-15", + third_party_surveyor_identifier="THIRDPARTY", ) # Act @@ -468,11 +472,13 @@ def test_abri_job_logging__real_portal_project_code__returns_true() -> None: id=deal_id, project_code="[Abri Stock Condition - Privately Funded - 062026 - 1247536318670]", confirmed_survey_date=None, + third_party_surveyor_identifier=None, ) new_deal = make_new_deal( deal_id, project_code="[Abri Stock Condition - Privately Funded - 062026 - 1247536318670]", confirmed_survey_date="2026-07-15", + third_party_surveyor_identifier="THIRDPARTY", ) # Act @@ -530,11 +536,13 @@ def test_abri_job_logging__associated_abri_project_id__returns_true() -> None: id=deal_id, project_code="Southern Retrofit", confirmed_survey_date=None, + third_party_surveyor_identifier=None, ) new_deal = make_new_deal( deal_id, project_code="Southern Retrofit", confirmed_survey_date="2026-07-15", + third_party_surveyor_identifier="THIRDPARTY", ) new_project = ProjectData( project_id=HubspotDealDiffer.ABRI_CONDITION_ASSOCIATED_PROJECT_ID, @@ -581,6 +589,39 @@ def test_abri_job_logging__non_abri_project_id_with_abri_code__returns_false() - assert result is False +def test_abri_job_logging__deal_already_has_client_booking_reference__returns_false() -> ( + None +): + deal_id = uuid.uuid4() + + # Arrange: the pair completes this scrape, but the deal already carries a + # Job Number, so the job exists and must not be logged again. + old_deal = make_old_deal( + id=deal_id, + project_code=ABRI_PROJECT_CODE, + confirmed_survey_date=None, + third_party_surveyor_identifier=None, + client_booking_reference="AD0226519", + ) + new_deal = make_new_deal( + deal_id, + project_code=ABRI_PROJECT_CODE, + confirmed_survey_date="2026-07-15", + third_party_surveyor_identifier="THIRDPARTY", + client_booking_reference="AD0226519", + ) + + # Act + result = HubspotDealDiffer.check_for_abri_job_logging( + new_deal=new_deal, + new_project=None, + old_deal=old_deal, + ) + + # Assert + assert result is False + + # ================================ # ABRI JOB AMENDMENT TRIGGER TESTS # ================================ @@ -589,16 +630,18 @@ def test_abri_job_logging__non_abri_project_id_with_abri_code__returns_false() - def test_abri_job_amendment__date_changed_on_abri_deal__returns_true() -> None: deal_id = uuid.uuid4() - # Arrange + # Arrange: a complete booking (date + surveyor) whose date moves. old_deal = make_old_deal( id=deal_id, project_code=ABRI_PROJECT_CODE, confirmed_survey_date=datetime(2026, 7, 1, tzinfo=timezone.utc), + third_party_surveyor_identifier="THIRDPARTY", ) new_deal = make_new_deal( deal_id, project_code=ABRI_PROJECT_CODE, confirmed_survey_date="2026-07-15", + third_party_surveyor_identifier="THIRDPARTY", ) # Act @@ -669,18 +712,20 @@ def test_abri_job_amendment__date_and_time_unchanged__returns_false() -> None: def test_abri_job_amendment__time_changed_on_abri_deal__returns_true() -> None: deal_id = uuid.uuid4() - # Arrange + # Arrange: a complete booking (date + surveyor) whose time moves. old_deal = make_old_deal( id=deal_id, project_code=ABRI_PROJECT_CODE, confirmed_survey_date=datetime(2026, 7, 15, tzinfo=timezone.utc), confirmed_survey_time="09:30", + third_party_surveyor_identifier="THIRDPARTY", ) new_deal = make_new_deal( deal_id, project_code=ABRI_PROJECT_CODE, confirmed_survey_date="2026-07-15", confirmed_survey_time="14:00", + third_party_surveyor_identifier="THIRDPARTY", ) # Act