Skip to content

Commit 76410ae

Browse files
kevinansfieldkevinansfield
committed
Changed all-of filter handling to be driven by field config
ref https://linear.app/ghost/issue/BER-3664/add-is-all-of-operator-to-member-label-filters - the parse side of is-all was label-specific: a hard-coded extractor recognised only label/labels keys, so any other set field adopting is-all would serialize the grouped form but silently fail to parse it back - replaced it with a shared extractor in filter-query-core that activates for any field declaring the is-all operator, with alias keys coming from the existing parseKeys mechanism instead of being hard-coded - converted setCodec's serialize branches into an operator-serializer table so adding a set operator is a table entry rather than a special-cased early return
1 parent 0bec214 commit 76410ae

6 files changed

Lines changed: 126 additions & 60 deletions

File tree

‎apps/posts/src/views/filters/filter-codecs.ts‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,22 @@ const DATE_OPERATOR_SYMBOLS: Record<string, string> = {
4949
'is-or-greater': '>='
5050
};
5151

52-
const SET_OPERATOR_SYMBOLS: Record<string, string> = {
53-
'is-any': '',
54-
'is-not-any': '-'
52+
type SetOperatorSerializer = (field: string, values: string[], config?: CodecConfig) => string;
53+
54+
function serializeSetMembership(symbol: string): SetOperatorSerializer {
55+
return (field, values, config) => {
56+
if (config?.serializeSingletonAsScalar && values.length === 1) {
57+
return `${field}:${symbol}${serializeScalarValue(values[0], config)}`;
58+
}
59+
60+
return `${field}:${symbol}[${values.map(value => serializeScalarValue(value, config)).join(',')}]`;
61+
};
62+
}
63+
64+
const SET_OPERATOR_SERIALIZERS: Record<string, SetOperatorSerializer> = {
65+
'is-any': serializeSetMembership(''),
66+
'is-not-any': serializeSetMembership('-'),
67+
'is-all': (field, values, config) => `(${values.map(value => `${field}:${serializeScalarValue(value, config)}`).join('+')})`
5568
};
5669

5770
const UNQUOTED_TOKEN_PATTERN = /^[A-Za-z0-9_.-]+$/;
@@ -269,25 +282,13 @@ export function setCodec(config?: CodecConfig): FilterCodec {
269282
return null;
270283
}
271284

272-
const values = normalizeMultiValue(predicate.values);
273-
274-
if (predicate.operator === 'is-all') {
275-
const clauses = values.map(value => `${field}:${serializeScalarValue(value, config)}`);
285+
const serializeOperator = SET_OPERATOR_SERIALIZERS[predicate.operator];
276286

277-
return [`(${clauses.join('+')})`];
278-
}
279-
280-
const operator = SET_OPERATOR_SYMBOLS[predicate.operator];
281-
282-
if (operator === undefined) {
287+
if (serializeOperator === undefined) {
283288
return null;
284289
}
285290

286-
if (config?.serializeSingletonAsScalar && values.length === 1) {
287-
return [`${field}:${operator}${serializeScalarValue(values[0], config)}`];
288-
}
289-
290-
return [`${field}:${operator}[${values.map(value => serializeScalarValue(value, config)).join(',')}]`];
291+
return [serializeOperator(field, normalizeMultiValue(predicate.values), config)];
291292
}
292293
};
293294
}

‎apps/posts/src/views/filters/filter-query-core.test.ts‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import {defineFields} from './filter-types';
22
import {describe, expect, it} from 'vitest';
3-
import {dispatchSimpleNodes, getFieldKeysByType, hasFieldKey, parseFilterToAst, serializePredicates} from './filter-query-core';
4-
import {numberCodec, scalarCodec} from './filter-codecs';
3+
import {dispatchSimpleNodes, extractAllOfPredicates, getFieldKeysByType, hasFieldKey, parseFilterToAst, serializePredicates} from './filter-query-core';
4+
import {numberCodec, scalarCodec, setCodec} from './filter-codecs';
55
import type {AstNode} from './filter-ast';
66
import type {FilterPredicate} from './filter-types';
77

@@ -120,3 +120,38 @@ describe('filter-query-core', () => {
120120
expect(hasFieldKey(ast, fieldKeys)).toBe(true);
121121
});
122122
});
123+
124+
describe('extractAllOfPredicates', () => {
125+
const allOfFields = defineFields({
126+
label: {
127+
operators: ['is-any', 'is-all', 'is-not-any'],
128+
parseKeys: ['labels'],
129+
ui: {
130+
label: 'Label',
131+
type: 'multiselect'
132+
},
133+
codec: setCodec()
134+
},
135+
tier: {
136+
operators: ['is-any', 'is-not-any'],
137+
ui: {
138+
label: 'Tier',
139+
type: 'multiselect'
140+
},
141+
codec: setCodec()
142+
}
143+
});
144+
145+
it('groups repeated clauses for fields declaring is-all, resolving alias keys', () => {
146+
const result = extractAllOfPredicates([{label: 'alpha'}, {labels: 'vip'}, {status: 'paid'}], allOfFields, 'UTC');
147+
148+
expect(result).toEqual({
149+
predicates: [{field: 'label', operator: 'is-all', values: ['alpha', 'vip']}],
150+
remaining: [{status: 'paid'}]
151+
});
152+
});
153+
154+
it('returns null when no field accumulates at least two all-of clauses', () => {
155+
expect(extractAllOfPredicates([{label: 'alpha'}, {tier: 'gold'}, {tier: 'silver'}], allOfFields, 'UTC')).toBeNull();
156+
});
157+
});

‎apps/posts/src/views/filters/filter-query-core.ts‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,64 @@ export function dispatchSimpleNodes<TFields extends Record<string, FilterField>>
7676
});
7777
}
7878

79+
/**
80+
* Pulls repeated positive scalar clauses for the same field out of a grouped
81+
* AND's children, e.g. `(label:a+label:b)` → one all-of predicate. A field is
82+
* eligible when it declares the `is-all` operator; alias keys come from
83+
* `parseKeys`. Fields need at least two clauses to form a group — lone
84+
* clauses stay in `remaining`. Returns null when nothing qualifies.
85+
*/
86+
export function extractAllOfPredicates<TFields extends Record<string, FilterField>>(
87+
children: AstNode[],
88+
fields: TFields,
89+
timezone: string
90+
): {predicates: ParsedPredicate[]; remaining: AstNode[]} | null {
91+
const clauses = children.map((child) => {
92+
const keys = Object.keys(child);
93+
const value = child[keys[0]];
94+
95+
if (keys.length !== 1 || keys[0].startsWith('$') || typeof value !== 'string') {
96+
return {child, allOf: null};
97+
}
98+
99+
const resolved = resolveField(fields, keys[0], timezone);
100+
101+
if (!resolved?.definition.operators.includes('is-all')) {
102+
return {child, allOf: null};
103+
}
104+
105+
return {child, allOf: {field: resolved.context.key, value}};
106+
});
107+
108+
const clauseCounts = new Map<string, number>();
109+
110+
for (const {allOf} of clauses) {
111+
if (allOf) {
112+
clauseCounts.set(allOf.field, (clauseCounts.get(allOf.field) ?? 0) + 1);
113+
}
114+
}
115+
116+
const predicatesByField = new Map<string, ParsedPredicate>();
117+
const remaining: AstNode[] = [];
118+
119+
for (const {child, allOf} of clauses) {
120+
if (!allOf || (clauseCounts.get(allOf.field) ?? 0) < 2) {
121+
remaining.push(child);
122+
continue;
123+
}
124+
125+
const predicate = predicatesByField.get(allOf.field) ?? {field: allOf.field, operator: 'is-all', values: []};
126+
predicate.values.push(allOf.value);
127+
predicatesByField.set(allOf.field, predicate);
128+
}
129+
130+
if (!predicatesByField.size) {
131+
return null;
132+
}
133+
134+
return {predicates: [...predicatesByField.values()], remaining};
135+
}
136+
79137
function canonicalizeClauses(clauses: string[]): string[] {
80138
return [...clauses].sort((left, right) => left.localeCompare(right));
81139
}

‎apps/posts/src/views/members/member-fields.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ const baseMemberFields = defineFields({
143143
},
144144
label: {
145145
operators: LABEL_SET_OPERATORS,
146+
parseKeys: ['labels'],
146147
ui: {
147148
label: 'Label',
148149
type: 'multiselect',

‎apps/posts/src/views/members/member-filter-query.test.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,12 @@ describe('member-filter-query', () => {
5858
]);
5959
});
6060

61+
it('parses grouped label all-of filters using the labels alias key', () => {
62+
expect(stripIds(parseMemberFilter('(labels:alpha+labels:vip)', 'UTC'))).toEqual([
63+
{field: 'label', operator: 'is-all', values: ['alpha', 'vip']}
64+
]);
65+
});
66+
6167
it('parses grouped label all-of filters alongside other predicates', () => {
6268
expect(stripIds(parseMemberFilter('(label:alpha+label:vip)+status:paid', 'UTC'))).toEqual([
6369
{field: 'label', operator: 'is-all', values: ['alpha', 'vip']},

‎apps/posts/src/views/members/member-filter-query.ts‎

Lines changed: 5 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {dispatchSimpleNodes, getFieldKeysByType, hasFieldKey, parseFilterToAst, serializePredicates, stampPredicates} from '../filters/filter-query-core';
1+
import {dispatchSimpleNodes, extractAllOfPredicates, getFieldKeysByType, hasFieldKey, parseFilterToAst, serializePredicates, stampPredicates} from '../filters/filter-query-core';
22
import {memberFields} from './member-fields';
33
import {resolveField} from '../filters/resolve-field';
44
import type {AstNode} from '../filters/filter-ast';
@@ -183,41 +183,6 @@ function matchFeedbackGroupedNode(node: AstNode): ParsedPredicate | null {
183183
};
184184
}
185185

186-
function extractLabelAllOfPredicate(children: AstNode[]): {predicate: ParsedPredicate; remaining: AstNode[]} | null {
187-
const labelValues: string[] = [];
188-
const remaining: AstNode[] = [];
189-
190-
for (const child of children) {
191-
const keys = Object.keys(child);
192-
const key = keys[0];
193-
const value = child[key];
194-
195-
if (
196-
keys.length === 1 &&
197-
(key === 'label' || key === 'labels') &&
198-
typeof value === 'string'
199-
) {
200-
labelValues.push(value);
201-
continue;
202-
}
203-
204-
remaining.push(child);
205-
}
206-
207-
if (labelValues.length < 2) {
208-
return null;
209-
}
210-
211-
return {
212-
predicate: {
213-
field: 'label',
214-
operator: 'is-all',
215-
values: labelValues
216-
},
217-
remaining
218-
};
219-
}
220-
221186
const MEMBER_COMPOUND_MATCHERS: CompoundMatcher[] = [
222187
matchSubscribedNode,
223188
matchNewsletterGroupedNode,
@@ -259,12 +224,12 @@ function parseMemberNode(node: AstNode, timezone: string, isGroupedContext = fal
259224
const compound = getCompoundChildren(node);
260225

261226
if (compound?.operator === '$and') {
262-
const labelAllOf = isGroupedContext ? extractLabelAllOfPredicate(compound.children) : null;
227+
const allOf = isGroupedContext ? extractAllOfPredicates(compound.children, memberFields, timezone) : null;
263228

264-
if (labelAllOf) {
229+
if (allOf) {
265230
return [
266-
labelAllOf.predicate,
267-
...labelAllOf.remaining.flatMap(child => parseMemberNode(child, timezone, true))
231+
...allOf.predicates,
232+
...allOf.remaining.flatMap(child => parseMemberNode(child, timezone, true))
268233
];
269234
}
270235

0 commit comments

Comments
 (0)