diff --git a/api/app/settings/common.py b/api/app/settings/common.py index 9ffd74f8f9cc..62be8cfa0361 100644 --- a/api/app/settings/common.py +++ b/api/app/settings/common.py @@ -1187,17 +1187,6 @@ # Hubspot settings HUBSPOT_ACCESS_TOKEN = env.str("HUBSPOT_ACCESS_TOKEN", None) ENABLE_HUBSPOT_LEAD_TRACKING = env.bool("ENABLE_HUBSPOT_LEAD_TRACKING", False) -HUBSPOT_IGNORE_DOMAINS = env.list( - "HUBSPOT_IGNORE_DOMAINS", - subcast=str, - default=[], -) -HUBSPOT_IGNORE_DOMAINS_REGEX = env.str("HUBSPOT_IGNORE_DOMAINS_REGEX", "") -HUBSPOT_IGNORE_ORGANISATION_DOMAINS = env.list( - "HUBSPOT_IGNORE_ORGANISATION_DOMAINS", - subcast=str, - default=[], -) # Number of minutes to wait for a user that has signed up to # join or create an organisation before creating a lead in diff --git a/api/integrations/lead_tracking/hubspot/client.py b/api/integrations/lead_tracking/hubspot/client.py index 717c82f85ada..f02f2892977a 100644 --- a/api/integrations/lead_tracking/hubspot/client.py +++ b/api/integrations/lead_tracking/hubspot/client.py @@ -5,7 +5,6 @@ import hubspot # type: ignore[import-untyped] import requests from django.conf import settings -from hubspot.crm.associations.v4 import AssociationSpec # type: ignore[import-untyped] from hubspot.crm.companies import ( # type: ignore[import-untyped] PublicObjectSearchRequest, SimplePublicObjectInput, @@ -116,21 +115,6 @@ def create_lead_form( ) return response.json() # type: ignore[no-any-return] - def associate_contact_to_company(self, contact_id: str, company_id: str) -> None: - association_spec = [ - AssociationSpec( - association_category="HUBSPOT_DEFINED", association_type_id=1 - ) - ] - - self.client.crm.associations.v4.basic_api.create( - object_type="contacts", - object_id=contact_id, - to_object_type="companies", - to_object_id=company_id, - association_spec=association_spec, - ) - def get_company_by_domain(self, domain: str) -> dict[str, Any] | None: """ Domain should be unique in Hubspot by design, so we should only ever have 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 9ee71ac1b162..20b82e6c35e5 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, @@ -305,13 +304,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_client.py b/api/tests/unit/integrations/lead_tracking/hubspot/test_unit_hubspot_client.py index 3f7b63dad5d5..07b839f78dd1 100644 --- a/api/tests/unit/integrations/lead_tracking/hubspot/test_unit_hubspot_client.py +++ b/api/tests/unit/integrations/lead_tracking/hubspot/test_unit_hubspot_client.py @@ -3,7 +3,6 @@ import pytest import responses -from hubspot.crm.associations.v4 import AssociationSpec # type: ignore[import-untyped] from pytest_mock import MockerFixture from rest_framework import status @@ -216,32 +215,6 @@ def test_create_company__without_organisation_info__creates_with_name_and_domain } -def test_associate_contact_to_company__valid_ids__calls_hubspot_api( - hubspot_client: HubspotClient, -) -> None: - # Given - company_id = "456" - contact_id = "123" - - # When - hubspot_client.associate_contact_to_company( - contact_id=contact_id, company_id=company_id - ) - - # Then - hubspot_client.client.crm.associations.v4.basic_api.create.assert_called_once_with( - object_type="contacts", - object_id=contact_id, - to_object_type="companies", - to_object_id=company_id, - association_spec=[ - AssociationSpec( - association_category="HUBSPOT_DEFINED", association_type_id=1 - ) - ], - ) - - @pytest.mark.parametrize( "kwargs, expected_properties", [ 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..a93f1049b1b0 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,9 @@ 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.create_lead_form.assert_not_called() mock_client_existing_contact.get_contact.assert_called_once_with(user) @@ -215,10 +200,9 @@ def test_create_lead__contact_not_found_initially__creates_contact_via_form( user=user, form_id=HUBSPOT_FORM_ID_SAAS ) mock_client.create_company.assert_not_called() # We rely on Hubspot creating contacts - mock_client.associate_contact_to_company.assert_not_called() -def test_create_lead__existing_hubspot_org__creates_contact_and_associates( +def test_create_lead__existing_hubspot_org__creates_contact_without_association( organisation: Organisation, mocker: MockerFixture, ) -> None: @@ -253,100 +237,6 @@ 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 @pytest.mark.parametrize( @@ -390,38 +280,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 - ) diff --git a/infrastructure/aws/production/ecs-task-definition-task-processor.json b/infrastructure/aws/production/ecs-task-definition-task-processor.json index a9e2e04eb2a4..89198054b675 100644 --- a/infrastructure/aws/production/ecs-task-definition-task-processor.json +++ b/infrastructure/aws/production/ecs-task-definition-task-processor.json @@ -145,10 +145,6 @@ "name": "ENABLE_HUBSPOT_LEAD_TRACKING", "value": "True" }, - { - "name": "HUBSPOT_IGNORE_DOMAINS", - "value": "flagsmith.com,solidstategroup.com,restmail.net,bullettrain.io,flagsmithe2etestdomain.io" - }, { "name": "GITHUB_APP_ID", "value": "811209"