fix(components)!: the filter builder's list value holds the spec list's entries, so a boolean switched to In is a rule the spec accepts

.changeset/26348-filter-builder-boolean-list-entry.md

@@ -0,0 +1,19 @@
+---
+'@object-ui/components': patch
+'@object-ui/plugin-list': patch
+---
+
+fix(components)!: the filter builder's list value holds the spec list's entries, so an `equals true` row switched to In is a rule the spec accepts
+
+Clause-②: yes (narrowing)
+Metadata-protocol: none
+
+<!-- adr-0087: not-required (runtime-interface-only ui/packages/components/src/custom/filter-builder.tsx#FilterBuilderRow) A published runtime TypeScript interface of a React component with no metadata surface: no Zod schema, no spec declaration and no stored representation, so `objectstack migrate meta` has nothing to rewrite; the affected party is a TypeScript consumer, reached by its own compiler. -->
+
+**BREAKING** type-surface change, compile-time only. `FilterBuilderRow` is exported from `@object-ui/components`. Its `value` is now exactly `ViewFilterRule['value']` from `@objectstack/spec/ui`: the builder's own wider list member, which also admitted boolean entries, is gone. A writer that put a boolean into a list value no longer compiles.
+
+The defect it fixes: in the filter builder, a row `equals true` on a select column switched to In emitted `value: [true]`, and `ViewFilterRuleSchema` refuses a boolean list entry, so the builder produced a rule the spec rejects. A boolean carried into a list or a range is now written as its string spelling, so the same switch emits `["true"]`. That is the conversion the builder already made for a boolean that reaches a text column, and every column that offers a list operator is one. The option whose value is `"true"` stays ticked, because the list controls compare entries as strings.
+
+`UserFilters` (`@object-ui/plugin-list`) hands a lookup chip's selection to `LookupValuePicker` the same way. An authored `defaultValues` entry `true` on a lookup filter field used to reach the query beside a picked id as a boolean; it now arrives as `"true"`, still drawn as a chip the user can remove.
+
+**Migration.** Code that writes a list into a `FilterBuilderRow` value writes strings and numbers only. Write a boolean entry as its string spelling, the way the builder does, or keep the boolean as a scalar under `equals`. Code that reads `row.value` needs no change: a list it reads already holds only strings and numbers.

ui/packages/components/docs/FilterBuilder.md

@@ -136,9 +136,10 @@ interface FilterBuilderRow {
   id: string;                    // Row identifier (required)
   field: string;                 // Field value
   operator: FilterBuilderOperator; // Operator id (see below)
-  value?: ViewFilterRule['value'] | (string | number | boolean)[];
+  value?: ViewFilterRule['value'];
   // `ViewFilterRule` from `@objectstack/spec/ui`: optional (value-less operators
-  // carry none) and may be null; the builder's own list may also hold booleans
+  // carry none) and may be null; a list holds strings and numbers, so a boolean
+  // carried into `in` / `not_in` / `between` is written as `"true"` / `"false"`
 }
 ```
 

ui/packages/components/src/__tests__/filter-builder-boolean-list-26348.test.tsx

@@ -0,0 +1,143 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * The builder's list values hold what the spec rule's list holds (#26348).
+ *
+ * `ViewFilterRuleSchema` (`@objectstack/spec/ui`) takes `in` / `not_in` /
+ * `between` values as arrays of strings and numbers. The builder's row used to
+ * declare a wider list member of its own, `(string | number | boolean)[]`, and
+ * its one list writer for a scalar, `normalizeToArray`, kept a boolean a
+ * boolean. So an `equals true` row (a boolean scalar, which the spec admits) on
+ * a `select` column, switched to `In`, emitted `[true]`, and the schema refused
+ * the rule the builder had just produced.
+ *
+ * The emitted list now matches the spec's list element at the source: a boolean
+ * carried into a list or a range is written as its string spelling, the
+ * conversion the builder already makes for a boolean that reaches a text
+ * column. The row's value is the spec rule's value and nothing wider.
+ *
+ * Type-level pins are judged by `tsc -p tsconfig.test.json`; a green vitest run
+ * says nothing about them.
+ */
+import { describe, it, expect, vi } from 'vitest';
+import React from 'react';
+import { render, screen, fireEvent, waitFor } from '@testing-library/react';
+import '@testing-library/jest-dom';
+import { ViewFilterRuleSchema, type ViewFilterRule } from '@objectstack/spec/ui';
+import type { FilterGroup } from '@object-ui/types';
+import {
+  FilterBuilder,
+  reshapeFilterValue,
+  retypeFilterValue,
+  type FilterBuilderProps,
+  type FilterBuilderRow,
+} from '../custom/filter-builder';
+
+type Equal<A, B> =
+  (<T>() => T extends A ? 1 : 2) extends (<T>() => T extends B ? 1 : 2) ? true : false;
+function expectType<T extends true>(_: T = true as T): void { /* compile-time only */ }
+
+/* ── The shape ────────────────────────────────────────────────────────────── */
+
+// The row's value IS the spec rule's value, with no wider list member beside it.
+expectType<Equal<FilterBuilderRow['value'], ViewFilterRule['value']>>();
+// So a boolean list is refused where it is written, as the schema refuses it.
+const booleanList: FilterBuilderRow = {
+  id: 'c1',
+  field: 'stage',
+  operator: 'in',
+  // @ts-expect-error a list entry is a string or a number, as the spec's is
+  value: [true],
+};
+// A boolean SCALAR stays admitted: the spec admits it, and `equals true` is a row.
+const booleanScalar: FilterBuilderRow = { id: 'c1', field: 'active', operator: 'equals', value: true };
+void booleanList;
+void booleanScalar;
+
+const FIELDS: NonNullable<FilterBuilderProps['fields']> = [
+  {
+    value: 'stage',
+    label: 'Stage',
+    type: 'select',
+    options: [
+      { value: 'true', label: 'True' },
+      { value: 'won', label: 'Won' },
+    ],
+  },
+  { value: 'closed_at', label: 'Closed at', type: 'date' },
+];
+
+function renderAuthored(row: FilterGroup['conditions'][number]) {
+  const onChange = vi.fn();
+  // Hoisted out of the JSX: the sync effect re-seeds state when the `value`
+  // prop's identity changes, which would undo the interaction under test.
+  const value: FilterGroup = { logic: 'and', conditions: [row] };
+  render(<FilterBuilder fields={FIELDS} value={value} onChange={onChange} />);
+  return onChange;
+}
+
+function lastRow(onChange: ReturnType<typeof vi.fn>): FilterBuilderRow {
+  expect(onChange.mock.calls.length, 'the builder never called onChange').toBeGreaterThan(0);
+  return onChange.mock.calls.at(-1)?.[0].conditions[0];
+}
+
+/** Drive the row's REAL operator dropdown (index 1: field, operator, value). */
+async function pickOperator(label: string) {
+  fireEvent.keyDown(screen.getAllByRole('combobox')[1], { key: 'ArrowDown' });
+  const option = await waitFor(() => {
+    const found = screen.getAllByRole('option').find((o) => o.textContent === label);
+    expect(found, `no "${label}" option in the operator dropdown`).toBeTruthy();
+    return found!;
+  });
+  fireEvent.click(option);
+}
+
+function specAccepts(row: FilterBuilderRow): boolean {
+  return ViewFilterRuleSchema.safeParse({ field: row.field, operator: row.operator, value: row.value })
+    .success;
+}
+
+describe('#26348 — a boolean carried into a list is a list the spec accepts', () => {
+  it('the schema refuses the list the builder used to emit (non-vacuity)', () => {
+    expect(ViewFilterRuleSchema.safeParse({ field: 'stage', operator: 'in', value: [true] }).success).toBe(false);
+    expect(ViewFilterRuleSchema.safeParse({ field: 'stage', operator: 'in', value: ['true'] }).success).toBe(true);
+  });
+
+  it('an `equals true` row on a select column, switched to In, emits ["true"]', async () => {
+    const onChange = renderAuthored({ id: 'c1', field: 'stage', operator: 'equals', value: true });
+    await pickOperator('In');
+    const row = lastRow(onChange);
+    expect(row.operator).toBe('in');
+    expect(row.value).toEqual(['true']);
+    expect(specAccepts(row)).toBe(true);
+    // Still drawn: the list control compares entries with `String()`, so the
+    // option whose value is "true" is the one ticked.
+    const ticked = screen.getAllByRole('checkbox').filter((c) => c.getAttribute('data-state') === 'checked');
+    expect(ticked).toHaveLength(1);
+    expect(ticked[0].closest('label')).toHaveTextContent('True');
+  });
+
+  it('`equals false` switched to Not in emits ["false"]', async () => {
+    const onChange = renderAuthored({ id: 'c1', field: 'stage', operator: 'equals', value: false });
+    await pickOperator('Not in');
+    const row = lastRow(onChange);
+    expect(row.value).toEqual(['false']);
+    expect(specAccepts(row)).toBe(true);
+  });
+
+  it('the helpers write a boolean list or range entry as its string spelling', () => {
+    expect(reshapeFilterValue(true, 'in')).toEqual(['true']);
+    expect(reshapeFilterValue(false, 'not_in')).toEqual(['false']);
+    expect(reshapeFilterValue(true, 'between')).toEqual(['true', '']);
+    // A scalar target is untouched: `equals true` keeps its boolean.
+    expect(reshapeFilterValue(true, 'equals')).toBe(true);
+    // A field switch into a list column converts the same way.
+    expect(retypeFilterValue(true, 'select', 'in')).toEqual(['true']);
+  });
+});

ui/packages/components/src/__tests__/filter-builder-name-authority-9306.test.tsx

@@ -97,14 +97,14 @@ const offVocabulary: FilterBuilderRow = {
   value: 1,
 };
 
-// The value extension: the SPEC rule's value, by reference (#26306), plus the
-// builder's own boolean-bearing list — never the
+// The value extension: the SPEC rule's value, by reference (#26306), and
+// nothing wider since #26348 — never the
 // authority's `value?: any`, which would switch the typing of every exported
 // value helper off in silence. Optional, as the spec's is: a value-less
 // operator is authored with no value, and a required member refused the
 // authored group the renderer hands over. The per-case pins live in
 // `filter-builder-spec-row-26306.test.tsx`.
-expectType<Equal<FilterBuilderRow['value'], ViewFilterRule['value'] | (string | number | boolean)[]>>();
+expectType<Equal<FilterBuilderRow['value'], ViewFilterRule['value']>>();
 const rowWithoutValue: FilterBuilderRow = { id: 'c1', field: 'a', operator: 'is_null' };
 // …and still not `any`: a value the spec refuses is refused here.
 const objectValue: FilterBuilderRow = {

ui/packages/components/src/__tests__/filter-builder-spec-row-26306.test.tsx

@@ -22,7 +22,7 @@
  *   |--------------------------------|------------------------|---------------------------------|
  *   | value omitted (unary operator) | `value?`               | `value` required                |
  *   | value `null`                   | admitted               | refused                         |
- *   | boolean entry in a list / pair | refused                | admitted (kept, see below)      |
+ *   | boolean entry in a list / pair | refused                | admitted (narrowed by #26348)   |
  *   | operator                       | `VIEW_FILTER_OPERATORS` | those + opt-in `exists` / `notExists` |
  *
  * The operator row is not a divergence on the way IN: every spec id is a
@@ -36,12 +36,11 @@
  * threw on `.trim()` and on a text column came back as the string
  * `"undefined"`.
  *
- * The third case runs the other way — the builder is WIDER than the spec, so it
- * does not keep an authored group out — and is left as it is here: the row
- * keeps its boolean list member beside the spec's value, and narrowing it is a
- * breaking change to a published type with a consumer in `@object-ui/plugin-list`
- * (`UserFilters` hands `LookupValuePicker` a boolean-bearing list). The pin
- * below states the width as it is, so the narrowing has to change it.
+ * The third case runs the other way — the builder was WIDER than the spec, so it
+ * did not keep an authored group out — and was left as it was here. #26348
+ * narrowed it: the row's value is now the spec's value and nothing wider, and
+ * the writers that produced a boolean list entry write its string spelling.
+ * Its pins live in `filter-builder-boolean-list-26348.test.tsx`.
  *
  * Type-level pins are judged by `tsc -p tsconfig.test.json`; a green vitest run
  * says nothing about them.
@@ -66,9 +65,8 @@ type Assignable<From, To> = [From] extends [To] ? true : false;
 
 /* ── The shapes ───────────────────────────────────────────────────────────── */
 
-// The row's value IS the spec rule's value — by reference, not a copy of it —
-// plus the builder's own wider list member.
-expectType<Equal<FilterBuilderRow['value'], ViewFilterRule['value'] | (string | number | boolean)[]>>();
+// The row's value IS the spec rule's value — by reference, not a copy of it.
+expectType<Equal<FilterBuilderRow['value'], ViewFilterRule['value']>>();
 expectType<Assignable<ViewFilterRule['value'], FilterBuilderRow['value']>>();
 // The renderer's two reads cross with no cast: the authored group and the
 // authored field list are what the builder's props take.