feat: remember show advanced filters (#18170)

* feat: remember show advanced filters * fix advanced filters check
PostHog · Oct 25, 2023 · 367b44a · 367b44a
1 parent 4fc05e3
commit 367b44a
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 22 deletions.
diff --git a/frontend/src/scenes/session-recordings/filters/SessionRecordingsFilters.tsx b/frontend/src/scenes/session-recordings/filters/SessionRecordingsFilters.tsx
@@ -101,8 +101,9 @@ export function SessionRecordingsFilters({
                     size="small"
                     onClick={() => setShowAdvancedFilters(!showAdvancedFilters)}
                     disabledReason={
-                        hasAdvancedFilters &&
-                        'You are only allowed person filters and a single pageview event to switch back to simple filters'
+                        hasAdvancedFilters
+                            ? 'You are only allowed person filters and a single pageview event (filtered by current url) to switch back to simple filters'
+                            : undefined
                     }
                     data-attr={`session-recordings-show-${showAdvancedFilters ? 'simple' : 'advanced'}-filters`}
                 >

diff --git a/frontend/src/scenes/session-recordings/playlist/sessionRecordingsPlaylistLogic.ts b/frontend/src/scenes/session-recordings/playlist/sessionRecordingsPlaylistLogic.ts
@@ -79,30 +79,64 @@ export const getDefaultFilters = (personUUID?: PersonUUID): RecordingFilters =>
     return personUUID ? DEFAULT_PERSON_RECORDING_FILTERS : DEFAULT_RECORDING_FILTERS
 }
 
+function isPageViewFilter(filter: Record<string, any>): boolean {
+    return filter.name === '$pageview'
+}
+function isCurrentURLPageViewFilter(eventsFilter: Record<string, any>): boolean {
+    const hasSingleProperty = Array.isArray(eventsFilter.properties) && eventsFilter.properties?.length === 1
+    const isCurrentURLProperty = hasSingleProperty && eventsFilter.properties[0].key === '$current_url'
+    return isPageViewFilter(eventsFilter) && isCurrentURLProperty
+}
+
+// checks are stored against filter keys so that the type system enforces adding a check when we add new filters
+const advancedFilterChecks: Record<
+    keyof RecordingFilters,
+    (filters: RecordingFilters, defaultFilters: RecordingFilters) => boolean
+> = {
+    actions: (filters) => (filters.actions ? filters.actions.length > 0 : false),
+    events: function (filters: RecordingFilters): boolean {
+        const eventsFilters = filters.events || []
+        // simple filters allow a single $pageview event filter with $current_url as the selected property
+        // anything else is advanced
+        return (
+            eventsFilters.length > 1 ||
+            (!!eventsFilters[0] &&
+                (!isPageViewFilter(eventsFilters[0]) || !isCurrentURLPageViewFilter(eventsFilters[0])))
+        )
+    },
+    properties: function (): boolean {
+        // TODO is this right? should we ever care about properties for choosing between advanced and simple?
+        return false
+    },
+    date_from: (filters, defaultFilters) => filters.date_from != defaultFilters.date_from,
+    date_to: (filters, defaultFilters) => filters.date_to != defaultFilters.date_to,
+    session_recording_duration: (filters, defaultFilters) =>
+        !equal(filters.session_recording_duration, defaultFilters.session_recording_duration),
+    duration_type_filter: (filters, defaultFilters) =>
+        filters.duration_type_filter !== defaultFilters.duration_type_filter,
+    console_search_query: (filters) =>
+        filters.console_search_query ? filters.console_search_query.trim().length > 0 : false,
+    console_logs: (filters) => (filters.console_logs ? filters.console_logs.length > 0 : false),
+    filter_test_accounts: (filters) => filters.filter_test_accounts ?? false,
+}
+
 export const addedAdvancedFilters = (
     filters: RecordingFilters | undefined,
     defaultFilters: RecordingFilters
 ): boolean => {
-    if (!filters) {
+    // if there are no filters or if some filters are not present then the page is still booting up
+    if (!filters || filters.session_recording_duration === undefined || filters.date_from === undefined) {
         return false
     }
 
-    const hasActions = filters.actions ? filters.actions.length > 0 : false
-    const hasChangedDateFrom = filters.date_from != defaultFilters.date_from
-    const hasChangedDateTo = filters.date_to != defaultFilters.date_to
-    const hasConsoleLogsFilters = filters.console_logs ? filters.console_logs.length > 0 : false
-    const hasChangedDuration = !equal(filters.session_recording_duration, defaultFilters.session_recording_duration)
-    const eventsFilters = filters.events || []
-    const hasAdvancedEvents = eventsFilters.length > 1 || (!!eventsFilters[0] && eventsFilters[0].name != '$pageview')
-
-    return (
-        hasActions ||
-        hasAdvancedEvents ||
-        hasChangedDuration ||
-        hasChangedDateFrom ||
-        hasChangedDateTo ||
-        hasConsoleLogsFilters
-    )
+    // keeps results with the keys for printing when debugging
+    const checkResults = Object.keys(advancedFilterChecks).map((key) => ({
+        key,
+        result: advancedFilterChecks[key](filters, defaultFilters),
+    }))
+
+    // if any check is true, then this is an advanced filter
+    return checkResults.some((checkResult) => checkResult.result)
 }
 
 export const defaultPageviewPropertyEntityFilter = (
@@ -348,8 +382,14 @@ export const sessionRecordingsPlaylistLogic = kea<sessionRecordingsPlaylistLogic
         showAdvancedFilters: [
             addedAdvancedFilters(props.filters, getDefaultFilters(props.personUUID)),
             {
-                setFilters: (showingAdvancedFilters, { filters }) =>
-                    addedAdvancedFilters(filters, getDefaultFilters(props.personUUID)) ? true : showingAdvancedFilters,
+                persist: true,
+            },
+            {
+                setFilters: (showingAdvancedFilters, { filters }) => {
+                    return addedAdvancedFilters(filters, getDefaultFilters(props.personUUID))
+                        ? true
+                        : showingAdvancedFilters
+                },
                 setShowAdvancedFilters: (_, { showAdvancedFilters }) => showAdvancedFilters,
             },
         ],

diff --git a/package.json b/package.json
@@ -225,7 +225,6 @@
         "@types/react-resizable": "^3.0.4",
         "@types/react-syntax-highlighter": "^15.5.7",
         "@types/react-textfit": "^1.1.0",
-        "@types/react-transition-group": "^4.4.4",
         "@types/react-virtualized": "^9.21.14",
         "@types/testing-library__jest-dom": "^5.14.5",
         "@types/zxcvbn": "^4.4.0",