fix: keep begin_checkout user_id reactive in subscription flows#8726
fix: keep begin_checkout user_id reactive in subscription flows#8726benceruleanlu merged 2 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughRefactors Firebase auth store usage to use Pinia's Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎭 Playwright Tests:
|
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 02/07/2026, 11:25:05 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 22.2 kB (baseline 22.2 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 854 kB (baseline 854 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 69 kB (baseline 69 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 409 kB (baseline 409 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 11 added / 11 removed User & Accounts — 16.1 kB (baseline 16.1 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed Editors & Dialogs — 781 B (baseline 781 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 1 added / 1 removed UI Components — 36.6 kB (baseline 36.6 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 2.11 MB (baseline 2.11 MB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 12 added / 12 removed Utilities & Hooks — 237 kB (baseline 237 kB) • 🔴 +132 BHelpers, composables, and utility bundles
Status: 12 added / 12 removed Vendor & Third-Party — 8.77 MB (baseline 8.77 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.15 MB (baseline 7.15 MB) • 🔴 +43 BBundles that do not match a named category
Status: 54 added / 54 removed |
There was a problem hiding this comment.
Pull request overview
This PR fixes stale user_id values in begin_checkout telemetry for cloud subscription flows by ensuring userId is read reactively at event-dispatch time, matching delayed auth-state updates.
Changes:
- Switch subscription checkout telemetry to use
storeToRefs(useFirebaseAuthStore())and readuserId.valuewhen callingtrackBeginCheckout. - Update
performSubscriptionCheckoutto avoid destructured store snapshots and callgetFirebaseAuthHeader()from the store instance. - Add regression tests that mutate
userIdafter mount / after checkout starts and assert telemetry uses the latest value.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/platform/cloud/subscription/utils/subscriptionCheckoutUtil.ts |
Make userId reactive via storeToRefs and read .value at telemetry dispatch time. |
src/platform/cloud/subscription/utils/subscriptionCheckoutUtil.test.ts |
Add deferred-auth regression test verifying late userId updates are reflected. |
src/platform/cloud/subscription/components/PricingTable.vue |
Use storeToRefs for reactive userId reads in the “change plan” telemetry path. |
src/platform/cloud/subscription/components/PricingTable.test.ts |
Add regression test verifying userId updates after mount are used in telemetry. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/platform/cloud/subscription/utils/subscriptionCheckoutUtil.ts (1)
64-84:⚠️ Potential issue | 🟡 MinorDouble
response.text()call in error handling could throw.If the response body stream has already been consumed by the failed
response.json()call on Line 67, the subsequentresponse.text()call on Line 72 may throw because the body stream is already read. In practice, most runtime implementations allow only one body read — the second read throwsTypeError: body stream already read.This is a pre-existing issue (not introduced by this PR), but it's worth noting since it's in the changed file.
Proposed fix: clone the response or read text first
if (!response.ok) { let errorMessage = 'Failed to initiate checkout' try { - const errorData = await response.json() + const errorText = await response.text() + const errorData = JSON.parse(errorText) errorMessage = errorData.message || errorMessage } catch { - // If JSON parsing fails, try to get text response or use HTTP status - try { - const errorText = await response.text() - errorMessage = - errorText || `HTTP ${response.status} ${response.statusText}` - } catch { - errorMessage = `HTTP ${response.status} ${response.statusText}` - } + // errorText is not accessible here, fall back to HTTP status + errorMessage = `HTTP ${response.status} ${response.statusText}` }Alternatively, restructure to read text once, then try to parse as JSON:
if (!response.ok) { let errorMessage = `HTTP ${response.status} ${response.statusText}` try { + const errorText = await response.text() + try { + const errorData = JSON.parse(errorText) + errorMessage = errorData.message || errorMessage + } catch { + errorMessage = errorText || errorMessage + } - const errorData = await response.json() - errorMessage = errorData.message || errorMessage } catch { - try { - const errorText = await response.text() - errorMessage = - errorText || `HTTP ${response.status} ${response.statusText}` - } catch { - errorMessage = `HTTP ${response.status} ${response.statusText}` - } + // Body unreadable, keep HTTP status fallback }
🧹 Nitpick comments (1)
src/platform/cloud/subscription/utils/subscriptionCheckoutUtil.test.ts (1)
36-41: Mock store relies on lazy computed evaluation — subtle but correct.The mock returns a fresh
reactive({ userId: computed(() => mockUserId.value) })on eachuseFirebaseAuthStore()call. SincemockUserIdis a plain (non-reactive) object, Vue won't track it as a dependency — meaning the computed will not re-evaluate after its first read. The new test on Line 119 works because the computed is lazily created and never read until aftermockUserId.valueis mutated.This is fine for the current single-read scenario, but be aware that if the SUT ever reads
userId.valuetwice (e.g., once for the guard and once for the payload), the second read would return the stale cached value. If that ever becomes the case, wrapmockUserIdwithref()instead:- mockUserId: { value: 'user-123' as string | undefined }, + mockUserId: { value: 'user-123' as string | undefined }, // works for single-read; switch to ref() if multi-read neededNo action needed now.
| Promise.resolve({ Authorization: 'Bearer test-token' }) | ||
| ), | ||
| mockUserId: { value: 'user-123' }, | ||
| mockUserId: { value: 'user-123' as string | undefined }, |
There was a problem hiding this comment.
no need for this assertion
| mockUserId: { value: 'user-123' as string | undefined }, | |
| mockUserId: { value: 'user-123' }, |
## Summary Use reactive `userId` reads for `begin_checkout` telemetry so delayed auth state updates are reflected at event time instead of using a stale snapshot. ## Changes - **What**: switched subscription checkout telemetry paths to `storeToRefs(useFirebaseAuthStore())` and read `userId.value` when dispatching `trackBeginCheckout`. - **What**: added regression tests that mutate `userId` after setup / after checkout starts and assert telemetry uses the updated ID. ## Review Focus - Verify `PricingTable` and `performSubscriptionCheckout` still emit exactly one `begin_checkout` event per action, with `checkout_type: change` and `checkout_type: new` in their respective paths. - Verify the new tests would fail with stale store destructuring (manually validated during development). ## Screenshots (if applicable) N/A ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8726-fix-keep-begin_checkout-user_id-reactive-in-subscription-flows-3006d73d365081888c84c0335ab52e09) by [Unito](https://www.unito.io) (cherry picked from commit 815be49)
Summary
Use reactive
userIdreads forbegin_checkouttelemetry so delayed auth state updates are reflected at event time instead of using a stale snapshot.Changes
storeToRefs(useFirebaseAuthStore())and readuserId.valuewhen dispatchingtrackBeginCheckout.userIdafter setup / after checkout starts and assert telemetry uses the updated ID.Review Focus
PricingTableandperformSubscriptionCheckoutstill emit exactly onebegin_checkoutevent per action, withcheckout_type: changeandcheckout_type: newin their respective paths.Screenshots (if applicable)
N/A
┆Issue is synchronized with this Notion page by Unito