Skip to content

Commit fe218a0

Browse files
rob-ghostrob-ghost
committed
🎨 Used NQL $all operator for member label all-of filters
Replaced the parenthesised-grouping workaround for "is all of" label filters with NQL's native all-of operator. The admin codecs and member filter query now serialize and parse the $all form directly, dropping the outer-grouping probe and the bespoke all-of extraction. Consumes the operator from a prerelease published under the `next` dist-tag (TryGhost/NQL#156) via the catalog plus an override that forces it onto transitive consumers like @tryghost/bookshelf-filter. Bump to the released versions once #156 merges to latest.
1 parent 4884ed7 commit fe218a0

12 files changed

Lines changed: 211 additions & 206 deletions

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

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,15 +272,36 @@ describe('setCodec', () => {
272272
expect(setCodec().serialize(predicate, labelContext)).toEqual(['label:[alpha,vip]']);
273273
});
274274

275-
it('serializes all-of set membership as AND clauses', () => {
275+
it('parses all-of set membership ($all) into an is-all predicate', () => {
276+
expect(setCodec().parse(nql.parse('label:[vip+alpha]') as never, labelContext)).toEqual({
277+
field: 'label',
278+
operator: 'is-all',
279+
values: ['vip', 'alpha']
280+
});
281+
});
282+
283+
it('serializes all-of set membership as a +-separated value list', () => {
276284
const predicate: FilterPredicate = {
277285
id: '1',
278286
field: 'label',
279287
operator: 'is-all',
280288
values: ['vip', 'alpha']
281289
};
282290

283-
expect(setCodec().serialize(predicate, labelContext)).toEqual(['(label:alpha+label:vip)']);
291+
expect(setCodec().serialize(predicate, labelContext)).toEqual(['label:[alpha+vip]']);
292+
});
293+
294+
it('serializes a single-value all-of as any-of (a one-value all is identical to any)', () => {
295+
const predicate: FilterPredicate = {
296+
id: '1',
297+
field: 'label',
298+
operator: 'is-all',
299+
values: ['alpha']
300+
};
301+
302+
// `[alpha]` is `$in` regardless, so emit the any-of form rather than a
303+
// one-element all-of that would just round-trip back to is-any anyway.
304+
expect(setCodec().serialize(predicate, labelContext)).toEqual(['label:[alpha]']);
284305
});
285306

286307
it('can serialize singleton string values as quoted scalars', () => {

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

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,21 @@ function serializeSetMembership(symbol: string): SetOperatorSerializer {
6161
};
6262
}
6363

64+
const serializeAnyMembership = serializeSetMembership('');
65+
6466
const SET_OPERATOR_SERIALIZERS: Record<string, SetOperatorSerializer> = {
65-
'is-any': serializeSetMembership(''),
67+
'is-any': serializeAnyMembership,
6668
'is-not-any': serializeSetMembership('-'),
67-
'is-all': (field, values, config) => `(${values.map(value => `${field}:${serializeScalarValue(value, config)}`).join('+')})`
69+
'is-all': (field, values, config) => {
70+
// NQL's all-of operator is a `+`-separated value list: `label:[a+b]`.
71+
// A single value is identical to any-of (and `[a]` is `$in` regardless),
72+
// so only emit the all-of form for two or more values.
73+
if (values.length === 1) {
74+
return serializeAnyMembership(field, values, config);
75+
}
76+
77+
return `${field}:[${values.map(value => serializeScalarValue(value, config)).join('+')}]`;
78+
}
6879
};
6980

7081
const UNQUOTED_TOKEN_PATTERN = /^[A-Za-z0-9_.-]+$/;
@@ -135,7 +146,7 @@ export function scalarCodec(config?: CodecConfig): FilterCodec {
135146
const comparator = extractComparator(node as Record<string, unknown>);
136147
const field = getCodecField(config, ctx.key);
137148

138-
if (!comparator || comparator.field !== field) {
149+
if (!comparator || comparator.field !== field && comparator.field !== ctx.matchedKey) {
139150
return null;
140151
}
141152

@@ -178,7 +189,7 @@ export function textCodec(config?: CodecConfig): FilterCodec {
178189
const comparator = extractComparator(node as Record<string, unknown>);
179190
const field = getCodecField(config, ctx.key);
180191

181-
if (!comparator || comparator.field !== field) {
192+
if (!comparator || comparator.field !== field && comparator.field !== ctx.matchedKey) {
182193
return null;
183194
}
184195

@@ -237,7 +248,7 @@ export function setCodec(config?: CodecConfig): FilterCodec {
237248
const comparator = extractComparator(node as Record<string, unknown>);
238249
const field = getCodecField(config, ctx.key);
239250

240-
if (!comparator || comparator.field !== field) {
251+
if (!comparator || comparator.field !== field && comparator.field !== ctx.matchedKey) {
241252
return null;
242253
}
243254

@@ -249,6 +260,14 @@ export function setCodec(config?: CodecConfig): FilterCodec {
249260
};
250261
}
251262

263+
if (comparator.operator === '$all' && Array.isArray(comparator.value)) {
264+
return {
265+
field: ctx.key,
266+
operator: 'is-all',
267+
values: comparator.value
268+
};
269+
}
270+
252271
if (comparator.operator === '$nin' && Array.isArray(comparator.value)) {
253272
return {
254273
field: ctx.key,
@@ -299,7 +318,7 @@ export function numberCodec(config?: CodecConfig): FilterCodec {
299318
const comparator = extractComparator(node as Record<string, unknown>);
300319
const field = getCodecField(config, ctx.key);
301320

302-
if (!comparator || comparator.field !== field || typeof comparator.value !== 'number') {
321+
if (!comparator || comparator.field !== field && comparator.field !== ctx.matchedKey || typeof comparator.value !== 'number') {
303322
return null;
304323
}
305324

@@ -371,7 +390,7 @@ export function dateCodec(config?: CodecConfig): FilterCodec {
371390
const comparator = extractComparator(node as Record<string, unknown>);
372391
const field = getCodecField(config, ctx.key);
373392

374-
if (!comparator || comparator.field !== field) {
393+
if (!comparator || comparator.field !== field && comparator.field !== ctx.matchedKey) {
375394
return null;
376395
}
377396

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

Lines changed: 2 additions & 37 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, extractAllOfPredicates, getFieldKeysByType, hasFieldKey, parseFilterToAst, serializePredicates} from './filter-query-core';
4-
import {numberCodec, scalarCodec, setCodec} from './filter-codecs';
3+
import {dispatchSimpleNodes, getFieldKeysByType, hasFieldKey, parseFilterToAst, serializePredicates} from './filter-query-core';
4+
import {numberCodec, scalarCodec} from './filter-codecs';
55
import type {AstNode} from './filter-ast';
66
import type {FilterPredicate} from './filter-types';
77

@@ -120,38 +120,3 @@ 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: 0 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -76,64 +76,6 @@ 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-
13779
function canonicalizeClauses(clauses: string[]): string[] {
13880
return [...clauses].sort((left, right) => left.localeCompare(right));
13981
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,13 @@ export interface FilterPredicate {
88
export type ParsedPredicate = Omit<FilterPredicate, 'id'>;
99

1010
export interface CodecContext {
11+
// Canonical field key the codec serializes to / reports predicates under.
1112
key: string;
13+
// The key actually matched in the source NQL — equals `key` for an exact or
14+
// pattern match, but is the alias (a `parseKeys` entry) when one was used.
15+
// Codecs additionally accept a node whose field is this key. Set by
16+
// `resolveField`; absent on hand-built contexts (which use canonical keys).
17+
matchedKey?: string;
1218
pattern: string;
1319
params: Record<string, string>;
1420
timezone: string;

‎apps/posts/src/views/filters/resolve-field.test.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ describe('resolveField', () => {
4747
definition: fields.status,
4848
context: {
4949
key: 'status',
50+
matchedKey: 'status',
5051
pattern: 'status',
5152
params: {},
5253
timezone: 'UTC'
@@ -61,6 +62,7 @@ describe('resolveField', () => {
6162
definition: fields['newsletters.:slug'],
6263
context: {
6364
key: 'newsletters.weekly',
65+
matchedKey: 'newsletters.weekly',
6466
pattern: 'newsletters.:slug',
6567
params: {
6668
slug: 'weekly'
@@ -84,6 +86,7 @@ describe('resolveField', () => {
8486
definition: fields.author,
8587
context: {
8688
key: 'author',
89+
matchedKey: 'member_id',
8790
pattern: 'author',
8891
params: {},
8992
timezone: 'UTC'

‎apps/posts/src/views/filters/resolve-field.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ export function resolveField<TFields extends Record<string, FilterField>>(fields
4040
definition: exactDefinition,
4141
context: {
4242
key,
43+
matchedKey: key,
4344
pattern: key,
4445
params: {},
4546
timezone
@@ -56,6 +57,7 @@ export function resolveField<TFields extends Record<string, FilterField>>(fields
5657
definition,
5758
context: {
5859
key: fieldKey,
60+
matchedKey: key,
5961
pattern: fieldKey,
6062
params: {},
6163
timezone
@@ -78,6 +80,7 @@ export function resolveField<TFields extends Record<string, FilterField>>(fields
7880
definition,
7981
context: {
8082
key,
83+
matchedKey: key,
8184
pattern,
8285
params,
8386
timezone

0 commit comments

Comments
 (0)