From ab93452cdfc0fd0d840eb0a94e1622be48425526 Mon Sep 17 00:00:00 2001 From: Jordi Enric <37541088+jordienr@users.noreply.github.com> Date: Mon, 8 Jun 2026 11:31:47 +0200 Subject: [PATCH] fix(unified-logs): apply sidebar facet filters on click (#46589) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem Clicking a checkbox in the logs filter sidebar (Log Type, Status, Method, etc.) doesn't do anything. The box ticks, but the log list and counts don't change. Only the search bar at the top actually filters. On top of that, opening a URL that already has filters renders the sidebar checkboxes unticked, so the active filters are invisible. ## Solution The top search bar and the sidebar each store a selected filter in a different internal format. A recent change taught the query builder (`columnFiltersToLogsFilters`) to understand only the search bar's wrapped `{ operator, values }` format, so anything clicked in the sidebar (a bare `string[]`) was thrown away before it reached the query, and nothing refetched. - Accept the sidebar's bare format, treating it as a plain "equals" filter (which is exactly what a checkbox means). Sidebar clicks apply immediately again. - Seed the sidebar checkboxes from the URL: equality filter groups are seeded as a bare `string[]` (the shape the checkbox reads), so they render ticked on load. Non-eq groups (neq/ilike from the top bar) stay wrapped so their operator survives a round-trip. - Keep the time-range picker out of the `filter` URL param so it doesn't get swept in by mistake. - Extracted the URL-building (`buildFilterSearchUpdate`) and URL-seeding (`logsFiltersToColumnFilters`) into pure helpers so the click-to-query and URL-load wiring are unit tested, not just the transform. ## How to test Open Unified Logs and click a Log Type / Status / Method checkbox in the left sidebar. The list and counts should update right away, without touching the top search bar. Reload the page (or open a shared URL with filters): the matching sidebar checkboxes should be ticked. `UnifiedLogs.filters.test.ts` covers both filter formats, the time-range exclusion, cleared filters, the click-to-URL wiring, and the URL-load round-trip (including that a neq filter is not downgraded to eq). ## Summary by CodeRabbit * **Tests** * Added coverage for filter ↔ URL conversions, checkbox grouping, operator preservation, null/cleared handling, and timerange routing. * **Bug Fixes** * Consistently normalize and serialize varied filter input shapes. * Omit non-allowlisted columns from URL filters. * Ensure timerange uses its dedicated URL key and is removed when cleared. * **Refactor** * Centralized filter serialization/deserialization and simplified URL update wiring. --------- Co-authored-by: Claude Opus 4.8 --- .../UnifiedLogs/UnifiedLogs.filters.test.ts | 124 ++++++++++++++++++ .../UnifiedLogs/UnifiedLogs.filters.ts | 44 ++++++- .../interfaces/UnifiedLogs/UnifiedLogs.tsx | 27 +--- 3 files changed, 168 insertions(+), 27 deletions(-) create mode 100644 apps/studio/components/interfaces/UnifiedLogs/UnifiedLogs.filters.test.ts diff --git a/apps/studio/components/interfaces/UnifiedLogs/UnifiedLogs.filters.test.ts b/apps/studio/components/interfaces/UnifiedLogs/UnifiedLogs.filters.test.ts new file mode 100644 index 00000000000..68538d1680d --- /dev/null +++ b/apps/studio/components/interfaces/UnifiedLogs/UnifiedLogs.filters.test.ts @@ -0,0 +1,124 @@ +import { describe, expect, it } from 'vitest' + +import { + buildFilterSearchUpdate, + columnFiltersToLogsFilters, + logsFiltersToColumnFilters, + logsFiltersToUrlParams, + parseLogsFilterUrlParams, +} from './UnifiedLogs.filters' + +describe('columnFiltersToLogsFilters', () => { + it('serializes a bare string[] (sidebar checkbox) using the default `=` operator', () => { + const filters = columnFiltersToLogsFilters([ + { id: 'log_type', value: ['postgres', 'postgrest'] }, + ]) + expect(filters).toEqual([ + { column: 'log_type', operator: '=', value: 'postgres' }, + { column: 'log_type', operator: '=', value: 'postgrest' }, + ]) + expect(logsFiltersToUrlParams(filters)).toEqual([ + 'log_type:eq:postgres', + 'log_type:eq:postgrest', + ]) + }) + + it('preserves the operator from a wrapped value (top filter bar)', () => { + const filters = columnFiltersToLogsFilters([ + { id: 'method', value: { operator: '<>', values: ['GET'] } }, + ]) + expect(filters).toEqual([{ column: 'method', operator: '<>', value: 'GET' }]) + }) + + it('treats a bare non-array scalar as a single `=` value', () => { + const filters = columnFiltersToLogsFilters([{ id: 'status', value: '500' }]) + expect(filters).toEqual([{ column: 'status', operator: '=', value: '500' }]) + }) + + it('excludes columns not in filterableNames (e.g. the `date` timerange brush)', () => { + const filterable = new Set(['log_type', 'method']) + const filters = columnFiltersToLogsFilters( + [ + { id: 'date', value: [new Date('2026-05-08T00:00:00Z'), new Date('2026-05-08T01:00:00Z')] }, + { id: 'log_type', value: ['postgres'] }, + ], + filterable + ) + expect(filters).toEqual([{ column: 'log_type', operator: '=', value: 'postgres' }]) + }) + + it('skips null/undefined values (cleared filters)', () => { + const filters = columnFiltersToLogsFilters([ + { id: 'log_type', value: null }, + { id: 'method', value: undefined }, + ]) + expect(filters).toEqual([]) + }) +}) + +describe('logsFiltersToColumnFilters', () => { + it('seeds an `=` group as a bare string[] so sidebar checkboxes render ticked', () => { + const columnFilters = logsFiltersToColumnFilters([ + { column: 'log_type', operator: '=', value: 'postgres' }, + { column: 'log_type', operator: '=', value: 'postgrest' }, + ]) + expect(columnFilters).toEqual([{ id: 'log_type', value: ['postgres', 'postgrest'] }]) + }) + + it('keeps non-eq groups wrapped so the operator survives a round-trip', () => { + const columnFilters = logsFiltersToColumnFilters([ + { column: 'event_message', operator: '~~*', value: 'error' }, + ]) + expect(columnFilters).toEqual([ + { id: 'event_message', value: { operator: '~~*', values: ['error'] } }, + ]) + }) + + it('round-trips an eq URL filter back to the same param without flipping the operator', () => { + const seeded = logsFiltersToColumnFilters(parseLogsFilterUrlParams(['method:eq:GET'])) + const update = buildFilterSearchUpdate(seeded, [{ value: 'method', type: 'checkbox' }]) + expect(update.filter).toEqual(['method:eq:GET']) + }) + + it('round-trips a neq URL filter without downgrading it to eq', () => { + const seeded = logsFiltersToColumnFilters(parseLogsFilterUrlParams(['method:neq:GET'])) + const update = buildFilterSearchUpdate(seeded, [{ value: 'method', type: 'checkbox' }]) + expect(update.filter).toEqual(['method:neq:GET']) + }) +}) + +describe('buildFilterSearchUpdate', () => { + const fields = [ + { value: 'date', type: 'timerange' }, + { value: 'log_type', type: 'checkbox' }, + { value: 'method', type: 'checkbox' }, + ] + + it('serializes a bare sidebar checkbox into the `filter` param (the regression)', () => { + const update = buildFilterSearchUpdate([{ id: 'log_type', value: ['postgres'] }], fields) + expect(update.filter).toEqual(['log_type:eq:postgres']) + }) + + it('clears the `filter` param to null when no equality filters are set', () => { + const update = buildFilterSearchUpdate([{ id: 'log_type', value: null }], fields) + expect(update.filter).toBeNull() + }) + + it('routes a timerange to its own URL key, never into `filter`', () => { + const range = [new Date('2026-05-08T00:00:00Z'), new Date('2026-05-08T01:00:00Z')] + const update = buildFilterSearchUpdate( + [ + { id: 'date', value: range }, + { id: 'method', value: ['GET'] }, + ], + fields + ) + expect(update.filter).toEqual(['method:eq:GET']) + expect(update.date).toBe(range) + }) + + it('nulls an absent timerange key so a cleared brush is removed from the URL', () => { + const update = buildFilterSearchUpdate([{ id: 'method', value: ['GET'] }], fields) + expect(update.date).toBeNull() + }) +}) diff --git a/apps/studio/components/interfaces/UnifiedLogs/UnifiedLogs.filters.ts b/apps/studio/components/interfaces/UnifiedLogs/UnifiedLogs.filters.ts index e327798e678..32614bf9088 100644 --- a/apps/studio/components/interfaces/UnifiedLogs/UnifiedLogs.filters.ts +++ b/apps/studio/components/interfaces/UnifiedLogs/UnifiedLogs.filters.ts @@ -81,15 +81,51 @@ export const groupLogsFiltersByColumn = ( return grouped } +export const logsFiltersToColumnFilters = ( + filters: LogsFilter[] +): { id: string; value: string[] | LogsColumnFilterValue }[] => { + return Object.entries(groupLogsFiltersByColumn(filters)).map(([id, group]) => + group.operator === '=' ? { id, value: group.values } : { id, value: group } + ) +} + export const columnFiltersToLogsFilters = ( - columnFilters: { id: string; value: unknown }[] + columnFilters: { id: string; value: unknown }[], + filterableNames?: Set ): LogsFilter[] => { const filters: LogsFilter[] = [] for (const { id, value } of columnFilters) { - if (!isLogsFilterColumnValue(value)) continue - for (const v of value.values) { - filters.push({ column: id, operator: value.operator, value: String(v) }) + if (filterableNames && !filterableNames.has(id)) continue + if (value === null || value === undefined) continue + const fallback: LogsColumnFilterValue = { + operator: '=', + values: (Array.isArray(value) ? value : [value]).map(String), + } + const { operator, values } = isLogsFilterColumnValue(value) ? value : fallback + for (const v of values) { + filters.push({ column: id, operator, value: v }) } } return filters } + +export const buildFilterSearchUpdate = ( + columnFilters: { id: string; value: unknown }[], + filterFields: { value: string; type: string }[] +): Record => { + const filterableNames = new Set( + filterFields.filter((field) => field.type !== 'timerange').map((field) => field.value) + ) + const filterEntries = logsFiltersToUrlParams( + columnFiltersToLogsFilters(columnFilters, filterableNames) + ) + const update: Record = { + filter: filterEntries.length > 0 ? filterEntries : null, + } + for (const field of filterFields) { + if (field.type !== 'timerange') continue + const current = columnFilters.find((c) => c.id === field.value)?.value + update[field.value] = current ?? null + } + return update +} diff --git a/apps/studio/components/interfaces/UnifiedLogs/UnifiedLogs.tsx b/apps/studio/components/interfaces/UnifiedLogs/UnifiedLogs.tsx index 58884413e67..a04be8fa290 100644 --- a/apps/studio/components/interfaces/UnifiedLogs/UnifiedLogs.tsx +++ b/apps/studio/components/interfaces/UnifiedLogs/UnifiedLogs.tsx @@ -39,9 +39,8 @@ import { ServiceFlowPanel } from './ServiceFlowPanel' import { SEARCH_PARAMS_PARSER } from './UnifiedLogs.constants' import { filterFields as defaultFilterFields } from './UnifiedLogs.fields' import { - columnFiltersToLogsFilters, - groupLogsFiltersByColumn, - logsFiltersToUrlParams, + buildFilterSearchUpdate, + logsFiltersToColumnFilters, parseLogsFilterUrlParams, } from './UnifiedLogs.filters' import { useLiveMode, useResetFocus } from './UnifiedLogs.hooks' @@ -92,12 +91,7 @@ export const UnifiedLogs = () => { const defaultColumnSorting = search.sort ? [search.sort] : [] const defaultColumnVisibility = { uuid: false } - // Column filters are seeded from the repeatable `filter` URL param. Each entry - // (`col:opAbbrev:value`) is grouped per column into a wrapped { operator, values } - // shape that the query builder reads back. - const defaultColumnFilters = Object.entries( - groupLogsFiltersByColumn(parseLogsFilterUrlParams(search.filter)) - ).map(([id, value]) => ({ id, value })) + const defaultColumnFilters = logsFiltersToColumnFilters(parseLogsFilterUrlParams(search.filter)) const [topBarHeight, setTopBarHeight] = useState(0) const topBarRef = useRef(null) @@ -294,21 +288,8 @@ export const UnifiedLogs = () => { }) }, [facets]) - // Debounced filter application to avoid too many API calls when user clicks multiple filters quickly. - // All equality/pattern column filters serialize into the repeatable `filter` URL param. Slider/timerange - // column filters keep their dedicated per-column URL keys so things like the timeline brush still - // round-trip — they have their own range semantics and aren't covered by eq/neq/like. const applyFilterSearch = () => { - const filterEntries = logsFiltersToUrlParams(columnFiltersToLogsFilters(columnFilters)) - const update: Record = { - filter: filterEntries.length > 0 ? filterEntries : null, - } - for (const field of filterFields) { - if (field.type !== 'timerange') continue - const current = columnFilters.find((c) => c.id === field.value)?.value - update[field.value] = current ?? null - } - setSearch(update) + setSearch(buildFilterSearchUpdate(columnFilters, filterFields)) } const debouncedApplyFilterSearch = useDebounce(applyFilterSearch, 250)