Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions .changeset/lookup-inline-resolution-align.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
---
"@object-ui/fields": patch
---

fix(fields): align inline lookup value resolution with the read cell (external-id strings, tolerant id match)

Follow-up to #2125. `LookupField`'s inline display now resolves every value
shape the read cell (`LookupCellRenderer`) does:

- **JSON-encoded external-id references** (`'{"externalId":"Website Relaunch"}'`)
are parsed and shown by their external id, and excluded from the hydration
fetch (so we never `findOne` with a raw JSON string). `recordToOption` gained
an `externalId` fallback for both the value and the label.
- **Tolerant id matching** — a `String()`-coerced fallback (`findOptionLoose`)
resolves a numeric cell value against a string-keyed option (and vice versa),
matching the read cell's `String(a) === String(b)` comparison. Only consulted
when the strict match misses, so homogeneous option lists are unaffected.

Also adds explicit inline-editor tests for `user` / `owner` fields (they
delegate to `LookupField` via `UserField`), completing coverage for the full
relational set wired inline in #2122.
73 changes: 73 additions & 0 deletions packages/fields/src/complex-widgets.test.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -620,6 +620,79 @@ describe('Complex & Relationship Widgets', () => {
});
});

describe('LookupField — inline value-shape alignment with the read cell', () => {
// Rounds out #2125: user/owner inline editing, JSON-encoded external-id
// reference values, and String()-tolerant id matching — so the inline
// editor resolves every value shape the read cell (LookupCellRenderer)
// does, instead of falling back to the "Select…" placeholder.
const mockDataSource = {
find: vi.fn(),
findOne: vi.fn(),
create: vi.fn(),
update: vi.fn(),
delete: vi.fn(),
};

beforeEach(() => {
vi.clearAllMocks();
try { localStorage.clear(); } catch { /* jsdom */ }
});

it('user field resolves an expanded-object value inline (via UserField → LookupField)', () => {
render(
<FieldEditWidget
field={{ name: 'assignee', type: 'user' } as any}
value={{ id: 'u1', name: 'Ada Lovelace' }}
onChange={vi.fn()}
/>
);
expect(screen.getByText('Ada Lovelace')).toBeInTheDocument();
});

it('owner field resolves an expanded-object value inline', () => {
render(
<FieldEditWidget
field={{ name: 'owner', type: 'owner' } as any}
value={{ id: 'u2', name: 'Grace Hopper' }}
onChange={vi.fn()}
/>
);
expect(screen.getByText('Grace Hopper')).toBeInTheDocument();
});

it('resolves a JSON-encoded external-id reference string without a bogus fetch', () => {
render(
<LookupField
value={'{"externalId":"Website Relaunch"}'}
onChange={vi.fn()}
field={{ name: 'project', type: 'lookup', reference: 'projects' } as any}
readonly={false}
dataSource={mockDataSource}
compact
/>
);
// The external id is used as the display label (mirrors the read cell).
expect(screen.getByText('Website Relaunch')).toBeInTheDocument();
// Never fetch by passing the raw JSON string as an id.
expect(mockDataSource.findOne).not.toHaveBeenCalled();
expect(mockDataSource.find).not.toHaveBeenCalled();
});

it('resolves a numeric cell value against a string-keyed option (tolerant match)', () => {
render(
<LookupField
value={1}
onChange={vi.fn()}
field={{ name: 'category', type: 'lookup', options: [{ value: '1', label: 'One' }, { value: '2', label: 'Two' }] } as any}
readonly={false}
compact
/>
);
// Strict === misses (number 1 vs string '1'); the loose fallback resolves it.
expect(screen.getByText('One')).toBeInTheDocument();
});
});

describe('MasterDetailField', () => {
const items = [
{ id: '1', label: 'Item 1' },
Expand Down
72 changes: 60 additions & 12 deletions packages/fields/src/widgets/LookupField.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -90,7 +90,7 @@ function recordToOption(
titleFormat?: string | null,
objectDef?: any,
): LookupOption {
const val = record[idField] ?? record.id ?? record._id;
const val = record[idField] ?? record.id ?? record._id ?? record.externalId;
const templated = titleFormat ? formatRecordTitle(record, titleFormat) : null;

// Object-level resolver fallback (displayNameField + derivation), excluding
Expand All@@ -114,11 +114,31 @@ function recordToOption(
record.full_name ??
record.title ??
record.subject ??
record.externalId ??
String(val);
const description = descriptionField ? record[descriptionField] : undefined;
return { value: val, label: String(label), description, ...record };
}

/**
* A reference value can arrive JSON-encoded — e.g. an unresolved external-id
* reference `'{"externalId":"Website Relaunch"}'`. Parse such a string into its
* object form so the inline editor resolves it through the same path as a
* server-`$expand`ed record. Returns null for anything that isn't a JSON object
* string. Mirrors the read cell (`LookupCellRenderer`) so the two stay aligned.
*/
function parseReferenceObjectString(v: any): Record<string, any> | null {
if (typeof v !== 'string') return null;
const s = v.trim();
if (!s.startsWith('{') || !s.endsWith('}')) return null;
try {
const parsed = JSON.parse(s);
return parsed && typeof parsed === 'object' && !Array.isArray(parsed) ? parsed : null;
} catch {
return null;
}
}

/**
* Map a LookupColumnDef.type to a filter input type for the filter bar.
* Returns undefined if the field type is not filterable.
Expand DownExpand Up@@ -466,11 +486,14 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel
const raw: any[] = multiple
? Array.isArray(value) ? value : []
: value != null && value !== '' ? [value] : [];
// Expanded-reference values (server `$expand`) already arrive as the related
// record object and resolve directly in `resolveSelectedOption` — only bare
// ids need a fetch. Passing an object to `findOne` would query for a bogus
// id and leave the trigger stuck on the placeholder.
const ids = raw.filter((v) => v != null && v !== '' && typeof v !== 'object');
// Expanded-reference values (server `$expand`, or their JSON-encoded string
// form) already carry their display fields and resolve directly in
// `resolveSelectedOption` — only bare ids need a fetch. Passing an object (or
// a JSON string) to `findOne` would query for a bogus id and leave the
// trigger stuck on the placeholder.
const ids = raw.filter(
(v) => v != null && v !== '' && typeof v !== 'object' && !parseReferenceObjectString(v),
);
if (!ids.length) return;
// Only fetch records we haven't resolved yet.
const unresolved = ids.filter((v) => !findOption(v));
Expand DownExpand Up@@ -522,11 +545,29 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel
[staticOptions, fetchedOptions, pickerResolvedRecords],
);

// String-coerced fallback for `findOption` — matches the read cell's tolerant
// `String(a) === String(b)` comparison so a numeric cell value still resolves
// against a string-keyed option (and vice versa). Only consulted when the
// strict match misses, so homogeneous option lists are unaffected.
const findOptionLoose = useCallback(
(v: any): LookupOption | undefined => {
const key = String(v);
return (
staticOptions.find(opt => String(opt.value) === key) ??
fetchedOptions.find(opt => String(opt.value) === key) ??
pickerResolvedRecords.find(opt => String(opt.value) === key)
);
},
[staticOptions, fetchedOptions, pickerResolvedRecords],
);

// Collapse an expanded-reference value (the related record object returned by
// server `$expand`) to its bare id — used for option matching / highlighting.
const normalizeId = useCallback(
(raw: any): any =>
raw != null && typeof raw === 'object' ? (raw[idField] ?? raw.id ?? raw._id) : raw,
(raw: any): any => {
const obj = raw != null && typeof raw === 'object' ? raw : parseReferenceObjectString(raw);
return obj ? (obj[idField] ?? obj.id ?? obj._id ?? obj.externalId) : raw;
},
[idField],
);

Expand All@@ -537,12 +578,19 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel
const resolveSelectedOption = useCallback(
(raw: any): LookupOption | undefined => {
if (raw == null || raw === '') return undefined;
if (typeof raw === 'object') {
return recordToOption(raw, displayField, idField, effectiveDescriptionField, refTitleFormat, refObjectSchema);
// An expanded-reference object (server `$expand`) — or its JSON-encoded
// string form, e.g. an external-id reference `'{"externalId":"…"}'` — is
// mapped directly, mirroring the read cell (`LookupCellRenderer`).
const asObject = typeof raw === 'object' ? raw : parseReferenceObjectString(raw);
if (asObject) {
return recordToOption(asObject, displayField, idField, effectiveDescriptionField, refTitleFormat, refObjectSchema);
}
return findOption(raw);
// Bare id: strict match first, then a String()-coerced fallback so a
// numeric cell value still resolves against a string-keyed option (and
// vice versa) — matching the read cell's tolerant comparison.
return findOption(raw) ?? findOptionLoose(raw);
},
[findOption, displayField, idField, effectiveDescriptionField, refTitleFormat, refObjectSchema],
[findOption, findOptionLoose, displayField, idField, effectiveDescriptionField, refTitleFormat, refObjectSchema],
);

const selectedOptions = multiple
Expand Down
Loading