mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-02-10 01:50:08 +00:00
fix: keep begin_checkout user_id reactive in subscription flows (#8726)
## 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)
This commit is contained in:
@@ -1,7 +1,7 @@
|
||||
import { createTestingPinia } from '@pinia/testing'
|
||||
import { flushPromises, mount } from '@vue/test-utils'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { computed, ref } from 'vue'
|
||||
import { computed, reactive, ref } from 'vue'
|
||||
import { createI18n } from 'vue-i18n'
|
||||
|
||||
import PricingTable from '@/platform/cloud/subscription/components/PricingTable.vue'
|
||||
@@ -15,6 +15,7 @@ const mockIsYearlySubscription = ref(false)
|
||||
const mockAccessBillingPortal = vi.fn()
|
||||
const mockReportError = vi.fn()
|
||||
const mockTrackBeginCheckout = vi.fn()
|
||||
const mockUserId = ref<string | undefined>('user-123')
|
||||
const mockGetFirebaseAuthHeader = vi.fn(() =>
|
||||
Promise.resolve({ Authorization: 'Bearer test-token' })
|
||||
)
|
||||
@@ -55,10 +56,11 @@ vi.mock('@/composables/useErrorHandling', () => ({
|
||||
}))
|
||||
|
||||
vi.mock('@/stores/firebaseAuthStore', () => ({
|
||||
useFirebaseAuthStore: () => ({
|
||||
getFirebaseAuthHeader: mockGetFirebaseAuthHeader,
|
||||
userId: 'user-123'
|
||||
}),
|
||||
useFirebaseAuthStore: () =>
|
||||
reactive({
|
||||
getFirebaseAuthHeader: mockGetFirebaseAuthHeader,
|
||||
userId: computed(() => mockUserId.value)
|
||||
}),
|
||||
FirebaseAuthStoreError: class extends Error {}
|
||||
}))
|
||||
|
||||
@@ -151,6 +153,7 @@ describe('PricingTable', () => {
|
||||
mockIsActiveSubscription.value = false
|
||||
mockSubscriptionTier.value = null
|
||||
mockIsYearlySubscription.value = false
|
||||
mockUserId.value = 'user-123'
|
||||
mockTrackBeginCheckout.mockReset()
|
||||
vi.mocked(global.fetch).mockResolvedValue({
|
||||
ok: true,
|
||||
@@ -201,6 +204,33 @@ describe('PricingTable', () => {
|
||||
expect(mockAccessBillingPortal).toHaveBeenCalledWith('pro-yearly')
|
||||
})
|
||||
|
||||
it('should use the latest userId value when it changes after mount', async () => {
|
||||
mockIsActiveSubscription.value = true
|
||||
mockSubscriptionTier.value = 'STANDARD'
|
||||
mockUserId.value = 'user-early'
|
||||
|
||||
const wrapper = createWrapper()
|
||||
await flushPromises()
|
||||
|
||||
mockUserId.value = 'user-late'
|
||||
|
||||
const creatorButton = wrapper
|
||||
.findAll('button')
|
||||
.find((btn) => btn.text().includes('Creator'))
|
||||
|
||||
await creatorButton?.trigger('click')
|
||||
await flushPromises()
|
||||
|
||||
expect(mockTrackBeginCheckout).toHaveBeenCalledTimes(1)
|
||||
expect(mockTrackBeginCheckout).toHaveBeenCalledWith({
|
||||
user_id: 'user-late',
|
||||
tier: 'creator',
|
||||
cycle: 'yearly',
|
||||
checkout_type: 'change',
|
||||
previous_tier: 'standard'
|
||||
})
|
||||
})
|
||||
|
||||
it('should not call accessBillingPortal when clicking current plan', async () => {
|
||||
mockIsActiveSubscription.value = true
|
||||
mockSubscriptionTier.value = 'CREATOR'
|
||||
|
||||
@@ -243,6 +243,7 @@
|
||||
|
||||
<script setup lang="ts">
|
||||
import { cn } from '@comfyorg/tailwind-utils'
|
||||
import { storeToRefs } from 'pinia'
|
||||
import Popover from 'primevue/popover'
|
||||
import SelectButton from 'primevue/selectbutton'
|
||||
import type { ToggleButtonPassThroughMethodOptions } from 'primevue/togglebutton'
|
||||
@@ -333,7 +334,7 @@ const tiers: PricingTierConfig[] = [
|
||||
const { isActiveSubscription, subscriptionTier, isYearlySubscription } =
|
||||
useSubscription()
|
||||
const telemetry = useTelemetry()
|
||||
const { userId } = useFirebaseAuthStore()
|
||||
const { userId } = storeToRefs(useFirebaseAuthStore())
|
||||
const { accessBillingPortal, reportError } = useFirebaseAuthActions()
|
||||
const { wrapWithErrorHandlingAsync } = useErrorHandling()
|
||||
|
||||
@@ -415,9 +416,9 @@ const handleSubscribe = wrapWithErrorHandlingAsync(
|
||||
try {
|
||||
if (isActiveSubscription.value) {
|
||||
const checkoutAttribution = getCheckoutAttribution()
|
||||
if (userId) {
|
||||
if (userId.value) {
|
||||
telemetry?.trackBeginCheckout({
|
||||
user_id: userId,
|
||||
user_id: userId.value,
|
||||
tier: tierKey,
|
||||
cycle: currentBillingCycle.value,
|
||||
checkout_type: 'change',
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { computed, reactive } from 'vue'
|
||||
|
||||
import { performSubscriptionCheckout } from './subscriptionCheckoutUtil'
|
||||
|
||||
@@ -15,7 +16,7 @@ const {
|
||||
mockGetAuthHeader: vi.fn(() =>
|
||||
Promise.resolve({ Authorization: 'Bearer test-token' })
|
||||
),
|
||||
mockUserId: { value: 'user-123' },
|
||||
mockUserId: { value: 'user-123' as string | undefined },
|
||||
mockIsCloud: { value: true },
|
||||
mockGetCheckoutAttribution: vi.fn(() => ({
|
||||
ga_client_id: 'ga-client-id',
|
||||
@@ -32,12 +33,12 @@ vi.mock('@/platform/telemetry', () => ({
|
||||
}))
|
||||
|
||||
vi.mock('@/stores/firebaseAuthStore', () => ({
|
||||
useFirebaseAuthStore: vi.fn(() => ({
|
||||
getFirebaseAuthHeader: mockGetAuthHeader,
|
||||
get userId() {
|
||||
return mockUserId.value
|
||||
}
|
||||
})),
|
||||
useFirebaseAuthStore: vi.fn(() =>
|
||||
reactive({
|
||||
getFirebaseAuthHeader: mockGetAuthHeader,
|
||||
userId: computed(() => mockUserId.value)
|
||||
})
|
||||
),
|
||||
FirebaseAuthStoreError: class extends Error {}
|
||||
}))
|
||||
|
||||
@@ -53,6 +54,15 @@ vi.mock('@/platform/telemetry/utils/checkoutAttribution', () => ({
|
||||
|
||||
global.fetch = vi.fn()
|
||||
|
||||
function createDeferred<T>() {
|
||||
let resolve: (value: T) => void = () => {}
|
||||
const promise = new Promise<T>((res) => {
|
||||
resolve = res
|
||||
})
|
||||
|
||||
return { promise, resolve }
|
||||
}
|
||||
|
||||
describe('performSubscriptionCheckout', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
@@ -105,4 +115,35 @@ describe('performSubscriptionCheckout', () => {
|
||||
)
|
||||
expect(openSpy).toHaveBeenCalledWith(checkoutUrl, '_blank')
|
||||
})
|
||||
|
||||
it('uses the latest userId when it changes after checkout starts', async () => {
|
||||
const checkoutUrl = 'https://checkout.stripe.com/test'
|
||||
const openSpy = vi.spyOn(window, 'open').mockImplementation(() => null)
|
||||
const authHeader = createDeferred<{ Authorization: string }>()
|
||||
|
||||
mockUserId.value = 'user-early'
|
||||
mockGetAuthHeader.mockImplementationOnce(() => authHeader.promise)
|
||||
vi.mocked(global.fetch).mockResolvedValue({
|
||||
ok: true,
|
||||
json: async () => ({ checkout_url: checkoutUrl })
|
||||
} as Response)
|
||||
|
||||
const checkoutPromise = performSubscriptionCheckout('pro', 'yearly', true)
|
||||
|
||||
mockUserId.value = 'user-late'
|
||||
authHeader.resolve({ Authorization: 'Bearer test-token' })
|
||||
|
||||
await checkoutPromise
|
||||
|
||||
expect(mockTelemetry.trackBeginCheckout).toHaveBeenCalledTimes(1)
|
||||
expect(mockTelemetry.trackBeginCheckout).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
user_id: 'user-late',
|
||||
tier: 'pro',
|
||||
cycle: 'yearly',
|
||||
checkout_type: 'new'
|
||||
})
|
||||
)
|
||||
expect(openSpy).toHaveBeenCalledWith(checkoutUrl, '_blank')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
import { storeToRefs } from 'pinia'
|
||||
|
||||
import { getComfyApiBaseUrl } from '@/config/comfyApi'
|
||||
import { t } from '@/i18n'
|
||||
import { isCloud } from '@/platform/distribution/types'
|
||||
@@ -37,9 +39,10 @@ export async function performSubscriptionCheckout(
|
||||
): Promise<void> {
|
||||
if (!isCloud) return
|
||||
|
||||
const { getFirebaseAuthHeader, userId } = useFirebaseAuthStore()
|
||||
const firebaseAuthStore = useFirebaseAuthStore()
|
||||
const { userId } = storeToRefs(firebaseAuthStore)
|
||||
const telemetry = useTelemetry()
|
||||
const authHeader = await getFirebaseAuthHeader()
|
||||
const authHeader = await firebaseAuthStore.getFirebaseAuthHeader()
|
||||
|
||||
if (!authHeader) {
|
||||
throw new FirebaseAuthStoreError(t('toastMessages.userNotAuthenticated'))
|
||||
@@ -84,9 +87,9 @@ export async function performSubscriptionCheckout(
|
||||
const data = await response.json()
|
||||
|
||||
if (data.checkout_url) {
|
||||
if (userId) {
|
||||
if (userId.value) {
|
||||
telemetry?.trackBeginCheckout({
|
||||
user_id: userId,
|
||||
user_id: userId.value,
|
||||
tier: tierKey,
|
||||
cycle: currentBillingCycle,
|
||||
checkout_type: 'new',
|
||||
|
||||
Reference in New Issue
Block a user