From a91475fc498c7b298f3dd44727aeaa246be852e0 Mon Sep 17 00:00:00 2001 From: Phill <14913130+phillram@users.noreply.github.com> Date: Thu, 1 Oct 2026 10:49:19 -0700 Subject: [PATCH 1/6] fix(flags): label recent feature flags with their key A feature flag keeps its description in `name`, and most flags leave it empty, so the taxonomic filter had nothing to label a flag with in its Recent category. Two causes. A pick stored as a recent kept only `name` and `id`, which dropped the flag key. The recent row then labelled itself from `name` alone and never asked the source group for a label. The definition popover titled itself from `name` too, so it read "(empty string)" for any flag with no description, in the Recent list and in the main flag list. Recents now keep the fields a group labels its rows with, the recent row falls back to the group's `getName`, and the popover title does the same. Flag recents already stored without a key are dropped, so the blank rows go away instead of waiting out the 30 day expiry. Generated-By: PostHog Desktop Task-Id: 1b898a61-0065-4b4d-b7b2-84f843040805 --- .../DefinitionPopoverContents.tsx | 6 ++++- .../src/lib/components/FlagSelector.test.tsx | 5 ++-- frontend/src/lib/components/FlagSelector.tsx | 13 +++++----- .../TaxonomicFilter/InfiniteList.tsx | 5 +++- .../TaxonomicFilterKeyOnly.test.tsx | 25 +++++++++++++++++++ .../hooks/useTaxonomicFilter.ts | 6 ++--- .../recentTaxonomicFiltersLogic.test.ts | 23 +++++++++++++++++ .../recentTaxonomicFiltersLogic.ts | 18 +++++++++++++ .../TaxonomicFilter/taxonomicFilterLogic.tsx | 3 ++- .../utils/suggestedContextFilters.test.ts | 17 ++++++++++++- .../utils/suggestedContextFilters.ts | 8 ++++++ 11 files changed, 111 insertions(+), 18 deletions(-) diff --git a/frontend/src/lib/components/DefinitionPopover/DefinitionPopoverContents.tsx b/frontend/src/lib/components/DefinitionPopover/DefinitionPopoverContents.tsx index 8147ed374d8f..48688e2dac7f 100644 --- a/frontend/src/lib/components/DefinitionPopover/DefinitionPopoverContents.tsx +++ b/frontend/src/lib/components/DefinitionPopover/DefinitionPopoverContents.tsx @@ -756,8 +756,12 @@ export function ControlledDefinitionPopover({ { - // The picker strips `key` off recently-used flags, so a pick from the Recent tab labels itself - // with `name`, which on a flag holds a description like "Feature Flag for Early Access Feature - // Foo". The resolved key has to win over that. + // A picked item with no key labels itself with `name`, which on a flag holds a description like + // "Feature Flag for Early Access Feature Foo". The resolved key has to win over that. const RECENT_PICK = { id: 7, label: 'Feature Flag for Early Access Feature Foo' } test.each([ diff --git a/frontend/src/lib/components/FlagSelector.tsx b/frontend/src/lib/components/FlagSelector.tsx index cd4ab4477b87..29e32f0b3f82 100644 --- a/frontend/src/lib/components/FlagSelector.tsx +++ b/frontend/src/lib/components/FlagSelector.tsx @@ -34,10 +34,10 @@ export function flagSelectorButtonLabel({ initialButtonLabel: string | undefined }): string { // A pick only labels the button while it still agrees with `value`, so a pick the caller never - // stored can't linger. It ranks below `flagKey` because a pick from the recents list carries no - // key and falls back to `name`, which on a flag holds the description rather than a title. - // `flagKey` is '' both while the lookup is in flight and when it fails, which is the gap the - // pick covers. + // stored can't linger. It ranks below `flagKey` because the lookup holds the flag's current key, + // while a pick falls back to `name` when the picked item has no key, and `name` on a flag holds + // the description rather than a title. `flagKey` is '' both while the lookup is in flight and + // when it fails, which is the gap the pick covers. const pickedLabel = pickedFlag && pickedFlag.id === value ? pickedFlag.label : undefined return flagKey || pickedLabel || (initialButtonLabel ?? 'Select flag') } @@ -50,9 +50,8 @@ export function FlagSelector({ initialButtonLabel, }: FlagSelectorProps): JSX.Element { const [visible, setVisible] = useState(false) - // Recently-used flags are persisted with just `{ name, id }` (no `key`), so a pick from the - // recents list has nothing to label the button with until the live lookup resolves. Hold - // whatever the picker handed us to cover that gap. + // The live lookup of a freshly picked flag takes a moment, so hold the label the picker handed + // us to cover that gap. const [selectedFlag, setSelectedFlag] = useState(undefined) const { featureFlag } = useValues(featureFlagLogic({ id: value || 'link' })) diff --git a/frontend/src/lib/components/TaxonomicFilter/InfiniteList.tsx b/frontend/src/lib/components/TaxonomicFilter/InfiniteList.tsx index 37bb512a558a..5cb71c986f83 100644 --- a/frontend/src/lib/components/TaxonomicFilter/InfiniteList.tsx +++ b/frontend/src/lib/components/TaxonomicFilter/InfiniteList.tsx @@ -249,7 +249,10 @@ const renderItemContents = ({ ) } const coreDef = getCoreFilterDefinition(item.name, itemGroup.type) - const label = coreDef?.label || item.name || '' + // `getName` holds the label for a group that does not label its rows with `name`, such as a + // feature flag, where `name` holds the description and the row shows `key`. The popover for + // the same row resolves its label the same way. + const label = coreDef?.label || itemGroup.getName?.(item) || item.name || '' return (
{icon} diff --git a/frontend/src/lib/components/TaxonomicFilter/TaxonomicFilterKeyOnly.test.tsx b/frontend/src/lib/components/TaxonomicFilter/TaxonomicFilterKeyOnly.test.tsx index 6ee5dba9e5fc..ae31d095c815 100644 --- a/frontend/src/lib/components/TaxonomicFilter/TaxonomicFilterKeyOnly.test.tsx +++ b/frontend/src/lib/components/TaxonomicFilter/TaxonomicFilterKeyOnly.test.tsx @@ -38,6 +38,7 @@ describe('TaxonomicFilter selectingKeyOnly mode', () => { '/api/projects/:team/property_definitions': mockGetPropertyDefinitions, '/api/projects/:team/actions': { results: [] }, '/api/environments/:team/persons/properties': [{ id: 1, name: 'location', count: 1 }], + '/api/projects/:team/feature_flags/': { count: 0, results: [] }, }, post: { '/api/environments/:team/query': { results: [] }, @@ -84,6 +85,30 @@ describe('TaxonomicFilter selectingKeyOnly mode', () => { await userEvent.click(await screen.findByTestId('taxonomic-category-dropdown-item-recent_filters')) } + describe('feature flag recents', () => { + it('labels a recent flag row with the flag key, because a flag keeps its description in name', async () => { + recents.actions.recordRecentFilter({ + groupType: TaxonomicFilterGroupType.FeatureFlags, + groupName: 'Feature Flags', + value: 101, + item: { name: '', id: 101, key: 'checkout-redesign' }, + selectingKeyOnly: true, + }) + + renderFilter({ taxonomicGroupTypes: [TaxonomicFilterGroupType.FeatureFlags], selectingKeyOnly: true }) + + await selectRecentCategory() + + await waitFor( + () => + expect(screen.getByTestId('prop-filter-recent_filters-0').textContent).toContain( + 'checkout-redesign' + ), + { timeout: RENDER_TIMEOUT_MS } + ) + }) + }) + describe('recording on selection', () => { it('records an EventProperty selection to recents when selectingKeyOnly is set', async () => { renderFilter({ selectingKeyOnly: true }) diff --git a/frontend/src/lib/components/TaxonomicFilter/hooks/useTaxonomicFilter.ts b/frontend/src/lib/components/TaxonomicFilter/hooks/useTaxonomicFilter.ts index 39e3fc0006ab..1e34bd93a1eb 100644 --- a/frontend/src/lib/components/TaxonomicFilter/hooks/useTaxonomicFilter.ts +++ b/frontend/src/lib/components/TaxonomicFilter/hooks/useTaxonomicFilter.ts @@ -26,6 +26,7 @@ import { useCallback, useMemo, useRef, useState } from 'react' import { hasRecentContext, + pickMinimalRecentItem, recentTaxonomicFiltersLogic, stripRecentContext, } from 'lib/components/TaxonomicFilter/recentTaxonomicFiltersLogic' @@ -500,10 +501,7 @@ export function useTaxonomicFilter(opts: UseTaxonomicFilterOptions): TaxonomicFi allGroups.find((g) => g.type === stripped.group) : undefined const sourceGroupType = recentContext?.sourceGroupType ?? declaredGroup?.type ?? group.type - const cleanItem = { - name: stripped.name, - ...(stripped.id ? { id: stripped.id } : {}), - } + const cleanItem = pickMinimalRecentItem(stripped) const sourceGroupName = recentContext?.sourceGroupName ?? declaredGroup?.name ?? group.name const propertyFilterFromRecent = recentContext?.propertyFilter // Defer one tick — keeps the recents write off the diff --git a/frontend/src/lib/components/TaxonomicFilter/recentTaxonomicFiltersLogic.test.ts b/frontend/src/lib/components/TaxonomicFilter/recentTaxonomicFiltersLogic.test.ts index ee77e6cb3e50..7f4d080fc3f2 100644 --- a/frontend/src/lib/components/TaxonomicFilter/recentTaxonomicFiltersLogic.test.ts +++ b/frontend/src/lib/components/TaxonomicFilter/recentTaxonomicFiltersLogic.test.ts @@ -3,6 +3,7 @@ import { LogPropertyFilter, PersonPropertyFilter, PropertyFilterType, PropertyOp import { MAX_RECENT_FILTERS, + pickMinimalRecentItem, RECENT_FILTER_MAX_AGE_MS, recentTaxonomicFiltersLogic, } from './recentTaxonomicFiltersLogic' @@ -645,4 +646,26 @@ describe('recentTaxonomicFiltersLogic', () => { expect(filters[0].value).toBe(`person-key-${MAX_RECENT_FILTERS - 1}`) }) }) + + describe('pickMinimalRecentItem', () => { + it.each([ + [ + 'keeps the key of a feature flag, whose name holds the description', + { id: 7, key: 'my-flag', name: '', active: true, filters: { groups: [] } }, + { name: '', id: 7, key: 'my-flag' }, + ], + [ + 'keeps the title and short id of a notebook', + { short_id: 'ab12cd', title: 'Launch notes', content: { type: 'doc' } }, + { title: 'Launch notes', short_id: 'ab12cd' }, + ], + [ + 'drops everything a group cannot label a row with', + { name: '$pageview', id: 'uuid-1', description: 'A page view', tags: ['web'] }, + { name: '$pageview', id: 'uuid-1' }, + ], + ])('%s', (_label: string, item: Record, expected: Record) => { + expect(pickMinimalRecentItem(item)).toEqual(expected) + }) + }) }) diff --git a/frontend/src/lib/components/TaxonomicFilter/recentTaxonomicFiltersLogic.ts b/frontend/src/lib/components/TaxonomicFilter/recentTaxonomicFiltersLogic.ts index d5c3780c5f70..190f0da91a2e 100644 --- a/frontend/src/lib/components/TaxonomicFilter/recentTaxonomicFiltersLogic.ts +++ b/frontend/src/lib/components/TaxonomicFilter/recentTaxonomicFiltersLogic.ts @@ -36,6 +36,24 @@ export interface RecentTaxonomicFilter { propertyFilter?: AnyPropertyFilter } +/** + * Fields kept on a stored recent, beyond `name`. A recent goes to localStorage on every pick, so + * an entry holds only what a source group's `getName` reads to label its row. `name` alone is not + * enough: on a feature flag `name` holds the description, which most flags leave empty, and the row + * shows `key`. On a notebook the label comes from `title` or `short_id`. + */ +const RECENT_ITEM_LABEL_FIELDS = ['id', 'key', 'title', 'short_id'] as const + +export function pickMinimalRecentItem(item: Record): Record { + const picked: Record = { name: item.name } + for (const field of RECENT_ITEM_LABEL_FIELDS) { + if (item[field]) { + picked[field] = item[field] + } + } + return picked +} + export interface RecentItemContext { sourceGroupType: TaxonomicFilterGroupType sourceGroupName: string diff --git a/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx b/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx index 6eafe47371df..ab2b6f3e4c55 100644 --- a/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx +++ b/frontend/src/lib/components/TaxonomicFilter/taxonomicFilterLogic.tsx @@ -28,6 +28,7 @@ import { infiniteListLogic } from 'lib/components/TaxonomicFilter/infiniteListLo import type { infiniteListLogicType } from 'lib/components/TaxonomicFilter/infiniteListLogic' import { hasRecentContext, + pickMinimalRecentItem, recentTaxonomicFiltersLogic, stripRecentContext, } from 'lib/components/TaxonomicFilter/recentTaxonomicFiltersLogic' @@ -2655,7 +2656,7 @@ export const taxonomicFilterLogic = kea([ setTimeout(() => { if (recentTaxonomicFiltersLogic.isMounted()) { const stripped = hasRecentContext(item) ? stripRecentContext(item) : item - const cleanItem = { name: stripped.name, ...(stripped.id ? { id: stripped.id } : {}) } + const cleanItem = pickMinimalRecentItem(stripped) const sourceGroupName = hasRecentContext(item) ? item._recentContext.sourceGroupName : group.name diff --git a/frontend/src/lib/components/TaxonomicFilter/utils/suggestedContextFilters.test.ts b/frontend/src/lib/components/TaxonomicFilter/utils/suggestedContextFilters.test.ts index 284c69d1f932..2798d2b45e8e 100644 --- a/frontend/src/lib/components/TaxonomicFilter/utils/suggestedContextFilters.test.ts +++ b/frontend/src/lib/components/TaxonomicFilter/utils/suggestedContextFilters.test.ts @@ -3,7 +3,7 @@ import { PropertyOperator } from '~/types' import { TaxonomicDefinitionTypes, TaxonomicFilterGroupType } from '../types' import { filterPinnedForContext, filterRecentsForContext } from './suggestedContextFilters' -const { Events, EventProperties, Cohorts } = TaxonomicFilterGroupType +const { Events, EventProperties, Cohorts, FeatureFlags } = TaxonomicFilterGroupType function recent( sourceGroupType: TaxonomicFilterGroupType, @@ -71,6 +71,21 @@ describe('suggestedContextFilters', () => { }) }) + describe('filterRecentsForContext feature flags', () => { + const flagRecent = (id: number, item: Record): TaxonomicDefinitionTypes => + ({ + name: '', + ...item, + _recentContext: { sourceGroupType: FeatureFlags, sourceValue: id }, + }) as unknown as TaxonomicDefinitionTypes + + it('keeps a flag recent that carries its key and drops one stored without', () => { + const out = filterRecentsForContext([flagRecent(1, { key: 'my-flag' }), flagRecent(2, {})], [FeatureFlags]) + expect(out).toHaveLength(1) + expect((out[0] as unknown as { key: string }).key).toBe('my-flag') + }) + }) + describe('filterRecentsForContext bare-key expansion (value mode)', () => { const complete = (key: string, value: string): Record => ({ propertyFilter: { key, operator: PropertyOperator.Exact, value }, diff --git a/frontend/src/lib/components/TaxonomicFilter/utils/suggestedContextFilters.ts b/frontend/src/lib/components/TaxonomicFilter/utils/suggestedContextFilters.ts index 4ad9eb19cb90..5b745d12389a 100644 --- a/frontend/src/lib/components/TaxonomicFilter/utils/suggestedContextFilters.ts +++ b/frontend/src/lib/components/TaxonomicFilter/utils/suggestedContextFilters.ts @@ -32,6 +32,14 @@ export function filterRecentsForContext( if (!hasRecentContext(item) || !availableTypes.has(item._recentContext.sourceGroupType)) { return false } + // A flag recent stored without `key` holds only the flag description, which most flags + // leave empty, so its row renders blank. Drop it; the next pick stores a labelled entry. + if ( + item._recentContext.sourceGroupType === TaxonomicFilterGroupType.FeatureFlags && + !('key' in item && item.key) + ) { + return false + } // A group's excluded values (e.g. `message` for the logs group-by picker) must be dropped // from the Recent tab too, not just the group's own option list. Otherwise an excluded key // recorded elsewhere leaks back in as a selectable recent. From ee5620d1adcdda8966ab2ff72a71350ff22aa7dd Mon Sep 17 00:00:00 2001 From: Phill <14913130+phillram@users.noreply.github.com> Date: Thu, 1 Oct 2026 11:11:49 -0700 Subject: [PATCH 2/6] fix(flags): label recent groups and saved replay filters too A saved replay filter labels itself with `derived_name` when it has no name, and a group labels itself with `group_key`. Both fields were dropped when a pick was stored as a recent, so those rows read "Unnamed" or stayed blank for the same reason a feature flag did. Generated-By: PostHog Desktop Task-Id: 1b898a61-0065-4b4d-b7b2-84f843040805 --- .../TaxonomicFilter/recentTaxonomicFiltersLogic.test.ts | 5 +++++ .../TaxonomicFilter/recentTaxonomicFiltersLogic.ts | 5 +++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/frontend/src/lib/components/TaxonomicFilter/recentTaxonomicFiltersLogic.test.ts b/frontend/src/lib/components/TaxonomicFilter/recentTaxonomicFiltersLogic.test.ts index 7f4d080fc3f2..8258f96abe13 100644 --- a/frontend/src/lib/components/TaxonomicFilter/recentTaxonomicFiltersLogic.test.ts +++ b/frontend/src/lib/components/TaxonomicFilter/recentTaxonomicFiltersLogic.test.ts @@ -659,6 +659,11 @@ describe('recentTaxonomicFiltersLogic', () => { { short_id: 'ab12cd', title: 'Launch notes', content: { type: 'doc' } }, { title: 'Launch notes', short_id: 'ab12cd' }, ], + [ + 'keeps the group key of a group, whose properties are too heavy to store', + { group_key: 'org:acme', group_properties: { name: 'Acme', plan: 'pro' } }, + { group_key: 'org:acme' }, + ], [ 'drops everything a group cannot label a row with', { name: '$pageview', id: 'uuid-1', description: 'A page view', tags: ['web'] }, diff --git a/frontend/src/lib/components/TaxonomicFilter/recentTaxonomicFiltersLogic.ts b/frontend/src/lib/components/TaxonomicFilter/recentTaxonomicFiltersLogic.ts index 190f0da91a2e..f4b01a4f7b19 100644 --- a/frontend/src/lib/components/TaxonomicFilter/recentTaxonomicFiltersLogic.ts +++ b/frontend/src/lib/components/TaxonomicFilter/recentTaxonomicFiltersLogic.ts @@ -40,9 +40,10 @@ export interface RecentTaxonomicFilter { * Fields kept on a stored recent, beyond `name`. A recent goes to localStorage on every pick, so * an entry holds only what a source group's `getName` reads to label its row. `name` alone is not * enough: on a feature flag `name` holds the description, which most flags leave empty, and the row - * shows `key`. On a notebook the label comes from `title` or `short_id`. + * shows `key`. A notebook labels itself with `title` or `short_id`, a saved replay filter with + * `derived_name`, and a group with `group_key`. */ -const RECENT_ITEM_LABEL_FIELDS = ['id', 'key', 'title', 'short_id'] as const +const RECENT_ITEM_LABEL_FIELDS = ['id', 'key', 'title', 'short_id', 'derived_name', 'group_key'] as const export function pickMinimalRecentItem(item: Record): Record { const picked: Record = { name: item.name } From 36dc61c2c6a75d3d69adf97dc18b97e6711d885b Mon Sep 17 00:00:00 2001 From: Phill <14913130+phillram@users.noreply.github.com> Date: Fri, 2 Oct 2026 11:53:10 -0700 Subject: [PATCH 3/6] fix(flags): stop passing a recent flag summary off as the flag The flag picker handed callers whatever row was clicked. A row from the Feature Flags list is the flag; a row from the Recent category is a stored summary that holds only what labels it. Callers read the rest off that object, so a pick from Recent produced a broken link rather than a visibly empty one. A replay trigger persists `{id, key}` to the team and the key was undefined, so the trigger matched nothing. The survey editor stored the summary as `linked_flag`, leaving the variant selector without a flag to read. `pickedFeatureFlag` now resolves the id (falling back to the value the picker keys the row by), requires a key, and returns the flag only when the row really is one. The third `onChange` argument is optional, so a caller that needs the whole flag loads it by id, which the product tour field already did and the survey editor now does too. Generated-By: PostHog Desktop Task-Id: 1b898a61-0065-4b4d-b7b2-84f843040805 --- .../src/lib/components/FlagSelector.test.tsx | 95 +++++++++++++------ frontend/src/lib/components/FlagSelector.tsx | 57 ++++++++--- frontend/src/scenes/surveys/SurveyEdit.tsx | 30 +++++- 3 files changed, 138 insertions(+), 44 deletions(-) diff --git a/frontend/src/lib/components/FlagSelector.test.tsx b/frontend/src/lib/components/FlagSelector.test.tsx index a9bbc4dd6e32..e7f58607b3e8 100644 --- a/frontend/src/lib/components/FlagSelector.test.tsx +++ b/frontend/src/lib/components/FlagSelector.test.tsx @@ -1,29 +1,68 @@ -import { flagSelectorButtonLabel } from './FlagSelector' - -describe('flagSelectorButtonLabel', () => { - // A picked item with no key labels itself with `name`, which on a flag holds a description like - // "Feature Flag for Early Access Feature Foo". The resolved key has to win over that. - const RECENT_PICK = { id: 7, label: 'Feature Flag for Early Access Feature Foo' } - - test.each([ - ['resolved key wins over a recent pick', 'foo', 7, RECENT_PICK, 'Fallback', 'foo'], - ['pick stands in while the key is still loading', '', 7, RECENT_PICK, 'Fallback', RECENT_PICK.label], - ['pick stands in when the key lookup fails', '', 7, { id: 7, label: 'foo' }, 'Fallback', 'foo'], - ['pick the caller never stored is dropped', '', undefined, RECENT_PICK, 'Fallback', 'Fallback'], - ['pick for a different flag is dropped', '', 9, RECENT_PICK, 'Fallback', 'Fallback'], - ['falls back to the initial label with nothing picked', '', undefined, undefined, 'Fallback', 'Fallback'], - ['falls back to the default with no initial label', '', undefined, undefined, undefined, 'Select flag'], - ])( - '%s', - ( - _name: string, - flagKey: string, - value: number | undefined, - pickedFlag: { id: number; label: string } | undefined, - initialButtonLabel: string | undefined, - expected: string - ) => { - expect(flagSelectorButtonLabel({ flagKey, value, pickedFlag, initialButtonLabel })).toBe(expected) - } - ) +import { FeatureFlagBasicType } from '~/types' + +import { flagSelectorButtonLabel, pickedFeatureFlag } from './FlagSelector' + +describe('FlagSelector', () => { + describe('flagSelectorButtonLabel', () => { + const PICK = { id: 7, label: 'checkout-redesign' } + + test.each([ + ['resolved key wins over a pick', 'foo', 7, PICK, 'Fallback', 'foo'], + ['pick stands in while the key is still loading', '', 7, PICK, 'Fallback', PICK.label], + ['pick stands in when the key lookup fails', '', 7, { id: 7, label: 'foo' }, 'Fallback', 'foo'], + ['pick the caller never stored is dropped', '', undefined, PICK, 'Fallback', 'Fallback'], + ['pick for a different flag is dropped', '', 9, PICK, 'Fallback', 'Fallback'], + ['falls back to the initial label with nothing picked', '', undefined, undefined, 'Fallback', 'Fallback'], + ['falls back to the default with no initial label', '', undefined, undefined, undefined, 'Select flag'], + ])( + '%s', + ( + _name: string, + flagKey: string, + value: number | undefined, + pickedFlag: { id: number; label: string } | undefined, + initialButtonLabel: string | undefined, + expected: string + ) => { + expect(flagSelectorButtonLabel({ flagKey, value, pickedFlag, initialButtonLabel })).toBe(expected) + } + ) + }) + + describe('pickedFeatureFlag', () => { + const FULL_FLAG = { + id: 7, + key: 'checkout-redesign', + name: '', + active: true, + filters: { groups: [] }, + } as unknown as FeatureFlagBasicType + + it('hands back the whole flag when the row is one', () => { + expect(pickedFeatureFlag(FULL_FLAG, 7)).toEqual({ id: 7, key: 'checkout-redesign', flag: FULL_FLAG }) + }) + + // A Recent row holds only what labels it. Returning it as `flag` is what let callers store a + // feature flag with no key and no targeting config, which the replay trigger and the survey + // variant selector both read. + it('withholds the flag when the row is a stored summary', () => { + expect(pickedFeatureFlag({ name: '', id: 7, key: 'checkout-redesign' }, 7)).toEqual({ + id: 7, + key: 'checkout-redesign', + flag: undefined, + }) + }) + + it('takes the id from the picked value when the row carries none', () => { + expect(pickedFeatureFlag({ key: 'checkout-redesign' }, 7)?.id).toBe(7) + }) + + it.each([ + ['a row with no key', { name: '', id: 7 }, 7], + ['a row with an empty key', { name: '', id: 7, key: '' }, 7], + ['a row with no resolvable id', { key: 'checkout-redesign' }, null], + ])('is not a selection: %s', (_name: string, item: Record, value: number | null) => { + expect(pickedFeatureFlag(item, value)).toBeNull() + }) + }) }) diff --git a/frontend/src/lib/components/FlagSelector.tsx b/frontend/src/lib/components/FlagSelector.tsx index 29e32f0b3f82..b2d10b4a3cb2 100644 --- a/frontend/src/lib/components/FlagSelector.tsx +++ b/frontend/src/lib/components/FlagSelector.tsx @@ -2,7 +2,11 @@ import { useValues } from 'kea' import { useState } from 'react' import { TaxonomicFilter } from 'lib/components/TaxonomicFilter/TaxonomicFilter' -import { TaxonomicFilterGroupType, TaxonomicFilterLogicProps } from 'lib/components/TaxonomicFilter/types' +import { + TaxonomicFilterGroupType, + TaxonomicFilterLogicProps, + TaxonomicFilterValue, +} from 'lib/components/TaxonomicFilter/types' import { LemonButton } from 'lib/lemon-ui/LemonButton' import { Popover } from 'lib/lemon-ui/Popover' import { featureFlagLogic } from 'scenes/feature-flags/featureFlagLogic' @@ -11,12 +15,39 @@ import { FeatureFlagBasicType } from '~/types' interface FlagSelectorProps { value: number | undefined - onChange: (id: number, key: string, flag: FeatureFlagBasicType) => void + /** + * `flag` is absent when the picked row is a stored summary rather than the flag itself. A caller + * that reads the flag's configuration loads it from `id` in that case. + */ + onChange: (id: number, key: string, flag?: FeatureFlagBasicType) => void readOnly?: boolean disabledReason?: string initialButtonLabel?: string } +interface PickedFeatureFlag { + id: number + key: string + flag?: FeatureFlagBasicType +} + +/** + * A row from the Feature Flags list is the flag itself. A row from the Recent category is a stored + * summary of one: it holds the id and key its label needs, and none of the flag's configuration. A + * caller reads `filters` and `active` off the flag, so a summary must not stand in for one. + */ +export function pickedFeatureFlag(item: unknown, pickedValue: TaxonomicFilterValue): PickedFeatureFlag | null { + const row = typeof item === 'object' && item !== null ? (item as Record) : {} + // The Feature Flags group keys every row by flag id, so the picked value carries the id even + // when the row itself does not. + const id = typeof row.id === 'number' ? row.id : typeof pickedValue === 'number' ? pickedValue : null + const key = typeof row.key === 'string' && row.key ? row.key : null + if (id === null || key === null) { + return null + } + return { id, key, flag: 'filters' in row ? (row as unknown as FeatureFlagBasicType) : undefined } +} + interface PickedFlag { id: number label: string @@ -34,10 +65,9 @@ export function flagSelectorButtonLabel({ initialButtonLabel: string | undefined }): string { // A pick only labels the button while it still agrees with `value`, so a pick the caller never - // stored can't linger. It ranks below `flagKey` because the lookup holds the flag's current key, - // while a pick falls back to `name` when the picked item has no key, and `name` on a flag holds - // the description rather than a title. `flagKey` is '' both while the lookup is in flight and - // when it fails, which is the gap the pick covers. + // stored can't linger. It ranks below `flagKey` because the lookup holds the flag's current key. + // `flagKey` is '' both while the lookup is in flight and when it fails, which is the gap the + // pick covers. const pickedLabel = pickedFlag && pickedFlag.id === value ? pickedFlag.label : undefined return flagKey || pickedLabel || (initialButtonLabel ?? 'Select flag') } @@ -59,13 +89,16 @@ export function FlagSelector({ const taxonomicFilterLogicProps: TaxonomicFilterLogicProps = { groupType: TaxonomicFilterGroupType.FeatureFlags, value: value, - onChange: (_, __, item) => { - // The picker can hand back an item with no flag id; that isn't a selection. - if ('id' in item && item.id) { - setSelectedFlag({ id: item.id, label: item.key || item.name }) - onChange(item.id, item.key, item) - setVisible(false) + onChange: (_, pickedValue, item) => { + const picked = pickedFeatureFlag(item, pickedValue) + // A row the picker cannot resolve to a flag isn't a selection. No category renders one, + // so this only guards against a stored row shape that predates the fields above. + if (!picked) { + return } + setSelectedFlag({ id: picked.id, label: picked.key }) + onChange(picked.id, picked.key, picked.flag) + setVisible(false) }, taxonomicGroupTypes: [TaxonomicFilterGroupType.FeatureFlags], optionsFromProp: undefined, diff --git a/frontend/src/scenes/surveys/SurveyEdit.tsx b/frontend/src/scenes/surveys/SurveyEdit.tsx index 932dd63e7caa..da07a4965422 100644 --- a/frontend/src/scenes/surveys/SurveyEdit.tsx +++ b/frontend/src/scenes/surveys/SurveyEdit.tsx @@ -1454,10 +1454,32 @@ export default function SurveyEdit({ id }: { id: string }): JSX.Element { ) }) } else { - setSurveyValue( - 'linked_flag', - flag - ) + if (flag) { + setSurveyValue( + 'linked_flag', + flag + ) + } else { + // A pick from the Recent + // category carries only the + // flag id, and the variant + // selector below reads the + // flag itself. + api.featureFlags + .get(id) + .then((linkedFlag) => { + setSurveyValue( + 'linked_flag', + linkedFlag + ) + }) + .catch(() => { + setSurveyValue( + 'linked_flag_id', + null + ) + }) + } // Reset variant selection when flag changes const { linkedFlagVariant, From 9a3e80fcac802e860717356b3ddd427565ff209a Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Fri, 2 Oct 2026 19:34:37 +0000 Subject: [PATCH 4/6] fix(surveys): keep a picked flag when its lookup fails A flag picked from Recent is loaded by id so the variant selector can read it. Any failed lookup cleared linked_flag_id, so a temporary error followed by a save removed the survey's flag targeting. Only a 404 now ends the link, and it clears linked_flag with it. Any other failure keeps the pick and shows an error toast with a button that retries the lookup. Co-Authored-By: Claude Opus 5.5 Generated-By: PostHog Desktop Task-Id: 77ee946d-952c-4c91-95bc-424e31f3839d --- frontend/src/scenes/surveys/SurveyEdit.tsx | 37 ++++++++++++++-------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/frontend/src/scenes/surveys/SurveyEdit.tsx b/frontend/src/scenes/surveys/SurveyEdit.tsx index da07a4965422..ba8e8e978fc2 100644 --- a/frontend/src/scenes/surveys/SurveyEdit.tsx +++ b/frontend/src/scenes/surveys/SurveyEdit.tsx @@ -20,9 +20,11 @@ import { LemonTag, Link, Popover, + lemonToast, } from '@posthog/lemon-ui' import api from 'lib/api' +import { ApiError } from 'lib/api-error' import { FlagSelector } from 'lib/components/FlagSelector' import { ANY_VARIANT, variantOptions } from 'lib/components/IngestionControls/triggers/FlagTrigger/VariantSelector' import { PropertyValue } from 'lib/components/PropertyFilters/components/PropertyValue' @@ -374,6 +376,26 @@ export default function SurveyEdit({ id }: { id: string }): JSX.Element { setFlagPropertyErrors(null) } + function loadPickedLinkedFlag(flagId: number): void { + api.featureFlags + .get(flagId) + .then((linkedFlag) => { + setSurveyValue('linked_flag', linkedFlag) + }) + .catch((error) => { + // Only a missing flag ends the link. A failed request keeps the pick, because a save with no + // linked_flag_id removes the survey's flag targeting. + if (error instanceof ApiError && error.status === 404) { + setSurveyValue('linked_flag_id', null) + setSurveyValue('linked_flag', null) + return + } + lemonToast.error("Couldn't load the selected feature flag.", { + button: { label: 'Try again', action: () => loadPickedLinkedFlag(flagId) }, + }) + }) + } + const getFieldError = ( fieldKey: string ): { language: string; questionIndex: number; field: string; error: string } | undefined => { @@ -1465,20 +1487,7 @@ export default function SurveyEdit({ id }: { id: string }): JSX.Element { // flag id, and the variant // selector below reads the // flag itself. - api.featureFlags - .get(id) - .then((linkedFlag) => { - setSurveyValue( - 'linked_flag', - linkedFlag - ) - }) - .catch(() => { - setSurveyValue( - 'linked_flag_id', - null - ) - }) + loadPickedLinkedFlag(id) } // Reset variant selection when flag changes const { From 549f364c1b0a55fd70fc9939b271e810dcacfaec Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Fri, 2 Oct 2026 19:40:30 +0000 Subject: [PATCH 5/6] fix(flags): keep completed flag dependencies in recents The drop for keyless flag recents also removed completed flag dependencies. Both property-filter writers store those without `key`, and their row takes its label from the property filter, so they never rendered blank. A keyless flag recent is now dropped only when it has no completed property filter, or when the picker selects a key only and so strips that filter. Co-Authored-By: Claude Opus 5.5 Generated-By: PostHog Desktop Task-Id: 77ee946d-952c-4c91-95bc-424e31f3839d --- .../utils/suggestedContextFilters.test.ts | 34 +++++++++++++++++-- .../utils/suggestedContextFilters.ts | 13 +++++-- 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/frontend/src/lib/components/TaxonomicFilter/utils/suggestedContextFilters.test.ts b/frontend/src/lib/components/TaxonomicFilter/utils/suggestedContextFilters.test.ts index 2798d2b45e8e..be93ffb6f36a 100644 --- a/frontend/src/lib/components/TaxonomicFilter/utils/suggestedContextFilters.test.ts +++ b/frontend/src/lib/components/TaxonomicFilter/utils/suggestedContextFilters.test.ts @@ -1,4 +1,4 @@ -import { PropertyOperator } from '~/types' +import { PropertyFilterType, PropertyOperator } from '~/types' import { TaxonomicDefinitionTypes, TaxonomicFilterGroupType } from '../types' import { filterPinnedForContext, filterRecentsForContext } from './suggestedContextFilters' @@ -72,11 +72,15 @@ describe('suggestedContextFilters', () => { }) describe('filterRecentsForContext feature flags', () => { - const flagRecent = (id: number, item: Record): TaxonomicDefinitionTypes => + const flagRecent = ( + id: number, + item: Record, + context: Record = {} + ): TaxonomicDefinitionTypes => ({ name: '', ...item, - _recentContext: { sourceGroupType: FeatureFlags, sourceValue: id }, + _recentContext: { sourceGroupType: FeatureFlags, sourceValue: id, ...context }, }) as unknown as TaxonomicDefinitionTypes it('keeps a flag recent that carries its key and drops one stored without', () => { @@ -84,6 +88,30 @@ describe('suggestedContextFilters', () => { expect(out).toHaveLength(1) expect((out[0] as unknown as { key: string }).key).toBe('my-flag') }) + + // The shape propertyFilterLogic records for a completed flag dependency: no `key` on the item. + const dependencyRecent = flagRecent( + 7, + { name: 'my-flag' }, + { + propertyFilter: { + type: PropertyFilterType.Flag, + key: '7', + label: 'my-flag', + operator: PropertyOperator.FlagEvaluatesTo, + value: true, + }, + } + ) + + it.each([ + ['keeps a completed flag dependency in a value picker', undefined, ['my-flag', 'my-flag']], + ['drops a completed flag dependency in a key-only picker', true, []], + ])('%s', (_name: string, selectingKeyOnly: boolean | undefined, expected: string[]) => { + expect( + names(filterRecentsForContext([dependencyRecent], [FeatureFlags], undefined, selectingKeyOnly)) + ).toEqual(expected) + }) }) describe('filterRecentsForContext bare-key expansion (value mode)', () => { diff --git a/frontend/src/lib/components/TaxonomicFilter/utils/suggestedContextFilters.ts b/frontend/src/lib/components/TaxonomicFilter/utils/suggestedContextFilters.ts index 5b745d12389a..2637ef94faa4 100644 --- a/frontend/src/lib/components/TaxonomicFilter/utils/suggestedContextFilters.ts +++ b/frontend/src/lib/components/TaxonomicFilter/utils/suggestedContextFilters.ts @@ -1,4 +1,8 @@ -import { expandRecentsForDisplay, hasRecentContext } from 'lib/components/TaxonomicFilter/recentTaxonomicFiltersLogic' +import { + expandRecentsForDisplay, + hasRecentContext, + isCompleteRecentPropertyFilter, +} from 'lib/components/TaxonomicFilter/recentTaxonomicFiltersLogic' import { hasPinnedContext } from 'lib/components/TaxonomicFilter/taxonomicFilterPinnedPropertiesLogic' import { ExcludedOperators, @@ -6,6 +10,7 @@ import { SelectingKeyOnly, TaxonomicDefinitionTypes, TaxonomicFilterGroupType, + isKeyOnlyForGroup, } from 'lib/components/TaxonomicFilter/types' /* @@ -34,9 +39,13 @@ export function filterRecentsForContext( } // A flag recent stored without `key` holds only the flag description, which most flags // leave empty, so its row renders blank. Drop it; the next pick stores a labelled entry. + // A completed flag dependency is also stored without `key`, but its row takes its label from + // the property filter. Keep it, except in a key-only picker, which strips that filter. if ( item._recentContext.sourceGroupType === TaxonomicFilterGroupType.FeatureFlags && - !('key' in item && item.key) + !('key' in item && item.key) && + (isKeyOnlyForGroup(selectingKeyOnly, TaxonomicFilterGroupType.FeatureFlags) || + !isCompleteRecentPropertyFilter(item._recentContext.propertyFilter)) ) { return false } From 472fb46bd1efa812e42f374e3fc0969147961be5 Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Fri, 2 Oct 2026 19:45:43 +0000 Subject: [PATCH 6/6] fix(surveys): ignore a flag lookup that finishes after the pick changed A flag picked from Recent is loaded by id. If the user picked another flag or cleared the field before that lookup finished, its result wrote the old flag into the newer selection, and a failure cleared the newer selection. The lookup now applies its result only while its flag is still the selected one. A Recent pick clears the loaded flag first, so the previous flag's variants do not show for the new pick while the lookup runs. Every pick now goes through that one path. The branch for a selected id with no loaded flag fetched the previous id after the pick changed, so it could write the old flag into the new selection and kept the old variant. Co-Authored-By: Claude Opus 5.5 Generated-By: PostHog Desktop Task-Id: 77ee946d-952c-4c91-95bc-424e31f3839d --- frontend/src/scenes/surveys/SurveyEdit.tsx | 87 ++++++++-------------- 1 file changed, 33 insertions(+), 54 deletions(-) diff --git a/frontend/src/scenes/surveys/SurveyEdit.tsx b/frontend/src/scenes/surveys/SurveyEdit.tsx index ba8e8e978fc2..0c846f15f048 100644 --- a/frontend/src/scenes/surveys/SurveyEdit.tsx +++ b/frontend/src/scenes/surveys/SurveyEdit.tsx @@ -298,6 +298,7 @@ export default function SurveyEdit({ id }: { id: string }): JSX.Element { clearAiGeneratedTranslationField, } = useActions(surveyLogic) const { setPreferredEditor } = useActions(surveysLogic) + const mountedSurveyLogic = useMountedLogic(surveyLogic) const { featureFlags } = useValues(enabledFeaturesLogic) const surveyTranslationsEnabled = !!featureFlags[FEATURE_FLAGS.SURVEYS_TRANSLATIONS] const hostedEditorEnabled = !!featureFlags[FEATURE_FLAGS.SURVEYS_HOSTED_EDITOR] @@ -377,12 +378,20 @@ export default function SurveyEdit({ id }: { id: string }): JSX.Element { } function loadPickedLinkedFlag(flagId: number): void { + // The lookup can finish after the user picks another flag or clears the field. Its result + // applies only while its flag is still the selected one, so it cannot overwrite a newer pick. + const isStillSelected = (): boolean => mountedSurveyLogic.values.survey.linked_flag_id === flagId api.featureFlags .get(flagId) .then((linkedFlag) => { - setSurveyValue('linked_flag', linkedFlag) + if (isStillSelected()) { + setSurveyValue('linked_flag', linkedFlag) + } }) .catch((error) => { + if (!isStillSelected()) { + return + } // Only a missing flag ends the link. A failed request keeps the pick, because a save with no // linked_flag_id removes the survey's flag targeting. if (error instanceof ApiError && error.status === 404) { @@ -1443,61 +1452,31 @@ export default function SurveyEdit({ id }: { id: string }): JSX.Element { value={value} onChange={(id, _key, flag) => { onChange(id) - if ( - survey.linked_flag_id && - !survey.linked_flag - ) { - api.featureFlags - .get(survey.linked_flag_id) - .then((flag) => { - setSurveyValue( - 'linked_flag', - flag - ) - }) - .catch(() => { - // If flag doesn't exist anymore, clear the linked_flag_id - setSurveyValue( - 'linked_flag_id', - null - ) - // Reset variant selection when flag changes - const { - linkedFlagVariant, - ...conditions - } = - survey.conditions || - {} - setSurveyValue( - 'conditions', - { - ...conditions, - } - ) - }) + if (flag) { + setSurveyValue( + 'linked_flag', + flag + ) } else { - if (flag) { - setSurveyValue( - 'linked_flag', - flag - ) - } else { - // A pick from the Recent - // category carries only the - // flag id, and the variant - // selector below reads the - // flag itself. - loadPickedLinkedFlag(id) - } - // Reset variant selection when flag changes - const { - linkedFlagVariant, - ...conditions - } = survey.conditions || {} - setSurveyValue('conditions', { - ...conditions, - }) + // A pick from the Recent + // category carries only the + // flag id, and the variant + // selector below reads the + // flag itself. + setSurveyValue( + 'linked_flag', + null + ) + loadPickedLinkedFlag(id) } + // Reset variant selection when flag changes + const { + linkedFlagVariant, + ...conditions + } = survey.conditions || {} + setSurveyValue('conditions', { + ...conditions, + }) }} /> {value && (