Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(toolbar): support feature flag payload overrides in the flag toolbar #28058

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open

feat(toolbar): support feature flag payload overrides in the flag toolbar #28058

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
a006864
added support for setting payload overrides
dmarticus Jan 29, 2025
9bbe892
use latest version from posthog-js
dmarticus Jan 29, 2025
cf1cd93
merge master
dmarticus Jan 29, 2025
8f81c61
Update UI snapshots for `chromium` (2)
github-actions[bot] Jan 29, 2025
05850ce
greptile feedback
dmarticus Jan 29, 2025
b32cb58
Merge branch 'feat/support-payload-overrides-in-flag-toolbar' of gith…
dmarticus Jan 29, 2025
93c2630
Update UI snapshots for `chromium` (1)
github-actions[bot] Jan 29, 2025
5453459
Update UI snapshots for `chromium` (1)
github-actions[bot] Jan 29, 2025
7abe2aa
use latest posthog-js
dmarticus Jan 30, 2025
d081f45
Merge branch 'feat/support-payload-overrides-in-flag-toolbar' of gith…
dmarticus Jan 30, 2025
4629b76
heckin conflicts
dmarticus Jan 30, 2025
5cc6e60
fix docs
dmarticus Jan 30, 2025
6b308cc
Merge branch 'master' into feat/support-payload-overrides-in-flag-too…
dmarticus Jan 30, 2025
d72bd7d
Update UI snapshots for `chromium` (1)
github-actions[bot] Jan 30, 2025
c2bd8ab
Update UI snapshots for `chromium` (1)
github-actions[bot] Jan 30, 2025
c28c89e
Update UI snapshots for `chromium` (1)
github-actions[bot] Jan 30, 2025
0b9f2d7
fix tsc
dmarticus Jan 30, 2025
b099ee0
Merge branch 'feat/support-payload-overrides-in-flag-toolbar' of gith…
dmarticus Jan 30, 2025
6f906c7
Merge branch 'master' into feat/support-payload-overrides-in-flag-too…
dmarticus Jan 30, 2025
a14f8a1
Update UI snapshots for `chromium` (2)
github-actions[bot] Jan 30, 2025
843e840
Update UI snapshots for `chromium` (2)
github-actions[bot] Jan 30, 2025
d73e80c
override these
dmarticus Feb 4, 2025
e3a20b5
Update UI snapshots for `chromium` (2)
github-actions[bot] Feb 4, 2025
f35e1c9
Update UI snapshots for `chromium` (1)
github-actions[bot] Feb 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions frontend/src/scenes/experiments/ExperimentCodeSnippets.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export function JSSnippet({ flagKey, variant }: SnippetProps): JSX.Element {
<b>Test that it works</b>
</div>
<CodeSnippet language={Language.JavaScript} wrap>
{`posthog.featureFlags.override({'${flagKey}': '${variant}'})`}
{`posthog.featureFlags.overrideFeatureFlags({'${flagKey}': '${variant}'})`}
</CodeSnippet>
</div>
)
Expand Down Expand Up @@ -120,7 +120,7 @@ function App() {
}

// You can also test your code by overriding the feature flag:
posthog.featureFlags.override({'${flagKey}': '${variant}'})`}
posthog.featureFlags.overrideFeatureFlags({'${flagKey}': '${variant}'})`}
</CodeSnippet>
</>
)
Expand Down
154 changes: 99 additions & 55 deletions frontend/src/toolbar/flags/FlagsToolbarMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ import clsx from 'clsx'
import { useActions, useValues } from 'kea'
import { AnimatedCollapsible } from 'lib/components/AnimatedCollapsible'
import { IconOpenInNew } from 'lib/lemon-ui/icons'
import { LemonButton } from 'lib/lemon-ui/LemonButton'
import { LemonInput } from 'lib/lemon-ui/LemonInput'
import { LemonRadio } from 'lib/lemon-ui/LemonRadio'
import { LemonSwitch } from 'lib/lemon-ui/LemonSwitch'
import { LemonTextArea } from 'lib/lemon-ui/LemonTextArea'
import { Link } from 'lib/lemon-ui/Link'
import { Spinner } from 'lib/lemon-ui/Spinner'
import { useEffect } from 'react'
Expand All @@ -15,14 +17,16 @@ import { flagsToolbarLogic } from '~/toolbar/flags/flagsToolbarLogic'
import { toolbarConfigLogic } from '~/toolbar/toolbarConfigLogic'

export const FlagsToolbarMenu = (): JSX.Element => {
const { searchTerm, filteredFlags, userFlagsLoading } = useValues(flagsToolbarLogic)
const { searchTerm, filteredFlags, userFlagsLoading, draftPayloads, payloadErrors } = useValues(flagsToolbarLogic)
const {
setSearchTerm,
setOverriddenUserFlag,
deleteOverriddenUserFlag,
getUserFlags,
checkLocalOverrides,
setFeatureFlagValueFromPostHogClient,
setDraftPayload,
savePayloadOverride,
} = useActions(flagsToolbarLogic)
const { apiURL, posthog: posthogClient } = useValues(toolbarConfigLogic)

Expand All @@ -48,62 +52,100 @@ export const FlagsToolbarMenu = (): JSX.Element => {
<ToolbarMenu.Body>
<div className="mt-1">
{filteredFlags.length > 0 ? (
filteredFlags.map(({ feature_flag, value, hasOverride, hasVariants, currentValue }) => (
<div className={clsx('-mx-1 py-1 px-2', hasOverride && 'bg-mark')} key={feature_flag.key}>
<div className="flex flex-row items-center">
<div className="flex-1 truncate">
<Link
className="font-medium"
to={`${apiURL}${
feature_flag.id
? urls.featureFlag(feature_flag.id)
: urls.featureFlags()
}`}
subtle
target="_blank"
>
{feature_flag.key}
<IconOpenInNew />
</Link>
filteredFlags.map(
({ feature_flag, value, hasOverride, hasVariants, currentValue, payloadOverride }) => (
<div
className={clsx('-mx-1 py-1 px-2', hasOverride && 'bg-mark')}
key={feature_flag.key}
>
<div className="flex flex-row items-center">
<div className="flex-1 truncate">
<Link
className="font-medium"
to={`${apiURL}${
feature_flag.id
? urls.featureFlag(feature_flag.id)
: urls.featureFlags()
}`}
subtle
target="_blank"
>
{feature_flag.key}
<IconOpenInNew />
</Link>
</div>

<LemonSwitch
checked={!!currentValue}
onChange={(checked) => {
const newValue =
hasVariants && checked
? (feature_flag.filters?.multivariate?.variants[0]
?.key as string)
: checked
if (newValue === value && hasOverride) {
deleteOverriddenUserFlag(feature_flag.key)
} else {
setOverriddenUserFlag(feature_flag.key, newValue)
}
}}
/>
</div>

<LemonSwitch
checked={!!currentValue}
onChange={(checked) => {
const newValue =
hasVariants && checked
? (feature_flag.filters?.multivariate?.variants[0]?.key as string)
: checked
if (newValue === value && hasOverride) {
deleteOverriddenUserFlag(feature_flag.key)
} else {
setOverriddenUserFlag(feature_flag.key, newValue)
}
}}
/>
</div>
<AnimatedCollapsible collapsed={!currentValue}>
<>
{hasVariants ? (
<LemonRadio
className={clsx('pt-1 pl-4 w-full', hasOverride && 'bg-mark')}
value={typeof currentValue === 'string' ? currentValue : undefined}
options={
feature_flag.filters?.multivariate?.variants.map((variant) => ({
label: `${variant.key} - ${variant.name} (${variant.rollout_percentage}%)`,
value: variant.key,
})) || []
}
onChange={(newValue) => {
if (newValue === value && hasOverride) {
deleteOverriddenUserFlag(feature_flag.key)
} else {
setOverriddenUserFlag(feature_flag.key, newValue)
}
}}
/>
) : null}

<AnimatedCollapsible collapsed={!hasVariants || !currentValue}>
<LemonRadio
className={clsx('pt-1 pl-4 w-full', hasOverride && 'bg-mark')}
value={typeof currentValue === 'string' ? currentValue : undefined}
options={
feature_flag.filters?.multivariate?.variants.map((variant) => ({
label: `${variant.key} - ${variant.name} (${variant.rollout_percentage}%)`,
value: variant.key,
})) || []
}
onChange={(newValue) => {
if (newValue === value && hasOverride) {
deleteOverriddenUserFlag(feature_flag.key)
} else {
setOverriddenUserFlag(feature_flag.key, newValue)
}
}}
/>
</AnimatedCollapsible>
</div>
))
<div className={clsx('py-1', hasVariants && 'pl-4')}>
<label className="text-xs font-semibold">Payload</label>
<div className="flex gap-2 items-center mt-1">
<LemonTextArea
className={clsx(
'font-mono text-xs flex-1 !rounded',
payloadErrors[feature_flag.key] && 'border-danger'
)}
value={
draftPayloads[feature_flag.key] ??
(payloadOverride
? JSON.stringify(payloadOverride, null, 2)
: '')
}
onChange={(val) => setDraftPayload(feature_flag.key, val)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider debouncing setDraftPayload to avoid excessive state updates during typing

placeholder='{"key": "value"}'
minRows={2}
/>
<LemonButton
size="small"
type="primary"
onClick={() => savePayloadOverride(feature_flag.key)}
>
Save
</LemonButton>
</div>
</div>
</>
</AnimatedCollapsible>
</div>
)
)
) : (
<div className="flex flex-row items-center p-1">
{userFlagsLoading ? (
Expand All @@ -119,7 +161,9 @@ export const FlagsToolbarMenu = (): JSX.Element => {
</ToolbarMenu.Body>

<ToolbarMenu.Footer>
<span className="text-xs">Note: overriding feature flags will only affect this browser.</span>
<span className="text-xs">
Note: overriding feature flags and payloads will only affect this browser.
</span>
</ToolbarMenu.Footer>
</ToolbarMenu>
)
Expand Down
85 changes: 78 additions & 7 deletions frontend/src/toolbar/flags/flagsToolbarLogic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

import type { flagsToolbarLogicType } from './flagsToolbarLogicType'

type PayloadOverrides = Record<string, any>

export const flagsToolbarLogic = kea<flagsToolbarLogicType>([
path(['toolbar', 'flags', 'flagsToolbarLogic']),
connect(() => ({
Expand All @@ -22,11 +24,19 @@
flags,
variants,
}),
setOverriddenUserFlag: (flagKey: string, overrideValue: string | boolean) => ({ flagKey, overrideValue }),
setOverriddenUserFlag: (flagKey: string, overrideValue: string | boolean, payloadOverride?: any) => ({
flagKey,
overrideValue,
payloadOverride,
}),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: payloadOverride type should be more strictly defined than 'any' for type safety

setPayloadOverride: (flagKey: string, payload: any) => ({ flagKey, payload }),
deleteOverriddenUserFlag: (flagKey: string) => ({ flagKey }),
setSearchTerm: (searchTerm: string) => ({ searchTerm }),
checkLocalOverrides: true,
storeLocalOverrides: (localOverrides: Record<string, string | boolean>) => ({ localOverrides }),
setDraftPayload: (flagKey: string, draftPayload: string) => ({ flagKey, draftPayload }),
savePayloadOverride: (flagKey: string) => ({ flagKey }),
setPayloadError: (flagKey: string, error: string | null) => ({ flagKey, error }),
}),
loaders(({ values }) => ({
userFlags: [
Expand Down Expand Up @@ -70,11 +80,52 @@
},
},
],
payloadOverrides: [
{} as PayloadOverrides,
{
setPayloadOverride: (state, { flagKey, payload }) => ({
...state,
[flagKey]: payload,
}),
deleteOverriddenUserFlag: (state, { flagKey }) => {
const newState = { ...state }
delete newState[flagKey]
return newState
},
},
],
draftPayloads: [
{} as Record<string, string>,
{
setDraftPayload: (state, { flagKey, draftPayload }) => ({
...state,
[flagKey]: draftPayload,
}),
deleteOverriddenUserFlag: (state, { flagKey }) => {
const newState = { ...state }
delete newState[flagKey]
return newState
},
},
],
payloadErrors: [
{} as Record<string, string | null>,
{
setPayloadError: (state, { flagKey, error }) => ({
...state,
[flagKey]: error,
}),
setDraftPayload: (state, { flagKey }) => ({
...state,
[flagKey]: null,
}),
},
],
}),
selectors({
userFlagsWithOverrideInfo: [
(s) => [s.userFlags, s.localOverrides, s.posthogClientFlagValues],
(userFlags, localOverrides, posthogClientFlagValues) => {
(s) => [s.userFlags, s.localOverrides, s.posthogClientFlagValues, s.payloadOverrides],
(userFlags, localOverrides, posthogClientFlagValues, payloadOverrides) => {
return userFlags.map((flag) => {
const hasVariants = (flag.feature_flag.filters?.multivariate?.variants?.length || 0) > 0

Expand All @@ -88,6 +139,7 @@
hasVariants,
currentValue,
hasOverride: flag.feature_flag.key in localOverrides,
payloadOverride: payloadOverrides[flag.feature_flag.key],
}
})
},
Expand Down Expand Up @@ -115,12 +167,21 @@
actions.storeLocalOverrides(locallyOverrideFeatureFlags)
}
},
setOverriddenUserFlag: ({ flagKey, overrideValue }) => {
setOverriddenUserFlag: ({ flagKey, overrideValue, payloadOverride }) => {
const clientPostHog = values.posthog
if (clientPostHog) {
clientPostHog.featureFlags.override({ ...values.localOverrides, [flagKey]: overrideValue })
const payloads = payloadOverride ? { [flagKey]: payloadOverride } : undefined
clientPostHog.featureFlags.overrideFeatureFlags(

Check failure on line 174 in frontend/src/toolbar/flags/flagsToolbarLogic.ts

View workflow job for this annotation

GitHub Actions / Code quality checks 

Property 'overrideFeatureFlags' does not exist on type 'PostHogFeatureFlags'. Did you mean 'onFeatureFlags'?
{ ...values.localOverrides, [flagKey]: overrideValue },
{
payloads: payloads,
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: No validation of payloadOverride structure before passing to overrideFeatureFlags - could cause runtime errors if malformed

)
toolbarPosthogJS.capture('toolbar feature flag overridden')
actions.checkLocalOverrides()
if (payloadOverride) {
actions.setPayloadOverride(flagKey, payloadOverride)
}
clientPostHog.featureFlags.reloadFeatureFlags()
}
},
Expand All @@ -130,15 +191,25 @@
const updatedFlags = { ...values.localOverrides }
delete updatedFlags[flagKey]
if (Object.keys(updatedFlags).length > 0) {
clientPostHog.featureFlags.override({ ...updatedFlags })
clientPostHog.featureFlags.overrideFeatureFlags({ ...updatedFlags })

Check failure on line 194 in frontend/src/toolbar/flags/flagsToolbarLogic.ts

View workflow job for this annotation

GitHub Actions / Code quality checks 

Property 'overrideFeatureFlags' does not exist on type 'PostHogFeatureFlags'. Did you mean 'onFeatureFlags'?
} else {
clientPostHog.featureFlags.override(false)
clientPostHog.featureFlags.overrideFeatureFlags(false)

Check failure on line 196 in frontend/src/toolbar/flags/flagsToolbarLogic.ts

View workflow job for this annotation

GitHub Actions / Code quality checks 

Property 'overrideFeatureFlags' does not exist on type 'PostHogFeatureFlags'. Did you mean 'onFeatureFlags'?
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: State inconsistency possible - payloadOverrides state not cleared when feature flags are disabled

toolbarPosthogJS.capture('toolbar feature flag override removed')
actions.checkLocalOverrides()
clientPostHog.featureFlags.reloadFeatureFlags()
}
},
savePayloadOverride: ({ flagKey }) => {
try {
const payload = JSON.parse(values.draftPayloads[flagKey] || '')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Empty string JSON.parse() will throw - should check for truthy value before parsing

Suggested change
const payload = JSON.parse(values.draftPayloads[flagKey] || '')
const draftPayload = values.draftPayloads[flagKey]
const payload = draftPayload ? JSON.parse(draftPayload) : null

actions.setPayloadError(flagKey, null)
actions.setOverriddenUserFlag(flagKey, true, payload)
} catch (e) {
actions.setPayloadError(flagKey, 'Invalid JSON')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

console.error('Invalid JSON:', e)
}
},
})),
permanentlyMount(),
])
Expand Down
Loading