diff --git a/vite/src/views/migrations/migration/filters/filterRowTypes.ts b/vite/src/views/migrations/migration/filters/filterRowTypes.ts index 728f60367..1bdcd23a3 100644 --- a/vite/src/views/migrations/migration/filters/filterRowTypes.ts +++ b/vite/src/views/migrations/migration/filters/filterRowTypes.ts @@ -155,42 +155,62 @@ function stringMatcherToRule( 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, matcher: NumberMatcher | undefined, -): FilterRule | null { - if (matcher === undefined) return null; - if (matcher === null) return { field, operator: "is", values: [] }; +): FilterRule[] { + if (matcher === undefined) return []; + if (matcher === null) return [{ field, operator: "is", values: [] }]; if (typeof matcher === "number") - return { field, operator: "is", values: [String(matcher)] }; - if (matcher.$eq !== undefined && matcher.$eq !== null) - return { field, operator: "is", values: [String(matcher.$eq)] }; + return [{ field, operator: "is", values: [String(matcher)] }]; + + 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) - return { field, operator: "is_not", values: [String(matcher.$ne)] }; + rules.push({ field, operator: "is_not", values: [String(matcher.$ne)] }); 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) - 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) - return { field, operator: "gt", values: [String(matcher.$gt)] }; + rules.push({ field, operator: "gt", values: [String(matcher.$gt)] }); 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) - return { field, operator: "lt", values: [String(matcher.$lt)] }; + rules.push({ field, operator: "lt", values: [String(matcher.$lt)] }); if (matcher.$lte !== undefined) - return { field, operator: "lte", values: [String(matcher.$lte)] }; - return { field, operator: "is", values: [] }; + rules.push({ field, operator: "lte", values: [String(matcher.$lte)] }); + 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 | null { const nums = rule.values .map((v) => Number.parseFloat(v)) .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]; switch (rule.operator) { case "is": - return nums.length > 1 ? { $in: nums } : first; + return nums.length > 1 ? { $in: nums } : { $eq: first }; case "is_not": return { $ne: first }; case "in": @@ -206,10 +226,28 @@ function ruleToNumberMatcher(rule: FilterRule): NumberMatcher | undefined { case "lte": return { $lte: first }; default: - return first; + return { $eq: first }; } } +export function mergeNumberFragments( + fragments: Record[], +): 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 { if ( rule.operator === "in" || @@ -265,8 +303,7 @@ export function planFilterToGroups(filter: PlanFilter): FilterGroupData[] { const planIdRule = stringMatcherToRule("plan_id", filter.plan_id); if (planIdRule) mainRules.push(planIdRule); - const versionRule = numberMatcherToRule("version", filter.version); - if (versionRule) mainRules.push(versionRule); + mainRules.push(...numberMatcherToRules("version", filter.version)); if (filter.paid !== undefined) mainRules.push(booleanRule("paid", filter.paid)); @@ -342,15 +379,18 @@ export function groupsToPlanFilter(groups: FilterGroupData[]): PlanFilter { let hasItemFields = false; const itemInner: Record = {}; let itemMode: ArrayFilterMode = "$some"; + const versionFragments: Record[] = []; for (const rule of main.rules) { switch (rule.field) { case "plan_id": filter.plan_id = ruleToStringMatcher(rule); break; - case "version": - filter.version = ruleToNumberMatcher(rule); + case "version": { + const fragment = ruleToNumberMatcherFragment(rule); + if (fragment) versionFragments.push(fragment); break; + } case "paid": filter.paid = rule.values[0] === "true"; break; @@ -397,6 +437,9 @@ export function groupsToPlanFilter(groups: FilterGroupData[]): PlanFilter { : ({ [itemMode]: itemInner } as PlanFilter["item"]); } + const versionMatcher = mergeNumberFragments(versionFragments); + if (versionMatcher !== undefined) filter.version = versionMatcher; + if (groups.length > 1) { filter.$or = groups.slice(1).map((group) => groupsToPlanFilter([group])); } diff --git a/vite/tests/views/migrations/migration/filters/filterRowTypes.test.ts b/vite/tests/views/migrations/migration/filters/filterRowTypes.test.ts index 35313473d..4e2aff01c 100644 --- a/vite/tests/views/migrations/migration/filters/filterRowTypes.test.ts +++ b/vite/tests/views/migrations/migration/filters/filterRowTypes.test.ts @@ -241,3 +241,90 @@ describe("customerIdToStrings -> stringsToCustomerId roundtrip", () => { 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 2–4" 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: [] }, + ]); + }); +});