mirror of
https://github.com/supabase/supabase.git
synced 2026-10-11 12:25:05 +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? Bug fix. ## What is the current behavior? This PR addresses three issues with the implementation of the `dataApiRevokeOnCreateDefault` experiment ([GROWTH-858](https://linear.app/supabase/issue/GROWTH-858)) on the frontend side: 1. The `project_creation_default_privileges_exposed` event payload captures `dataApiEnabled`, which is the parent "Enable Data API" toggle. That value defaults to `true` for everyone in both arms, which doesn't help understand how people are interacting with the form. 2. The hook itself was gating on PostHog JS SDK values rather than our backend server values being sent from `/telemetry/feature-flags`. 3. The form on `/new/[slug]` captures `dataApiDefaultPrivileges` defaults once at mount via react-hook-form's `defaultValues`. If the flag is still loading when the page mounts, `useDataApiRevokeOnCreateDefaultEnabled()` returns `false` (coerced from undefined), and the form locks the field to the legacy default of `true`. The flag later resolving has no effect, and treatment users get the legacy default visually and in the exposure event. ## What is the new behavior? 1. Main-surface payload now sends `dataApiDefaultPrivileges` — the form field the experiment actually controls (`true` = legacy grants kept, `false` = revoked on create). Post-fix data will let us read out whether treatment users actually got the new default. 2. Hook is simplified: drop `orgCountReady`, drop the `onFeatureFlags` subscription, drop the `posthogClient` import. It now fires once when the flag resolves, period. Vercel surface is unchanged (still no `dataApiDefaultPrivileges` since there's no user-facing toggle there). Tests updated. 3. New useEffect in `/new/[slug]` watches the raw flag value and syncs `dataApiDefaultPrivileges` to the correct experiment-driven default when the flag resolves, gated on `getFieldState(...).isDirty` so we don't clobber intentional user input. ## Additional context Backend half of this fix is at supabase/platform#32933 (passes `org_count` and `signup_timestamp` in `personProperties` so the audience filter actually evaluates correctly). Both PRs are needed for the experiment to bucket at 5% and be measurable. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Changes** * Telemetry now records the selected default-privileges setting (dataApiDefaultPrivileges) in project-creation events; the previous dataApiEnabled field was removed. * Project-creation flows apply the experiment-driven default for that setting once the experiment resolves, but they do not overwrite user-edited choices. Vercel new-project flow syncs with the experiment until the user changes the checkbox. * **Tests** * Updated tests to validate tracking, deduplication, and sync/timing behaviors for dataApiDefaultPrivileges. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/supabase/supabase/pull/46085?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
256 lines
7.8 KiB
TypeScript
256 lines
7.8 KiB
TypeScript
import { renderHook } from '@testing-library/react'
|
|
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(),
|
|
}))
|
|
|
|
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)
|
|
})
|
|
|
|
afterEach(() => {
|
|
vi.clearAllMocks()
|
|
})
|
|
|
|
it('does not fire while the flag is undefined', () => {
|
|
vi.mocked(usePHFlag).mockReturnValue(undefined)
|
|
renderHook(() =>
|
|
useTrackDefaultPrivilegesExposure({
|
|
surface: 'main',
|
|
dataApiDefaultPrivileges: true,
|
|
hasUserModified: false,
|
|
})
|
|
)
|
|
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',
|
|
dataApiDefaultPrivileges: false,
|
|
hasUserModified: false,
|
|
})
|
|
)
|
|
expect(track).toHaveBeenCalledTimes(1)
|
|
expect(track).toHaveBeenCalledWith(
|
|
'project_creation_default_privileges_exposed',
|
|
{
|
|
surface: 'main',
|
|
dataApiDefaultPrivileges: false,
|
|
dataApiRevokeOnCreateDefaultEnabled: true,
|
|
},
|
|
undefined
|
|
)
|
|
})
|
|
|
|
it('fires once when the flag resolves to false on the main surface', () => {
|
|
vi.mocked(usePHFlag).mockReturnValue(false)
|
|
renderHook(() =>
|
|
useTrackDefaultPrivilegesExposure({
|
|
surface: 'main',
|
|
dataApiDefaultPrivileges: true,
|
|
hasUserModified: false,
|
|
})
|
|
)
|
|
expect(track).toHaveBeenCalledTimes(1)
|
|
expect(track).toHaveBeenCalledWith(
|
|
'project_creation_default_privileges_exposed',
|
|
{
|
|
surface: 'main',
|
|
dataApiDefaultPrivileges: true,
|
|
dataApiRevokeOnCreateDefaultEnabled: false,
|
|
},
|
|
undefined
|
|
)
|
|
})
|
|
|
|
it('does not fire while the form value is stale relative to the flag (waits for sync)', () => {
|
|
// Race: flag just resolved to true (treatment), but the caller-side sync
|
|
// useEffect hasn't run yet, so the form value is still the legacy `true`.
|
|
// Without the convergence gate, exposure would fire with the wrong value.
|
|
vi.mocked(usePHFlag).mockReturnValue(true)
|
|
renderHook(() =>
|
|
useTrackDefaultPrivilegesExposure({
|
|
surface: 'main',
|
|
dataApiDefaultPrivileges: true,
|
|
hasUserModified: false,
|
|
})
|
|
)
|
|
expect(track).not.toHaveBeenCalled()
|
|
})
|
|
|
|
it('fires on the next render after the form syncs to match the flag', () => {
|
|
vi.mocked(usePHFlag).mockReturnValue(true)
|
|
const { rerender } = renderHook(
|
|
({ dataApiDefaultPrivileges }: { dataApiDefaultPrivileges: boolean }) =>
|
|
useTrackDefaultPrivilegesExposure({
|
|
surface: 'main',
|
|
dataApiDefaultPrivileges,
|
|
hasUserModified: false,
|
|
}),
|
|
{ initialProps: { dataApiDefaultPrivileges: true } }
|
|
)
|
|
expect(track).not.toHaveBeenCalled()
|
|
|
|
// Caller-side sync runs and updates the form value to !flag.
|
|
rerender({ dataApiDefaultPrivileges: false })
|
|
expect(track).toHaveBeenCalledTimes(1)
|
|
expect(track).toHaveBeenCalledWith(
|
|
'project_creation_default_privileges_exposed',
|
|
expect.objectContaining({
|
|
dataApiDefaultPrivileges: false,
|
|
dataApiRevokeOnCreateDefaultEnabled: true,
|
|
}),
|
|
undefined
|
|
)
|
|
})
|
|
|
|
it('fires immediately with the dirty value when the user has modified the field', () => {
|
|
// User toggled the checkbox before the flag resolved, dirtying the field.
|
|
// The sync gate is bypassed; exposure fires with the user's explicit value.
|
|
vi.mocked(usePHFlag).mockReturnValue(true)
|
|
renderHook(() =>
|
|
useTrackDefaultPrivilegesExposure({
|
|
surface: 'main',
|
|
dataApiDefaultPrivileges: true, // form value disagrees with !flag=false
|
|
hasUserModified: true,
|
|
})
|
|
)
|
|
expect(track).toHaveBeenCalledTimes(1)
|
|
expect(track).toHaveBeenCalledWith(
|
|
'project_creation_default_privileges_exposed',
|
|
{
|
|
surface: 'main',
|
|
dataApiDefaultPrivileges: true,
|
|
dataApiRevokeOnCreateDefaultEnabled: true,
|
|
},
|
|
undefined
|
|
)
|
|
})
|
|
|
|
it('fires on the vercel surface with the form-flag convergence gate', () => {
|
|
vi.mocked(usePHFlag).mockReturnValue(true)
|
|
renderHook(() =>
|
|
useTrackDefaultPrivilegesExposure({
|
|
surface: 'vercel',
|
|
orgSlug: 'acme-org',
|
|
dataApiDefaultPrivileges: false,
|
|
hasUserModified: false,
|
|
})
|
|
)
|
|
expect(track).toHaveBeenCalledWith(
|
|
'project_creation_default_privileges_exposed',
|
|
{
|
|
surface: 'vercel',
|
|
dataApiDefaultPrivileges: false,
|
|
dataApiRevokeOnCreateDefaultEnabled: true,
|
|
},
|
|
{ organization: 'acme-org' }
|
|
)
|
|
})
|
|
|
|
it('skips emission on vercel surface when orgSlug is missing', () => {
|
|
vi.mocked(usePHFlag).mockReturnValue(true)
|
|
renderHook(() =>
|
|
useTrackDefaultPrivilegesExposure({
|
|
surface: 'vercel',
|
|
orgSlug: undefined,
|
|
dataApiDefaultPrivileges: false,
|
|
hasUserModified: false,
|
|
})
|
|
)
|
|
expect(track).not.toHaveBeenCalled()
|
|
})
|
|
|
|
it('deduplicates across re-renders', () => {
|
|
vi.mocked(usePHFlag).mockReturnValue(true)
|
|
const { rerender } = renderHook(() =>
|
|
useTrackDefaultPrivilegesExposure({
|
|
surface: 'main',
|
|
dataApiDefaultPrivileges: false,
|
|
hasUserModified: false,
|
|
})
|
|
)
|
|
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',
|
|
dataApiDefaultPrivileges: true,
|
|
hasUserModified: false,
|
|
})
|
|
)
|
|
vi.mocked(usePHFlag).mockReturnValue(true)
|
|
rerender()
|
|
expect(track).toHaveBeenCalledTimes(1)
|
|
expect(track).toHaveBeenCalledWith(
|
|
'project_creation_default_privileges_exposed',
|
|
expect.objectContaining({ dataApiRevokeOnCreateDefaultEnabled: false }),
|
|
undefined
|
|
)
|
|
})
|
|
})
|