From 2129eb5057671869ef51bc3ea2b187a77e19b25a Mon Sep 17 00:00:00 2001 From: Felix Hennig Date: Tue, 5 May 2026 08:42:19 +0200 Subject: [PATCH 1/5] initial implementation --- .../AdvancedQueryFilter.browser.spec.tsx | 16 +++++++++ .../genspectrum/AdvancedQueryFilter.tsx | 16 ++++++++- .../pageStateSelectors/BaselineSelector.tsx | 7 +++- .../src/lapis/siloFilterExpression.spec.ts | 35 ++++++++++++++++++- website/src/lapis/siloFilterExpression.ts | 29 +++++++++++++++ 5 files changed, 100 insertions(+), 3 deletions(-) diff --git a/website/src/components/genspectrum/AdvancedQueryFilter.browser.spec.tsx b/website/src/components/genspectrum/AdvancedQueryFilter.browser.spec.tsx index e224cb142..ce61838f9 100644 --- a/website/src/components/genspectrum/AdvancedQueryFilter.browser.spec.tsx +++ b/website/src/components/genspectrum/AdvancedQueryFilter.browser.spec.tsx @@ -105,6 +105,22 @@ describe('AdvancedQueryFilter', () => { await expect.element(getByTitle('Validating')).toBeVisible(); }); + // TODO: allowedFields - shows error when query references a disallowed metadata field + // - mock a successful parse returning e.g. { type: 'StringEquals', column: 'host', value: 'Human' } + // - render with allowedFields={['nextcladePangoLineage']} + // - assert error icon is shown and message contains the disallowed field name and the allowed list + // - assert onInput was NOT called + + // TODO: allowedFields - does not show error when all referenced fields are in the allowed list + // - mock a successful parse returning a Lineage filter on 'nextcladePangoLineage' + // - render with allowedFields={['nextcladePangoLineage']} + // - assert checkmark is shown and onInput was called with the query + + // TODO: allowedFields - mutation-only query passes even with a restrictive allowedFields list + // - mock a successful parse returning e.g. { type: 'NucleotideEquals', position: 123, symbol: 'A' } + // - render with allowedFields={['nextcladePangoLineage']} + // - assert checkmark is shown (no metadata columns referenced, so nothing to block) + it('shows error icon with network error tooltip when LAPIS is unreachable', async ({ routeMockers }) => { routeMockers.lapis.mockLapisDown(); diff --git a/website/src/components/genspectrum/AdvancedQueryFilter.tsx b/website/src/components/genspectrum/AdvancedQueryFilter.tsx index 3a51bf488..b8b53fce9 100644 --- a/website/src/components/genspectrum/AdvancedQueryFilter.tsx +++ b/website/src/components/genspectrum/AdvancedQueryFilter.tsx @@ -3,6 +3,7 @@ import { type FC, type InputEvent, useEffect, useRef, useState } from 'react'; import { getClientLogger } from '../../clientLogger.ts'; import { parseQuery } from '../../lapis/parseQuery.ts'; +import { extractMetadataFields } from '../../lapis/siloFilterExpression.ts'; const logger = getClientLogger('AdvancedQueryFilter'); @@ -19,9 +20,10 @@ type AdvancedQueryFilterProps = { onInput?: (newValue: string | undefined) => void; enabled: boolean; lapisUrl: string; + allowedFields?: string[]; }; -export const AdvancedQueryFilter: FC = ({ value, onInput, enabled, lapisUrl }) => { +export const AdvancedQueryFilter: FC = ({ value, onInput, enabled, lapisUrl, allowedFields }) => { const [inputValue, setInputValue] = useState(value); const [validationState, setValidationState] = useState({ type: 'idle' }); const userEditedRef = useRef(false); @@ -31,6 +33,18 @@ export const AdvancedQueryFilter: FC = ({ value, onInp onSuccess: (results, query) => { const result = results[0]; if (result.type === 'success') { + if (allowedFields !== undefined) { + const usedFields = extractMetadataFields(result.filter); + const disallowed = usedFields.filter((col) => !allowedFields.includes(col)); + if (disallowed.length > 0) { + const listed = disallowed.map((col) => `"${col}"`).join(', '); + setValidationState({ + type: 'error', + message: `Field ${listed} is not allowed. Allowed fields: ${allowedFields.join(', ')}.`, + }); + return; + } + } setValidationState({ type: 'valid' }); onInput?.(query); } else { diff --git a/website/src/components/pageStateSelectors/BaselineSelector.tsx b/website/src/components/pageStateSelectors/BaselineSelector.tsx index f56253eec..ff25f1ec5 100644 --- a/website/src/components/pageStateSelectors/BaselineSelector.tsx +++ b/website/src/components/pageStateSelectors/BaselineSelector.tsx @@ -34,6 +34,10 @@ export type NumberRangeFilterConfig = { sliderStep?: number; }; +export type AdvancedQueryFilterConfig = { + allowedFields?: string[]; +}; + export type BaselineFilterConfig = | ({ type: 'date'; @@ -41,7 +45,7 @@ export type BaselineFilterConfig = | ({ type: 'text' } & TextInputConfig) | ({ type: 'location' } & LocationFilterConfig) | ({ type: 'number' } & NumberRangeFilterConfig) - | { type: 'advancedQuery' }; + | ({ type: 'advancedQuery' } & AdvancedQueryFilterConfig); export function BaselineSelector({ baselineFilterConfigs, @@ -177,6 +181,7 @@ export function BaselineSelector({ value={datasetFilter.advancedQuery ?? ''} enabled={enableAdvancedQueryFilter} lapisUrl={lapisUrl} + allowedFields={config.allowedFields} /> ); } diff --git a/website/src/lapis/siloFilterExpression.spec.ts b/website/src/lapis/siloFilterExpression.spec.ts index ba3e4c113..09eec73ee 100644 --- a/website/src/lapis/siloFilterExpression.spec.ts +++ b/website/src/lapis/siloFilterExpression.spec.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from 'vitest'; -import { siloFilterExpressionSchema } from './siloFilterExpression.ts'; +import { SiloFilterExpression, extractMetadataFields, siloFilterExpressionSchema } from './siloFilterExpression.ts'; describe('siloFilterExpressionSchema', () => { test('should parse StringEquals', () => { @@ -230,6 +230,39 @@ describe('siloFilterExpressionSchema', () => { expect(result.success).toBe(false); }); +describe('extractMetadataFields', () => { + test('extracts column names from nested And/Or expressions', () => { + const expr: SiloFilterExpression = { + type: 'And', + children: [ + { type: 'Lineage', column: 'nextcladePangoLineage', value: 'BA.1', includeSublineages: true }, + { + type: 'Or', + children: [ + { type: 'StringEquals', column: 'country', value: 'Germany' }, + { type: 'DateBetween', column: 'date', from: '2024-01-01', to: null }, + ], + }, + ], + }; + + expect(extractMetadataFields(expr)).toEqual(['nextcladePangoLineage', 'country', 'date']); + }); + + test('returns no columns for mutation-only expressions', () => { + const expr: Parameters[0] = { + type: 'And', + children: [ + { type: 'NucleotideEquals', position: 123, symbol: 'A' }, + { type: 'HasAminoAcidMutation', sequenceName: 'S', position: 501 }, + ], + }; + + expect(extractMetadataFields(expr)).toEqual([]); + }); +}); + +describe('siloFilterExpressionSchema (parse)', () => { test('should accept nullable values', () => { const data = { type: 'StringEquals', diff --git a/website/src/lapis/siloFilterExpression.ts b/website/src/lapis/siloFilterExpression.ts index 2b3a1050b..6407d15a3 100644 --- a/website/src/lapis/siloFilterExpression.ts +++ b/website/src/lapis/siloFilterExpression.ts @@ -166,6 +166,35 @@ const nOfSchema: z.ZodType<{ }), ); +/** + * Given an expression, returns a list of all the metadata fields that are referenced + * in the expression. + */ +export function extractMetadataFields(expr: SiloFilterExpression): string[] { + switch (expr.type) { + case 'StringEquals': + case 'BooleanEquals': + case 'Lineage': + case 'DateBetween': + case 'IntEquals': + case 'IntBetween': + case 'FloatEquals': + case 'FloatBetween': + case 'StringSearch': + case 'PhyloDescendantOf': + return [expr.column]; + case 'And': + case 'Or': + case 'N-Of': + return expr.children.flatMap(extractMetadataFields); + case 'Not': + case 'Maybe': + return extractMetadataFields(expr.child); + default: + return []; + } +} + // Combined union for all SiloFilterExpression types. // This schema was initially LLM generated from the LAPIS code. export const siloFilterExpressionSchema = z.union([ From 4a994542b17d3131001b48ef94a81c959353e193 Mon Sep 17 00:00:00 2001 From: Felix Hennig Date: Tue, 5 May 2026 08:50:03 +0200 Subject: [PATCH 2/5] format --- .../src/components/genspectrum/AdvancedQueryFilter.tsx | 8 +++++++- website/src/lapis/siloFilterExpression.spec.ts | 1 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/website/src/components/genspectrum/AdvancedQueryFilter.tsx b/website/src/components/genspectrum/AdvancedQueryFilter.tsx index b8b53fce9..d0d3e195c 100644 --- a/website/src/components/genspectrum/AdvancedQueryFilter.tsx +++ b/website/src/components/genspectrum/AdvancedQueryFilter.tsx @@ -23,7 +23,13 @@ type AdvancedQueryFilterProps = { allowedFields?: string[]; }; -export const AdvancedQueryFilter: FC = ({ value, onInput, enabled, lapisUrl, allowedFields }) => { +export const AdvancedQueryFilter: FC = ({ + value, + onInput, + enabled, + lapisUrl, + allowedFields, +}) => { const [inputValue, setInputValue] = useState(value); const [validationState, setValidationState] = useState({ type: 'idle' }); const userEditedRef = useRef(false); diff --git a/website/src/lapis/siloFilterExpression.spec.ts b/website/src/lapis/siloFilterExpression.spec.ts index 09eec73ee..29c998960 100644 --- a/website/src/lapis/siloFilterExpression.spec.ts +++ b/website/src/lapis/siloFilterExpression.spec.ts @@ -229,6 +229,7 @@ describe('siloFilterExpressionSchema', () => { expect(result.success).toBe(false); }); +}); describe('extractMetadataFields', () => { test('extracts column names from nested And/Or expressions', () => { From 99bf5d101f541bcf2d3de123b5ecf34fff7a66a4 Mon Sep 17 00:00:00 2001 From: Felix Hennig Date: Tue, 5 May 2026 09:16:47 +0200 Subject: [PATCH 3/5] fix(website): use type-only import for SiloFilterExpression in spec Required by verbatimModuleSyntax. Co-Authored-By: Claude Sonnet 4.6 --- website/src/lapis/siloFilterExpression.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/src/lapis/siloFilterExpression.spec.ts b/website/src/lapis/siloFilterExpression.spec.ts index 29c998960..4439a2100 100644 --- a/website/src/lapis/siloFilterExpression.spec.ts +++ b/website/src/lapis/siloFilterExpression.spec.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from 'vitest'; -import { SiloFilterExpression, extractMetadataFields, siloFilterExpressionSchema } from './siloFilterExpression.ts'; +import { type SiloFilterExpression, extractMetadataFields, siloFilterExpressionSchema } from './siloFilterExpression.ts'; describe('siloFilterExpressionSchema', () => { test('should parse StringEquals', () => { From c8d9c312a1a9b9d2d67af6254224536ffe0c0bea Mon Sep 17 00:00:00 2001 From: Felix Hennig Date: Tue, 5 May 2026 10:41:02 +0200 Subject: [PATCH 4/5] format --- website/src/lapis/siloFilterExpression.spec.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/website/src/lapis/siloFilterExpression.spec.ts b/website/src/lapis/siloFilterExpression.spec.ts index 4439a2100..41c289c66 100644 --- a/website/src/lapis/siloFilterExpression.spec.ts +++ b/website/src/lapis/siloFilterExpression.spec.ts @@ -1,6 +1,10 @@ import { describe, expect, test } from 'vitest'; -import { type SiloFilterExpression, extractMetadataFields, siloFilterExpressionSchema } from './siloFilterExpression.ts'; +import { + type SiloFilterExpression, + extractMetadataFields, + siloFilterExpressionSchema, +} from './siloFilterExpression.ts'; describe('siloFilterExpressionSchema', () => { test('should parse StringEquals', () => { From 8145503bc181aaaf7bfd7fb2ae70fb20556f7993 Mon Sep 17 00:00:00 2001 From: Felix Hennig Date: Tue, 5 May 2026 10:48:47 +0200 Subject: [PATCH 5/5] test(website): implement allowedFields browser tests for AdvancedQueryFilter Co-Authored-By: Claude Sonnet 4.6 --- .../AdvancedQueryFilter.browser.spec.tsx | 97 ++++++++++++++++--- 1 file changed, 82 insertions(+), 15 deletions(-) diff --git a/website/src/components/genspectrum/AdvancedQueryFilter.browser.spec.tsx b/website/src/components/genspectrum/AdvancedQueryFilter.browser.spec.tsx index ce61838f9..1d6e4b5f2 100644 --- a/website/src/components/genspectrum/AdvancedQueryFilter.browser.spec.tsx +++ b/website/src/components/genspectrum/AdvancedQueryFilter.browser.spec.tsx @@ -105,21 +105,88 @@ describe('AdvancedQueryFilter', () => { await expect.element(getByTitle('Validating')).toBeVisible(); }); - // TODO: allowedFields - shows error when query references a disallowed metadata field - // - mock a successful parse returning e.g. { type: 'StringEquals', column: 'host', value: 'Human' } - // - render with allowedFields={['nextcladePangoLineage']} - // - assert error icon is shown and message contains the disallowed field name and the allowed list - // - assert onInput was NOT called - - // TODO: allowedFields - does not show error when all referenced fields are in the allowed list - // - mock a successful parse returning a Lineage filter on 'nextcladePangoLineage' - // - render with allowedFields={['nextcladePangoLineage']} - // - assert checkmark is shown and onInput was called with the query - - // TODO: allowedFields - mutation-only query passes even with a restrictive allowedFields list - // - mock a successful parse returning e.g. { type: 'NucleotideEquals', position: 123, symbol: 'A' } - // - render with allowedFields={['nextcladePangoLineage']} - // - assert checkmark is shown (no metadata columns referenced, so nothing to block) + it('allowedFields - shows error when query references a disallowed metadata field', async ({ routeMockers }) => { + const onInput = vi.fn(); + + routeMockers.lapis.mockPostQueryParse( + { queries: ['host:Human'], doFullValidation: true }, + { data: [{ type: 'success', filter: { type: 'StringEquals', column: 'host', value: 'Human' } }] }, + ); + + const { getByRole, getByLabelText, getByText } = render( + , + ); + + await userEvent.type(getByRole('textbox'), 'host:Human'); + + await expect.element(getByLabelText('Error')).toBeVisible(); + await expect.element(getByText(/"host"/)).toBeVisible(); + await expect.element(getByText(/nextcladePangoLineage/)).toBeVisible(); + expect(onInput).not.toHaveBeenCalled(); + }); + + it('allowedFields - does not show error when all referenced fields are in the allowed list', async ({ + routeMockers, + }) => { + const onInput = vi.fn(); + + routeMockers.lapis.mockPostQueryParse( + { queries: ['BA.1*'], doFullValidation: true }, + { + data: [ + { + type: 'success', + filter: { + type: 'Lineage', + column: 'nextcladePangoLineage', + value: 'BA.1', + includeSublineages: true, + }, + }, + ], + }, + ); + + const { getByRole, getByTitle } = render( + , + ); + + await userEvent.type(getByRole('textbox'), 'BA.1*'); + + await expect.element(getByTitle('Advanced query is valid')).toBeVisible(); + await expect.poll(() => onInput).toHaveBeenCalledWith('BA.1*'); + }); + + it('allowedFields - mutation-only query passes even with a restrictive allowedFields list', async ({ + routeMockers, + }) => { + routeMockers.lapis.mockPostQueryParse( + { queries: ['A123T'], doFullValidation: true }, + { data: [{ type: 'success', filter: { type: 'NucleotideEquals', position: 123, symbol: 'A' } }] }, + ); + + const { getByRole, getByTitle } = render( + , + ); + + await userEvent.type(getByRole('textbox'), 'A123T'); + + await expect.element(getByTitle('Advanced query is valid')).toBeVisible(); + }); it('shows error icon with network error tooltip when LAPIS is unreachable', async ({ routeMockers }) => { routeMockers.lapis.mockLapisDown();