mirror of
https://github.com/supabase/supabase.git
synced 2026-10-10 20:05:06 +03:00
## I have read the [CONTRIBUTING.md](https://github.com/supabase/supabase/blob/master/CONTRIBUTING.md) file. YES ## What kind of change does this PR introduce? Feature + two follow-on fixes — small, scoped to telemetry / experiment plumbing. ## What is the current behavior? PostHog feature flags evaluated in Studio only have access to the `gotrue_id` person property (set in `useTelemetryIdentify`) and the `organization`/`project` group associations from pageviews. Flags can't target users by org membership without a behavioral cohort, which refreshes on a ~hourly schedule and lags behind real-time signup state. This is blocking the rollout of the `dataApiRevokeOnCreateDefault` experiment ahead of the May 30 default-privileges breaking change — we need to target brand-new dashboard signups with no prior org membership, and there's no person property to filter on. ## What is the new behavior? Three changes, scoped tightly to make experiment targeting reliable for brand-new signups: ### 1. Mirror `org_count` to a PostHog person property (`apps/studio/lib/telemetry.tsx`) The Studio `Telemetry` component now mirrors the user's current org-list length to a PostHog person property `org_count` via `posthog.identify(user.id, { org_count })`. The effect: - Subscribes to `useOrganizationsQuery` (shares the same React Query cache as `useSelectedOrganizationQuery`, so no extra network requests). - Dedupes via a ref keyed on `{ userId, orgCount }` so we only call identify when the value actually changes — handles user-switch (logout/login as different user with same count) correctly. - Generic enough to be useful beyond this experiment — analytics segmentation by org membership, future flags that depend on multi-org behavior, etc. ### 2. Merge pre-init identify properties (`packages/common/posthog-client.ts`) The previous `pendingIdentification` slot was a single-write buffer — calling `posthogClient.identify()` before the PostHog SDK initialized would overwrite any prior queued identify. Latent until this PR added a second identify caller (`org_count`), which exposed the last-write-wins behavior on first-visitor-before-consent flows. Now merges properties across pre-init calls for the same user so both `{ gotrue_id }` and `{ org_count }` land on the person record when the SDK flushes. Caught during Codex review. ### 3. Gate the exposure event on `org_count` being present (`apps/studio/hooks/misc/useDataApiRevokeOnCreateDefault.ts`) `useTrackDefaultPrivilegesExposure` previously fired on the first non-undefined value of the `dataApiRevokeOnCreateDefault` flag. For brand-new signups, this races the `org_count` identify: the initial `/flags/` response (before targeting can match) returns the untargeted variant, the exposure locks it in via `hasTracked`, then our identify fires and a subsequent `/flags/` refresh updates the flag — but the exposure has already recorded the wrong variant. Fix: gate the exposure on `org_count` being present on the SDK person, subscribing via `onFeatureFlags` so we pick up the post-identify `/flags/` response. Adds `posthogClient.getPersonProperty` as the local-state reader. Without this, the experiment would have a ~5-15% noise floor on cohort assignment for new signups. ## Verification End-to-end verified locally against the staging PostHog project (34343): - Local Studio's PostHog SDK has `$stored_person_properties: { gotrue_id: <uuid>, org_count: 1 }` after sign-in. - Both `$set` events landed server-side within ~300ms of each other, and the staging person record now shows `org_count = 1.0` with `gotrue_id` preserved. - Targeting query `person.properties.org_count == 1` works end-to-end against staging. ## Additional context Ref: [GROWTH-853](https://linear.app/supabase/issue/GROWTH-853) Targeting plan for the flag once shipped: `person.org_count == 1` plus a behavioral filter on recent `sign_up` event, at 5% rollout. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Telemetry now records and syncs the user's organization count as an analytics person property and avoids redundant identifications when unchanged. * Analytics client now merges queued identification properties made before initialization and exposes a method to read stored person properties. * **Bug Fixes** * Tracking now waits for organization-count readiness before firing certain exposure events to prevent missing data. * **Tests** * Added/updated tests to cover person-property behavior and gating logic. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/supabase/supabase/pull/45946) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
156 lines
5.4 KiB
TypeScript
156 lines
5.4 KiB
TypeScript
import { renderHook } from '@testing-library/react'
|
|
import { posthogClient } from 'common'
|
|
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
|
|
|
import {
|
|
useDataApiRevokeOnCreateDefaultEnabled,
|
|
useTrackDefaultPrivilegesExposure,
|
|
} from '../useDataApiRevokeOnCreateDefault'
|
|
import { usePHFlag } from '@/hooks/ui/useFlag'
|
|
import * as constants from '@/lib/constants'
|
|
import { useTrack } from '@/lib/telemetry/track'
|
|
|
|
vi.mock('@/hooks/ui/useFlag', () => ({
|
|
usePHFlag: vi.fn(),
|
|
}))
|
|
|
|
vi.mock('@/lib/constants', async () => {
|
|
const actual = await vi.importActual<typeof import('@/lib/constants')>('@/lib/constants')
|
|
return {
|
|
...actual,
|
|
IS_TEST_ENV: false,
|
|
}
|
|
})
|
|
|
|
vi.mock('@/lib/telemetry/track', () => ({
|
|
useTrack: vi.fn(),
|
|
}))
|
|
|
|
vi.mock('common', async () => {
|
|
const actual = await vi.importActual<typeof import('common')>('common')
|
|
return {
|
|
...actual,
|
|
posthogClient: {
|
|
getPersonProperty: vi.fn(),
|
|
onFeatureFlags: vi.fn(() => () => {}),
|
|
},
|
|
}
|
|
})
|
|
|
|
describe('useDataApiRevokeOnCreateDefaultEnabled', () => {
|
|
afterEach(() => {
|
|
vi.restoreAllMocks()
|
|
vi.mocked(constants, { partial: true }).IS_TEST_ENV = false
|
|
})
|
|
|
|
it('returns false when the PostHog flag is undefined (not yet resolved)', () => {
|
|
vi.mocked(usePHFlag).mockReturnValue(undefined)
|
|
const { result } = renderHook(() => useDataApiRevokeOnCreateDefaultEnabled())
|
|
expect(result.current).toBe(false)
|
|
})
|
|
|
|
it('returns false when the PostHog flag is false', () => {
|
|
vi.mocked(usePHFlag).mockReturnValue(false)
|
|
const { result } = renderHook(() => useDataApiRevokeOnCreateDefaultEnabled())
|
|
expect(result.current).toBe(false)
|
|
})
|
|
|
|
it('returns true when the PostHog flag is true', () => {
|
|
vi.mocked(usePHFlag).mockReturnValue(true)
|
|
const { result } = renderHook(() => useDataApiRevokeOnCreateDefaultEnabled())
|
|
expect(result.current).toBe(true)
|
|
})
|
|
|
|
it('returns false in test env regardless of flag value', () => {
|
|
vi.mocked(constants, { partial: true }).IS_TEST_ENV = true
|
|
vi.mocked(usePHFlag).mockReturnValue(true)
|
|
const { result } = renderHook(() => useDataApiRevokeOnCreateDefaultEnabled())
|
|
expect(result.current).toBe(false)
|
|
})
|
|
})
|
|
|
|
describe('useTrackDefaultPrivilegesExposure', () => {
|
|
const track = vi.fn()
|
|
|
|
beforeEach(() => {
|
|
vi.mocked(useTrack).mockReturnValue(track)
|
|
// Default: org_count is set on the person — most tests want to exercise
|
|
// the post-targeting-resolution behavior. The gate-blocked case has its
|
|
// own test below.
|
|
vi.mocked(posthogClient.getPersonProperty).mockReturnValue(1)
|
|
vi.mocked(posthogClient.onFeatureFlags).mockReturnValue(() => {})
|
|
})
|
|
|
|
afterEach(() => {
|
|
vi.clearAllMocks()
|
|
})
|
|
|
|
it('does not fire while the flag is undefined', () => {
|
|
vi.mocked(usePHFlag).mockReturnValue(undefined)
|
|
renderHook(() => useTrackDefaultPrivilegesExposure({ surface: 'main', dataApiEnabled: true }))
|
|
expect(track).not.toHaveBeenCalled()
|
|
})
|
|
|
|
it('does not fire while org_count is missing from the SDK person', () => {
|
|
vi.mocked(usePHFlag).mockReturnValue(true)
|
|
vi.mocked(posthogClient.getPersonProperty).mockReturnValue(undefined)
|
|
renderHook(() => useTrackDefaultPrivilegesExposure({ surface: 'main', dataApiEnabled: true }))
|
|
expect(track).not.toHaveBeenCalled()
|
|
})
|
|
|
|
it('fires once when the flag resolves to true on the main surface', () => {
|
|
vi.mocked(usePHFlag).mockReturnValue(true)
|
|
renderHook(() => useTrackDefaultPrivilegesExposure({ surface: 'main', dataApiEnabled: true }))
|
|
expect(track).toHaveBeenCalledTimes(1)
|
|
expect(track).toHaveBeenCalledWith('project_creation_default_privileges_exposed', {
|
|
surface: 'main',
|
|
dataApiEnabled: true,
|
|
dataApiRevokeOnCreateDefaultEnabled: true,
|
|
})
|
|
})
|
|
|
|
it('fires once when the flag resolves to false on the main surface', () => {
|
|
vi.mocked(usePHFlag).mockReturnValue(false)
|
|
renderHook(() => useTrackDefaultPrivilegesExposure({ surface: 'main', dataApiEnabled: false }))
|
|
expect(track).toHaveBeenCalledTimes(1)
|
|
expect(track).toHaveBeenCalledWith('project_creation_default_privileges_exposed', {
|
|
surface: 'main',
|
|
dataApiEnabled: false,
|
|
dataApiRevokeOnCreateDefaultEnabled: false,
|
|
})
|
|
})
|
|
|
|
it('omits dataApiEnabled on the vercel surface', () => {
|
|
vi.mocked(usePHFlag).mockReturnValue(true)
|
|
renderHook(() => useTrackDefaultPrivilegesExposure({ surface: 'vercel' }))
|
|
expect(track).toHaveBeenCalledWith('project_creation_default_privileges_exposed', {
|
|
surface: 'vercel',
|
|
dataApiRevokeOnCreateDefaultEnabled: true,
|
|
})
|
|
})
|
|
|
|
it('deduplicates across re-renders', () => {
|
|
vi.mocked(usePHFlag).mockReturnValue(true)
|
|
const { rerender } = renderHook(() =>
|
|
useTrackDefaultPrivilegesExposure({ surface: 'main', dataApiEnabled: true })
|
|
)
|
|
rerender()
|
|
rerender()
|
|
expect(track).toHaveBeenCalledTimes(1)
|
|
})
|
|
|
|
it('does not re-fire if the flag flips after initial exposure', () => {
|
|
vi.mocked(usePHFlag).mockReturnValue(false)
|
|
const { rerender } = renderHook(() =>
|
|
useTrackDefaultPrivilegesExposure({ surface: 'main', dataApiEnabled: true })
|
|
)
|
|
vi.mocked(usePHFlag).mockReturnValue(true)
|
|
rerender()
|
|
expect(track).toHaveBeenCalledTimes(1)
|
|
expect(track).toHaveBeenCalledWith(
|
|
'project_creation_default_privileges_exposed',
|
|
expect.objectContaining({ dataApiRevokeOnCreateDefaultEnabled: false })
|
|
)
|
|
})
|
|
})
|