From f5b5a7f89c6fdb0871ddcc09513a9d4c50553cc0 Mon Sep 17 00:00:00 2001 From: Phill <14913130+phillram@users.noreply.github.com> Date: Fri, 2 Oct 2026 12:12:58 -0700 Subject: [PATCH 1/2] fix(replay): apply a saved filter picked from recent A row in the picker's Recent category is a summary kept in local storage: it holds the short id the row is keyed by, and not the filters, which are too heavy to store. The apply path required the filters to be on the picked row, so the guard failed and the listener returned without applying anything or reporting why. Resolve the picked short id against the already-loaded saved filter list instead. `resolveSavedFilter` is pure, so the recent, list, missing and no-filters cases are testable without mounting the replay scene. Generated-By: PostHog Desktop Task-Id: 1b898a61-0065-4b4d-b7b2-84f843040805 --- .../universalFiltersLogic.test.ts | 28 ++++++++++++++- .../UniversalFilters/universalFiltersLogic.ts | 36 ++++++++++++++++--- 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/frontend/src/lib/components/UniversalFilters/universalFiltersLogic.test.ts b/frontend/src/lib/components/UniversalFilters/universalFiltersLogic.test.ts index 221e747363ba..2631d4064c67 100644 --- a/frontend/src/lib/components/UniversalFilters/universalFiltersLogic.test.ts +++ b/frontend/src/lib/components/UniversalFilters/universalFiltersLogic.test.ts @@ -6,11 +6,12 @@ import { FilterLogicalOperator, PropertyFilterType, PropertyOperator, + SessionRecordingPlaylistType, UniversalFiltersGroup, } from '~/types' import { QuickFilterItem, TaxonomicFilterGroup, TaxonomicFilterGroupType } from '../TaxonomicFilter/types' -import { universalFiltersLogic } from './universalFiltersLogic' +import { resolveSavedFilter, universalFiltersLogic } from './universalFiltersLogic' const propertyFilter: AnyPropertyFilter = { key: '$geoip_country_code', @@ -377,4 +378,29 @@ describe('universalFiltersLogic', () => { }) }) }) + + describe('resolveSavedFilter', () => { + const savedFilter = { + short_id: 'abc123', + name: 'Rage clicks', + filters: { date_from: '-7d' }, + } as unknown as SessionRecordingPlaylistType + const unfiltered = { short_id: 'def456', name: 'Draft' } as unknown as SessionRecordingPlaylistType + + it('resolves a Recent row, which carries a short id and no filters', () => { + const recent = { name: 'Rage clicks', short_id: 'abc123' } + expect(resolveSavedFilter(recent, 'abc123', [savedFilter])).toBe(savedFilter) + }) + + it('keeps the saved filter the list hands back without consulting the list', () => { + expect(resolveSavedFilter(savedFilter, 'abc123', [])).toBe(savedFilter) + }) + + it.each([ + ['a short id no saved filter matches', 'gone', [savedFilter]], + ['a match that has no filters to apply', 'def456', [unfiltered]], + ])('resolves nothing for %s', (_name: string, shortId: string, list: SessionRecordingPlaylistType[]) => { + expect(resolveSavedFilter({ short_id: shortId }, shortId, list)).toBeNull() + }) + }) }) diff --git a/frontend/src/lib/components/UniversalFilters/universalFiltersLogic.ts b/frontend/src/lib/components/UniversalFilters/universalFiltersLogic.ts index 7076f8409288..abd31b430bdf 100644 --- a/frontend/src/lib/components/UniversalFilters/universalFiltersLogic.ts +++ b/frontend/src/lib/components/UniversalFilters/universalFiltersLogic.ts @@ -39,12 +39,32 @@ import { } from '../TaxonomicFilter/types' import { DEFAULT_UNIVERSAL_GROUP_FILTER } from './constants' -function isApplicableSavedFilter( - item: unknown -): item is SessionRecordingPlaylistType & { filters: NonNullable } { +type ApplicableSavedFilter = SessionRecordingPlaylistType & { + filters: NonNullable +} + +function isApplicableSavedFilter(item: unknown): item is ApplicableSavedFilter { return typeof item === 'object' && item !== null && 'short_id' in item && 'filters' in item && item.filters != null } +/** + * The Saved filters list hands back the saved filter itself. A row from the Recent category is a + * stored summary of one: it carries the short id the row is keyed by and never the filters, which + * are too heavy to keep in local storage. Resolve that summary against the loaded list so both + * rows apply the same filter. + */ +export function resolveSavedFilter( + item: unknown, + shortId: TaxonomicFilterValue, + savedFilters: SessionRecordingPlaylistType[] +): ApplicableSavedFilter | null { + if (isApplicableSavedFilter(item)) { + return item + } + const match = savedFilters.find((saved) => saved.short_id === shortId) + return isApplicableSavedFilter(match) ? match : null +} + function recordRecentFromPropertyFilter(propertyFilter: AnyPropertyFilter): void { if (!recentTaxonomicFiltersLogic.isMounted()) { return @@ -260,8 +280,14 @@ export const universalFiltersLogic = kea([ addGroupFilter: ({ taxonomicGroup, propertyKey, item }) => { if (taxonomicGroup.type === TaxonomicFilterGroupType.ReplaySavedFilters) { - if (isApplicableSavedFilter(item)) { - sessionRecordingSavedFiltersLogic.findMounted()?.actions.requestApplySavedFilter(item) + const savedFiltersLogic = sessionRecordingSavedFiltersLogic.findMounted() + const savedFilter = resolveSavedFilter( + item, + propertyKey, + savedFiltersLogic?.values.savedFilters.results ?? [] + ) + if (savedFilter) { + savedFiltersLogic?.actions.requestApplySavedFilter(savedFilter) } return } From 0c001e0369d5dca182685df0245bc114cf94f2c8 Mon Sep 17 00:00:00 2001 From: Phill <14913130+phillram@users.noreply.github.com> Date: Fri, 2 Oct 2026 12:35:28 -0700 Subject: [PATCH 2/2] fix(replay): fetch a recent saved filter the loaded page misses Resolving the picked short id against `savedFilters.results` alone was too narrow. That list holds one page of 30, and the saved filters panel narrows it further with its search and created-by filters, so a filter the picker offers can be absent from it. The pick then applied nothing and said nothing. The replay logic now owns the resolution through `requestApplySavedFilterByShortId`. It uses the loaded list first, falls back to one fetch by short id, and reports an unresolvable pick with a toast instead of returning in silence. Tests: the resolution cases move to the replay logic (loaded hit with no request, a miss that fetches, and an unresolvable id), and the universal filters tests now cover the wiring from the picker, which the pure helper tests did not reach. Generated-By: PostHog Desktop Task-Id: f48638a7-ba40-4124-bb01-b1c3e42b429a --- .../universalFiltersLogic.test.ts | 41 +++++++++---- .../UniversalFilters/universalFiltersLogic.ts | 32 +++------- .../sessionRecordingSavedFiltersLogic.test.ts | 59 ++++++++++++++++++- .../sessionRecordingSavedFiltersLogic.ts | 20 +++++++ 4 files changed, 113 insertions(+), 39 deletions(-) diff --git a/frontend/src/lib/components/UniversalFilters/universalFiltersLogic.test.ts b/frontend/src/lib/components/UniversalFilters/universalFiltersLogic.test.ts index 2631d4064c67..0b35414e0e2c 100644 --- a/frontend/src/lib/components/UniversalFilters/universalFiltersLogic.test.ts +++ b/frontend/src/lib/components/UniversalFilters/universalFiltersLogic.test.ts @@ -1,5 +1,7 @@ import { expectLogic } from 'kea-test-utils' +import { sessionRecordingSavedFiltersLogic } from 'scenes/session-recordings/filters/sessionRecordingSavedFiltersLogic' + import { initKeaTests } from '~/test/init' import { AnyPropertyFilter, @@ -11,7 +13,7 @@ import { } from '~/types' import { QuickFilterItem, TaxonomicFilterGroup, TaxonomicFilterGroupType } from '../TaxonomicFilter/types' -import { resolveSavedFilter, universalFiltersLogic } from './universalFiltersLogic' +import { universalFiltersLogic } from './universalFiltersLogic' const propertyFilter: AnyPropertyFilter = { key: '$geoip_country_code', @@ -379,28 +381,41 @@ describe('universalFiltersLogic', () => { }) }) - describe('resolveSavedFilter', () => { + describe('addGroupFilter with a saved replay filter', () => { const savedFilter = { short_id: 'abc123', name: 'Rage clicks', filters: { date_from: '-7d' }, } as unknown as SessionRecordingPlaylistType - const unfiltered = { short_id: 'def456', name: 'Draft' } as unknown as SessionRecordingPlaylistType + const savedFiltersGroup = { type: TaxonomicFilterGroupType.ReplaySavedFilters } as TaxonomicFilterGroup - it('resolves a Recent row, which carries a short id and no filters', () => { - const recent = { name: 'Rage clicks', short_id: 'abc123' } - expect(resolveSavedFilter(recent, 'abc123', [savedFilter])).toBe(savedFilter) + beforeEach(() => { + sessionRecordingSavedFiltersLogic.mount() }) - it('keeps the saved filter the list hands back without consulting the list', () => { - expect(resolveSavedFilter(savedFilter, 'abc123', [])).toBe(savedFilter) + it('applies the saved filter the Saved filters list hands back', async () => { + await expectLogic(sessionRecordingSavedFiltersLogic, () => { + logic.actions.addGroupFilter(savedFiltersGroup, 'abc123', savedFilter) + }).toDispatchActions([ + sessionRecordingSavedFiltersLogic.actionCreators.requestApplySavedFilter(savedFilter), + ]) }) - it.each([ - ['a short id no saved filter matches', 'gone', [savedFilter]], - ['a match that has no filters to apply', 'def456', [unfiltered]], - ])('resolves nothing for %s', (_name: string, shortId: string, list: SessionRecordingPlaylistType[]) => { - expect(resolveSavedFilter({ short_id: shortId }, shortId, list)).toBeNull() + it('asks for a Recent row by short id, because the row holds no filters', async () => { + const recentRow = { + name: 'Rage clicks', + _recentContext: { + sourceGroupType: TaxonomicFilterGroupType.ReplaySavedFilters, + sourceGroupName: 'Saved filters', + sourceValue: 'abc123', + }, + } + + await expectLogic(sessionRecordingSavedFiltersLogic, () => { + logic.actions.addGroupFilter(savedFiltersGroup, 'abc123', recentRow) + }).toDispatchActions([ + sessionRecordingSavedFiltersLogic.actionCreators.requestApplySavedFilterByShortId('abc123'), + ]) }) }) }) diff --git a/frontend/src/lib/components/UniversalFilters/universalFiltersLogic.ts b/frontend/src/lib/components/UniversalFilters/universalFiltersLogic.ts index abd31b430bdf..c934c332cf6c 100644 --- a/frontend/src/lib/components/UniversalFilters/universalFiltersLogic.ts +++ b/frontend/src/lib/components/UniversalFilters/universalFiltersLogic.ts @@ -47,24 +47,6 @@ function isApplicableSavedFilter(item: unknown): item is ApplicableSavedFilter { return typeof item === 'object' && item !== null && 'short_id' in item && 'filters' in item && item.filters != null } -/** - * The Saved filters list hands back the saved filter itself. A row from the Recent category is a - * stored summary of one: it carries the short id the row is keyed by and never the filters, which - * are too heavy to keep in local storage. Resolve that summary against the loaded list so both - * rows apply the same filter. - */ -export function resolveSavedFilter( - item: unknown, - shortId: TaxonomicFilterValue, - savedFilters: SessionRecordingPlaylistType[] -): ApplicableSavedFilter | null { - if (isApplicableSavedFilter(item)) { - return item - } - const match = savedFilters.find((saved) => saved.short_id === shortId) - return isApplicableSavedFilter(match) ? match : null -} - function recordRecentFromPropertyFilter(propertyFilter: AnyPropertyFilter): void { if (!recentTaxonomicFiltersLogic.isMounted()) { return @@ -281,13 +263,13 @@ export const universalFiltersLogic = kea([ addGroupFilter: ({ taxonomicGroup, propertyKey, item }) => { if (taxonomicGroup.type === TaxonomicFilterGroupType.ReplaySavedFilters) { const savedFiltersLogic = sessionRecordingSavedFiltersLogic.findMounted() - const savedFilter = resolveSavedFilter( - item, - propertyKey, - savedFiltersLogic?.values.savedFilters.results ?? [] - ) - if (savedFilter) { - savedFiltersLogic?.actions.requestApplySavedFilter(savedFilter) + if (isApplicableSavedFilter(item)) { + savedFiltersLogic?.actions.requestApplySavedFilter(item) + } else if (propertyKey) { + // A row from the Recent category is a stored summary of a saved filter: it holds + // the short id the row is keyed by and never the filters, which are too heavy to + // keep in local storage. The replay logic resolves the short id back to a filter. + savedFiltersLogic?.actions.requestApplySavedFilterByShortId(String(propertyKey)) } return } diff --git a/frontend/src/scenes/session-recordings/filters/sessionRecordingSavedFiltersLogic.test.ts b/frontend/src/scenes/session-recordings/filters/sessionRecordingSavedFiltersLogic.test.ts index 850c23e718e5..06975bdb4822 100644 --- a/frontend/src/scenes/session-recordings/filters/sessionRecordingSavedFiltersLogic.test.ts +++ b/frontend/src/scenes/session-recordings/filters/sessionRecordingSavedFiltersLogic.test.ts @@ -1,6 +1,7 @@ import { router } from 'kea-router' import { expectLogic } from 'kea-test-utils' +import { lemonToast } from 'lib/lemon-ui/LemonToast/LemonToast' import { removeProjectIdIfPresent } from 'lib/utils/kea-router' import { sessionRecordingSavedFiltersLogic } from 'scenes/session-recordings/filters/sessionRecordingSavedFiltersLogic' import { playlistFiltersLogic } from 'scenes/session-recordings/playlist/playlistFiltersLogic' @@ -13,6 +14,7 @@ import { ReplayTabs } from '~/types' describe('sessionRecordingSavedFiltersLogic', () => { let logic: ReturnType let savedFiltersRequestCount: number + let savedFilterRequestCount: number const savedFilter = { id: 'abc', short_id: 'short_abc', @@ -23,13 +25,17 @@ describe('sessionRecordingSavedFiltersLogic', () => { beforeEach(() => { savedFiltersRequestCount = 0 + savedFilterRequestCount = 0 useMocks({ get: { '/api/projects/:team/session_recording_playlists': () => { savedFiltersRequestCount += 1 return { results: [], count: 0 } }, - '/api/projects/:team/session_recording_playlists/:id': savedFilter, + '/api/projects/:team/session_recording_playlists/:id': () => { + savedFilterRequestCount += 1 + return savedFilter + }, }, }) initKeaTests() @@ -87,6 +93,57 @@ describe('sessionRecordingSavedFiltersLogic', () => { expect(removeProjectIdIfPresent(router.values.location.pathname)).toBe(urls.replay()) }) + describe('requestApplySavedFilterByShortId', () => { + it('applies a saved filter the loaded page holds, without fetching it again', async () => { + useMocks({ + get: { + '/api/projects/:team/session_recording_playlists': () => ({ + results: [savedFilter], + count: 1, + }), + }, + }) + logic.mount() + logic.actions.loadSavedFilters() + await expectLogic(logic).toFinishAllListeners() + + await expectLogic(logic, () => { + logic.actions.requestApplySavedFilterByShortId(savedFilter.short_id) + }) + .toFinishAllListeners() + .toMatchValues({ pendingFilterApplication: savedFilter }) + expect(savedFilterRequestCount).toBe(0) + }) + + it('fetches a saved filter the loaded page does not hold', async () => { + logic.mount() + + await expectLogic(logic, () => { + logic.actions.requestApplySavedFilterByShortId(savedFilter.short_id) + }) + .toFinishAllListeners() + .toMatchValues({ pendingFilterApplication: savedFilter }) + expect(savedFilterRequestCount).toBe(1) + }) + + it('reports a short id that resolves to no saved filter', async () => { + const errorToast = jest.spyOn(lemonToast, 'error').mockReturnValue('' as any) + useMocks({ + get: { + '/api/projects/:team/session_recording_playlists/:id': () => [404, { detail: 'Not found.' }], + }, + }) + logic.mount() + + await expectLogic(logic, () => { + logic.actions.requestApplySavedFilterByShortId('gone') + }) + .toFinishAllListeners() + .toMatchValues({ pendingFilterApplication: null }) + expect(errorToast).toHaveBeenCalled() + }) + }) + it('does not redirect a saved filter load that resolves after the user navigated away', async () => { router.actions.push(urls.replay(ReplayTabs.Home), { savedFilterId: savedFilter.short_id }) diff --git a/frontend/src/scenes/session-recordings/filters/sessionRecordingSavedFiltersLogic.ts b/frontend/src/scenes/session-recordings/filters/sessionRecordingSavedFiltersLogic.ts index c4f3b0beae0d..c97b59a13a0b 100644 --- a/frontend/src/scenes/session-recordings/filters/sessionRecordingSavedFiltersLogic.ts +++ b/frontend/src/scenes/session-recordings/filters/sessionRecordingSavedFiltersLogic.ts @@ -3,6 +3,7 @@ import { loaders } from 'kea-loaders' import { router } from 'kea-router' import api from 'lib/api' +import { lemonToast } from 'lib/lemon-ui/LemonToast/LemonToast' import { PaginationManual } from 'lib/lemon-ui/PaginationControl' import { removeProjectIdIfPresent } from 'lib/utils/kea-router' import { objectClean } from 'lib/utils/objects' @@ -102,6 +103,9 @@ export interface sessionRecordingSavedFiltersLogicActions { requestApplySavedFilter: (filter: SessionRecordingPlaylistType) => { filter: SessionRecordingPlaylistType } + requestApplySavedFilterByShortId: (shortId: SessionRecordingPlaylistType['short_id']) => { + shortId: SessionRecordingPlaylistType['short_id'] + } setAppliedSavedFilter: (appliedSavedFilter: SessionRecordingPlaylistType | null) => { appliedSavedFilter: SessionRecordingPlaylistType | null } @@ -186,6 +190,7 @@ export const sessionRecordingSavedFiltersLogic = kea ({ appliedSavedFilter }), requestApplySavedFilter: (filter: SessionRecordingPlaylistType) => ({ filter }), + requestApplySavedFilterByShortId: (shortId: SessionRecordingPlaylistType['short_id']) => ({ shortId }), clearPendingFilterApplication: true, })), reducers(() => ({ @@ -275,6 +280,21 @@ export const sessionRecordingSavedFiltersLogic = kea ({ + requestApplySavedFilterByShortId: async ({ shortId }) => { + const loadedFilter = values.savedFilters.results.find((savedFilter) => savedFilter.short_id === shortId) + if (loadedFilter?.filters) { + actions.requestApplySavedFilter(loadedFilter) + return + } + // `savedFilters` holds one page of the list, narrowed by the search and the created-by + // filter of the saved filters panel, so a filter the picker offers can be absent from it. + const fetchedFilter = await api.recordings.getPlaylist(shortId).catch(() => null) + if (fetchedFilter?.filters) { + actions.requestApplySavedFilter(fetchedFilter) + return + } + lemonToast.error('Could not apply that saved filter. It may have been deleted.') + }, setIsFiltersExpanded: ({ isFiltersExpanded }) => { if (isFiltersExpanded) { actions.loadSavedFiltersIfNeeded()