Use stable PostHog identity for newsletter signups#6588
Conversation
Merging this PR will not alter performance
Comparing 
Footnotes
|
Greptile SummaryThis PR refactors PostHog newsletter tracking to stop using email as the
Confidence Score: 4/5Safe to merge after verifying that no active PostHog dashboards, funnels, or cohorts rely on the newsletter_signup event name. The tracking logic itself is correct — email is properly kept out of distinct_id and the identify call only fires when a canonical UUID is available. The one concern is the event name change from newsletter_signup to newsletter_subscribed: since signup.py was the only caller of the old function, all future newsletter signups land under the new name. Existing PostHog insights filtering on newsletter_signup will silently stop receiving data. packages/reflex-site-shared/src/reflex_site_shared/backend/signup.py — the event name change here has analytics consequences worth confirming before merge. Important Files Changed
Reviews (3): Last reviewed commit: "Merge origin/main into fix-posthog-newsl..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| yield track_newsletter_posthog_subscription(email) | ||
| yield IndexState.send_contact_to_webhook(email) | ||
| yield IndexState.add_contact_to_loops(email) |
There was a problem hiding this comment.
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.
| 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) |
| if (Object.keys(personProps).length > 0) {{ | ||
| eventProps.$set = personProps; | ||
| }} | ||
| if (canonicalDistinctId && canonicalDistinctId !== props.email) {{ | ||
| posthog.identify(String(canonicalDistinctId), personProps); | ||
| }} |
There was a problem hiding this comment.
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!
|
@copilot resolve the merge conflicts in this pull request |
Resolved the merge conflicts in 84e6a63. The two conflicting areas were:
|
|
@copilot resolve the merge conflicts in this pull request |
Resolved in e1eb12f. The only conflict was in |
| }, | ||
| ) | ||
| return | ||
| yield track_newsletter_posthog_subscription(email) |
There was a problem hiding this comment.
track_newsletter_posthog_submission emitted newsletter_signup; track_newsletter_posthog_subscription emits newsletter_subscribed. Since signup.py was the only caller of the old function (no other callers found in the repo), all future newsletter signups will land under the new event name. Any existing PostHog dashboards, funnels, or cohorts that filter on newsletter_signup will silently stop receiving new data. Verify whether any PostHog insights need to be updated or duplicated to cover both event names before merging.
Summary
$setperson propertiesposthog.identifywhen a canonical non-email ID is providedtrack_newsletter_posthog_subscription(email, contact_uuid)as the new preferred API for newsletter trackingtrack_newsletter_posthog_submission(form_data)for backwards compatibility (now uses the improved_capture_posthog_person_eventunder the hood)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.pyuv 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.pyuv run --package reflex-site-shared python - <<'PY' ...script assertions for newsletter PostHog payload