Skip to content

Commit 29ba3a8

Browse files
baozhoutaoclaude
andauthored
fix(detail): multi-value lookup is selectable in inline edit (#2957)
The pencil affordance on a detail page rendered a `multiple: true` lookup as a SINGLE-select picker: picking a record replaced the value and closed the popover, so a second record could never be added. Root cause: `DetailSection` (and `HeaderHighlight`) enriched each view field from the object schema with a hand-written key whitelist that never included `multiple` — nor any of the other relational/picker keys the form path gets for free (display/description/id fields, quick-create, record-picker columns / page size / base filters, people-picker config, dependent-lookup gating). The two whitelists had already drifted apart from each other twice. Both now go through a shared `enrichDetailField`, which carries the documented key set (view field always wins). With `multiple` flowing through, the select branch of `InlineFieldInput` must stop flattening the value with `String()`, so a multi-value picklist gets the chip picker too (`multiselect` included). Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 4db7eb3 commit 29ba3a8

6 files changed

Lines changed: 355 additions & 54 deletions

File tree

packages/plugin-detail/src/DetailSection.tsx

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import { useSafeFieldLabel } from '@object-ui/react';
3434
import { PermissionFacetLink } from './renderers/PermissionFacetLink';
3535
import { NON_EDITABLE_SYSTEM_FIELDS } from './systemFields';
3636
import { InlineFieldInput, TEXTUAL_REF_FALLBACK_TYPES } from './InlineFieldInput';
37+
import { enrichDetailField } from './fieldEnrichment';
3738

3839
/**
3940
* Section-header icon. `fieldGroups[].icon` declares a Lucide name (spec),
@@ -212,28 +213,7 @@ export const DetailSection: React.FC<DetailSectionProps> = ({
212213
// like select options, currency code, lookup target, etc. are
213214
// available in either mode.
214215
const objectDefField = objectSchema?.fields?.[field.name];
215-
const enrichedField: Record<string, any> = { ...field };
216-
if (objectDefField) {
217-
if (!field.type && objectDefField.type) enrichedField.type = objectDefField.type;
218-
if (objectDefField.options && !enrichedField.options) enrichedField.options = objectDefField.options;
219-
if (objectDefField.currency && !enrichedField.currency) enrichedField.currency = objectDefField.currency;
220-
if (objectDefField.precision !== undefined && enrichedField.precision === undefined) enrichedField.precision = objectDefField.precision;
221-
if ((objectDefField as any).scale !== undefined && (enrichedField as any).scale === undefined) (enrichedField as any).scale = (objectDefField as any).scale;
222-
// Numeric range/step constraints (objectui#2572 item 3) — without these
223-
// the metadata's `min: 0` never reaches the numeric edit widgets.
224-
if ((objectDefField as any).min !== undefined && (enrichedField as any).min === undefined) (enrichedField as any).min = (objectDefField as any).min;
225-
if ((objectDefField as any).max !== undefined && (enrichedField as any).max === undefined) (enrichedField as any).max = (objectDefField as any).max;
226-
if ((objectDefField as any).step !== undefined && (enrichedField as any).step === undefined) (enrichedField as any).step = (objectDefField as any).step;
227-
if (objectDefField.format && !enrichedField.format) enrichedField.format = objectDefField.format;
228-
// Per-field widget override (ADR-0056 P2) — carry it from the object
229-
// metadata so the inline-edit widget branch (and read cell) can honor a
230-
// structured editor even when the synthesized section field didn't include it.
231-
if ((objectDefField as any).widget && !enrichedField.widget) enrichedField.widget = (objectDefField as any).widget;
232-
const refTarget = objectDefField.reference_to || objectDefField.reference;
233-
if (refTarget && !enrichedField.reference_to) enrichedField.reference_to = refTarget;
234-
if (objectDefField.reference_field && !enrichedField.reference_field) enrichedField.reference_field = objectDefField.reference_field;
235-
if ((objectDefField as any).dueLike !== undefined && enrichedField.dueLike === undefined) enrichedField.dueLike = (objectDefField as any).dueLike;
236-
}
216+
const enrichedField = enrichDetailField(field as Record<string, any>, objectDefField);
237217
if (objectName && Array.isArray(enrichedField.options) && enrichedField.options.length > 0) {
238218
enrichedField.options = translateOptions(objectName, field.name, enrichedField.options as any);
239219
}

packages/plugin-detail/src/HeaderHighlight.tsx

Lines changed: 10 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { getCellRenderer, resolveCellRendererType } from '@object-ui/fields';
2020
import { useSafeFieldLabel, useInlineEdit } from '@object-ui/react';
2121
import { Check, X, Pencil } from 'lucide-react';
2222
import { InlineFieldInput, TEXTUAL_REF_FALLBACK_TYPES } from './InlineFieldInput';
23+
import { enrichDetailField } from './fieldEnrichment';
2324
import { NON_EDITABLE_SYSTEM_FIELDS } from './systemFields';
2425
import { useDetailTranslation } from './useDetailTranslation';
2526

@@ -82,35 +83,15 @@ export const HeaderHighlight: React.FC<HeaderHighlightProps> = ({
8283
// Enrich field metadata from objectSchema
8384
const objectDefField = objectSchema?.fields?.[field.name];
8485
const resolvedType = field.type || objectDefField?.type;
85-
// Backend object schemas use the ObjectStack-convention `reference`
86-
// key (DetailSection normalizes the same pair) — without it the
87-
// lookup editor has no target object to hydrate or search against.
88-
const refTarget =
89-
objectDefField?.reference_to || (objectDefField as any)?.reference;
90-
const enrichedField = {
91-
name: field.name,
92-
label: field.label,
93-
type: resolvedType || 'text',
94-
...(objectDefField?.options && { options: objectDefField.options }),
95-
...(objectDefField?.currency && { currency: objectDefField.currency }),
96-
// The SPEC channel for a per-field currency (a bare `currency`
97-
// key is designer/DB-only) — resolveFieldCurrency reads
98-
// currencyConfig.defaultCurrency second (#2548).
99-
...(objectDefField?.currencyConfig && { currencyConfig: objectDefField.currencyConfig }),
100-
...(objectDefField?.precision !== undefined && { precision: objectDefField.precision }),
101-
...((objectDefField as any)?.scale !== undefined && { scale: (objectDefField as any).scale }),
102-
// Numeric range/step constraints (objectui#2572 item 3) — keep in
103-
// sync with DetailSection's enrichment so both editors honor them.
104-
...((objectDefField as any)?.min !== undefined && { min: (objectDefField as any).min }),
105-
...((objectDefField as any)?.max !== undefined && { max: (objectDefField as any).max }),
106-
...((objectDefField as any)?.step !== undefined && { step: (objectDefField as any).step }),
107-
...(objectDefField?.format && { format: objectDefField.format }),
108-
...(refTarget && { reference_to: refTarget }),
109-
...((objectDefField as any)?.reference_field && {
110-
reference_field: (objectDefField as any).reference_field,
111-
}),
112-
...((objectDefField as any)?.widget && { widget: (objectDefField as any).widget }),
113-
};
86+
// Shared with DetailSection (`enrichDetailField`) so the highlights
87+
// strip and the details body resolve an identical field shape —
88+
// including the relational keys a lookup picker needs (`multiple`,
89+
// display/id fields, picker config), and the `reference` /
90+
// `reference_to` spelling pair backend schemas use.
91+
const enrichedField = enrichDetailField(
92+
{ name: field.name, label: field.label, type: resolvedType || 'text' },
93+
objectDefField,
94+
);
11495

11596
// Live value = the user's draft edit for this field, else the record
11697
// value. Read as `draft[name] ?? data[name]` (objectui#2407 P2).

packages/plugin-detail/src/InlineFieldInput.tsx

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,23 @@ export const InlineFieldInput: React.FC<InlineFieldInputProps> = ({
113113
);
114114
}
115115
// Picklist → real Select widget so users see localized option labels and
116-
// can't free-type invalid values.
117-
if (editType === 'select' && Array.isArray(field.options) && field.options.length > 0) {
118-
return (
116+
// can't free-type invalid values. A `multiple` picklist (spec canon: `select`
117+
// + `multiple: true`; `multiselect` is the widget-level alias) selects
118+
// zero-or-more values — `SelectField` delegates to the chip picker, so the
119+
// value must stay an ARRAY here instead of being flattened by `String()`.
120+
const isMultiSelect = editType === 'multiselect' || !!field.multiple;
121+
if (
122+
(editType === 'select' || editType === 'multiselect') &&
123+
Array.isArray(field.options) &&
124+
field.options.length > 0
125+
) {
126+
return isMultiSelect ? (
127+
<SelectField
128+
field={{ ...(field as any), multiple: true }}
129+
value={Array.isArray(value) ? value : value == null || value === '' ? [] : [value]}
130+
onChange={(v) => onChange(v)}
131+
/>
132+
) : (
119133
<SelectField
120134
field={field as any}
121135
value={value == null ? '' : String(value)}
Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* Multi-value fields in inline edit (the pencil affordance on a detail page).
11+
*
12+
* Regression: `DetailSection`'s field enrichment copied a hand-written key
13+
* whitelist that never included `multiple`, so a lookup declared
14+
* `multiple: true` reached the picker as a SINGLE-select field — picking a
15+
* record replaced the value and closed the popover, making it impossible to
16+
* select more than one. The read cell showed one chip for the same reason.
17+
*
18+
* The enrichment now goes through the shared `enrichDetailField`, which carries
19+
* `multiple` (and the rest of the picker configuration) from the object schema.
20+
*/
21+
22+
import * as React from 'react';
23+
import { describe, it, expect, vi, beforeEach } from 'vitest';
24+
import { render, screen, fireEvent, waitFor, act } from '@testing-library/react';
25+
import { DetailSection } from '../DetailSection';
26+
import type { DetailViewSection } from '@object-ui/types';
27+
28+
const tags = [
29+
{ id: 't1', name: 'Urgent' },
30+
{ id: 't2', name: 'Backlog' },
31+
];
32+
33+
function makeDataSource() {
34+
const find = vi.fn(async () => ({ data: tags, total: tags.length }));
35+
return { find } as any;
36+
}
37+
38+
const objectSchema = {
39+
fields: {
40+
tags: {
41+
type: 'lookup',
42+
reference_to: 'tags',
43+
display_field: 'name',
44+
multiple: true,
45+
},
46+
priority: {
47+
type: 'select',
48+
multiple: true,
49+
options: [
50+
{ label: 'High', value: 'high' },
51+
{ label: 'Low', value: 'low' },
52+
],
53+
},
54+
},
55+
};
56+
57+
const section = {
58+
fields: [{ name: 'tags', label: 'Tags' }],
59+
} as DetailViewSection;
60+
61+
/**
62+
* Stand-in for `DetailView`'s inline-edit state: it merges the draft back into
63+
* `data` on every change, exactly as `{ ...data, ...editedValues }` does, so a
64+
* second pick sees the first one.
65+
*/
66+
function EditingHost({
67+
onChange,
68+
initial,
69+
fields = section,
70+
}: {
71+
onChange: (name: string, value: any) => void;
72+
initial: Record<string, any>;
73+
fields?: DetailViewSection;
74+
}) {
75+
const [data, setData] = React.useState(initial);
76+
return (
77+
<DetailSection
78+
section={fields}
79+
data={data}
80+
objectSchema={objectSchema}
81+
dataSource={makeDataSource()}
82+
isEditing
83+
onFieldChange={(name, value) => {
84+
setData((prev) => ({ ...prev, [name]: value }));
85+
onChange(name, value);
86+
}}
87+
/>
88+
);
89+
}
90+
91+
beforeEach(() => {
92+
try {
93+
localStorage.clear();
94+
} catch {
95+
/* happy-dom */
96+
}
97+
});
98+
99+
describe('DetailSection inline edit — multi-value lookup', () => {
100+
it('accumulates picks instead of replacing the previous one', async () => {
101+
const onChange = vi.fn();
102+
render(<EditingHost onChange={onChange} initial={{ tags: [] }} />);
103+
104+
await act(async () => {
105+
fireEvent.click(screen.getByTestId('lookup-trigger-tags'));
106+
});
107+
108+
await waitFor(() => expect(screen.getByRole('option', { name: /Urgent/ })).toBeInTheDocument());
109+
110+
await act(async () => {
111+
fireEvent.click(screen.getByRole('option', { name: /Urgent/ }));
112+
});
113+
expect(onChange).toHaveBeenLastCalledWith('tags', ['t1']);
114+
115+
// The popover stays open in multi mode — the second pick must ADD to the
116+
// value, which is the exact step that was impossible before the fix.
117+
await act(async () => {
118+
fireEvent.click(screen.getByRole('option', { name: /Backlog/ }));
119+
});
120+
expect(onChange).toHaveBeenLastCalledWith('tags', ['t1', 't2']);
121+
});
122+
123+
it('renders every selected record as a chip, including `$expand`-ed values', async () => {
124+
render(
125+
<EditingHost
126+
onChange={vi.fn()}
127+
initial={{ tags: [{ id: 't1', name: 'Urgent' }, { id: 't2', name: 'Backlog' }] }}
128+
/>,
129+
);
130+
131+
await waitFor(() => {
132+
expect(screen.getByText('Urgent')).toBeInTheDocument();
133+
expect(screen.getByText('Backlog')).toBeInTheDocument();
134+
});
135+
});
136+
137+
it('renders a multi-value picklist as the chip picker, not a stringified value', async () => {
138+
const onChange = vi.fn();
139+
render(
140+
<EditingHost
141+
onChange={onChange}
142+
initial={{ priority: ['high'] }}
143+
fields={{ fields: [{ name: 'priority', label: 'Priority' }] } as DetailViewSection}
144+
/>,
145+
);
146+
147+
// A single-select dropdown would have coerced the array through String();
148+
// the multi picker offers every option as a toggleable chip.
149+
const high = screen.getByTestId('multiselect-option-high');
150+
expect(high).toHaveAttribute('aria-pressed', 'true');
151+
expect(screen.getByTestId('multiselect-option-low')).toHaveAttribute('aria-pressed', 'false');
152+
153+
await act(async () => {
154+
fireEvent.click(screen.getByTestId('multiselect-option-low'));
155+
});
156+
expect(onChange).toHaveBeenLastCalledWith('priority', ['high', 'low']);
157+
});
158+
});
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
import { describe, it, expect } from 'vitest';
10+
import { enrichDetailField } from '../fieldEnrichment';
11+
12+
describe('enrichDetailField', () => {
13+
it('carries the relational picker configuration a lookup editor needs', () => {
14+
const enriched = enrichDetailField(
15+
{ name: 'tags', label: 'Tags' },
16+
{
17+
type: 'lookup',
18+
reference: 'tags',
19+
multiple: true,
20+
display_field: 'name',
21+
id_field: 'code',
22+
lookup_filters: [{ field: 'active', operator: 'eq', value: true }],
23+
depends_on: ['category'],
24+
picker: 'search',
25+
},
26+
);
27+
28+
expect(enriched).toMatchObject({
29+
type: 'lookup',
30+
// ObjectStack's `reference` spelling normalizes onto the canonical key.
31+
reference_to: 'tags',
32+
multiple: true,
33+
display_field: 'name',
34+
id_field: 'code',
35+
picker: 'search',
36+
depends_on: ['category'],
37+
});
38+
expect(enriched.lookup_filters).toEqual([{ field: 'active', operator: 'eq', value: true }]);
39+
});
40+
41+
it('keeps the view field as the override — the schema only fills gaps', () => {
42+
const enriched = enrichDetailField(
43+
{ name: 'amount', type: 'currency', currency: 'EUR' },
44+
{ type: 'number', currency: 'USD', scale: 2, min: 0 },
45+
);
46+
47+
expect(enriched.type).toBe('currency');
48+
expect(enriched.currency).toBe('EUR');
49+
expect(enriched.scale).toBe(2);
50+
expect(enriched.min).toBe(0);
51+
});
52+
53+
it('never copies `readonly` (the hosts gate editability off the schema directly)', () => {
54+
const enriched = enrichDetailField({ name: 'code' }, { type: 'text', readonly: true });
55+
expect(enriched.readonly).toBeUndefined();
56+
});
57+
58+
it('is a no-op passthrough when the field has no object metadata', () => {
59+
expect(enrichDetailField({ name: 'ad_hoc', type: 'text' }, undefined)).toEqual({
60+
name: 'ad_hoc',
61+
type: 'text',
62+
});
63+
});
64+
});

0 commit comments

Comments
 (0)