chore(taxonomic-filter): share the context filters and cover the menu

frontend/src/lib/components/TaxonomicFilter/hooks/useTaxonomicFilter.test.tsx

@@ -9,6 +9,7 @@ import { performQuery } from '~/queries/query'
 import { initKeaTests } from '~/test/init'
 
 import { recentTaxonomicFiltersLogic } from '../recentTaxonomicFiltersLogic'
+import { taxonomicFilterPinnedPropertiesLogic } from '../taxonomicFilterPinnedPropertiesLogic'
 import { TaxonomicFilterGroupType } from '../types'
 import { useTaxonomicFilter } from './useTaxonomicFilter'
 import { __clearTaxonomicResourceCache } from './useTaxonomicResource'
@@ -479,4 +480,36 @@ describe('useTaxonomicFilter', () => {
         expect(input.showNumericalPropsOnly).toBe(true)
         expect(input.enableKeywordShortcuts).toBe(true)
     })
+
+    // Pins are stored globally, so this override can carry items from groups the picker does not
+    // offer. Anything that reads it needs a list the picker can represent.
+    it('filters the Pinned override to groups this picker offers', () => {
+        const pinnedLogic = taxonomicFilterPinnedPropertiesLogic.build()
+        pinnedLogic.mount()
+        pinnedLogic.actions.setPinnedFilters([
+            {
+                groupType: TaxonomicFilterGroupType.Events,
+                groupName: 'Events',
+                value: 'signed up',
+                item: { name: 'signed up' },
+                timestamp: 0,
+            },
+            {
+                groupType: TaxonomicFilterGroupType.Cohorts,
+                groupName: 'Cohorts',
+                value: 1,
+                item: { name: 'Power users' },
+                timestamp: 0,
+            },
+        ])
+
+        const { result } = renderHook(
+            () => useTaxonomicFilter({ taxonomicGroupTypes: [TaxonomicFilterGroupType.Events] }),
+            { wrapper }
+        )
+
+        const pinnedGroup = result.current.groups.find((g) => g.type === TaxonomicFilterGroupType.PinnedFilters)!
+        const pinned = result.current.getGroupListInput(pinnedGroup).localOverride ?? []
+        expect(pinned.map((item: any) => item.name)).toEqual(['signed up'])
+    })
 })

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

@@ -21,7 +21,10 @@ import {
     TaxonomicDefinitionTypes,
     TaxonomicFilterGroupType,
 } from 'lib/components/TaxonomicFilter/types'
-import { filterRecentsForContext } from 'lib/components/TaxonomicFilter/utils/suggestedContextFilters'
+import {
+    filterPinnedForContext,
+    filterRecentsForContext,
+} from 'lib/components/TaxonomicFilter/utils/suggestedContextFilters'
 import { dataWarehouseSettingsSceneLogic } from 'scenes/data-warehouse/settings/dataWarehouseSettingsSceneLogic'
 
 import { actionsModel } from '~/models/actionsModel'
@@ -61,6 +64,10 @@ export function useTaxonomicLocalOverrides(context: {
             ),
         [recentFilterItems, taxonomicGroupTypes, excludedOperators, selectingKeyOnly, excludedProperties]
     )
+    const contextFilteredPinnedItems = useMemo(
+        () => filterPinnedForContext(pinnedFilterItems, taxonomicGroupTypes),
+        [pinnedFilterItems, taxonomicGroupTypes]
+    )
 
     return useCallback(
         (groupType: TaxonomicFilterGroupType): TaxonomicDefinitionTypes[] | undefined => {
@@ -70,7 +77,7 @@ export function useTaxonomicLocalOverrides(context: {
                 case TaxonomicFilterGroupType.RecentFilters:
                     return contextFilteredRecentItems
                 case TaxonomicFilterGroupType.PinnedFilters:
-                    return pinnedFilterItems
+                    return contextFilteredPinnedItems
                 case TaxonomicFilterGroupType.Dashboards:
                     return nameSortedDashboards as unknown as TaxonomicDefinitionTypes[]
                 case TaxonomicFilterGroupType.Experiments:
@@ -86,7 +93,7 @@ export function useTaxonomicLocalOverrides(context: {
         [
             actionsSorted,
             contextFilteredRecentItems,
-            pinnedFilterItems,
+            contextFilteredPinnedItems,
             nameSortedDashboards,
             experiments,
             dataWarehouseTablesAndViews,

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

@@ -19,7 +19,6 @@ import posthog from 'posthog-js'
 import api, { ApiMethodOptions } from 'lib/api'
 import { formatPropertyLabel } from 'lib/components/PropertyFilters/utils'
 import {
-    expandRecentsForDisplay,
     hasRecentContext,
     recentTaxonomicFiltersLogic,
 } from 'lib/components/TaxonomicFilter/recentTaxonomicFiltersLogic'
@@ -35,6 +34,7 @@ import {
     InfiniteListLogicProps,
     META_GROUP_TYPES,
     QuickFilterItem,
+    SelectingKeyOnly,
     SkeletonItem,
     isQuickFilterItem,
     isSkeletonItem,
@@ -59,6 +59,10 @@ import {
 } from 'lib/components/TaxonomicFilter/utils/floatRecentPinned'
 import { floatToFront } from 'lib/components/TaxonomicFilter/utils/floatToFront'
 import { promoteMatchingProperties } from 'lib/components/TaxonomicFilter/utils/promoteProperties'
+import {
+    filterPinnedForContext,
+    filterRecentsForContext,
+} from 'lib/components/TaxonomicFilter/utils/suggestedContextFilters'
 import { FEATURE_FLAGS } from 'lib/constants'
 import { createFuse } from 'lib/utils/fuseSearch'
 import { mapGroupQueryResponse } from 'lib/utils/groups'
@@ -510,7 +514,7 @@ export interface infiniteListLogicMeta {
             recentFilterItems: TaxonomicDefinitionTypes[],
             taxonomicGroupTypes: TaxonomicFilterGroupType[],
             arg: ExcludedOperators | undefined,
-            arg2: import('lib/components/TaxonomicFilter/types').SelectingKeyOnly | undefined,
+            arg2: SelectingKeyOnly | undefined,
             arg3: import('lib/components/TaxonomicFilter/types').TaxonomicFilterGroupValueMap | undefined
         ) => TaxonomicDefinitionTypes[]
         contextFilteredPinnedItems: (
@@ -1118,52 +1122,23 @@ export const infiniteListLogic = kea<infiniteListLogicType>([
                 recentFilterItems: TaxonomicDefinitionTypes[],
                 taxonomicGroupTypes: TaxonomicFilterGroupType[],
                 excludedOperators: ExcludedOperators | undefined,
-                selectingKeyOnly: boolean | undefined,
+                selectingKeyOnly: SelectingKeyOnly | undefined,
                 excludedProperties: ExcludedProperties | undefined
-            ): TaxonomicDefinitionTypes[] => {
-                if (!recentFilterItems?.length) {
-                    return []
-                }
-                const availableTypes = new Set(taxonomicGroupTypes)
-                const inScope = recentFilterItems.filter((item) => {
-                    if (!hasRecentContext(item) || !availableTypes.has(item._recentContext.sourceGroupType)) {
-                        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.
-                    const excludedValues = excludedProperties?.[item._recentContext.sourceGroupType]
-                    if (excludedValues?.length && excludedValues.includes(item._recentContext.sourceValue)) {
-                        return false
-                    }
-                    const excludedForGroup = excludedOperators?.[item._recentContext.sourceGroupType]
-                    if (excludedForGroup?.length) {
-                        const propertyFilter = item._recentContext.propertyFilter
-                        const operator =
-                            propertyFilter && 'operator' in propertyFilter ? propertyFilter.operator : undefined
-                        if (operator && excludedForGroup.includes(operator)) {
-                            return false
-                        }
-                    }
-                    return true
-                })
-                return expandRecentsForDisplay(inScope, selectingKeyOnly)
-            },
+            ): TaxonomicDefinitionTypes[] =>
+                filterRecentsForContext(
+                    recentFilterItems,
+                    taxonomicGroupTypes,
+                    excludedOperators,
+                    selectingKeyOnly,
+                    excludedProperties
+                ),
         ],
         contextFilteredPinnedItems: [
             (s) => [s.pinnedFilterItems, s.taxonomicGroupTypes],
             (
                 pinnedFilterItems: TaxonomicDefinitionTypes[],
                 taxonomicGroupTypes: TaxonomicFilterGroupType[]
-            ): TaxonomicDefinitionTypes[] => {
-                if (!pinnedFilterItems?.length) {
-                    return []
-                }
-                const availableTypes = new Set(taxonomicGroupTypes)
-                return pinnedFilterItems.filter(
-                    (item) => hasPinnedContext(item) && availableTypes.has(item._pinnedContext.sourceGroupType)
-                )
-            },
+            ): TaxonomicDefinitionTypes[] => filterPinnedForContext(pinnedFilterItems, taxonomicGroupTypes),
         ],
         // This list is the filter's only substantive (non-meta) group. There are no separate
         // Recent/Pinned tabs leading the filter, so this list floats recent/pinned items to

frontend/src/lib/components/TaxonomicFilter/menu/TaxonomicFilterMenu.pinned.test.tsx

@@ -0,0 +1,107 @@
+import '@testing-library/jest-dom'
+
+import { cleanup, render, screen, waitFor } from '@testing-library/react'
+import userEvent from '@testing-library/user-event'
+import { Provider } from 'kea'
+
+import { useMocks } from '~/mocks/jest'
+import { actionsModel } from '~/models/actionsModel'
+import { groupsModel } from '~/models/groupsModel'
+import { performQuery } from '~/queries/query'
+import { initKeaTests } from '~/test/init'
+import { emptyPaginated } from '~/test/mocks/taxonomicFilterApiMock'
+
+import { TaxonomicFilterHeadless } from '../headless'
+import { __clearTaxonomicResourceCache } from '../hooks/useTaxonomicResource'
+import { taxonomicFilterPinnedPropertiesLogic } from '../taxonomicFilterPinnedPropertiesLogic'
+import { TaxonomicFilterGroupType } from '../types'
+import { TaxonomicFilterMenu } from './TaxonomicFilterMenu'
+
+jest.mock('~/queries/query', () => ({
+    performQuery: jest.fn(),
+}))
+
+jest.mock('posthog-js', () => ({
+    __esModule: true,
+    default: { capture: jest.fn() },
+}))
+
+jest.mock('lib/api', () => require('~/test/mocks/taxonomicFilterApiMock').buildTaxonomicFilterApiMock())
+
+const apiGet = jest.requireMock('lib/api').default.get as jest.MockedFunction<any>
+
+// Two content groups, so the picker does not take the sole-group path that
+// promotes pinned items into the single list.
+const GROUP_TYPES = [TaxonomicFilterGroupType.Events, TaxonomicFilterGroupType.EventProperties]
+
+function renderMenu(): ReturnType<typeof render> {
+    return render(
+        <Provider>
+            <TaxonomicFilterHeadless.Root taxonomicGroupTypes={GROUP_TYPES} onChange={jest.fn()}>
+                <TaxonomicFilterMenu />
+            </TaxonomicFilterHeadless.Root>
+        </Provider>
+    )
+}
+
+function pinnedRowTexts(): string[] {
+    return Array.from(document.querySelectorAll('[data-slot="taxonomic-filter-menu-row"]')).map(
+        (el) => el.textContent ?? ''
+    )
+}
+
+describe('TaxonomicFilterMenu pinned rows', () => {
+    beforeEach(() => {
+        __clearTaxonomicResourceCache()
+        apiGet.mockReset()
+        apiGet.mockImplementation(emptyPaginated)
+        ;(performQuery as jest.Mock).mockResolvedValue({ tables: {}, joins: [] })
+        useMocks({})
+        localStorage.clear()
+        initKeaTests()
+        actionsModel.mount()
+        groupsModel.mount()
+    })
+
+    afterEach(() => cleanup())
+
+    // Pins are stored globally, so a pin can outlive the picker it was made in.
+    // Without the context filter the cohort still renders: resolveShortcutGroup
+    // falls back to the first non-meta group, so it appears as an Events row and
+    // commits a cohort id under the Events group.
+    it('leaves out a pin whose source group this picker does not offer', async () => {
+        const pinnedLogic = taxonomicFilterPinnedPropertiesLogic.build()
+        pinnedLogic.mount()
+        pinnedLogic.actions.setPinnedFilters([
+            {
+                groupType: TaxonomicFilterGroupType.Events,
+                groupName: 'Events',
+                value: 'signed up',
+                item: { name: 'signed up' },
+                timestamp: 0,
+            },
+            {
+                groupType: TaxonomicFilterGroupType.Cohorts,
+                groupName: 'Cohorts',
+                value: 1,
+                item: { name: 'Power users' },
+                timestamp: 0,
+            },
+        ])
+
+        renderMenu()
+
+        await userEvent.click(screen.getByTestId('taxonomic-filter-menu-trigger'))
+        await waitFor(() => {
+            expect(screen.getByText('Pinned')).toBeInTheDocument()
+        })
+        await userEvent.click(screen.getByText('Pinned'))
+
+        await waitFor(() => {
+            expect(pinnedRowTexts().length).toBeGreaterThan(0)
+        })
+        const rows = pinnedRowTexts()
+        expect(rows.some((text) => text.includes('signed up'))).toBe(true)
+        expect(rows.some((text) => text.includes('Power users'))).toBe(false)
+    })
+})

frontend/src/lib/components/TaxonomicFilter/utils/suggestedContextFilters.ts

@@ -9,12 +9,8 @@ import {
 } from 'lib/components/TaxonomicFilter/types'
 
 /*
- * These mirror the legacy `infiniteListLogic` context-filter selectors
- * (`contextFilteredRecentItems` / `contextFilteredPinnedItems`). It's a
- * deliberate fork, not a shared util: the rebuild and the legacy kea picker
- * are independent code paths, and the legacy one is being retired with the
- * rest of `infiniteListLogic`. Until then, behaviour changes here that should
- * also apply to the legacy picker must be made in both places.
+ * The legacy `infiniteListLogic` picker and the rebuild both call these helpers,
+ * so a change to recent or pinned behavior reaches both.
  */
 
 /** Recents whose source group is one of the picker's groups, with operators the
@@ -37,7 +33,8 @@ export function filterRecentsForContext(
             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.
+        // 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.
         const excludedValues = excludedProperties?.[item._recentContext.sourceGroupType]
         if (excludedValues?.length && excludedValues.includes(item._recentContext.sourceValue)) {
             return false