feat(workflows): add team config + settings UI for messaging engagement events#50122
Conversation
|
Last reviewed commit: 1ab9a02 |
|
ReviewHog Alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
sortafreel
left a comment
There was a problem hiding this comment.
ReviewHog Report
Feature
Files: nodejs/src/cdp/services/messaging/helpers/tracking-code.ts, nodejs/src/cdp/services/messaging/email.service.ts
Issues: 4 issues
What were the main changes
- Extends email tracking code format to optionally include distinct_id while keeping backward compatibility with existing functionId:invocationId payloads.
- Updates tracking-code parsing to safely reconstruct distinct_id when it contains colons, and propagates distinct_id through pixel URL generation.
- Adds standard PostHog event capture on email send/fail in the send path with workflow and email metadata properties.
Full analysis
This chunk extends the email tracking identifier format so one ph_id token can carry functionId, invocationId, and optionally distinctId, while still accepting legacy two-part payloads. In tracking-code.ts, the helper API is made explicit with ParsedTrackingCode, generation methods now accept an optional distinctId, and parsing uses a fixed-first-two + rest rejoin strategy to preserve distinctId values that may contain :. The overall pattern remains a compact URL-safe base64 transport format, so existing links/tags continue to work and old payloads decode without schema migration.
In email.service.ts, the send path now propagates invocation.state.globals.event.distinct_id into the SES ph_id email tag (generateEmailTrackingCode(result.invocation, distinctId)), which is the critical integration point for later asynchronous SES webhook attribution. The same method also appends standard PostHog capture events ($messaging_email_sent / $messaging_email_failed) to result.capturedPostHogEvents with workflow and email metadata, but only when a distinct ID exists. This follows the existing invocation-result contract rather than introducing a direct side-effect call inside the service.
Architecturally, this chunk connects synchronous send execution and downstream telemetry pipelines: EmailService.executeSendEmail is called by HogExecutor, returned results are consumed by cyclotron worker processing, and capturedPostHogEvents are ultimately emitted via HogFunctionMonitoringService + InternalCaptureService. On the tracking side, the updated token format is consumed by SES webhook parsing and direct pixel/redirect handlers through shared helper functions, giving one consistent identity propagation path across direct opens/clicks and SES-originated engagement signals. There is one existing review-thread nit in this chunk about clarifying the inline colon-rejoin comment, but no architectural disagreement in the discussion.
Business logic
Files: nodejs/src/cdp/services/monitoring/hog-function-monitoring.service.ts, nodejs/src/cdp/services/messaging/helpers/ses.ts, nodejs/src/cdp/services/messaging/email-tracking.service.ts
Issues: 7 issues
What were the main changes
- Adds queuePostHogEvent to HogFunctionMonitoringService to enqueue arbitrary PostHog events by resolving team token and normalizing timestamp.
- Enhances SES webhook processing to return distinct_id and engagement properties ($email_to, optional $link_url) alongside metric mapping.
- Refactors email tracking handlers to parse combined ph_id (with legacy ph_fn_id/ph_inv_id fallback), emit standardized $messaging_email_* events, and attach source/workflow/link properties for opens, clicks, and SES events.
- Review hotspot: email-tracking currently calls async queuePostHogEvent without await before flush, which can cause dropped events.
Full analysis
This chunk extends the CDP messaging pipeline so asynchronous email engagement signals (pixel opens, redirect clicks, SES webhooks) are emitted as first-class PostHog events, not only as workflow app metrics. The architectural addition is queuePostHogEvent on HogFunctionMonitoringService, which reuses the existing buffered monitoring pattern (eventsToCapture + flush) but accepts a team_id, resolves the team API token through TeamManager, normalizes timestamp, and queues an InternalCaptureEvent for InternalCaptureService. This is the missing path for events that happen after the original invocation lifecycle, where capturedPostHogEvents on invocation results is no longer available.
EmailTrackingService is refactored into a normalization/dispatch layer: tracking URLs now encode ph_id with optional distinct_id, request handlers decode either the new combined ph_id format or legacy ph_fn_id/ph_inv_id, and trackMetric resolves whether the source ID belongs to a Hog function or Hog flow before emitting data. It still queues legacy app metrics (metric_kind: "email") for existing monitoring, and now additionally emits standardized $messaging_email_* events with consistent properties ($workflow_id, $messaging_source, plus optional $link_url/$email_to). This keeps backward compatibility while aligning engagement telemetry with standard PostHog event semantics for insights, funnels, and automation triggers.
SesWebhookHandler is upgraded from metric-name mapping to richer event extraction: it now returns distinctId and event properties from SES records, including recipient and clicked link. That output feeds directly into EmailTrackingService.handleSesWebhook, which iterates metrics and forwards them through the same trackMetric pipeline used by pixel/redirect endpoints, giving one integration path for direct and SES sources. The main implementation caveat already identified in review is that queuePostHogEvent is async but currently called without await before flush, creating a race where PostHog events can be dropped if flush executes before queueing completes.
|
Thanks for adding this feature |
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, please remove the |
|
We would love this feature being added. |
|
+1, this feature would allow us to start migrating to workflows and run tighter experiments 🦔🧪 |
|
I have an actual use case now: We want to record all emails sent to our users in our CRM. With this event we would be able to trigger a sync into Hubspot. |
|
Hoping this one can bubble back to the top for several folks who are waiting on it as a blocker. |
|
thanks guys it's still on our radar |
When emails are sent, opened, clicked, bounced, or marked as spam via Workflows, the system now captures standard PostHog events alongside the existing app_metrics2 records. This lets users build insights, funnels, and dashboards from messaging engagement data. Key changes: - Extend email tracking code to include distinct_id so async events (opens, clicks, bounces) can be attributed to the correct person - Add $messaging_email_* event capture in EmailTrackingService for pixel opens, link clicks, and SES webhook events - Add $messaging_email_sent/failed event capture in EmailService during the synchronous send path - Add queuePostHogEvent() to HogFunctionMonitoringService for direct event capture outside invocation results - Fix pixel/redirect handlers to parse the combined ph_id parameter (previously only read legacy separate ph_fn_id/ph_inv_id params) - Maintain backwards compatibility with old tracking code format
1ab9a02 to
0bffb37
Compare
…gen MCP types, fix settings loading state
…timestamp, person-id fallback, add team API tests
|
🎭 Playwright didn't run on this PR — your changes touch code that could affect E2E behavior, but Playwright is opt-in via label now to keep CI cost down. Add the Most PRs don't need this. Real regressions still get caught on master and fix-forward. |
|
Size Change: +5.36 kB (+0.01%) Total Size: 81.9 MB 📦 View Changed
ℹ️ View Unchanged
|
Migration SQL ChangesHey 👋, we've detected some migrations on this PR. Here's the SQL output for each migration, make sure they make sense:
|
🔍 Migration Risk AnalysisWe've analyzed your migrations for potential risks. Summary: 1 Safe | 0 Needs Review | 0 Blocked ✅ SafeBrief or no lock, backwards compatible Last updated: 2026-06-05 09:30 UTC (64bfe84) |
|
✅ Visual changes approved by @dmarchuk — baseline updated in 57 changed. |
…recondition in team-config tests
The migration & plugin-server changes must merge separately per the migration-isolation CI check (.github/workflows/ci-migrations-service-separation-check.yml). This PR keeps the schema change, model, API serializer, frontend, and IDOR registration. The plugin-server work lives in the follow-up branch feat/messaging-posthog-events-plugin-server and is safe to merge once the table exists in production.
…hog-events # Conflicts: # products/workflows/backend/migrations/max_migration.txt # products/workflows/backend/models/__init__.py
|
Reviews (2): Last reviewed commit: "Merge remote-tracking branch 'origin/mas..." | Re-trigger Greptile |
|
Reiterating a note from PR description: this isn't the full functionality yet, the PR needs to be split in two. This is just adding the configuration, the actual emitting of events will be added in #60212. |
|
Reviews (3): Last reviewed commit: "feat(workflows): add docs link to engage..." | Re-trigger Greptile |
…aging_engagement_events
…hog-events # Conflicts: # frontend/snapshots.yml
48 updated Run: 4d9efeab-08d9-4111-b60e-d59d3f7b0139 Co-authored-by: dmarchuk <8395106+dmarchuk@users.noreply.github.com>
…nto feat/messaging-posthog-events # Conflicts: # frontend/snapshots.yml
48 updated Run: f26d90aa-e135-4d8f-a398-32d251963d9e Co-authored-by: dmarchuk <8395106+dmarchuk@users.noreply.github.com>
Summary
Adds the opt-in configuration for capturing messaging engagement activity as standard PostHog events — the team-level setting, schema, API, and settings UI. The actual event emission (the plugin-server consumer that fires
$messaging_email_*events) lives in the follow-up #60212, which reads the table this PR creates.This PR alone changes no runtime behaviour: it creates an off-by-default toggle. Nothing is emitted until #60212 ships.
What this PR does
TeamWorkflowsConfig— new team-extension model + migration (workflows_teamworkflowsconfig, singlecapture_messaging_engagement_eventsboolean, default off). Follows theTeamCustomerAnalyticsConfig/ team-extension pattern.workflows_configon the team API —GETreturns{ capture_messaging_engagement_events: false }for teams without a row;PATCHcreates/updates it. Nested-serializer validation surfaces the inner field path.Workflows → Engagement eventssection (gated behind theWORKFLOWS_ENGAGEMENT_EVENTSflag) with aLemonSwitchbound tocurrentTeamLoading. Description links out to the docs page for the full event reference.TeamWorkflowsConfigregistered inbaseline_unmigrated.txt+ the semgrep IDOR rule.hogli build:openapi.Field naming
The boolean is
capture_messaging_engagement_events(notcapture_engagement_events): the feature's domain is messaging (events are$messaging_*), and the name is channel-agnostic so a single toggle can cover SMS/push later without a schema change. The config object already carries theworkflows_confignamespace.Coordination
workflows_teamworkflowsconfig. Must deploy after this PR's migration./docs/workflows/engagement-events) that the SettingsdocsUrland the editor "Learn more" link point at.Deploy order
$messaging_email_*for opted-in teams.Safe in both directions: toggling on between (1) and (2) just waits for the consumer; if (2) lands first, the consumer's lazy-loader falls back to the default (capture off) for any team whose lookup 42P01s.
Test plan
pytest products/workflows/backend/test/test_team_workflows_config.py— 6 tests (default for fresh teams, PATCH enable/disable, isolation from other team fields, non-boolean rejection, unknown-key tolerance).ruffclean, frontend typecheck clean.makemigrations --checkclean (migration0008_teamworkflowsconfig, after master's0007_migrate_hog_flow_models).TeamWorkflowsConfigschema +workflows_configfield onTeam/PatchedTeam.Publish to changelog?
Yes, once #60212 also merges (neither half delivers value alone): "Workflows: opt-in capture of email engagement events (
$messaging_email_*) as standard PostHog events for insights, funnels, and dashboards."🤖 Generated with Claude Code