diff --git a/api/integrations/lead_tracking/hubspot/lead_tracker.py b/api/integrations/lead_tracking/hubspot/lead_tracker.py index 501e34af7678..d3cbb6f62168 100644 --- a/api/integrations/lead_tracking/hubspot/lead_tracker.py +++ b/api/integrations/lead_tracking/hubspot/lead_tracker.py @@ -5,11 +5,7 @@ from django.conf import settings from integrations.lead_tracking.lead_tracking import LeadTracker -from organisations.models import ( - HubspotOrganisation, - Organisation, - Subscription, -) +from organisations.models import Organisation from users.models import FFAdminUser, HubspotLead, HubspotTracker from .client import HubspotClient @@ -17,54 +13,11 @@ logger = logging.getLogger(__name__) -try: - import re2 as re # type: ignore[import-untyped] - - logger.info("Using re2 library for regex.") -except ImportError: - logger.warning("Unable to import re2. Falling back to re.") - import re - class HubspotLeadTracker(LeadTracker): @staticmethod def should_track(user: FFAdminUser) -> bool: - if not settings.ENABLE_HUBSPOT_LEAD_TRACKING: - return False - - domain = user.email_domain - - if settings.HUBSPOT_IGNORE_DOMAINS_REGEX and re.match( - settings.HUBSPOT_IGNORE_DOMAINS_REGEX, domain - ): - return False - - if ( - settings.HUBSPOT_IGNORE_DOMAINS - and domain in settings.HUBSPOT_IGNORE_DOMAINS - ): - return False - - return True - - def update_company_active_subscription( - self, subscription: Subscription - ) -> dict[str, Any] | None: - if not subscription.plan: - return None - - organisation = subscription.organisation - - # Check if we're missing the associated hubspot id. - if not getattr(organisation, "hubspot_organisation", None): - return None - - response: dict[str, Any] | None = self.client.update_company( - active_subscription=subscription.plan, - hubspot_company_id=organisation.hubspot_organisation.hubspot_id, - ) - - return response + return settings.ENABLE_HUBSPOT_LEAD_TRACKING def create_user_hubspot_contact(self, user: FFAdminUser) -> str | None: tracker = HubspotTracker.objects.filter(user=user).first() @@ -96,17 +49,9 @@ def create_user_hubspot_contact(self, user: FFAdminUser) -> str | None: return hubspot_contact_id def create_lead(self, user: FFAdminUser, organisation: Organisation) -> None: - hubspot_contact_id = self._get_or_create_user_hubspot_id(user) - if not hubspot_contact_id: - return - hubspot_org_id = self._get_organisation_hubspot_id(user, organisation) - if not hubspot_org_id: - return - - self.client.associate_contact_to_company( - contact_id=hubspot_contact_id, - company_id=hubspot_org_id, - ) + # Only create the contact. HubSpot handles company creation and + # association automatically from the contact's email domain. + self._get_or_create_user_hubspot_id(user) def _get_new_contact_with_retry( self, user: FFAdminUser, max_retries: int = 3 @@ -138,58 +83,5 @@ def _get_or_create_user_hubspot_id(self, user: FFAdminUser) -> str | None: return hubspot_contact_id - def _get_organisation_hubspot_id( - self, - user: FFAdminUser, - organisation: Organisation, - ) -> str | None: - """ - Return the Hubspot API's id for an organisation. - """ - if getattr(organisation, "hubspot_organisation", None): - return organisation.hubspot_organisation.hubspot_id - - if user.email_domain in settings.HUBSPOT_IGNORE_ORGANISATION_DOMAINS: - return None - - domain = user.email_domain - company_kwargs = {"domain": domain} - company_kwargs["name"] = organisation.name - company_kwargs["organisation_id"] = organisation.id - company_kwargs["active_subscription"] = organisation.subscription.plan - - # As Hubspot creates/associates companies automatically based on contact domain - # we need to get the hubspot id when this user creates the company for the first time - # and update the company name - company = self._get_hubspot_company_by_domain(domain) - if not company: - return None - org_hubspot_id: str = company["id"] - - # Update the company in Hubspot with the name of the created - # organisation in Flagsmith, and its numeric ID. - self.client.update_company( - name=organisation.name, - hubspot_company_id=org_hubspot_id, - flagsmith_organisation_id=organisation.id, - ) - - # Store the organisation data in the database since we are - # unable to look them up via a unique identifier. - HubspotOrganisation.objects.create( - organisation=organisation, - hubspot_id=org_hubspot_id, - ) - - return org_hubspot_id - - def _get_hubspot_company_by_domain( - self, - domain: str, - ) -> dict[str, Any]: - company = self.client.get_company_by_domain(domain) - - return company # type: ignore[no-any-return] - def _get_client(self) -> HubspotClient: return HubspotClient() diff --git a/api/integrations/lead_tracking/hubspot/tasks.py b/api/integrations/lead_tracking/hubspot/tasks.py index 5ea81438221b..e3b8b21da8ef 100644 --- a/api/integrations/lead_tracking/hubspot/tasks.py +++ b/api/integrations/lead_tracking/hubspot/tasks.py @@ -51,19 +51,6 @@ def create_hubspot_contact_for_user(user_id: int) -> None: hubspot_lead_tracker.create_user_hubspot_contact(user) -@register_task_handler() -def update_hubspot_active_subscription(subscription_id: int) -> None: - assert settings.ENABLE_HUBSPOT_LEAD_TRACKING - - from organisations.models import Subscription - - from .lead_tracker import HubspotLeadTracker - - subscription = Subscription.objects.get(id=subscription_id) - hubspot_lead_tracker = HubspotLeadTracker() - hubspot_lead_tracker.update_company_active_subscription(subscription) - - @register_task_handler() def create_self_hosted_onboarding_lead_task( email: str, first_name: str, last_name: str, hubspot_cookie: str = "" diff --git a/api/organisations/models.py b/api/organisations/models.py index bc24160e7a1c..9dc40228ea7b 100644 --- a/api/organisations/models.py +++ b/api/organisations/models.py @@ -21,7 +21,6 @@ from features.versioning.constants import DEFAULT_VERSION_LIMIT_DAYS from integrations.lead_tracking.hubspot.tasks import ( track_hubspot_lead_v2, - update_hubspot_active_subscription, ) from organisations.chargebee import ( # type: ignore[attr-defined] get_customer_id_from_subscription_id, @@ -303,13 +302,6 @@ def update_api_limit_access_block(self): # type: ignore[no-untyped-def] self.organisation.block_access_to_admin = False self.organisation.save() - @hook(AFTER_SAVE, when="plan", has_changed=True) - def update_hubspot_active_subscription(self): # type: ignore[no-untyped-def] - if not settings.ENABLE_HUBSPOT_LEAD_TRACKING: - return - - update_hubspot_active_subscription.delay(args=(self.id,)) - def save_as_free_subscription(self): # type: ignore[no-untyped-def] """ Wipes a subscription to a normal free plan. diff --git a/api/tests/unit/integrations/lead_tracking/hubspot/test_unit_hubspot_lead_tracking.py b/api/tests/unit/integrations/lead_tracking/hubspot/test_unit_hubspot_lead_tracking.py index 6513e228840e..3fbedd52f60b 100644 --- a/api/tests/unit/integrations/lead_tracking/hubspot/test_unit_hubspot_lead_tracking.py +++ b/api/tests/unit/integrations/lead_tracking/hubspot/test_unit_hubspot_lead_tracking.py @@ -145,14 +145,6 @@ def test_track_hubspot_lead_v2__new_user_added_to_org__creates_associations( "organisations.models.track_hubspot_lead_v2" ) - mock_client_existing_contact.get_company_by_domain.return_value = { - "id": HUBSPOT_COMPANY_ID, - "properties": {"name": domain}, - } - mock_client_existing_contact.update_company.return_value = { - "id": HUBSPOT_COMPANY_ID, - "properties": {"name": organisation.name}, - } assert getattr(organisation, "hubspot_organisation", None) is None # When user.add_organisation(organisation, role=OrganisationRole.ADMIN) @@ -165,16 +157,10 @@ def test_track_hubspot_lead_v2__new_user_added_to_org__creates_associations( # Triggering it manually to void the delay track_hubspot_lead_v2(user.id, organisation.id) - organisation.refresh_from_db() - assert organisation.hubspot_organisation is not None - assert organisation.hubspot_organisation.hubspot_id == HUBSPOT_COMPANY_ID + # create_lead only creates the contact, not the company association mock_client_existing_contact.create_company.assert_not_called() - - mock_client_existing_contact.associate_contact_to_company.assert_called_once_with( - contact_id=HUBSPOT_USER_ID, - company_id=HUBSPOT_COMPANY_ID, - ) + mock_client_existing_contact.associate_contact_to_company.assert_not_called() mock_client_existing_contact.create_lead_form.assert_not_called() mock_client_existing_contact.get_contact.assert_called_once_with(user) @@ -253,100 +239,7 @@ def test_create_lead__existing_hubspot_org__creates_contact_and_associates( mock_client.create_lead_form.assert_called_once_with( user=user, form_id=HUBSPOT_FORM_ID_SAAS ) - mock_client.associate_contact_to_company.assert_called_once_with( - contact_id=HUBSPOT_USER_ID, - company_id=HUBSPOT_COMPANY_ID, - ) - - -def test_create_lead__filtered_domain__skips_company_creation( - organisation: Organisation, - settings: SettingsWrapper, - mock_client_existing_contact: MagicMock, - enable_hubspot: None, - mocker: MockerFixture, -) -> None: - # Given - settings.HUBSPOT_IGNORE_ORGANISATION_DOMAINS = ["example.com"] - - user = FFAdminUser.objects.create( - email="new.user@example.com", - first_name="Frank", - last_name="Louis", - marketing_consent_given=True, - ) - - # When - tracker = HubspotLeadTracker() - tracker.create_lead(user=user, organisation=organisation) - - # Then - assert HubspotLead.objects.filter(user=user, hubspot_id=HUBSPOT_USER_ID).exists() - mock_client_existing_contact.get_contact.assert_called_once_with(user) - mock_client_existing_contact.create_company.assert_not_called() - mock_client_existing_contact.associate_contact_to_company.assert_not_called() - - -def test_update_company_active_subscription__valid_subscription__calls_update_company( - db: None, mocker: MockerFixture -) -> None: - # Given - mock_client = mocker.MagicMock() - mock_response = {"id": "123"} - mock_client.update_company.return_value = mock_response - - tracker = HubspotLeadTracker() - tracker.client = mock_client - - mock_org = mocker.MagicMock() - mock_org.hubspot_organisation.hubspot_id = "hubspot-org-1" - - mock_subscription = mocker.MagicMock() - mock_subscription.plan = "scaleup" - mock_subscription.organisation = mock_org - - # When - result = tracker.update_company_active_subscription(subscription=mock_subscription) - - # Then - assert result == mock_response - mock_client.update_company.assert_called_once_with( - active_subscription="scaleup", - hubspot_company_id="hubspot-org-1", - ) - - -def test_update_company_active_subscription__no_hubspot_org__returns_none( - mocker: MockerFixture, -) -> None: - # Given - subscription = mocker.MagicMock() - subscription.plan = "pro" - subscription.organisation = mocker.MagicMock() - subscription.organisation.hubspot_organisation = None - - # When - tracker = HubspotLeadTracker() - result = tracker.update_company_active_subscription(subscription) - - # Then - assert result is None - - -def test_update_company_active_subscription__no_plan__returns_none( - mocker: MockerFixture, -) -> None: - # Given - subscription = mocker.MagicMock() - subscription.plan = None - subscription.organisation = mocker.MagicMock() - - # When - tracker = HubspotLeadTracker() - result = tracker.update_company_active_subscription(subscription) - - # Then - assert result is None + mock_client.associate_contact_to_company.assert_not_called() @pytest.mark.parametrize( @@ -390,38 +283,6 @@ def test_create_user_hubspot_contact__get_contact_retries__returns_expected_id( assert mock_client.get_contact.call_count == expected_call_count -@pytest.mark.parametrize( - "hubspot_contact_id, hubspot_org_id", - [ - (None, "org_123"), - ("contact_123", None), - ], -) -def test_create_lead__missing_contact_or_org_id__skips_association( - mocker: MockerFixture, - hubspot_contact_id: str | None, - hubspot_org_id: str | None, - staff_user: FFAdminUser, - organisation: Organisation, -) -> None: - # Given - mock_client = mocker.MagicMock() - tracker = HubspotLeadTracker() - - mocker.patch.object( - tracker, "_get_or_create_user_hubspot_id", return_value=hubspot_contact_id - ) - mocker.patch.object( - tracker, "_get_organisation_hubspot_id", return_value=hubspot_org_id - ) - - # When - tracker.create_lead(staff_user, organisation) - - # Then - mock_client.associate_contact_to_company.assert_not_called() - - def test_register_hubspot_tracker_and_track_user__no_explicit_user__falls_back_to_request_user( mocker: MockerFixture, staff_user: FFAdminUser, settings: SettingsWrapper ) -> None: diff --git a/api/tests/unit/integrations/lead_tracking/hubspot/test_unit_hubspot_tasks.py b/api/tests/unit/integrations/lead_tracking/hubspot/test_unit_hubspot_tasks.py index 1776770b166d..40457d490b73 100644 --- a/api/tests/unit/integrations/lead_tracking/hubspot/test_unit_hubspot_tasks.py +++ b/api/tests/unit/integrations/lead_tracking/hubspot/test_unit_hubspot_tasks.py @@ -5,9 +5,7 @@ from integrations.lead_tracking.hubspot.tasks import ( create_hubspot_contact_for_user, track_hubspot_lead_v2, - update_hubspot_active_subscription, ) -from organisations.models import Organisation from users.models import FFAdminUser @@ -79,38 +77,3 @@ def test_track_hubspot_lead__should_track_false__does_not_create_lead( # Then mock_create_lead.assert_not_called() - - -def test_update_hubspot_active_subscription__tracking_disabled__raises_assertion_error( - settings: SettingsWrapper, - admin_user: FFAdminUser, - mocker: MockerFixture, -) -> None: - # Given - settings.ENABLE_HUBSPOT_LEAD_TRACKING = False - - # When / Then - with pytest.raises(AssertionError): - update_hubspot_active_subscription(subscription_id=1) - - -def test_update_hubspot_active_subscription__tracking_enabled__calls_update( - db: None, - settings: SettingsWrapper, - organisation: Organisation, - mocker: MockerFixture, -) -> None: - # Given - settings.ENABLE_HUBSPOT_LEAD_TRACKING = True - - mock_update_company_active_subscription = mocker.patch( - "integrations.lead_tracking.hubspot.lead_tracker.HubspotLeadTracker.update_company_active_subscription" - ) - - # When - update_hubspot_active_subscription(organisation.subscription.id) - - # Then - mock_update_company_active_subscription.assert_called_once_with( - organisation.subscription - )