[Payment due @ikevin127] Split workspace_created conversion event into two events for ad platforms - #92446
Conversation
…orms Splits the single `workspace_created` conversion event into two so Google, Meta, and Reddit can optimize bids toward higher-value leads: - `workspace_created_sales_eligible` fires when all of: onboarding intent is "Manage my team", company size is 5+ employees, and the email domain is private. It uses the standard "Lead" Meta/Reddit event so the platforms' pre-trained conversion models optimize toward these leads. - `workspace_created` fires for all other workspace creations and now uses a custom Meta/Reddit event, so the higher-volume, lower-value conversions don't dilute the standard "Lead" optimization. The decision lives in getWorkspaceCreatedAnalyticsEvent and is applied at both firing sites (createWorkspace and categorizeTrackedExpense). LinkedIn is paused and intentionally left unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@ikevin127 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13dca42223
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| */ | ||
| function getWorkspaceCreatedAnalyticsEvent(engagementChoice: string | undefined, companySize: string | undefined, email: string): GoogleTagManagerEvent { | ||
| const isSalesEligible = | ||
| engagementChoice === CONST.ONBOARDING_CHOICES.MANAGE_TEAM && !!companySize && SALES_ELIGIBLE_COMPANY_SIZES.has(companySize) && !!email && !isEmailPublicDomain(email); |
There was a problem hiding this comment.
When a new user signs up with a phone-number login (the app stores those as SMS logins, e.g. session.email ending in CONST.SMS.DOMAIN as checked in useHasPhoneNumberLogin), this condition treats it as a private-domain email because it only verifies the value is non-empty and not in the public-domain set. A Manage-my-team user with 5+ employees but no actual email domain would therefore fire workspace_created_sales_eligible, even though the documented criterion is a private email domain; please reject SMS/invalid logins or require a valid non-SMS email before returning the sales-eligible event.
Useful? React with 👍 / 👎.
|
@youssef-lr If this PR needs my C+ review, please tag me once all workflows are 🟢 and PR si ready for review. |
|
This doesn't need a C+ review @ikevin127. Sorry it was supposed to be created as a draft. |
…44512-app-split-workspace-created-event
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
Hey Kevin! I added you back as a reviewer. Please note this only needs to be tested on Web. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 855dfffcb1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| WORKSPACE_CREATED: { | ||
| NAME: 'workspace_created', | ||
| META: 'workspace_created', | ||
| REDDIT: 'Lead', |
There was a problem hiding this comment.
For non-sales-eligible workspace creations, this still configures the Reddit pixel event as Lead. publishEvent only applies IS_CUSTOM_PIXEL_EVENT to Meta and still calls rdt('track', pixelEvent.REDDIT, ...), so any workspace that falls back to workspace_created continues to hit Reddit's standard Lead conversion and dilutes the sales-eligible Lead optimization the split is meant to protect. Use Reddit's custom event form/name for this branch as well.
Useful? React with 👍 / 👎.
Reviewer Checklist
Screenshots/VideosScreen.Recording.2026-06-12.at.16.01.24.mov |
|
🟢 The PR successfully splits the
@youssef-lr This is after the initial PR overview look, I'm continuing the code review 🔄 |
| REDDIT: 'Lead', | ||
| LINKEDIN: 25474804, | ||
| }, | ||
| PAID_ADOPTION: { | ||
| NAME: 'paid_adoption', | ||
| META: 'Purchase', | ||
| REDDIT: 'Purchase', | ||
| LINKEDIN: 25474820, |
There was a problem hiding this comment.
🟡 MEDIUM — PR Description Mismatch
The PR description says "LinkedIn is paused and intentionally left unchanged" and "Uses a custom Meta/Reddit event", but the code completely removes all rdt (Reddit) and lintrk (LinkedIn) tracking from GoogleTagManager/index.ts. This is a significant inconsistency, either the code is correct and the description needs updating, or the description is correct and the code is wrong.
Fix: Update the PR description to explicitly state that Reddit and LinkedIn tracking is being removed from all pixel events, and this PR only targets Meta. Alternatively, if the intent is to keep them, the code must be restored.
Question: Were Reddit and LinkedIn intentionally dropped, or is this an accidental deletion ? Since the commit history shows "Remove reddit & meta" followed by "Apply changes only to Google & Meta", this seems intentional, but the PR description must be updated.
|
|
||
| // Standard events (e.g. "Lead") tap into Meta's pre-trained conversion models, so we only mark an event as | ||
| // custom when we intentionally don't want it optimized against the standard event. | ||
| const isCustomPixelEvent = 'IS_CUSTOM_PIXEL_EVENT' in pixelEvent && pixelEvent.IS_CUSTOM_PIXEL_EVENT; |
There was a problem hiding this comment.
🟠 HIGH — Missing test coverage for trackCustom vs track branching
The PR introduces a new isCustomPixelEvent flag that changes the fbq call REQ method, but there is no test verifying this behavior. The existing GoogleTagManagerTest.tsx only mocks the module and tests the calling code, not the internal fbq invocation logic. This means the fbq('trackCustom', ...) vs fbq('track', ...) distinction is completely untested.
Fix: Add a test for
GoogleTagManager.publishEventthat:
- Mocks
window.dataLayerandwindow.fbq- Verifies that
WORKSPACE_CREATEDtriggersfbq('trackCustom', 'workspace_created', ...)- Verifies that
WORKSPACE_CREATED_SALES_ELIGIBLEtriggersfbq('track', 'Lead', ...)
Example of what untested code could hide: A future developer could accidentally change isCustomPixelEvent ? 'trackCustom' : 'track' to always use 'track' and the existing tests would still pass, but Meta would incorrectly optimize the wrong conversions.
🟡 MEDIUM — IS_CUSTOM_PIXEL_EVENT could be more type-safe
The check IS_CUSTOM_PIXEL_EVENT in pixelEvent && pixelEvent.IS_CUSTOM_PIXEL_EVENT works at runtime but the optional property isn't strongly typed in the typeof CONST.ANALYTICS.EVENT[...] type. The PIXEL_EVENTS array is typed as const from CONST.ANALYTICS.EVENT, which includes all event configs. This is fine but could be cleaner with an explicit type.
Suggestion: Consider defining a typed interface for pixel events rather than relying on runtime 'in' checks. Not a blocker.
| @@ -204,6 +204,50 @@ describe('GoogleTagManagerTest', () => { | |||
| expect(GoogleTagManager.publishEvent).toHaveBeenCalledWith(CONST.ANALYTICS.EVENT.WORKSPACE_CREATED.NAME, 123456, email); | |||
| }); | |||
|
|
|||
There was a problem hiding this comment.
🟠 HIGH — Missing test for categorizeTrackedExpense with sales-eligible criteria
The categorizeTrackedExpense test starting at line 251 still expects WORKSPACE_CREATED.NAME with default params. There is no test verifying that when trackExpense is called with sales-eligible createdWorkspaceParams (i.e., engagementChoice: MANAGE_TEAM, companySize: MICRO_MEDIUM, private email), it correctly publishes WORKSPACE_CREATED_SALES_ELIGIBLE.NAME.
Fix: Add a new test case:
test('workspace_created_sales_eligible - categorizeTrackedExpense', async () => { // TODO: Mock createdWorkspaceParams with sales-eligible criteria // and verify the correct event is published });
🟡 MEDIUM — The categorizeTrackedExpense test (line 251) does not override createdWorkspaceParams:
The trackExpense call in the existing test does not include createdWorkspaceParams (it only appears to accept specific fields).
Should verify: does the test's trackExpense function even populate createdWorkspaceParams ? Looking at the test, the params passed to trackExpense do not include createdWorkspaceParams. This means at runtime createdWorkspaceParams is likely undefined, which causes getWorkspaceCreatedAnalyticsEvent(undefined, undefined, ...) to return the STANDARD event. The test will pass, but there's no verification that the function works correctly when createdWorkspaceParams IS populated.
Fix: Add a test case for
categorizeTrackedExpensethat includes explicitcreatedWorkspaceParamswith sales-eligible values.
ikevin127
left a comment
ikevin127
left a comment
There was a problem hiding this comment.
🟢 LGTM - Tests well
NAB: The core logic is sound but the PR needs:
- Potential description correction
- 2-3 more test cases.
Once these are addressed, it's ready to merge ✅ See comments above for details.
melvin-bot
Bot
changed the title
|
🎯 @ikevin127, thanks for reviewing and testing this PR! 🎉 A payment issue will be created for your review once this PR is deployed to production. If payment is not needed (e.g., regression PR review fix etc), react with 👎 to this comment to prevent the payment issue from being created. |
|
@youssef-lr please handle comments from @ikevin127 above, for thoroughness |
…44512-app-split-workspace-created-event
Address review feedback:
- Add GoogleTagManagerPublishEventTest covering the real publishEvent
fbq('trackCustom') vs fbq('track') branching, which the existing
module-mocked test could not exercise.
- Document why the categorizeTrackedExpense path always publishes the
standard workspace_created event (it builds the workspace with the
TRACK_WORKSPACE intent, which is never sales-eligible).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Updated! |
|
Thank you! @lakchote LGTM, on to you. |
|
🚧 @lakchote has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/lakchote in version: 9.4.10-0 🚀
Bundle Size Analysis (Sentry): |
|
🤖 No help site changes required. I reviewed the changes in this PR and confirmed they do not require any updates to the help site files under Why: This PR changes only internal marketing/analytics conversion tracking — splitting the I also searched No draft PR was created since there is nothing to document. @youssef-lr, please confirm you agree no help site changes are needed. If you believe a user-facing behavior did change and should be documented, let me know and I'll draft the docs PR. |
|
🚀 Deployed to staging by https://github.com/lakchote in version: 9.4.12-0 🚀
Bundle Size Analysis (Sentry): |
|
🤖 Help site review — no docs changes required. This PR changes only internal conversion-pixel/analytics instrumentation for workspace creation:
None of this alters any user-facing behavior — as the PR itself notes, "the user-facing workspace-creation flow is unchanged." The events fire silently to ad-platform dashboards and are only meaningful to Marketing via GTM/Meta. The Expensify help site ( Conclusion: No help site article needs to be created or updated, so no draft docs PR was opened. @youssef-lr, if you believe any customer-facing documentation is affected that I may have missed, let me know and I'll create the docs PR. |
|
🚀 Deployed to staging by https://github.com/lakchote in version: 9.4.14-0 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.4.14-1 🚀
|
|
🤖 Payment issue created: #93958 |
5 participants
Explanation of Change
Splits the single
workspace_createdconversion event into two separate events across Google and Meta so the ad platforms can optimize bids toward higher-value leads. As part of this change, all Reddit (rdt) and LinkedIn (lintrk) pixel tracking is removed — we now only fire conversion events to Google (dataLayer) and Meta (fbq).New behavior
workspace_created_sales_eligible— fired when all of the following are true:Manage my teamThis event uses the standard Meta
Leadevent so Meta's pre-trained conversion models can optimize toward these high-value leads.workspace_created— fired for all other workspace creations. It now uses a custom Meta event (workspace_created) so the higher-volume, lower-value conversions don't dilute the standardLeadoptimization for the sales-eligible segment.Implementation
CONST.ANALYTICS.EVENT.WORKSPACE_CREATED_SALES_ELIGIBLEand markedWORKSPACE_CREATEDwithIS_CUSTOM_PIXEL_EVENTsoGoogleTagManager.publishEventemitsfbq('trackCustom', …)for it and a standardfbq('track', …)call for the sales-eligible event.rdt) and LinkedIn (lintrk) pixel tracking fromGoogleTagManager/index.ts, along with the now-unusedREDDIT/LINKEDINfields on the analytics event constants.getWorkspaceCreatedAnalyticsEvent, which encapsulates the sales-eligibility criteria and returns the correct event name.createWorkspace(Policy) andcategorizeTrackedExpense(TrackExpense).Fixed Issues
$ https://github.com/Expensify/Expensify/issues/644512
PROPOSAL: N/A (internal)
Tests
These changes are covered by automated unit tests (
tests/unit/getWorkspaceCreatedAnalyticsEventTest.ts,tests/unit/GoogleTagManagerPublishEventTest.ts, andtests/unit/GoogleTagManagerTest.tsx). To validate manually:USE_THIRD_PARTY_SCRIPTS=truein.env) and open the JS console.you@yourcompany.com), and create a workspace.[GTM] event publishedlog shows eventworkspace_created_sales_eligible, and on web thatfbq('track', 'Lead', …)fires.you@gmail.com), a company size of 1-4, or an intent other than Manage my team.workspace_created, and on web that the Meta call uses the custom event form (fbq('trackCustom', 'workspace_created', …)).Offline tests
Conversion events only fire when online (they depend on third-party pixel scripts). No offline-specific behavior changes.
QA Steps
Same as Tests. Conversion-pixel firing is best verified by Marketing against the GTM/Meta dashboards once deployed; the user-facing workspace-creation flow is unchanged.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos