Skip to content

Use stable PostHog identity for newsletter signups#6588

Open
Alek99 wants to merge 1 commit into
mainfrom
fix-posthog-newsletter-distinct-id
Open

Use stable PostHog identity for newsletter signups#6588
Alek99 wants to merge 1 commit into
mainfrom
fix-posthog-newsletter-distinct-id

Conversation

@Alek99
Copy link
Copy Markdown
Member

@Alek99 Alek99 commented May 30, 2026

Summary

  • stop using email as the PostHog distinct ID for demo/intro form tracking
  • capture newsletter signups in PostHog with email in $set person properties
  • only call posthog.identify when a canonical non-email ID is provided

cc @Alek99

Testing

  • uv run ruff format packages/reflex-components-internal/src/reflex_components_internal/blocks/telemetry/posthog.py packages/reflex-components-internal/src/reflex_components_internal/blocks/telemetry/__init__.py packages/reflex-site-shared/src/reflex_site_shared/backend/signup.py
  • uv run ruff check packages/reflex-components-internal/src/reflex_components_internal/blocks/telemetry/posthog.py packages/reflex-components-internal/src/reflex_components_internal/blocks/telemetry/__init__.py packages/reflex-site-shared/src/reflex_site_shared/backend/signup.py
  • uv run --package reflex-site-shared python - <<'PY' ... script assertions for newsletter PostHog payload

@Alek99 Alek99 requested a review from a team as a code owner May 30, 2026 00:10
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 30, 2026

Merging this PR will not alter performance

✅ 24 untouched benchmarks


Comparing fix-posthog-newsletter-distinct-id (8c4adad) with main (e88acf7)1

Open in CodSpeed

Footnotes

  1. No successful run was found on main (7281317) during the generation of this report, so e88acf7 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 30, 2026

Greptile Summary

This PR stops using email addresses as PostHog distinct IDs for demo/intro form tracking and newsletter signups. It introduces a shared _capture_posthog_person_event helper that routes PII fields into $set person properties and only calls posthog.identify when a stable canonical UUID is available.

  • Refactors _track_form_posthog to delegate to the new helper, removing the unconditional posthog.identify(email) call in favour of identify-only-when-UUID-present semantics.
  • Adds track_newsletter_posthog_subscription (with contact_uuid support) and wires it into the signup background handler in signup.py.

Confidence Score: 4/5

Safe to merge; the only actionable finding is that track_newsletter_posthog_subscription fires in signup even when no email was collected, producing noise-only events in PostHog.

The core identity-management logic is well-constructed and the JS helper correctly isolates PII into person properties. The one concrete gap is in signup.py: the tracking call is not gated on email being present, so a form submission with no email address still emits a newsletter_subscribed event with no identity data. There is also a minor redundancy where $set person properties are written twice (via identify and then again on the capture call) when a canonical UUID is present.

signup.py — the unconditional track_newsletter_posthog_subscription(email) call.

Important Files Changed

Filename Overview
packages/reflex-components-internal/src/reflex_components_internal/blocks/telemetry/posthog.py Replaces email-as-distinct-id pattern with a proper canonical-ID flow; adds _capture_posthog_person_event helper and track_newsletter_posthog_subscription public function. Logic is sound; minor redundancy in $set when identify is also called.
packages/reflex-site-shared/src/reflex_site_shared/backend/signup.py Adds PostHog newsletter tracking call in the signup handler; tracking fires unconditionally even when email is None (no email provided in form), generating an identity-less event.
packages/reflex-components-internal/src/reflex_components_internal/blocks/telemetry/init.py Exports the new track_newsletter_posthog_subscription symbol; no issues.

Reviews (1): Last reviewed commit: "Use stable PostHog identity for newslett..." | Re-trigger Greptile

Comment on lines +108 to 110
yield track_newsletter_posthog_subscription(email)
yield IndexState.send_contact_to_webhook(email)
yield IndexState.add_contact_to_loops(email)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 PostHog fires a newsletter_subscribed event even when email is None. If the user submits the form without an email address, email_to_validate is falsy and the if block is skipped entirely — so email stays None and tracking fires with an empty props object. The resulting event has no identity and no person properties, creating noise in PostHog analytics.

Suggested change
yield track_newsletter_posthog_subscription(email)
yield IndexState.send_contact_to_webhook(email)
yield IndexState.add_contact_to_loops(email)
if email:
yield track_newsletter_posthog_subscription(email)
yield IndexState.send_contact_to_webhook(email)
yield IndexState.add_contact_to_loops(email)

Comment on lines +79 to +84
if (Object.keys(personProps).length > 0) {{
eventProps.$set = personProps;
}}
if (canonicalDistinctId && canonicalDistinctId !== props.email) {{
posthog.identify(String(canonicalDistinctId), personProps);
}}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Double person-property write when canonical ID is present

When canonicalDistinctId is truthy, posthog.identify(canonicalDistinctId, personProps) is called and then posthog.capture(event_name, {..., $set: personProps}) is called immediately after with the same personProps in $set. PostHog merges both writes, so this is harmless in practice, but the $set payload on the capture event is redundant when identify already carries the same data. Removing the $set attachment when canonicalDistinctId is present would eliminate the duplicate write, though this is a minor analytics hygiene concern.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant