Skip to content

fix(hubspot): simplify integration to only create contacts #7147

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Holmus wants to merge 3 commits into
base: main
Choose a base branch
from
+8 −313
Open

fix(hubspot): simplify integration to only create contacts #7147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ba4aa50
fix(Integrations): Let HubSpot handle company creation and association
Holmus Apr 7, 2026
d4d1f88
Remove subscription sync - ChargeBee is source of truth
Holmus Apr 9, 2026
60856fb
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 5 additions & 113 deletions api/integrations/lead_tracking/hubspot/lead_tracker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,66 +5,19 @@
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
from .constants import HUBSPOT_FORM_ID_SAAS

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(
Copy link
Copy Markdown
Contributor

@Zaimwa9 Zaimwa9 Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's clean those HUBSPOT_XXX from the settings please

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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
13 changes: 0 additions & 13 deletions api/integrations/lead_tracking/hubspot/tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ""
Expand Down
8 changes: 0 additions & 8 deletions api/organisations/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down
145 changes: 3 additions & 142 deletions api/tests/unit/integrations/lead_tracking/hubspot/test_unit_hubspot_lead_tracking.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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()
Copy link
Copy Markdown
Contributor

@Zaimwa9 Zaimwa9 Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test name should reflect that associations are not created.

(Atm: test_create_lead__existing_hubspot_org__creates_contact_and_associates)

Copy link
Copy Markdown
Contributor

@Zaimwa9 Zaimwa9 Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, associate_contact_to_company is only used in test now so we might want to clean it all up

mock_client_existing_contact.create_lead_form.assert_not_called()
mock_client_existing_contact.get_contact.assert_called_once_with(user)

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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:
Expand Down
Loading
Loading