Skip to content

Commit 788f4ab

Browse files
hotlonghotlong
authored
Merge pull request #884 from objectstack-ai/copilot/fix-field-type-display-issues
2 parents f8f6f9d + 9ca772b commit 788f4ab

File tree

3 files changed

+328
-9
lines changed

3 files changed

+328
-9
lines changed

‎ROADMAP.md‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1059,6 +1059,34 @@ The `FlowDesigner` is a canvas-based flow editor that bridges the gap between th
10591059

10601060
**Tests:** Added 3 new tests: 1 in `DashboardRenderer.widgetData.test.tsx` verifying metric widgets with I18nLabel trend labels render correctly, and 2 in `MetricCard.test.tsx` verifying I18nLabel resolution for title and description. All 159 dashboard tests pass.
10611061

1062+
### Field Type Display Issues — Lookup, User, Select, Status Renderers (February 2026)
1063+
1064+
**Root Cause:** Multiple renderer defects caused incorrect field value display across views:
1065+
1066+
1. **`LookupCellRenderer`** — Destructured only `value`, ignoring the `field` prop. When the API returned a raw primitive ID (e.g. `customer: 2`), the renderer fell through to `String(value)` and showed `"2"` instead of the related record's name. No attempt was made to resolve IDs via `field.options`.
1067+
1068+
2. **`UserCellRenderer`** — Did not guard against primitive values (number/string user IDs). Accessing `.name` / `.username` on a number returned `undefined`, silently falling through to `"User"` as the generic label.
1069+
1070+
3. **`getCellRenderer` standardMap** — `lookup` and `master_detail` were mapped to `SelectCellRenderer` instead of `LookupCellRenderer` in the fallback map. Although the fieldRegistry pre-registration shadowed this bug, it was semantically incorrect.
1071+
1072+
4. **`status`, `user`, `owner` types** — Not pre-registered in `fieldRegistry`. All went through the `standardMap` path, making their association with renderers implicit and invisible.
1073+
1074+
**Fix:**
1075+
- `LookupCellRenderer`: now accepts the `field` prop and resolves primitive IDs against `field.options` (matching by `String(opt.value) === String(val)` for type-safe comparison). Arrays of primitive IDs are resolved via the same logic. Null/empty-string guard updated from `!value` to `value == null || value === ''` to handle `0` correctly.
1076+
- `UserCellRenderer`: primitive values (typeof !== 'object') return a plain `<span>` with the string representation. Array items that are not objects are also handled gracefully.
1077+
- `getCellRenderer` standardMap: `lookup` and `master_detail` now correctly reference `LookupCellRenderer`.
1078+
- `fieldRegistry` now explicitly registers `status` → `SelectCellRenderer`, `user` → `UserCellRenderer`, and `owner` → `UserCellRenderer` alongside the existing `lookup`/`master_detail`/`select` registrations.
1079+
1080+
**Tests:** Added 36 new tests in `cell-renderers.test.tsx`:
1081+
- `getCellRenderer` registry assertions for `lookup`, `master_detail`, `status`, `user`, `owner` types
1082+
- `TextCellRenderer`: null, undefined, empty string, numeric zero (0 renders "0" not "-"), boolean false
1083+
- `LookupCellRenderer`: null, empty-string, primitive ID (number), primitive ID (string), unresolved primitive, object with name/label/_id, array of objects, array of primitive IDs resolved via options
1084+
- `UserCellRenderer`: null, primitive number ID, primitive string ID, object with name, object with username, array of user objects
1085+
1086+
5. **`TextCellRenderer`** — Used `value || '-'` which incorrectly rendered `'-'` for numeric `0` (falsy zero). Updated to `(value != null && value !== '') ? String(value) : '-'` for consistent null-only suppression.
1087+
1088+
All 313 `@object-ui/fields` tests pass.
1089+
10621090
---
10631091

10641092
## ⚠️ Risk Management

‎packages/fields/src/__tests__/cell-renderers.test.tsx‎

Lines changed: 258 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ import React from 'react';
1111
import {
1212
getCellRenderer,
1313
SelectCellRenderer,
14+
LookupCellRenderer,
15+
UserCellRenderer,
16+
TextCellRenderer,
1417
DateCellRenderer,
1518
BooleanCellRenderer,
1619
formatDate,
@@ -61,10 +64,70 @@ describe('getCellRenderer', () => {
6164
const renderer = getCellRenderer('unknown-type');
6265
expect(renderer).toBeDefined();
6366
});
67+
68+
it('should return LookupCellRenderer for lookup type', () => {
69+
const renderer = getCellRenderer('lookup');
70+
expect(renderer).toBe(LookupCellRenderer);
71+
});
72+
73+
it('should return LookupCellRenderer for master_detail type', () => {
74+
const renderer = getCellRenderer('master_detail');
75+
expect(renderer).toBe(LookupCellRenderer);
76+
});
77+
78+
it('should return SelectCellRenderer for status type', () => {
79+
const renderer = getCellRenderer('status');
80+
expect(renderer).toBe(SelectCellRenderer);
81+
});
82+
83+
it('should return UserCellRenderer for user type', () => {
84+
const renderer = getCellRenderer('user');
85+
expect(renderer).toBe(UserCellRenderer);
86+
});
87+
88+
it('should return UserCellRenderer for owner type', () => {
89+
const renderer = getCellRenderer('owner');
90+
expect(renderer).toBe(UserCellRenderer);
91+
});
92+
});
93+
94+
// =========================================================================
95+
// 2. TextCellRenderer
96+
// =========================================================================
97+
describe('TextCellRenderer', () => {
98+
it('should render text value', () => {
99+
render(<TextCellRenderer value="hello" field={{ name: 'title', type: 'text' } as any} />);
100+
expect(screen.getByText('hello')).toBeInTheDocument();
101+
});
102+
103+
it('should render dash for null', () => {
104+
render(<TextCellRenderer value={null} field={{ name: 'title', type: 'text' } as any} />);
105+
expect(screen.getByText('-')).toBeInTheDocument();
106+
});
107+
108+
it('should render dash for undefined', () => {
109+
render(<TextCellRenderer value={undefined} field={{ name: 'title', type: 'text' } as any} />);
110+
expect(screen.getByText('-')).toBeInTheDocument();
111+
});
112+
113+
it('should render dash for empty string', () => {
114+
render(<TextCellRenderer value="" field={{ name: 'title', type: 'text' } as any} />);
115+
expect(screen.getByText('-')).toBeInTheDocument();
116+
});
117+
118+
it('should render "0" for numeric zero (not dash)', () => {
119+
render(<TextCellRenderer value={0} field={{ name: 'count', type: 'text' } as any} />);
120+
expect(screen.getByText('0')).toBeInTheDocument();
121+
});
122+
123+
it('should render "false" for boolean false (not dash)', () => {
124+
render(<TextCellRenderer value={false} field={{ name: 'flag', type: 'text' } as any} />);
125+
expect(screen.getByText('false')).toBeInTheDocument();
126+
});
64127
});
65128

66129
// =========================================================================
67-
// 2. SelectCellRenderer
130+
// 3. SelectCellRenderer
68131
// =========================================================================
69132
describe('SelectCellRenderer', () => {
70133
it('should render badge with explicit color from options', () => {
@@ -477,3 +540,197 @@ describe('formatDate', () => {
477540
expect(result).toContain('2026');
478541
});
479542
});
543+
544+
// =========================================================================
545+
// 7. LookupCellRenderer
546+
// =========================================================================
547+
describe('LookupCellRenderer', () => {
548+
it('should render dash for null value', () => {
549+
render(
550+
<LookupCellRenderer
551+
value={null}
552+
field={{ name: 'customer', type: 'lookup' } as any}
553+
/>
554+
);
555+
expect(screen.getByText('-')).toBeInTheDocument();
556+
});
557+
558+
it('should render dash for empty string', () => {
559+
render(
560+
<LookupCellRenderer
561+
value=""
562+
field={{ name: 'customer', type: 'lookup' } as any}
563+
/>
564+
);
565+
expect(screen.getByText('-')).toBeInTheDocument();
566+
});
567+
568+
it('should resolve primitive ID to label via field options', () => {
569+
render(
570+
<LookupCellRenderer
571+
value={2}
572+
field={{
573+
name: 'customer',
574+
type: 'lookup',
575+
options: [
576+
{ value: 1, label: 'Alice' },
577+
{ value: 2, label: 'Bob' },
578+
],
579+
} as any}
580+
/>
581+
);
582+
expect(screen.getByText('Bob')).toBeInTheDocument();
583+
});
584+
585+
it('should resolve string ID to label via field options', () => {
586+
render(
587+
<LookupCellRenderer
588+
value="contact_1"
589+
field={{
590+
name: 'customer',
591+
type: 'lookup',
592+
options: [{ value: 'contact_1', label: 'Alice Smith' }],
593+
} as any}
594+
/>
595+
);
596+
expect(screen.getByText('Alice Smith')).toBeInTheDocument();
597+
});
598+
599+
it('should render raw primitive when no options available', () => {
600+
render(
601+
<LookupCellRenderer
602+
value={42}
603+
field={{ name: 'customer', type: 'lookup' } as any}
604+
/>
605+
);
606+
expect(screen.getByText('42')).toBeInTheDocument();
607+
});
608+
609+
it('should render object name when value is an object', () => {
610+
render(
611+
<LookupCellRenderer
612+
value={{ name: 'Acme Corp', _id: '123' }}
613+
field={{ name: 'account', type: 'lookup' } as any}
614+
/>
615+
);
616+
expect(screen.getByText('Acme Corp')).toBeInTheDocument();
617+
});
618+
619+
it('should render object label when name is missing', () => {
620+
render(
621+
<LookupCellRenderer
622+
value={{ label: 'Widget Co', _id: '456' }}
623+
field={{ name: 'account', type: 'lookup' } as any}
624+
/>
625+
);
626+
expect(screen.getByText('Widget Co')).toBeInTheDocument();
627+
});
628+
629+
it('should render object _id as fallback', () => {
630+
render(
631+
<LookupCellRenderer
632+
value={{ _id: '789' }}
633+
field={{ name: 'account', type: 'lookup' } as any}
634+
/>
635+
);
636+
expect(screen.getByText('789')).toBeInTheDocument();
637+
});
638+
639+
it('should render tags for array of objects', () => {
640+
render(
641+
<LookupCellRenderer
642+
value={[{ name: 'Alice' }, { name: 'Bob' }]}
643+
field={{ name: 'contacts', type: 'lookup' } as any}
644+
/>
645+
);
646+
expect(screen.getByText('Alice')).toBeInTheDocument();
647+
expect(screen.getByText('Bob')).toBeInTheDocument();
648+
});
649+
650+
it('should resolve array of primitive IDs via options', () => {
651+
render(
652+
<LookupCellRenderer
653+
value={[1, 3]}
654+
field={{
655+
name: 'contacts',
656+
type: 'lookup',
657+
options: [
658+
{ value: 1, label: 'Alice' },
659+
{ value: 2, label: 'Bob' },
660+
{ value: 3, label: 'Charlie' },
661+
],
662+
} as any}
663+
/>
664+
);
665+
expect(screen.getByText('Alice')).toBeInTheDocument();
666+
expect(screen.getByText('Charlie')).toBeInTheDocument();
667+
expect(screen.queryByText('Bob')).not.toBeInTheDocument();
668+
});
669+
});
670+
671+
// =========================================================================
672+
// 8. UserCellRenderer
673+
// =========================================================================
674+
describe('UserCellRenderer', () => {
675+
it('should render dash for null value', () => {
676+
render(
677+
<UserCellRenderer
678+
value={null}
679+
field={{ name: 'owner', type: 'user' } as any}
680+
/>
681+
);
682+
expect(screen.getByText('-')).toBeInTheDocument();
683+
});
684+
685+
it('should render text for primitive user ID (number)', () => {
686+
render(
687+
<UserCellRenderer
688+
value={5}
689+
field={{ name: 'owner', type: 'user' } as any}
690+
/>
691+
);
692+
expect(screen.getByText('5')).toBeInTheDocument();
693+
});
694+
695+
it('should render text for primitive user ID (string)', () => {
696+
render(
697+
<UserCellRenderer
698+
value="user_abc"
699+
field={{ name: 'owner', type: 'user' } as any}
700+
/>
701+
);
702+
expect(screen.getByText('user_abc')).toBeInTheDocument();
703+
});
704+
705+
it('should render avatar and name for object value', () => {
706+
render(
707+
<UserCellRenderer
708+
value={{ name: 'John Doe', username: 'jdoe' }}
709+
field={{ name: 'owner', type: 'user' } as any}
710+
/>
711+
);
712+
expect(screen.getByText('John Doe')).toBeInTheDocument();
713+
});
714+
715+
it('should use username when name is missing', () => {
716+
render(
717+
<UserCellRenderer
718+
value={{ username: 'jdoe' }}
719+
field={{ name: 'owner', type: 'user' } as any}
720+
/>
721+
);
722+
expect(screen.getByText('jdoe')).toBeInTheDocument();
723+
});
724+
725+
it('should render multiple avatars for array of user objects', () => {
726+
render(
727+
<UserCellRenderer
728+
value={[{ name: 'Alice' }, { name: 'Bob' }]}
729+
field={{ name: 'assignees', type: 'user' } as any}
730+
/>
731+
);
732+
// Avatar fallbacks contain initials
733+
expect(screen.getByTitle('Alice')).toBeInTheDocument();
734+
expect(screen.getByTitle('Bob')).toBeInTheDocument();
735+
});
736+
});

0 commit comments

Comments
 (0)