fix: filter row types

This commit is contained in:
johnyeo
2026-05-26 20:19:46 +01:00
parent 3e9a635c54
commit 0adc1648c7
2 changed files with 153 additions and 23 deletions

View File

@@ -155,42 +155,62 @@ function stringMatcherToRule(
return { field, operator: "is", values: [] }; return { field, operator: "is", values: [] };
} }
function numberMatcherToRule( /**
* Convert a NumberMatcher to one OR MORE FilterRules. A combined matcher like
* `{ $gte: 2, $lte: 4 }` emits two rules (a "≥ 2" rule and a "≤ 4" rule) so
* neither constraint is silently dropped. `groupsToPlanFilter` re-merges them
* by field on save.
*/
export function numberMatcherToRules(
field: FilterField, field: FilterField,
matcher: NumberMatcher | undefined, matcher: NumberMatcher | undefined,
): FilterRule | null { ): FilterRule[] {
if (matcher === undefined) return null; if (matcher === undefined) return [];
if (matcher === null) return { field, operator: "is", values: [] }; if (matcher === null) return [{ field, operator: "is", values: [] }];
if (typeof matcher === "number") if (typeof matcher === "number")
return { field, operator: "is", values: [String(matcher)] }; return [{ field, operator: "is", values: [String(matcher)] }];
if (matcher.$eq !== undefined && matcher.$eq !== null)
return { field, operator: "is", values: [String(matcher.$eq)] }; const rules: FilterRule[] = [];
if (matcher.$eq !== undefined) {
if (matcher.$eq === null) rules.push({ field, operator: "is", values: [] });
else rules.push({ field, operator: "is", values: [String(matcher.$eq)] });
}
if (matcher.$ne !== undefined && matcher.$ne !== null) if (matcher.$ne !== undefined && matcher.$ne !== null)
return { field, operator: "is_not", values: [String(matcher.$ne)] }; rules.push({ field, operator: "is_not", values: [String(matcher.$ne)] });
if (matcher.$in !== undefined) if (matcher.$in !== undefined)
return { field, operator: "in", values: matcher.$in.map(String) }; rules.push({ field, operator: "in", values: matcher.$in.map(String) });
if (matcher.$nin !== undefined) if (matcher.$nin !== undefined)
return { field, operator: "not_in", values: matcher.$nin.map(String) }; rules.push({ field, operator: "not_in", values: matcher.$nin.map(String) });
if (matcher.$gt !== undefined) if (matcher.$gt !== undefined)
return { field, operator: "gt", values: [String(matcher.$gt)] }; rules.push({ field, operator: "gt", values: [String(matcher.$gt)] });
if (matcher.$gte !== undefined) if (matcher.$gte !== undefined)
return { field, operator: "gte", values: [String(matcher.$gte)] }; rules.push({ field, operator: "gte", values: [String(matcher.$gte)] });
if (matcher.$lt !== undefined) if (matcher.$lt !== undefined)
return { field, operator: "lt", values: [String(matcher.$lt)] }; rules.push({ field, operator: "lt", values: [String(matcher.$lt)] });
if (matcher.$lte !== undefined) if (matcher.$lte !== undefined)
return { field, operator: "lte", values: [String(matcher.$lte)] }; rules.push({ field, operator: "lte", values: [String(matcher.$lte)] });
return { field, operator: "is", values: [] }; return rules;
} }
function ruleToNumberMatcher(rule: FilterRule): NumberMatcher | undefined { /**
* Convert a single FilterRule into a NumberMatcher fragment that can be
* merged with other fragments for the same field. An empty `"is"` rule round-
* trips from `version: null` and must preserve the explicit null match.
*/
function ruleToNumberMatcherFragment(
rule: FilterRule,
): Record<string, unknown> | null {
const nums = rule.values const nums = rule.values
.map((v) => Number.parseFloat(v)) .map((v) => Number.parseFloat(v))
.filter((n) => !Number.isNaN(n)); .filter((n) => !Number.isNaN(n));
if (nums.length === 0) return undefined; if (nums.length === 0) {
if (rule.operator === "is") return { $eq: null };
return null;
}
const first = nums[0]; const first = nums[0];
switch (rule.operator) { switch (rule.operator) {
case "is": case "is":
return nums.length > 1 ? { $in: nums } : first; return nums.length > 1 ? { $in: nums } : { $eq: first };
case "is_not": case "is_not":
return { $ne: first }; return { $ne: first };
case "in": case "in":
@@ -206,10 +226,28 @@ function ruleToNumberMatcher(rule: FilterRule): NumberMatcher | undefined {
case "lte": case "lte":
return { $lte: first }; return { $lte: first };
default: default:
return first; return { $eq: first };
} }
} }
export function mergeNumberFragments(
fragments: Record<string, unknown>[],
): NumberMatcher | undefined {
if (fragments.length === 0) return undefined;
if (fragments.length === 1) {
const fragment = fragments[0];
const keys = Object.keys(fragment);
if (keys.length === 1 && "$eq" in fragment) {
// Simplify single-eq fragments back to bare value (matches the
// canonical "bare = $eq" convention) — handles version: 1 → 1
// and version: null → null.
return fragment.$eq as NumberMatcher;
}
return fragment as NumberMatcher;
}
return Object.assign({}, ...fragments) as NumberMatcher;
}
function ruleToStringMatcher(rule: FilterRule): StringMatcher { function ruleToStringMatcher(rule: FilterRule): StringMatcher {
if ( if (
rule.operator === "in" || rule.operator === "in" ||
@@ -265,8 +303,7 @@ export function planFilterToGroups(filter: PlanFilter): FilterGroupData[] {
const planIdRule = stringMatcherToRule("plan_id", filter.plan_id); const planIdRule = stringMatcherToRule("plan_id", filter.plan_id);
if (planIdRule) mainRules.push(planIdRule); if (planIdRule) mainRules.push(planIdRule);
const versionRule = numberMatcherToRule("version", filter.version); mainRules.push(...numberMatcherToRules("version", filter.version));
if (versionRule) mainRules.push(versionRule);
if (filter.paid !== undefined) if (filter.paid !== undefined)
mainRules.push(booleanRule("paid", filter.paid)); mainRules.push(booleanRule("paid", filter.paid));
@@ -342,15 +379,18 @@ export function groupsToPlanFilter(groups: FilterGroupData[]): PlanFilter {
let hasItemFields = false; let hasItemFields = false;
const itemInner: Record<string, unknown> = {}; const itemInner: Record<string, unknown> = {};
let itemMode: ArrayFilterMode = "$some"; let itemMode: ArrayFilterMode = "$some";
const versionFragments: Record<string, unknown>[] = [];
for (const rule of main.rules) { for (const rule of main.rules) {
switch (rule.field) { switch (rule.field) {
case "plan_id": case "plan_id":
filter.plan_id = ruleToStringMatcher(rule); filter.plan_id = ruleToStringMatcher(rule);
break; break;
case "version": case "version": {
filter.version = ruleToNumberMatcher(rule); const fragment = ruleToNumberMatcherFragment(rule);
if (fragment) versionFragments.push(fragment);
break; break;
}
case "paid": case "paid":
filter.paid = rule.values[0] === "true"; filter.paid = rule.values[0] === "true";
break; break;
@@ -397,6 +437,9 @@ export function groupsToPlanFilter(groups: FilterGroupData[]): PlanFilter {
: ({ [itemMode]: itemInner } as PlanFilter["item"]); : ({ [itemMode]: itemInner } as PlanFilter["item"]);
} }
const versionMatcher = mergeNumberFragments(versionFragments);
if (versionMatcher !== undefined) filter.version = versionMatcher;
if (groups.length > 1) { if (groups.length > 1) {
filter.$or = groups.slice(1).map((group) => groupsToPlanFilter([group])); filter.$or = groups.slice(1).map((group) => groupsToPlanFilter([group]));
} }

View File

@@ -241,3 +241,90 @@ describe("customerIdToStrings -> stringsToCustomerId roundtrip", () => {
expect(stringsToCustomerId(customerIdToStrings(undefined))).toBeUndefined(); expect(stringsToCustomerId(customerIdToStrings(undefined))).toBeUndefined();
}); });
}); });
describe("planFilterToGroups -> groupsToPlanFilter: version (NumberMatcher)", () => {
const roundtrip = (input: PlanFilter): PlanFilter => {
const groups = planFilterToGroups(input);
return groupsToPlanFilter(groups);
};
test("bare number roundtrips", () => {
expect(roundtrip({ version: 2 })).toEqual({ version: 2 });
});
test("explicit null roundtrips", () => {
// Regression: previously the UI dropped `version: null` because
// ruleToNumberMatcher returned undefined for an empty values array.
expect(roundtrip({ version: null })).toEqual({ version: null });
});
test("$eq number roundtrips (simplified to bare value)", () => {
expect(roundtrip({ version: { $eq: 3 } })).toEqual({ version: 3 });
});
test("$ne number roundtrips", () => {
const input = { version: { $ne: 1 } };
expect(roundtrip(input)).toEqual(input);
});
test("$in roundtrips", () => {
const input = { version: { $in: [1, 2, 3] } };
expect(roundtrip(input)).toEqual(input);
});
test("$nin roundtrips", () => {
const input = { version: { $nin: [4, 5] } };
expect(roundtrip(input)).toEqual(input);
});
test("$gte roundtrips", () => {
const input = { version: { $gte: 2 } };
expect(roundtrip(input)).toEqual(input);
});
test("$lt roundtrips", () => {
const input = { version: { $lt: 5 } };
expect(roundtrip(input)).toEqual(input);
});
test("$gte + $lte (range) preserves BOTH constraints", () => {
// Regression: previously emitted only the first operator, silently
// broadening "versions 24" to "versions ≥ 2".
const input = { version: { $gte: 2, $lte: 4 } };
expect(roundtrip(input)).toEqual(input);
});
test("$gt + $lt (open range) preserves BOTH constraints", () => {
const input = { version: { $gt: 1, $lt: 10 } };
expect(roundtrip(input)).toEqual(input);
});
});
describe("planFilterToGroups: version emits multiple rules for combined matchers", () => {
test("single operator → single rule", () => {
const groups = planFilterToGroups({ version: { $gte: 2 } });
const versionRules = groups[0].rules.filter((r) => r.field === "version");
expect(versionRules.length).toBe(1);
expect(versionRules[0]).toEqual({
field: "version",
operator: "gte",
values: ["2"],
});
});
test("$gte + $lte → two rules in the same group", () => {
const groups = planFilterToGroups({ version: { $gte: 2, $lte: 4 } });
const versionRules = groups[0].rules.filter((r) => r.field === "version");
expect(versionRules.length).toBe(2);
const operators = versionRules.map((r) => r.operator).sort();
expect(operators).toEqual(["gte", "lte"]);
});
test("null → single empty `is` rule", () => {
const groups = planFilterToGroups({ version: null });
const versionRules = groups[0].rules.filter((r) => r.field === "version");
expect(versionRules).toEqual([
{ field: "version", operator: "is", values: [] },
]);
});
});