fix(fields): 记录选择器筛选面板保留 select 选项作者声明的类型 (#3422)

.changeset/record-picker-filter-select-option-type.md

@@ -0,0 +1,5 @@
+---
+'@object-ui/fields': patch
+---
+
+The record picker's filter panel now sends the AUTHORED value of a picked `select` filter option instead of the control's stringified form. Radix `Select` speaks strings — options render as `String(opt.value)` and `onValueChange` returns that string — and the panel stored it as-is, so a filter option whose value is a number or a boolean queried `{ level: "1" }` against records storing `level: 1`. The user picked an option that plainly has records and got an empty list. This bit the `lookup_filters` auto-derivation in particular: `lookup_filters: [{ field: 'level', operator: 'in', value: [1, 2, 3] }]` derives options whose values keep the author's type (`LookupFilterDef.value` is `unknown`), so every non-string option in the picker was unfilterable. The control's string is now mapped back through `col.options` at the control boundary — the control speaks string, the payload keeps the authored type — reusing the `matchOptionValue` / `toControlValue` semantics `@object-ui/components` introduced for the standalone form's select (#3090), rather than coercing in `filterValuesToRecord`, which would have to guess whether `"1"` meant `1` or `"1"`. Options coming from the object schema are strings by spec and round-trip unchanged, and the `number` / `boolean` filter inputs (which already converted explicitly) are untouched — the `select` branch is lifted to their standard, resolving the three-way inconsistency (#3422).

packages/fields/src/widgets/RecordPickerDialog.filterOptions.test.tsx

@@ -39,6 +39,11 @@
  *   keep winning over the schema (including the `in`-operator options
  *   auto-derived from `lookupFilters`). They guard the precedence and the
  *   non-leniency the fix chose, not a defect it repaired.
+ *
+ * The second suite in this file covers #3422 — what the panel DOWNLOADS once an
+ * option is picked. Having the right options to choose from (#3336) and sending
+ * the right value for the chosen one are the two halves of the same control, so
+ * they are pinned side by side; see that suite's own header for its directions.
  */
 
 import React from 'react';
@@ -112,10 +117,14 @@ const phaseFilterColumn = { field: 'project_phase', label: 'Project Phase', type
  * `findBy…`: the filter bar only exists once the filter columns are resolved,
  * which in the LookupField path waits on the referenced object's schema fetch.
  */
-async function openFilterSelect(): Promise<Element> {
+async function openFilterPanel(): Promise<HTMLElement> {
   const bar = await screen.findByTestId('record-picker-filter-bar');
   fireEvent.click(bar.querySelector('button')!);
-  const panel = await screen.findByTestId('record-picker-filter-panel');
+  return (await screen.findByTestId('record-picker-filter-panel')) as HTMLElement;
+}
+
+async function openFilterSelect(): Promise<Element> {
+  const panel = await openFilterPanel();
   const trigger = panel.querySelector('[role="combobox"]');
   expect(trigger).toBeTruthy();
   await act(async () => {
@@ -332,6 +341,242 @@ describe('RecordPickerDialog filter panel — select options come from the schem
   });
 });
 
+/**
+ * Record Picker FILTER PANEL — the picked option keeps its AUTHORED type
+ * (#3422).
+ *
+ * Radix `Select` speaks strings: options render as `String(opt.value)` and
+ * `onValueChange` returns that string. The panel used to store the string
+ * as-is, and `filterValuesToRecord`'s `select` branch passes its value through
+ * untouched — so a `lookup_filters`-derived option whose authored value is a
+ * number or a boolean queried `{ level: "1" }` against records storing
+ * `level: 1`. The user picked an option that plainly has records and got an
+ * empty list.
+ *
+ * The fix maps the control's string back through `col.options` at the control
+ * boundary (`matchOptionValue`, the #3090 semantics from
+ * `packages/components/src/renderers/form/option-value.ts`) — NOT a coercion
+ * in `filterValuesToRecord`, which would have to guess whether `"1"` meant `1`
+ * or `"1"`.
+ *
+ * ## Directions, decided before the run
+ *
+ * - **The two reproducers are RED before / GREEN after**: `$filter.level` was
+ *   the string `"1"`, `$filter.active` the string `"false"`. They assert
+ *   `toBe(1)` / `toBe(false)` plus an explicit `typeof`, because `"1" == 1` and
+ *   a loose assertion would have passed on the broken build.
+ * - **Three cases are NO-OP pins, green before and after** (marked
+ *   individually): schema-derived options are already strings and must
+ *   round-trip byte-identical (the fix must not start morphing the path
+ *   #3336 just made work); and the `number` / `boolean` filter inputs, which
+ *   already converted explicitly, must keep doing exactly that — the
+ *   three-branch inconsistency the issue flagged is resolved by lifting
+ *   `select` up to them, not by touching them.
+ * - **The collision case documents a tie-break, not a repair.** Two options
+ *   whose `String()` forms collide (`1` and `'1'`) cannot both be addressed by
+ *   a string-speaking control — Radix sees two `SelectItem`s with the same
+ *   `value`. The first match wins; the case pins that so the resolution is a
+ *   stated rule rather than an accident of `Array.prototype.find`.
+ */
+describe('RecordPickerDialog filter panel — picked option keeps its authored type (#3422)', () => {
+  const tasks = [{ id: 't1', name: 'Recalibrate press', level: 1, active: false }];
+
+  function makeTaskDataSource() {
+    const find = vi.fn(async () => ({ data: tasks, total: tasks.length }));
+    const getObjectSchema = vi.fn(async () => ({
+      name: 'tasks',
+      fields: {
+        name: { type: 'text', label: 'Name' },
+        level: { type: 'number', label: 'Level' },
+        active: { type: 'boolean', label: 'Active' },
+      },
+      highlightFields: ['name'],
+    }));
+    return { find, getObjectSchema } as any;
+  }
+
+  /** The `$filter` of the most recent `find` call. */
+  function lastFilter(ds: any): Record<string, any> {
+    return ds.find.mock.calls[ds.find.mock.calls.length - 1][1].$filter;
+  }
+
+  /**
+   * REPRODUCER (red before the fix). `lookup_filters` with an `in` array of
+   * numbers derives a select whose option values are numbers
+   * (`LookupFilterDef.value` is `unknown`, so the author's type survives into
+   * `options`). Picking one must query the number.
+   */
+  it('sends the authored NUMBER for a lookupFilters-derived select option', async () => {
+    const ds = makeTaskDataSource();
+    render(
+      <RecordPickerDialog
+        open
+        onOpenChange={() => {}}
+        dataSource={ds}
+        objectName="tasks"
+        onSelect={() => {}}
+        cellRenderer={getCellRenderer}
+        lookupFilters={[{ field: 'level', operator: 'in', value: [1, 2, 3] }]}
+      />,
+    );
+
+    await waitFor(() => expect(ds.find).toHaveBeenCalled());
+    await openFilterSelect();
+
+    await act(async () => {
+      fireEvent.click(await screen.findByRole('option', { name: '1' }));
+    });
+
+    // Pre-fix this was the string "1"; `toBe` distinguishes them, `==` would not.
+    await waitFor(() => expect(lastFilter(ds).level).toBe(1));
+    expect(typeof lastFilter(ds).level).toBe('number');
+  });
+
+  /** REPRODUCER (red before the fix) — the boolean half of the same morph. */
+  it('sends the authored BOOLEAN for a lookupFilters-derived select option', async () => {
+    const ds = makeTaskDataSource();
+    render(
+      <RecordPickerDialog
+        open
+        onOpenChange={() => {}}
+        dataSource={ds}
+        objectName="tasks"
+        onSelect={() => {}}
+        cellRenderer={getCellRenderer}
+        lookupFilters={[{ field: 'active', operator: 'in', value: [true, false] }]}
+      />,
+    );
+
+    await waitFor(() => expect(ds.find).toHaveBeenCalled());
+    await openFilterSelect();
+
+    await act(async () => {
+      fireEvent.click(await screen.findByRole('option', { name: 'false' }));
+    });
+
+    // Pre-fix this was the string "false" — which is TRUTHY, so the query asked
+    // for the opposite of what the user picked wherever the backend coerces.
+    await waitFor(() => expect(lastFilter(ds).active).toBe(false));
+    expect(typeof lastFilter(ds).active).toBe('boolean');
+  });
+
+  /**
+   * NO-OP PIN (green before and after). Schema-derived options are strings by
+   * spec (`SelectOptionSchema.value` is `string`), so the #3336 path must come
+   * out of the round-trip byte-identical — the fix restores the authored type,
+   * it does not invent one.
+   */
+  it('round-trips a schema-derived string option unchanged', async () => {
+    const ds = makeDataSource();
+    render(
+      <RecordPickerDialog
+        open
+        onOpenChange={() => {}}
+        dataSource={ds}
+        objectName="projects"
+        columns={[{ field: 'project_phase', label: 'Project Phase', type: 'select' }]}
+        onSelect={() => {}}
+        cellRenderer={getCellRenderer}
+        fieldsMeta={projectFields}
+        filterColumns={[phaseFilterColumn]}
+      />,
+    );
+
+    await waitFor(() => expect(ds.find).toHaveBeenCalled());
+    await openFilterSelect();
+
+    await act(async () => {
+      fireEvent.click(await screen.findByRole('option', { name: '03 Manufacturing' }));
+    });
+
+    await waitFor(() => expect(lastFilter(ds).project_phase).toBe('manufacturing'));
+    expect(typeof lastFilter(ds).project_phase).toBe('string');
+  });
+
+  /**
+   * TIE-BREAK PIN. `1` and `'1'` both render as the control string `"1"`, so
+   * the control cannot tell them apart — Radix receives two `SelectItem`s with
+   * the same `value` and React two children with the same key. The lookup
+   * resolves to the FIRST declared match, whichever entry was clicked. This
+   * documents the rule; it does not claim such an option list is usable.
+   */
+  it('resolves a String() collision to the first declared option', async () => {
+    const ds = makeTaskDataSource();
+    render(
+      <RecordPickerDialog
+        open
+        onOpenChange={() => {}}
+        dataSource={ds}
+        objectName="tasks"
+        onSelect={() => {}}
+        cellRenderer={getCellRenderer}
+        filterColumns={[
+          {
+            field: 'level',
+            label: 'Level',
+            type: 'select',
+            options: [
+              { label: 'Number one', value: 1 },
+              { label: 'String one', value: '1' },
+            ],
+          },
+        ]}
+      />,
+    );
+
+    await waitFor(() => expect(ds.find).toHaveBeenCalled());
+    await openFilterSelect();
+
+    // Clicking the SECOND entry still yields the first match — that is the
+    // documented consequence of a collision, not a bug in this assertion.
+    await act(async () => {
+      fireEvent.click(await screen.findByRole('option', { name: 'String one' }));
+    });
+
+    await waitFor(() => expect(lastFilter(ds).level).toBe(1));
+    expect(typeof lastFilter(ds).level).toBe('number');
+  });
+
+  /**
+   * NO-OP PIN (green before and after). The `number` and `boolean` filter
+   * inputs already converted explicitly (`Number(raw)` / `Boolean(checked)`);
+   * the issue flagged the three branches disagreeing, and the fix resolves that
+   * by lifting `select` to their standard — these two must be untouched.
+   */
+  it('leaves the number and boolean filter inputs converting as before', async () => {
+    const ds = makeTaskDataSource();
+    render(
+      <RecordPickerDialog
+        open
+        onOpenChange={() => {}}
+        dataSource={ds}
+        objectName="tasks"
+        onSelect={() => {}}
+        cellRenderer={getCellRenderer}
+        filterColumns={[
+          { field: 'level', label: 'Level', type: 'number' },
+          { field: 'active', label: 'Active', type: 'boolean' },
+        ]}
+      />,
+    );
+
+    await waitFor(() => expect(ds.find).toHaveBeenCalled());
+    const panel = await openFilterPanel();
+
+    await act(async () => {
+      fireEvent.change(panel.querySelector('input[type="number"]')!, { target: { value: '2' } });
+    });
+    await waitFor(() => expect(lastFilter(ds).level).toBe(2));
+    expect(typeof lastFilter(ds).level).toBe('number');
+
+    await act(async () => {
+      fireEvent.click(panel.querySelector('[role="checkbox"]')!);
+    });
+    await waitFor(() => expect(lastFilter(ds).active).toBe(true));
+    expect(typeof lastFilter(ds).active).toBe('boolean');
+  });
+});
+
 describe('LookupField → picker filter panel wiring (#3336)', () => {
   const lookup = {
     name: 'project',

packages/fields/src/widgets/RecordPickerDialog.tsx

@@ -220,10 +220,72 @@ function resolveSchemaOptions(
   return translateOptions(objectName, fieldName, options);
 }
 
+/**
+ * Option-value round-trip helpers for the filter panel's `select` input
+ * (#3422).
+ *
+ * Radix `Select` speaks strings only: an option renders as
+ * `value={String(opt.value)}` and `onValueChange` hands that same string back.
+ * A filter option's value, however, is whatever the metadata author wrote —
+ * `lookup_filters: [{ field: 'level', operator: 'in', value: [1, 2, 3] }]`
+ * derives options with NUMBER values (`LookupFilterDef.value` is `unknown`) —
+ * so writing the control's string straight into `$filter` queried
+ * `{ level: "1" }` against records storing `level: 1`, and the panel returned
+ * nothing for an option that plainly has records.
+ *
+ * The remap therefore happens at the CONTROL boundary, not in
+ * `filterValuesToRecord`: the control speaks string, the payload keeps the
+ * authored type. Coercing downstream would mean guessing whether `"1"` meant
+ * `1` or `"1"` — a guess the option list already answers exactly.
+ *
+ * Replicated from `matchOptionValue` / `toControlValue` in
+ * `packages/components/src/renderers/form/option-value.ts` (#3090), which
+ * solved the identical morph for the standalone form's select. Those two are
+ * module-private to `@object-ui/components` (its public barrel does not export
+ * them and the package publishes no deep subpath), so `@object-ui/fields`
+ * keeps its own copy of the four lines rather than widening another package's
+ * API to share them.
+ */
+
+/**
+ * Stringify a value for a string-speaking control, preserving null/undefined
+ * (an absent value must stay absent, not become `"undefined"`).
+ */
+function toControlValue(value: unknown): string | undefined {
+  return value == null ? undefined : String(value);
+}
+
+/**
+ * Map a control's string back to the authored option value, so a numeric /
+ * boolean / object option round-trips with its type intact. Falls back to the
+ * raw string when nothing matches (a stale value, or a column that declares no
+ * options) — the pre-#3422 behaviour, so unmatched paths change nothing.
+ *
+ * When two options share a `String()` form — `1` and `'1'`, or two object
+ * values that both print `[object Object]` — the FIRST match wins. Such a list
+ * is already unrenderable as a dropdown (Radix would receive two `SelectItem`s
+ * with the same `value`, and React two children with the same key), so this
+ * tie-break exists to be deterministic, not to make an ambiguous option list
+ * work.
+ */
+function matchOptionValue(
+  options: ReadonlyArray<{ value: unknown }> | undefined,
+  raw: string,
+): unknown {
+  const hit = options?.find(o => String(o.value) === raw);
+  return hit ? hit.value : raw;
+}
+
 /**
  * Convert user-entered filter bar values into a $filter Record.
  * Each key is a field name, each value the user-entered value.
  * Empty/null values are ignored.
+ *
+ * Note the empty string is the "no filter on this field" sentinel here (and
+ * the `select` control's "nothing picked" value), so an option whose authored
+ * value is `''` cannot be expressed as an active filter. That predates #3422
+ * and is unchanged by it — the round-trip above restores an option's TYPE, it
+ * does not redefine what counts as an empty selection.
  */
 function filterValuesToRecord(
   values: Record<string, any>,
@@ -828,9 +890,11 @@ export function RecordPickerDialog({
           return (
             <div className="space-y-1">
               <Label className="text-xs text-muted-foreground">{label}</Label>
+              {/* The control speaks string; the stored filter value keeps the
+                  authored option type (#3422 — see `matchOptionValue`). */}
               <Select
-                value={val !== undefined && val !== null ? String(val) : ''}
-                onValueChange={v => handleFilterChange(col.field, v)}
+                value={toControlValue(val) ?? ''}
+                onValueChange={v => handleFilterChange(col.field, matchOptionValue(col.options, v))}
               >
                 <SelectTrigger className="h-8 text-xs">
                   <SelectValue placeholder={t('lookup.filterPlaceholder', { label })} />