fix(flags): make recent feature flags show and link correctly

frontend/src/lib/components/DefinitionPopover/DefinitionPopoverContents.tsx

@@ -115,7 +115,7 @@
     )
 }
 
 function DefinitionView({ group }: { group: TaxonomicFilterGroup }): JSX.Element {
     const {
         definition,
         localDefinition,
@@ -488,7 +488,7 @@
                 <div className="flex flex-col justify-between gap-4">
                     <DefinitionPopover.Section>
                         {dataWarehousePopoverFields.map(
                             ({
                                 key,
                                 label,
                                 description,
@@ -585,7 +585,7 @@
     return <></>
 }
 
 function DefinitionEdit(): JSX.Element {
     const {
         definition,
         localDefinition,
@@ -711,7 +711,7 @@
     definitionPopoverRenderer?: DefinitionPopoverRenderer
 }
 
 export function ControlledDefinitionPopover({
     visible,
     item,
     group,
@@ -756,8 +756,12 @@
                 <DefinitionPopover.Wrapper>
                     <DefinitionPopover.Header
                         title={
+                            // A group that does not title its items with `name` falls back to
+                            // `getName`, which keeps a feature flag with no description from showing
+                            // as "(empty string)". A property keeps its raw key, because
+                            // PropertyKeyInfo resolves the label from it.
                             <PropertyKeyInfo
-                                value={item.name ?? ''}
+                                value={item.name || group.getName?.(item) || ''}
                                 type={group.type}
                                 disablePopover
                                 disableIcon={!!icon}

frontend/src/lib/components/FlagSelector.test.tsx

@@ -1,30 +1,68 @@
-import { flagSelectorButtonLabel } from './FlagSelector'
-
-describe('flagSelectorButtonLabel', () => {
-    // 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.
-    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<string, unknown>, value: number | null) => {
+            expect(pickedFeatureFlag(item, value)).toBeNull()
+        })
+    })
 })

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<string, unknown>) : {}
+    // 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,8 +65,7 @@ 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.
+    // 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
@@ -50,23 +80,25 @@ 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<PickedFlag | undefined>(undefined)
 
     const { featureFlag } = useValues(featureFlagLogic({ id: value || 'link' }))
 
     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,

frontend/src/lib/components/TaxonomicFilter/InfiniteList.tsx

@@ -195,7 +195,7 @@
         <div className="taxonomic-list-row-contents-icon">{itemGroup.getIcon(item)}</div>
     ) : null
 
 const renderItemContents = ({
     item,
     listGroupType,
     itemGroup,
@@ -249,7 +249,10 @@
             )
         }
         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 (
             <div className="taxonomic-list-row-contents min-w-0">
                 {icon}
@@ -431,7 +434,7 @@
     )
 }
 
 export const InfiniteListRow = ({
     index: rowIndex,
     style,
     results,
@@ -701,7 +704,7 @@
 // when a search matches across many categories.
 const MAX_OTHER_GROUP_SWITCHES = 3
 
 function InfiniteListEmptyState(): JSX.Element {
     const {
         searchQuery,
         activeTab,
@@ -878,7 +881,7 @@
     )
 }
 
 export function InfiniteList({ popupAnchorElement, definitionPopoverRenderer }: InfiniteListProps): JSX.Element {
     const {
         mouseInteractionsEnabled,
         eventNames,

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 })

frontend/src/lib/components/TaxonomicFilter/hooks/useTaxonomicFilter.ts

@@ -26,6 +26,7 @@
 
 import {
     hasRecentContext,
+    pickMinimalRecentItem,
     recentTaxonomicFiltersLogic,
     stripRecentContext,
 } from 'lib/components/TaxonomicFilter/recentTaxonomicFiltersLogic'
@@ -203,7 +204,7 @@
     TaxonomicFilterGroupType.EmailAddresses,
 ]
 
 function resolveTaxonomicGroupTypes(
     requested: TaxonomicFilterGroupType[],
     available: Set<TaxonomicFilterGroupType>,
     eventNames: string[]
@@ -476,7 +477,7 @@
     }, [])
 
     const selectItem = useCallback(
         (group: TaxonomicFilterGroup, valueIn: TaxonomicFilterValue | null, item: any) => {
             // Mirror the legacy `taxonomicFilterLogic.selectItem` recent
             // recording so menu commits show up in the dropdown's
             // "Recent" entry. Quick-filter items and the synthetic
@@ -500,10 +501,7 @@
                           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

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,31 @@ 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' },
+            ],
+            [
+                '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'] },
+                { name: '$pageview', id: 'uuid-1' },
+            ],
+        ])('%s', (_label: string, item: Record<string, any>, expected: Record<string, any>) => {
+            expect(pickMinimalRecentItem(item)).toEqual(expected)
+        })
+    })
 })

frontend/src/lib/components/TaxonomicFilter/recentTaxonomicFiltersLogic.ts

@@ -36,6 +36,25 @@ 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`. 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', 'derived_name', 'group_key'] as const
+
+export function pickMinimalRecentItem(item: Record<string, any>): Record<string, any> {
+    const picked: Record<string, any> = { 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

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<taxonomicFilterLogicType>([
                     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