Skip to content

Commit b00dc61

Browse files
Copilothotlong
Copilot
and
hotlong
committed
fix: address all review comments — spec filter bridge, type normalization, memoized views
- Implement robust spec-style filter bridge: parse ['field','=',val], [['f','=',v],...], and ['and'|'or',...] formats to FilterGroup, and convert back to spec format on save - Map operators between spec ('=','!=','>',etc.) and FilterBuilder IDs (equals, notEquals, etc.) - Normalize ObjectUI field types to FilterBuilder types (currency→number, datetime→date, etc.) - Fix Checkbox onCheckedChange to handle CheckedState properly (c === true) - Memoize merged views array in ObjectView.tsx to prevent unnecessary re-renders - Add tests for nested filter arrays, and/or logic, and field type normalization Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
1 parent bcf39e1 commit b00dc61

3 files changed

Lines changed: 239 additions & 25 deletions

File tree

‎apps/console/src/__tests__/ViewConfigPanel.test.tsx‎

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ const mockActiveView = {
8686
label: 'All Records',
8787
type: 'grid',
8888
columns: ['name', 'stage', 'amount'],
89-
filter: [{ field: 'stage', operator: '=', value: 'active' }],
89+
filter: ['stage', '=', 'active'], // spec-style single triplet
9090
sort: [{ field: 'name', order: 'asc' }],
9191
};
9292

@@ -460,7 +460,7 @@ describe('ViewConfigPanel', () => {
460460
const fb = screen.getByTestId('mock-filter-builder');
461461
expect(fb).toHaveAttribute('data-condition-count', '1');
462462
expect(fb).toHaveAttribute('data-field-count', '3');
463-
expect(screen.getByTestId('filter-condition-0')).toHaveTextContent('stage = active');
463+
expect(screen.getByTestId('filter-condition-0')).toHaveTextContent('stage equals active');
464464
});
465465

466466
it('renders inline SortBuilder with correct items from activeView', () => {
@@ -701,4 +701,66 @@ describe('ViewConfigPanel', () => {
701701
expect(onViewUpdate).toHaveBeenCalledWith('showSearch', false);
702702
expect(onViewUpdate).toHaveBeenCalledWith('showFilters', false);
703703
});
704+
705+
// ── Spec-style filter bridge tests ──
706+
707+
it('parses nested spec-style filter array [[field,op,val],[field,op,val]]', () => {
708+
render(
709+
<ViewConfigPanel
710+
open={true}
711+
onClose={vi.fn()}
712+
activeView={{
713+
...mockActiveView,
714+
filter: [['stage', '=', 'active'], ['name', '!=', 'Test']],
715+
}}
716+
objectDef={mockObjectDef}
717+
/>
718+
);
719+
720+
const fb = screen.getByTestId('mock-filter-builder');
721+
expect(fb).toHaveAttribute('data-condition-count', '2');
722+
expect(screen.getByTestId('filter-condition-0')).toHaveTextContent('stage equals active');
723+
expect(screen.getByTestId('filter-condition-1')).toHaveTextContent('name notEquals Test');
724+
});
725+
726+
it('parses and/or logic prefix: ["or", [...], [...]]', () => {
727+
render(
728+
<ViewConfigPanel
729+
open={true}
730+
onClose={vi.fn()}
731+
activeView={{
732+
...mockActiveView,
733+
filter: ['or', ['stage', '=', 'active'], ['stage', '=', 'pending']],
734+
}}
735+
objectDef={mockObjectDef}
736+
/>
737+
);
738+
739+
const fb = screen.getByTestId('mock-filter-builder');
740+
expect(fb).toHaveAttribute('data-condition-count', '2');
741+
});
742+
743+
it('normalizes field types for FilterBuilder (currency→number)', () => {
744+
render(
745+
<ViewConfigPanel
746+
open={true}
747+
onClose={vi.fn()}
748+
activeView={mockActiveView}
749+
objectDef={{
750+
...mockObjectDef,
751+
fields: {
752+
name: { label: 'Name', type: 'text' },
753+
revenue: { label: 'Revenue', type: 'currency' },
754+
created: { label: 'Created', type: 'datetime' },
755+
active: { label: 'Active', type: 'boolean' },
756+
status: { label: 'Status', type: 'picklist' },
757+
},
758+
}}
759+
/>
760+
);
761+
762+
// The mock FilterBuilder receives normalized fields via data-field-count
763+
const fb = screen.getByTestId('mock-filter-builder');
764+
expect(fb).toHaveAttribute('data-field-count', '5');
765+
});
704766
});

‎apps/console/src/components/ObjectView.tsx‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,17 @@ export function ObjectView({ dataSource, objects, onEdit, onRowClick }: any) {
333333
);
334334
}, [activeView, objectDef, objectName, refreshKey]);
335335

336+
// Memoize the merged views array so PluginObjectView doesn't get a new
337+
// reference on every render (which would trigger unnecessary data refetches).
338+
const mergedViews = useMemo(() =>
339+
views.map((v: any) =>
340+
v.id === activeViewId && viewDraft && viewDraft.id === v.id
341+
? { ...v, ...viewDraft }
342+
: v
343+
),
344+
[views, activeViewId, viewDraft]
345+
);
346+
336347
// Build the ObjectViewSchema for the plugin — reads from activeView (which merges draft)
337348
const objectViewSchema = useMemo(() => ({
338349
type: 'object-view' as const,
@@ -480,11 +491,7 @@ export function ObjectView({ dataSource, objects, onEdit, onRowClick }: any) {
480491
key={refreshKey}
481492
schema={objectViewSchema}
482493
dataSource={dataSource}
483-
views={views.map((v: any) =>
484-
v.id === activeViewId && viewDraft && viewDraft.id === v.id
485-
? { ...v, ...viewDraft }
486-
: v
487-
)}
494+
views={mergedViews}
488495
activeViewId={activeViewId}
489496
onViewChange={handleViewChange}
490497
onEdit={(record: any) => onEdit?.(record)}

‎apps/console/src/components/ViewConfigPanel.tsx‎

Lines changed: 163 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,154 @@ import type { FilterGroup, SortItem } from '@object-ui/components';
1818
import { X, Save, RotateCcw } from 'lucide-react';
1919
import { useObjectTranslation } from '@object-ui/i18n';
2020

21+
// ---------------------------------------------------------------------------
22+
// Operator mapping: @objectstack/spec ↔ FilterBuilder
23+
// ---------------------------------------------------------------------------
24+
const SPEC_TO_BUILDER_OP: Record<string, string> = {
25+
'=': 'equals',
26+
'==': 'equals',
27+
'!=': 'notEquals',
28+
'<>': 'notEquals',
29+
'>': 'greaterThan',
30+
'<': 'lessThan',
31+
'>=': 'greaterOrEqual',
32+
'<=': 'lessOrEqual',
33+
'contains': 'contains',
34+
'not_contains': 'notContains',
35+
'is_empty': 'isEmpty',
36+
'is_not_empty': 'isNotEmpty',
37+
'in': 'in',
38+
'not_in': 'notIn',
39+
'not in': 'notIn',
40+
'before': 'before',
41+
'after': 'after',
42+
'between': 'between',
43+
// Pass-through for already-normalized IDs
44+
'equals': 'equals',
45+
'notEquals': 'notEquals',
46+
'greaterThan': 'greaterThan',
47+
'lessThan': 'lessThan',
48+
'greaterOrEqual': 'greaterOrEqual',
49+
'lessOrEqual': 'lessOrEqual',
50+
'notContains': 'notContains',
51+
'isEmpty': 'isEmpty',
52+
'isNotEmpty': 'isNotEmpty',
53+
'notIn': 'notIn',
54+
};
55+
56+
const BUILDER_TO_SPEC_OP: Record<string, string> = {
57+
'equals': '=',
58+
'notEquals': '!=',
59+
'greaterThan': '>',
60+
'lessThan': '<',
61+
'greaterOrEqual': '>=',
62+
'lessOrEqual': '<=',
63+
'contains': 'contains',
64+
'notContains': 'not_contains',
65+
'isEmpty': 'is_empty',
66+
'isNotEmpty': 'is_not_empty',
67+
'in': 'in',
68+
'notIn': 'not in',
69+
'before': 'before',
70+
'after': 'after',
71+
'between': 'between',
72+
};
73+
74+
// ---------------------------------------------------------------------------
75+
// Field type normalization: ObjectUI → FilterBuilder
76+
// ---------------------------------------------------------------------------
77+
function normalizeFieldType(rawType?: string): 'text' | 'number' | 'boolean' | 'date' | 'select' {
78+
const t = (rawType || '').toLowerCase();
79+
if (['integer', 'int', 'float', 'double', 'number', 'currency', 'money', 'percent', 'rating'].includes(t)) return 'number';
80+
if (['date', 'datetime', 'datetime_tz', 'timestamp'].includes(t)) return 'date';
81+
if (['boolean', 'bool', 'checkbox', 'switch'].includes(t)) return 'boolean';
82+
if (['select', 'picklist', 'single_select', 'multi_select', 'enum'].includes(t)) return 'select';
83+
return 'text';
84+
}
85+
86+
// ---------------------------------------------------------------------------
87+
// Spec-style filter bridge: parse any supported format → FilterGroup conditions
88+
// Formats: ['field','=',val], [['f','=',v],['f2','!=',v2]], ['and'|'or', ...]
89+
// Also supports object-style: { field, operator, value }
90+
// ---------------------------------------------------------------------------
91+
function parseSpecFilter(raw: any): { logic: 'and' | 'or'; conditions: Array<{ id: string; field: string; operator: string; value: any }> } {
92+
if (!Array.isArray(raw) || raw.length === 0) {
93+
return { logic: 'and', conditions: [] };
94+
}
95+
96+
// Detect ['and', ...conditions] or ['or', ...conditions]
97+
if (typeof raw[0] === 'string' && (raw[0] === 'and' || raw[0] === 'or')) {
98+
const logic = raw[0] as 'and' | 'or';
99+
const rest = raw.slice(1);
100+
const conditions = rest.flatMap((item: any) => parseSingleOrNested(item));
101+
return { logic, conditions };
102+
}
103+
104+
// Detect single triplet: ['field', '=', value] (all primitives at top level)
105+
if (raw.length >= 2 && raw.length <= 3 && typeof raw[0] === 'string' && typeof raw[1] === 'string' && !Array.isArray(raw[0])) {
106+
// Check it's not an array of arrays
107+
if (!Array.isArray(raw[2])) {
108+
const cond = parseTriplet(raw);
109+
return { logic: 'and', conditions: cond ? [cond] : [] };
110+
}
111+
}
112+
113+
// Detect array of conditions: [[...], [...]] or [{...}, {...}]
114+
if (Array.isArray(raw[0]) || (typeof raw[0] === 'object' && raw[0] !== null && !Array.isArray(raw[0]))) {
115+
const conditions = raw.flatMap((item: any) => parseSingleOrNested(item));
116+
return { logic: 'and', conditions };
117+
}
118+
119+
// Fallback: try as single triplet
120+
const cond = parseTriplet(raw);
121+
return { logic: 'and', conditions: cond ? [cond] : [] };
122+
}
123+
124+
function parseTriplet(arr: any[]): { id: string; field: string; operator: string; value: any } | null {
125+
if (!Array.isArray(arr) || arr.length < 2) return null;
126+
const [field, op, value] = arr;
127+
if (typeof field !== 'string' || typeof op !== 'string') return null;
128+
return {
129+
id: crypto.randomUUID(),
130+
field,
131+
operator: SPEC_TO_BUILDER_OP[op] || op,
132+
value: value ?? '',
133+
};
134+
}
135+
136+
function parseSingleOrNested(item: any): Array<{ id: string; field: string; operator: string; value: any }> {
137+
if (Array.isArray(item)) {
138+
const triplet = parseTriplet(item);
139+
return triplet ? [triplet] : [];
140+
}
141+
if (typeof item === 'object' && item !== null && item.field) {
142+
return [{
143+
id: item.id || crypto.randomUUID(),
144+
field: item.field,
145+
operator: SPEC_TO_BUILDER_OP[item.operator] || item.operator || 'equals',
146+
value: item.value ?? '',
147+
}];
148+
}
149+
return [];
150+
}
151+
152+
/**
153+
* Convert FilterGroup conditions back to spec-style filter array.
154+
* Produces [['field','op',value], ...] for multiple conditions,
155+
* or ['field','op',value] for single condition,
156+
* or ['and'|'or', ...] when logic is 'or'.
157+
*/
158+
function toSpecFilter(logic: 'and' | 'or', conditions: Array<{ field: string; operator: string; value: any }>): any[] {
159+
const triplets = conditions
160+
.filter(c => c.field) // skip empty
161+
.map(c => [c.field, BUILDER_TO_SPEC_OP[c.operator] || c.operator, c.value]);
162+
163+
if (triplets.length === 0) return [];
164+
if (triplets.length === 1 && logic === 'and') return triplets[0];
165+
if (logic === 'or') return ['or', ...triplets];
166+
return triplets;
167+
}
168+
21169
/** View type labels for display */
22170
const VIEW_TYPE_LABELS: Record<string, string> = {
23171
grid: 'Grid',
@@ -162,20 +310,15 @@ export function ViewConfigPanel({ open, onClose, activeView, objectDef, onViewUp
162310
return Object.entries(objectDef.fields).map(([key, field]: [string, any]) => ({
163311
value: key,
164312
label: field.label || key,
165-
type: field.type || 'text',
313+
type: normalizeFieldType(field.type),
166314
options: field.options,
167315
}));
168316
}, [objectDef.fields]);
169317

170-
// Bridge: view filter array → FilterGroup
318+
// Bridge: view filter (any spec format) → FilterGroup
171319
const filterGroupValue = useMemo<FilterGroup>(() => {
172-
const conditions = (Array.isArray(draft.filter) ? draft.filter : []).map((f: any) => ({
173-
id: f.id || crypto.randomUUID(),
174-
field: f.field || '',
175-
operator: f.operator || 'equals',
176-
value: f.value ?? '',
177-
}));
178-
return { id: 'root', logic: 'and' as const, conditions };
320+
const parsed = parseSpecFilter(draft.filter);
321+
return { id: 'root', logic: parsed.logic, conditions: parsed.conditions };
179322
}, [draft.filter]);
180323

181324
// Bridge: view sort array → SortItem[]
@@ -187,15 +330,17 @@ export function ViewConfigPanel({ open, onClose, activeView, objectDef, onViewUp
187330
}));
188331
}, [draft.sort]);
189332

190-
/** Handle FilterBuilder changes → update draft.filter */
333+
/** Handle FilterBuilder changes → update draft.filter in spec format */
191334
const handleFilterChange = useCallback((group: FilterGroup) => {
192-
const filters = group.conditions.map(c => ({
193-
id: c.id,
194-
field: c.field,
195-
operator: c.operator,
196-
value: c.value,
197-
}));
198-
updateDraft('filter', filters);
335+
const specFilter = toSpecFilter(
336+
group.logic,
337+
group.conditions.map(c => ({
338+
field: c.field,
339+
operator: c.operator,
340+
value: c.value,
341+
}))
342+
);
343+
updateDraft('filter', specFilter);
199344
}, [updateDraft]);
200345

201346
/** Handle SortBuilder changes → update draft.sort */
@@ -283,7 +428,7 @@ export function ViewConfigPanel({ open, onClose, activeView, objectDef, onViewUp
283428
<Checkbox
284429
data-testid={`col-checkbox-${f.value}`}
285430
checked={checked}
286-
onCheckedChange={(c: boolean) => handleColumnToggle(f.value, c)}
431+
onCheckedChange={(c) => handleColumnToggle(f.value, c === true)}
287432
className="h-3.5 w-3.5"
288433
/>
289434
<span className="truncate">{f.label}</span>

0 commit comments

Comments
 (0)