Skip to content

Commit 450b214

Browse files
committed
Address comments and add tests
1 parent 184c8cf commit 450b214

File tree

3 files changed

+131
-2
lines changed

3 files changed

+131
-2
lines changed

kubernetes/loculus/values.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1394,7 +1394,7 @@ defaultOrganismConfig: &defaultOrganismConfig
13941394
defaultOrder: descending
13951395
multiFieldSearches:
13961396
- name: identifier
1397-
displayName: Identifier
1397+
displayName: Sample Identifier
13981398
fields:
13991399
- accessionVersion
14001400
- submissionId
Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
import { describe, expect, it } from 'vitest';
2+
3+
import { FieldFilterSet } from './SequenceFilters';
4+
import type { FieldValues, Metadata, MultiFieldSearch } from '../../../types/config.ts';
5+
import { MetadataFilterSchema } from '../../../utils/search.ts';
6+
7+
const makeFieldFilterSet = (
8+
fieldValues: FieldValues,
9+
metadataFields: Metadata[],
10+
multiFieldSearches: MultiFieldSearch[] = [],
11+
) => {
12+
return new FieldFilterSet(
13+
new MetadataFilterSchema(metadataFields, multiFieldSearches),
14+
fieldValues,
15+
{},
16+
{ nucleotideSegmentInfos: [], geneInfos: [] },
17+
{
18+
isMultiSegmented: false,
19+
segmentReferenceGenomes: {},
20+
segmentDisplayNames: {},
21+
useLapisMultiSegmentedEndpoint: false,
22+
},
23+
);
24+
};
25+
26+
const identifierMfs: MultiFieldSearch = {
27+
name: 'identifier',
28+
displayName: 'Sample Identifier',
29+
fields: ['accessionVersion', 'submissionId'],
30+
};
31+
32+
describe('FieldFilterSet.toApiParams — multi-field search advancedQuery', () => {
33+
it('generates advancedQuery when a multi-field search has a non-empty value', () => {
34+
const filter = makeFieldFilterSet({ identifier: 'test123' }, [], [identifierMfs]);
35+
const params = filter.toApiParams();
36+
expect(params.advancedQuery).toBe(
37+
"(accessionVersion.regex='(?i)test123' or submissionId.regex='(?i)test123')",
38+
);
39+
});
40+
41+
it('does not produce advancedQuery when value is empty string', () => {
42+
const filter = makeFieldFilterSet({ identifier: '' }, [], [identifierMfs]);
43+
const params = filter.toApiParams();
44+
expect(params.advancedQuery).toBeUndefined();
45+
});
46+
47+
it('does not produce advancedQuery when value is whitespace only', () => {
48+
const filter = makeFieldFilterSet({ identifier: ' ' }, [], [identifierMfs]);
49+
const params = filter.toApiParams();
50+
expect(params.advancedQuery).toBeUndefined();
51+
});
52+
53+
it('does not produce advancedQuery when multi-field key is absent', () => {
54+
const filter = makeFieldFilterSet({}, [], [identifierMfs]);
55+
const params = filter.toApiParams();
56+
expect(params.advancedQuery).toBeUndefined();
57+
});
58+
59+
it('trims surrounding whitespace from the search value', () => {
60+
const filter = makeFieldFilterSet({ identifier: ' acc123 ' }, [], [identifierMfs]);
61+
const params = filter.toApiParams();
62+
expect(params.advancedQuery).toBe(
63+
"(accessionVersion.regex='(?i)acc123' or submissionId.regex='(?i)acc123')",
64+
);
65+
});
66+
67+
it('escapes single quotes so LAPIS query string literals remain valid', () => {
68+
const filter = makeFieldFilterSet({ identifier: "O'Connor" }, [], [identifierMfs]);
69+
const params = filter.toApiParams();
70+
// The single quote inside the value must be escaped as \' in the query
71+
expect(params.advancedQuery).toBe(
72+
"(accessionVersion.regex='(?i)O\\'Connor' or submissionId.regex='(?i)O\\'Connor')",
73+
);
74+
});
75+
76+
it('escapes regex special characters in search values', () => {
77+
const filter = makeFieldFilterSet({ identifier: 'test.value' }, [], [identifierMfs]);
78+
const params = filter.toApiParams();
79+
expect(params.advancedQuery).toBe(
80+
"(accessionVersion.regex='(?i)test\\.value' or submissionId.regex='(?i)test\\.value')",
81+
);
82+
});
83+
84+
it('combines multiple active multi-field searches with "and"', () => {
85+
const contributorMfs: MultiFieldSearch = {
86+
name: 'contributor',
87+
displayName: 'Contributor',
88+
fields: ['authors'],
89+
};
90+
const filter = makeFieldFilterSet(
91+
{ identifier: 'acc123', contributor: 'Smith' },
92+
[],
93+
[identifierMfs, contributorMfs],
94+
);
95+
const params = filter.toApiParams();
96+
expect(params.advancedQuery).toBe(
97+
"(accessionVersion.regex='(?i)acc123' or submissionId.regex='(?i)acc123') and (authors.regex='(?i)Smith')",
98+
);
99+
});
100+
101+
it('only produces advancedQuery for the active multi-field search when the other is empty', () => {
102+
const contributorMfs: MultiFieldSearch = {
103+
name: 'contributor',
104+
displayName: 'Contributor',
105+
fields: ['authors'],
106+
};
107+
const filter = makeFieldFilterSet(
108+
{ identifier: 'acc123', contributor: '' },
109+
[],
110+
[identifierMfs, contributorMfs],
111+
);
112+
const params = filter.toApiParams();
113+
expect(params.advancedQuery).toBe(
114+
"(accessionVersion.regex='(?i)acc123' or submissionId.regex='(?i)acc123')",
115+
);
116+
});
117+
118+
it('excludes multi-field search keys from the regular filter params', () => {
119+
const filter = makeFieldFilterSet(
120+
{ identifier: 'test', country: 'Germany' },
121+
[{ name: 'country', type: 'string' as const }],
122+
[identifierMfs],
123+
);
124+
const params = filter.toApiParams();
125+
expect(params.identifier).toBeUndefined();
126+
expect(params.country).toBe('Germany');
127+
});
128+
});

website/src/components/SearchPage/DownloadDialog/SequenceFilters.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,8 @@ export class FieldFilterSet implements SequenceFilter {
151151
const value = this.fieldValues[mfs.name];
152152
if (value && typeof value === 'string' && value.trim()) {
153153
const regex = makeCaseInsensitiveLiteralSubstringRegex(value.trim());
154-
const fieldQueries = mfs.fields.map((f) => `${f}.regex='${regex}'`);
154+
const escapedRegex = regex.replace(/'/g, "\\'");
155+
const fieldQueries = mfs.fields.map((f) => `${f}.regex='${escapedRegex}'`);
155156
advancedQueryParts.push(`(${fieldQueries.join(' or ')})`);
156157
}
157158
}

0 commit comments

Comments
 (0)