-
Notifications
You must be signed in to change notification settings - Fork 2
Auto-inject $expand for lookup/master_detail fields in all data-fetching plugins #892
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
bcc0061
bb55339
7840c4a
6cd7971
f266b8c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| /** | ||
| * ObjectUI | ||
| * Copyright (c) 2024-present ObjectStack Inc. | ||
| * | ||
| * This source code is licensed under the MIT license found in the | ||
| * LICENSE file in the root directory of this source tree. | ||
| */ | ||
|
|
||
| import { describe, it, expect } from 'vitest'; | ||
| import { buildExpandFields } from '../expand-fields'; | ||
|
|
||
| describe('buildExpandFields', () => { | ||
| const sampleFields = { | ||
| name: { type: 'text', label: 'Name' }, | ||
| email: { type: 'email', label: 'Email' }, | ||
| account: { type: 'lookup', label: 'Account', reference_to: 'accounts' }, | ||
| parent: { type: 'master_detail', label: 'Parent', reference_to: 'contacts' }, | ||
| status: { type: 'select', label: 'Status' }, | ||
| }; | ||
|
|
||
| it('should return lookup and master_detail field names', () => { | ||
| const result = buildExpandFields(sampleFields); | ||
| expect(result).toEqual(['account', 'parent']); | ||
| }); | ||
|
|
||
| it('should return empty array when no lookup/master_detail fields exist', () => { | ||
| const fields = { | ||
| name: { type: 'text' }, | ||
| age: { type: 'number' }, | ||
| }; | ||
| expect(buildExpandFields(fields)).toEqual([]); | ||
| }); | ||
|
|
||
| it('should return empty array for null/undefined schema', () => { | ||
| expect(buildExpandFields(null)).toEqual([]); | ||
| expect(buildExpandFields(undefined)).toEqual([]); | ||
| }); | ||
|
|
||
| it('should return empty array for empty fields object', () => { | ||
| expect(buildExpandFields({})).toEqual([]); | ||
| }); | ||
|
|
||
| it('should filter by string columns when provided', () => { | ||
| const result = buildExpandFields(sampleFields, ['name', 'account']); | ||
| expect(result).toEqual(['account']); | ||
| }); | ||
|
|
||
| it('should filter by ListColumn objects with field property', () => { | ||
| const columns = [ | ||
| { field: 'name', label: 'Name' }, | ||
| { field: 'parent', label: 'Parent Contact' }, | ||
| ]; | ||
| const result = buildExpandFields(sampleFields, columns); | ||
| expect(result).toEqual(['parent']); | ||
| }); | ||
|
|
||
| it('should support columns with name property', () => { | ||
| const columns = [ | ||
| { name: 'account', label: 'Account' }, | ||
| ]; | ||
| const result = buildExpandFields(sampleFields, columns); | ||
| expect(result).toEqual(['account']); | ||
| }); | ||
|
|
||
| it('should support columns with fieldName property', () => { | ||
| const columns = [ | ||
| { fieldName: 'parent', label: 'Parent' }, | ||
| ]; | ||
| const result = buildExpandFields(sampleFields, columns); | ||
| expect(result).toEqual(['parent']); | ||
| }); | ||
|
|
||
| it('should return empty array when columns have no lookup fields', () => { | ||
| const result = buildExpandFields(sampleFields, ['name', 'email']); | ||
| expect(result).toEqual([]); | ||
| }); | ||
|
|
||
| it('should handle mixed string and object columns', () => { | ||
| const columns = [ | ||
| 'name', | ||
| { field: 'account' }, | ||
| 'parent', | ||
| ]; | ||
| const result = buildExpandFields(sampleFields, columns); | ||
| expect(result).toEqual(['account', 'parent']); | ||
| }); | ||
|
|
||
| it('should return all lookup fields when columns is empty array', () => { | ||
| // Empty columns array does not satisfy the length > 0 check, | ||
| // so no column restriction is applied → all lookup fields returned | ||
| const result = buildExpandFields(sampleFields, []); | ||
| expect(result).toEqual(['account', 'parent']); | ||
| }); | ||
|
|
||
| it('should handle malformed field definitions gracefully', () => { | ||
| const fields = { | ||
| name: null, | ||
| account: { type: 'lookup' }, | ||
| broken: 'not-an-object', | ||
| empty: {}, | ||
| }; | ||
| const result = buildExpandFields(fields as any); | ||
| expect(result).toEqual(['account']); | ||
| }); | ||
|
|
||
| it('should handle only lookup fields', () => { | ||
| const fields = { | ||
| ref1: { type: 'lookup', reference_to: 'obj1' }, | ||
| ref2: { type: 'lookup', reference_to: 'obj2' }, | ||
| }; | ||
| expect(buildExpandFields(fields)).toEqual(['ref1', 'ref2']); | ||
| }); | ||
|
|
||
| it('should handle only master_detail fields', () => { | ||
| const fields = { | ||
| detail1: { type: 'master_detail', reference_to: 'obj1' }, | ||
| }; | ||
| expect(buildExpandFields(fields)).toEqual(['detail1']); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| /** | ||
| * ObjectUI — expand-fields utility | ||
| * Copyright (c) 2024-present ObjectStack Inc. | ||
| * | ||
| * This source code is licensed under the MIT license found in the | ||
| * LICENSE file in the root directory of this source tree. | ||
| */ | ||
|
|
||
| /** | ||
| * Build an array of field names that should be included in `$expand` | ||
| * when fetching data. This scans the given object schema fields | ||
| * (and optional column configuration) for `lookup` and `master_detail` | ||
| * field types, so the backend (e.g. objectql) returns expanded objects | ||
| * instead of raw foreign-key IDs. | ||
| * | ||
| * @param schemaFields - Object map of field metadata from `getObjectSchema()`, | ||
| * e.g. `{ account: { type: 'lookup', reference_to: 'accounts' }, ... }`. | ||
| * @param columns - Optional explicit column list. When provided, only | ||
| * lookup/master_detail fields that appear in `columns` are expanded. | ||
| * Accepts `string[]` or `ListColumn[]` (objects with a `field` property). | ||
| * @returns Array of field names to pass as `$expand`. | ||
| * | ||
| * @example | ||
| * ```ts | ||
| * const fields = { | ||
| * name: { type: 'text' }, | ||
| * account: { type: 'lookup', reference_to: 'accounts' }, | ||
| * parent: { type: 'master_detail', reference_to: 'contacts' }, | ||
| * }; | ||
| * buildExpandFields(fields); | ||
| * // → ['account', 'parent'] | ||
| * | ||
| * buildExpandFields(fields, ['name', 'account']); | ||
| * // → ['account'] | ||
| * ``` | ||
| */ | ||
| export function buildExpandFields( | ||
| schemaFields?: Record<string, any> | null, | ||
| columns?: (string | { field?: string; name?: string; fieldName?: string })[], | ||
| ): string[] { | ||
| if (!schemaFields || typeof schemaFields !== 'object') { | ||
| return []; | ||
| } | ||
|
|
||
| // Collect all lookup / master_detail field names from the schema | ||
| const lookupFieldNames: string[] = []; | ||
| for (const [fieldName, fieldDef] of Object.entries(schemaFields)) { | ||
| if ( | ||
| fieldDef && | ||
| typeof fieldDef === 'object' && | ||
| (fieldDef.type === 'lookup' || fieldDef.type === 'master_detail') | ||
| ) { | ||
| lookupFieldNames.push(fieldName); | ||
| } | ||
| } | ||
|
|
||
| if (lookupFieldNames.length === 0) { | ||
| return []; | ||
| } | ||
|
|
||
| // When columns are provided, restrict expansion to visible columns only | ||
| if (columns && Array.isArray(columns) && columns.length > 0) { | ||
| const columnFieldNames = new Set<string>(); | ||
| for (const col of columns) { | ||
| if (typeof col === 'string') { | ||
| columnFieldNames.add(col); | ||
| } else if (col && typeof col === 'object') { | ||
| const name = col.field ?? col.name ?? col.fieldName; | ||
| if (name) columnFieldNames.add(name); | ||
| } | ||
| } | ||
| return lookupFieldNames.filter((f) => columnFieldNames.has(f)); | ||
| } | ||
|
|
||
| return lookupFieldNames; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,7 +28,7 @@ import { CalendarView, type CalendarEvent } from './CalendarView'; | |
| import { usePullToRefresh } from '@object-ui/mobile'; | ||
| import { useNavigationOverlay } from '@object-ui/react'; | ||
| import { NavigationOverlay } from '@object-ui/components'; | ||
| import { extractRecords } from '@object-ui/core'; | ||
| import { extractRecords, buildExpandFields } from '@object-ui/core'; | ||
|
|
||
| export interface CalendarSchema { | ||
| type: 'calendar'; | ||
|
|
@@ -215,9 +215,12 @@ export const ObjectCalendar: React.FC<ObjectCalendarProps> = ({ | |
|
|
||
| if (dataConfig?.provider === 'object') { | ||
| const objectName = dataConfig.object; | ||
| // Auto-inject $expand for lookup/master_detail fields | ||
| const expand = buildExpandFields(objectSchema?.fields); | ||
| const result = await dataSource.find(objectName, { | ||
| $filter: schema.filter, | ||
| $orderby: convertSortToQueryParams(schema.sort), | ||
| ...(expand.length > 0 ? { $expand: expand } : {}), | ||
|
Comment on lines
+218
to
+223
|
||
| }); | ||
|
|
||
| let items: any[] = extractRecords(result); | ||
|
|
@@ -242,7 +245,7 @@ export const ObjectCalendar: React.FC<ObjectCalendarProps> = ({ | |
|
|
||
| fetchData(); | ||
| return () => { isMounted = false; }; | ||
| }, [dataConfig, dataSource, hasInlineData, schema.filter, schema.sort, refreshKey]); | ||
| }, [dataConfig, dataSource, hasInlineData, schema.filter, schema.sort, refreshKey, objectSchema]); | ||
|
|
||
| // Fetch object schema for field metadata | ||
| useEffect(() => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,7 +27,7 @@ import type { ObjectGridSchema, DataSource, ViewData, GanttConfig } from '@objec | |
| import { GanttConfigSchema } from '@objectstack/spec/ui'; | ||
| import { useNavigationOverlay } from '@object-ui/react'; | ||
| import { NavigationOverlay } from '@object-ui/components'; | ||
| import { extractRecords } from '@object-ui/core'; | ||
| import { extractRecords, buildExpandFields } from '@object-ui/core'; | ||
| import { GanttView, type GanttTask } from './GanttView'; | ||
|
|
||
| export interface ObjectGanttProps { | ||
|
|
@@ -174,9 +174,12 @@ export const ObjectGantt: React.FC<ObjectGanttProps> = ({ | |
|
|
||
| if (dataConfig?.provider === 'object') { | ||
| const objectName = dataConfig.object; | ||
| // Auto-inject $expand for lookup/master_detail fields | ||
| const expand = buildExpandFields(objectSchema?.fields); | ||
| const result = await dataSource.find(objectName, { | ||
| $filter: schema.filter, | ||
| $orderby: convertSortToQueryParams(schema.sort), | ||
| ...(expand.length > 0 ? { $expand: expand } : {}), | ||
| }); | ||
|
Comment on lines
+177
to
183
|
||
| let items: any[] = extractRecords(result); | ||
| setData(items); | ||
|
|
@@ -193,7 +196,7 @@ export const ObjectGantt: React.FC<ObjectGanttProps> = ({ | |
| }; | ||
|
|
||
| fetchData(); | ||
| }, [dataConfig, dataSource, hasInlineData, schema.filter, schema.sort]); | ||
| }, [dataConfig, dataSource, hasInlineData, schema.filter, schema.sort, objectSchema]); | ||
|
|
||
| // Fetch object schema for field metadata | ||
| useEffect(() => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,7 +30,7 @@ import { | |
| Popover, PopoverContent, PopoverTrigger, | ||
| } from '@object-ui/components'; | ||
| import { usePullToRefresh } from '@object-ui/mobile'; | ||
| import { evaluatePlainCondition } from '@object-ui/core'; | ||
| import { evaluatePlainCondition, buildExpandFields } from '@object-ui/core'; | ||
| import { ChevronRight, ChevronDown, Download, Rows2, Rows3, Rows4, AlignJustify, Type, Hash, Calendar, CheckSquare, User, Tag, Clock } from 'lucide-react'; | ||
| import { useRowColor } from './useRowColor'; | ||
| import { useGroupedData } from './useGroupedData'; | ||
|
|
@@ -308,6 +308,12 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({ | |
| params.$orderby = `${(schema.defaultSort as any).field} ${(schema.defaultSort as any).order}`; | ||
| } | ||
|
|
||
| // Auto-inject $expand for lookup/master_detail fields | ||
| const expand = buildExpandFields(resolvedSchema?.fields, schemaColumns ?? schemaFields); | ||
| if (expand.length > 0) { | ||
| params.$expand = expand; | ||
| } | ||
|
|
||
|
Comment on lines
+311
to
+316
|
||
| const result = await dataSource.find(objectName, params); | ||
| if (cancelled) return; | ||
| setData(result.data || []); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,7 @@ import React, { useEffect, useState, useMemo } from 'react'; | |
| import type { DataSource } from '@object-ui/types'; | ||
| import { useDataScope, useNavigationOverlay } from '@object-ui/react'; | ||
| import { NavigationOverlay } from '@object-ui/components'; | ||
| import { extractRecords } from '@object-ui/core'; | ||
| import { extractRecords, buildExpandFields } from '@object-ui/core'; | ||
| import { KanbanRenderer } from './index'; | ||
| import { KanbanSchema } from './types'; | ||
|
|
||
|
|
@@ -61,9 +61,12 @@ export const ObjectKanban: React.FC<ObjectKanbanProps> = ({ | |
| if (!dataSource || typeof dataSource.find !== 'function' || !schema.objectName) return; | ||
| if (isMounted) setLoading(true); | ||
| try { | ||
| // Auto-inject $expand for lookup/master_detail fields | ||
| const expand = buildExpandFields(objectDef?.fields); | ||
| const results = await dataSource.find(schema.objectName, { | ||
| options: { $top: 100 }, | ||
| $filter: schema.filter | ||
| $filter: schema.filter, | ||
| ...(expand.length > 0 ? { $expand: expand } : {}), | ||
| }); | ||
|
Comment on lines
+64
to
70
|
||
|
|
||
| // Handle { value: [] } OData shape or { data: [] } shape or direct array | ||
|
|
@@ -88,7 +91,7 @@ export const ObjectKanban: React.FC<ObjectKanbanProps> = ({ | |
| fetchData(); | ||
| } | ||
| return () => { isMounted = false; }; | ||
| }, [schema.objectName, dataSource, boundData, schema.data, schema.filter, (props as any).data]); | ||
| }, [schema.objectName, dataSource, boundData, schema.data, schema.filter, (props as any).data, objectDef]); | ||
|
|
||
| // Determine which data to use: props.data -> bound -> inline -> fetched | ||
| const rawData = (props as any).data || boundData || schema.data || fetchedData; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,7 +19,7 @@ import { SchemaRenderer, useNavigationOverlay } from '@object-ui/react'; | |
| import { useDensityMode } from '@object-ui/react'; | ||
| import type { ListViewSchema } from '@object-ui/types'; | ||
| import { usePullToRefresh } from '@object-ui/mobile'; | ||
| import { evaluatePlainCondition, normalizeQuickFilter, normalizeQuickFilters } from '@object-ui/core'; | ||
| import { evaluatePlainCondition, normalizeQuickFilter, normalizeQuickFilters, buildExpandFields } from '@object-ui/core'; | ||
| import { useObjectTranslation } from '@object-ui/i18n'; | ||
|
|
||
| export interface ListViewProps { | ||
|
|
@@ -495,6 +495,12 @@ export const ListView: React.FC<ListViewProps> = ({ | |
| return () => { isMounted = false; }; | ||
| }, [schema.objectName, dataSource]); | ||
|
|
||
| // Auto-compute $expand fields from objectDef (lookup / master_detail) | ||
| const expandFields = React.useMemo( | ||
| () => buildExpandFields(objectDef?.fields, schema.fields), | ||
| [objectDef?.fields, schema.fields], | ||
| ); | ||
|
Comment on lines
+498
to
+502
|
||
|
|
||
| // Fetch data effect — supports schema.data (ViewDataSchema) provider modes | ||
| React.useEffect(() => { | ||
| let isMounted = true; | ||
|
|
@@ -567,6 +573,7 @@ export const ListView: React.FC<ListViewProps> = ({ | |
| $filter: finalFilter, | ||
| $orderby: sort, | ||
| $top: effectivePageSize, | ||
| ...(expandFields.length > 0 ? { $expand: expandFields } : {}), | ||
| ...(searchTerm ? { | ||
| $search: searchTerm, | ||
| ...(schema.searchableFields && schema.searchableFields.length > 0 | ||
|
|
@@ -606,7 +613,7 @@ export const ListView: React.FC<ListViewProps> = ({ | |
| fetchData(); | ||
|
|
||
| return () => { isMounted = false; }; | ||
| }, [schema.objectName, schema.data, dataSource, schema.filters, effectivePageSize, currentSort, currentFilters, activeQuickFilters, normalizedQuickFilters, userFilterConditions, refreshKey, searchTerm, schema.searchableFields]); // Re-fetch on filter/sort/search change | ||
| }, [schema.objectName, schema.data, dataSource, schema.filters, effectivePageSize, currentSort, currentFilters, activeQuickFilters, normalizedQuickFilters, userFilterConditions, refreshKey, searchTerm, schema.searchableFields, expandFields]); // Re-fetch on filter/sort/search change | ||
|
|
||
| // Available view types based on schema configuration | ||
| const availableViews = React.useMemo(() => { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
columnsare provided, the filtering only matches exact field names. If a visible column uses a nested path likeaccount.owner, the schema will containaccount(lookup) but the column set containsaccount.owner, so the lookup field is incorrectly excluded and$expandbecomes empty. Consider treating dot-path columns as implying their root field (and/or matching lookup fields when any column starts with${fieldName}.) and add a unit test for this case.