From 8ce76190442e5a7f8a9c1af038651521b7edb105 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Thu, 9 Apr 2026 09:14:20 +0000 Subject: [PATCH] refactor pashub trigger tests --- .../tests/test_hubspot_deal_differ.py | 470 ++++++++---------- etl/hubspot/hubspotDataTodB.py | 72 ++- 2 files changed, 233 insertions(+), 309 deletions(-) diff --git a/backend/hubspot_trigger_orchestrator/tests/test_hubspot_deal_differ.py b/backend/hubspot_trigger_orchestrator/tests/test_hubspot_deal_differ.py index ba6b80e4..75fa7927 100644 --- a/backend/hubspot_trigger_orchestrator/tests/test_hubspot_deal_differ.py +++ b/backend/hubspot_trigger_orchestrator/tests/test_hubspot_deal_differ.py @@ -1,359 +1,295 @@ from datetime import datetime -from typing import Dict +from typing import Any, Dict import uuid +import pytest + from backend.app.db.models.organisation import HubspotDealData from backend.hubspot_trigger_orchestrator.hubspot_deal_differ import HubspotDealDiffer -def test_pashub_trigger__outcome_note_added__returns_false() -> None: - # arrange - deal_id = uuid.uuid4() +BASE_TIME = datetime(2025, 12, 1, 12, 0, 0) - old_deal = HubspotDealData( - id=deal_id, + +def make_old_deal(**overrides: Any) -> HubspotDealData: + return HubspotDealData( + id=overrides.get("id", uuid.uuid4()), deal_id="1", - created_at=datetime(2025, 12, 1, 12, 0, 0), - updated_at=datetime(2025, 12, 1, 12, 0, 0), + created_at=BASE_TIME, + updated_at=BASE_TIME, + **{k: v for k, v in overrides.items() if k != "id"}, ) - new_deal: Dict[str, str] = { + + +def make_new_deal(deal_id: uuid.UUID, **overrides: Any) -> Dict[str, str]: + return { "id": str(deal_id), "deal_id": "1", - "outcome_notes": "test note", - "created_at": datetime(2025, 12, 1, 12, 0, 0).isoformat(), + "created_at": BASE_TIME.isoformat(), "updated_at": datetime(2025, 12, 1, 12, 30, 0).isoformat(), + **overrides, } - expected_output = False - # act - actual_output: bool = HubspotDealDiffer.check_for_pashub_trigger( - new_deal=new_deal, old_deal=old_deal +# ------------------------------------- +# Random change we aren't interested in +# ------------------------------------- + + +@pytest.mark.parametrize( + "new_overrides,expected", + [ + ({"outcome_notes": "test note"}, False), + ], +) +def test_pashub_trigger__outcome_note_added__returns_false( + new_overrides: Dict[str, str], + expected: bool, +) -> None: + deal_id = uuid.uuid4() + old_deal = make_old_deal(id=deal_id) + new_deal = make_new_deal(deal_id, **new_overrides) + + assert ( + HubspotDealDiffer.check_for_pashub_trigger( + new_deal=new_deal, + old_deal=old_deal, + ) + == expected ) - # assert - assert actual_output == expected_output + +# ------------------------- +# Pashub link changes +# ------------------------- -def test_pashub_trigger__pashub_link_changed__returns_true() -> None: - # arrange +@pytest.mark.parametrize( + "old_overrides,new_overrides,expected", + [ + ( + {"pashub_link": "www.google.co.uk"}, + {"pashub_link": "www.bbc.co.uk"}, + True, + ), + ], +) +def test_pashub_trigger__pashub_link_changed__returns_true( + old_overrides: Dict[str, str], + new_overrides: Dict[str, str], + expected: bool, +) -> None: + deal_id = uuid.uuid4() + old_deal = make_old_deal(id=deal_id, **old_overrides) + new_deal = make_new_deal(deal_id, **new_overrides) + + assert ( + HubspotDealDiffer.check_for_pashub_trigger( + new_deal=new_deal, + old_deal=old_deal, + ) + == expected + ) + + +# ------------------------- +# Coordination +# ------------------------- + + +@pytest.mark.parametrize( + "coordination_status,expected", + [ + ("v1 ioe/mtp complete", True), + ("v2 ioe/mtp complete", True), + ], +) +def test_pashub_trigger__coordination_completed_and_pashub_link_set__returns_true( + coordination_status: str, + expected: bool, +) -> None: deal_id = uuid.uuid4() - old_deal = HubspotDealData( + old_deal = make_old_deal( id=deal_id, - deal_id="1", - pashub_link="www.google.co.uk", - created_at=datetime(2025, 12, 1, 12, 0, 0), - updated_at=datetime(2025, 12, 1, 12, 0, 0), - ) - new_deal: Dict[str, str] = { - "id": str(deal_id), - "deal_id": "1", - "pashub_link": "www.bbc.co.uk", - "created_at": datetime(2025, 12, 1, 12, 0, 0).isoformat(), - "updated_at": datetime(2025, 12, 1, 12, 30, 0).isoformat(), - } - - expected_output = True - - # act - actual_output: bool = HubspotDealDiffer.check_for_pashub_trigger( - new_deal=new_deal, old_deal=old_deal - ) - - # assert - assert actual_output == expected_output - - -def test_pashub_trigger__coordination_completed_and_pashub_link_set__returns_true() -> ( - None -): - # arrange - deal_id = uuid.uuid4() - - old_deal = HubspotDealData( - id=deal_id, - deal_id="1", pashub_link="www.google.co.uk", coordination_status="random", - created_at=datetime(2025, 12, 1, 12, 0, 0), - updated_at=datetime(2025, 12, 1, 12, 0, 0), - ) - new_deal: Dict[str, str] = { - "id": str(deal_id), - "deal_id": "1", - "coordination_status": "v1 ioe/mtp complete", - "pashub_link": "www.google.co.uk", - "created_at": datetime(2025, 12, 1, 12, 0, 0).isoformat(), - "updated_at": datetime(2025, 12, 1, 12, 30, 0).isoformat(), - } - - expected_output = True - - # act - actual_output: bool = HubspotDealDiffer.check_for_pashub_trigger( - new_deal=new_deal, old_deal=old_deal ) - # assert - assert actual_output == expected_output - - -def test_pashub_trigger__coordination_completed_and_pashub_link_set__returns_true_2() -> ( - None -): - # arrange - deal_id = uuid.uuid4() - - old_deal = HubspotDealData( - id=deal_id, - deal_id="1", + new_deal = make_new_deal( + deal_id, pashub_link="www.google.co.uk", - coordination_status="random", - created_at=datetime(2025, 12, 1, 12, 0, 0), - updated_at=datetime(2025, 12, 1, 12, 0, 0), - ) - new_deal: Dict[str, str] = { - "id": str(deal_id), - "deal_id": "1", - "coordination_status": "v2 ioe/mtp complete", - "pashub_link": "www.google.co.uk", - "created_at": datetime(2025, 12, 1, 12, 0, 0).isoformat(), - "updated_at": datetime(2025, 12, 1, 12, 30, 0).isoformat(), - } - - expected_output = True - - # act - actual_output: bool = HubspotDealDiffer.check_for_pashub_trigger( - new_deal=new_deal, old_deal=old_deal + coordination_status=coordination_status, ) - # assert - assert actual_output == expected_output + assert ( + HubspotDealDiffer.check_for_pashub_trigger( + new_deal=new_deal, + old_deal=old_deal, + ) + == expected + ) def test_pashub_trigger__coordination_completed_and_pashub_link_not_set__returns_false() -> ( None ): - # arrange deal_id = uuid.uuid4() - old_deal = HubspotDealData( + old_deal = make_old_deal( id=deal_id, - deal_id="1", coordination_status="random", - created_at=datetime(2025, 12, 1, 12, 0, 0), - updated_at=datetime(2025, 12, 1, 12, 0, 0), - ) - new_deal: Dict[str, str] = { - "id": str(deal_id), - "deal_id": "1", - "coordination_status": "v2 ioe/mtp complete", - "created_at": datetime(2025, 12, 1, 12, 0, 0).isoformat(), - "updated_at": datetime(2025, 12, 1, 12, 30, 0).isoformat(), - } - - expected_output = False - - # act - actual_output: bool = HubspotDealDiffer.check_for_pashub_trigger( - new_deal=new_deal, old_deal=old_deal ) - # assert - assert actual_output == expected_output + new_deal = make_new_deal( + deal_id, + coordination_status="v2 ioe/mtp complete", + ) + + assert ( + HubspotDealDiffer.check_for_pashub_trigger( + new_deal=new_deal, + old_deal=old_deal, + ) + is False + ) + + +# ------------------------- +# Design +# ------------------------- def test_pashub_trigger__design_completed_and_pashub_link_set__returns_true() -> None: - # arrange deal_id = uuid.uuid4() - old_deal = HubspotDealData( + old_deal = make_old_deal( id=deal_id, - deal_id="1", pashub_link="www.google.co.uk", - created_at=datetime(2025, 12, 1, 12, 0, 0), - updated_at=datetime(2025, 12, 1, 12, 0, 0), - ) - new_deal: Dict[str, str] = { - "id": str(deal_id), - "deal_id": "1", - "pashub_link": "www.google.co.uk", - "design_status": "uploaded", - "created_at": datetime(2025, 12, 1, 12, 0, 0).isoformat(), - "updated_at": datetime(2025, 12, 1, 12, 30, 0).isoformat(), - } - - expected_output = True - - # act - actual_output: bool = HubspotDealDiffer.check_for_pashub_trigger( - new_deal=new_deal, old_deal=old_deal ) - # assert - assert actual_output == expected_output + new_deal = make_new_deal( + deal_id, + pashub_link="www.google.co.uk", + design_status="uploaded", + ) + + assert ( + HubspotDealDiffer.check_for_pashub_trigger( + new_deal=new_deal, + old_deal=old_deal, + ) + is True + ) def test_pashub_trigger__design_completed_and_pashub_link_not_set__returns_false() -> ( None ): - # arrange deal_id = uuid.uuid4() - old_deal = HubspotDealData( - id=deal_id, - deal_id="1", - created_at=datetime(2025, 12, 1, 12, 0, 0), - updated_at=datetime(2025, 12, 1, 12, 0, 0), - ) - new_deal: Dict[str, str] = { - "id": str(deal_id), - "deal_id": "1", - "design_status": "uploaded", - "created_at": datetime(2025, 12, 1, 12, 0, 0).isoformat(), - "updated_at": datetime(2025, 12, 1, 12, 30, 0).isoformat(), - } + old_deal = make_old_deal(id=deal_id) - expected_output = False - - # act - actual_output: bool = HubspotDealDiffer.check_for_pashub_trigger( - new_deal=new_deal, old_deal=old_deal + new_deal = make_new_deal( + deal_id, + design_status="uploaded", ) - # assert - assert actual_output == expected_output + assert ( + HubspotDealDiffer.check_for_pashub_trigger( + new_deal=new_deal, + old_deal=old_deal, + ) + is False + ) -def test_pashub_trigger__lodgement_completed_and_pashub_link_set__returns_true() -> ( - None -): - # arrange +# ------------------------- +# Lodgement +# ------------------------- + + +@pytest.mark.parametrize( + "lodgement_status,expected", + [ + ("lodgement complete", True), + ("measures lodged", True), + ], +) +def test_pashub_trigger__lodgement_completed_and_pashub_link_set__returns_true( + lodgement_status: str, + expected: bool, +) -> None: deal_id = uuid.uuid4() - old_deal = HubspotDealData( + old_deal = make_old_deal( id=deal_id, - deal_id="1", pashub_link="www.google.co.uk", - created_at=datetime(2025, 12, 1, 12, 0, 0), - updated_at=datetime(2025, 12, 1, 12, 0, 0), - ) - new_deal: Dict[str, str] = { - "id": str(deal_id), - "deal_id": "1", - "pashub_link": "www.google.co.uk", - "lodgement_status": "lodgement complete", - "created_at": datetime(2025, 12, 1, 12, 0, 0).isoformat(), - "updated_at": datetime(2025, 12, 1, 12, 30, 0).isoformat(), - } - - expected_output = True - - # act - actual_output: bool = HubspotDealDiffer.check_for_pashub_trigger( - new_deal=new_deal, old_deal=old_deal ) - # assert - assert actual_output == expected_output - - -def test_pashub_trigger__lodgement_completed_and_pashub_link_set__returns_true_2() -> ( - None -): - # arrange - deal_id = uuid.uuid4() - - old_deal = HubspotDealData( - id=deal_id, - deal_id="1", + new_deal = make_new_deal( + deal_id, pashub_link="www.google.co.uk", - created_at=datetime(2025, 12, 1, 12, 0, 0), - updated_at=datetime(2025, 12, 1, 12, 0, 0), - ) - new_deal: Dict[str, str] = { - "id": str(deal_id), - "deal_id": "1", - "pashub_link": "www.google.co.uk", - "lodgement_status": "measures lodged", - "created_at": datetime(2025, 12, 1, 12, 0, 0).isoformat(), - "updated_at": datetime(2025, 12, 1, 12, 30, 0).isoformat(), - } - - expected_output = True - - # act - actual_output: bool = HubspotDealDiffer.check_for_pashub_trigger( - new_deal=new_deal, old_deal=old_deal + lodgement_status=lodgement_status, ) - # assert - assert actual_output == expected_output + assert ( + HubspotDealDiffer.check_for_pashub_trigger( + new_deal=new_deal, + old_deal=old_deal, + ) + == expected + ) def test_pashub_trigger__lodgement_completed_and_pashub_link_not_set__returns_false() -> ( None ): - # arrange deal_id = uuid.uuid4() - old_deal = HubspotDealData( - id=deal_id, - deal_id="1", - created_at=datetime(2025, 12, 1, 12, 0, 0), - updated_at=datetime(2025, 12, 1, 12, 0, 0), - ) - new_deal: Dict[str, str] = { - "id": str(deal_id), - "deal_id": "1", - "design_status": "lodgement complete", - "created_at": datetime(2025, 12, 1, 12, 0, 0).isoformat(), - "updated_at": datetime(2025, 12, 1, 12, 30, 0).isoformat(), - } + old_deal = make_old_deal(id=deal_id) - expected_output = False - - # act - actual_output: bool = HubspotDealDiffer.check_for_pashub_trigger( - new_deal=new_deal, old_deal=old_deal + new_deal = make_new_deal( + deal_id, + design_status="lodgement complete", ) - # assert - assert actual_output == expected_output + assert ( + HubspotDealDiffer.check_for_pashub_trigger( + new_deal=new_deal, + old_deal=old_deal, + ) + is False + ) + + +# ------------------------- +# Negative case +# ------------------------- def test_pashub_trigger__coordination_design_lodgement_not_completed_and_pashub_link_set__returns_false() -> ( None ): - # arrange deal_id = uuid.uuid4() - old_deal = HubspotDealData( + old_deal = make_old_deal( id=deal_id, - deal_id="1", pashub_link="www.google.co.uk", - created_at=datetime(2025, 12, 1, 12, 0, 0), - updated_at=datetime(2025, 12, 1, 12, 0, 0), - ) - new_deal: Dict[str, str] = { - "id": str(deal_id), - "deal_id": "1", - "pashub_link": "www.google.co.uk", - "coordination_status": "not uploaded", - "design_status": "not uploaded", - "lodgement_status": "not uploaded", - "created_at": datetime(2025, 12, 1, 12, 0, 0).isoformat(), - "updated_at": datetime(2025, 12, 1, 12, 30, 0).isoformat(), - } - - expected_output = False - - # act - actual_output: bool = HubspotDealDiffer.check_for_pashub_trigger( - new_deal=new_deal, old_deal=old_deal ) - # assert - assert actual_output == expected_output + new_deal = make_new_deal( + deal_id, + pashub_link="www.google.co.uk", + coordination_status="not uploaded", + design_status="not uploaded", + lodgement_status="not uploaded", + ) + + assert ( + HubspotDealDiffer.check_for_pashub_trigger( + new_deal=new_deal, + old_deal=old_deal, + ) + is False + ) diff --git a/etl/hubspot/hubspotDataTodB.py b/etl/hubspot/hubspotDataTodB.py index f0beeee8..06cc3be9 100644 --- a/etl/hubspot/hubspotDataTodB.py +++ b/etl/hubspot/hubspotDataTodB.py @@ -291,54 +291,33 @@ class HubspotDataToDb: return False # Handle photo upload if it exists but S3 URL is missing - if ( - deal_in_db.major_condition_issue_photos - and not deal_in_db.major_condition_issue_evidence_s3_url - ): + if self._needs_photo_upload(deal_in_db): print( f"🖼️ Found photo for deal_id {deal_in_db.deal_id} — uploading to S3..." ) photo_url = hs_deal.get("major_condition_issue_photos") + if photo_url: - try: - # Download from HubSpot using fresh URL from hs_deal (not stale DB URL) - local_file = hubspot_client.download_file_from_url(photo_url) + self._upload_photo_to_s3( + deal_in_db, + photo_url, + hubspot_client, + verify=True, # 👈 key difference + ) - # Upload to S3 - bucket = "retrofit-data-dev" - s3_url = self.s3.upload_file( - local_file, bucket, prefix="hubspot/awaabs_law_evidence/" + # persist change + with db_read_session() as session: + db_record = session.get(HubspotDealData, deal_in_db.id) + db_record.major_condition_issue_evidence_s3_url = ( + deal_in_db.major_condition_issue_evidence_s3_url ) + session.add(db_record) + session.commit() - # Download again to verify integrity - downloaded = self.s3.download_from_url(s3_url) - if self._sha256(local_file) == self._sha256(downloaded): - print("✅ SHA256 match verified — upload successful.") - else: - print("❌ SHA256 mismatch — integrity check failed.") - raise ValueError("File integrity check failed after S3 upload.") - - # Update DB record with S3 URL - with db_read_session() as session: - db_record = session.get(HubspotDealData, deal_in_db.id) - db_record.major_condition_issue_evidence_s3_url = s3_url - session.add(db_record) - session.commit() - print( - f"✅ Updated DB with S3 URL for deal_id={deal_in_db.deal_id}" - ) - return False - except Exception as e: - print( - f"⚠️ Failed to download/upload photo for deal_id {deal_in_db.deal_id}: {e}" - ) - # Continue without the file — don't crash the entire update - finally: - if "local_file" in locals() and os.path.exists(local_file): - os.remove(local_file) + return False else: - print(f"⚠️ Photo URL missing for deal_id {deal_in_db.deal_id}") + print(f"⚠️ Photo URL missing for deal_id {deal_in_db.deal_id}") else: print(f"✅ No update or upload required for deal_id {deal_in_db.deal_id}.") @@ -534,10 +513,7 @@ class HubspotDataToDb: existing: HubspotDealData, hubspot_client: HubspotClient, ): - if ( - existing.major_condition_issue_photos - and not existing.major_condition_issue_evidence_s3_url - ): + if self._needs_photo_upload(existing): fresh_deal = hubspot_client.from_deal_id_get_info(existing.deal_id) photo_url = fresh_deal.get("major_condition_issue_photos") @@ -564,6 +540,7 @@ class HubspotDataToDb: record: HubspotDealData, photo_url: str, hubspot_client: HubspotClient, + verify: bool = False, ): try: local_file = hubspot_client.download_file_from_url(photo_url) @@ -574,6 +551,11 @@ class HubspotDataToDb: prefix="hubspot/awaabs_law_evidence/", ) + if verify: + downloaded = self.s3.download_from_url(s3_url) + if self._sha256(local_file) != self._sha256(downloaded): + raise ValueError("File integrity check failed after S3 upload.") + record.major_condition_issue_evidence_s3_url = s3_url except Exception as e: @@ -581,3 +563,9 @@ class HubspotDataToDb: finally: if "local_file" in locals() and os.path.exists(local_file): os.remove(local_file) + + def _needs_photo_upload(self, deal: HubspotDealData) -> bool: + return bool( + deal.major_condition_issue_photos + and not deal.major_condition_issue_evidence_s3_url + )