Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…adapt, and config alignment - ObjectChart: extract records from results.data and results.value formats - DashboardRenderer: auto-adapt series dataKey from aggregate.field - CRM dashboard: align yField to aggregate field 'amount' (was 'total') - Add extractRecords utility with 8 tests - Add 2 DashboardRenderer tests for aggregate auto-adaptation - Update ROADMAP.md Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
|
@copilot 仔细评估是否已经完成了任务中要求的所有开发,解决的所有相关的类似的问题 |
…action across all 6 components - Create shared extractRecords() utility in packages/core/src/utils/extract-records.ts - ObjectMap: was missing results.value extraction - ObjectCalendar: was missing results.records extraction - ObjectGantt: was missing results.value extraction - ObjectTimeline: was missing results.value, had inconsistent pattern - ObjectKanban: was missing results.records extraction - ObjectChart: re-exports from @object-ui/core for backward compatibility - Add 8 unit tests for extractRecords in @object-ui/core - Update ROADMAP.md Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
经过全面审查,发现 issue 第 3 点提到的"在所有数据组件中统一结果提取"确实没有完成。审计后发现 5 个组件存在同类数据提取不全的 bug:
已在
|
There was a problem hiding this comment.
Pull request overview
Fixes the “Revenue by Account” CRM dashboard widget rendering empty by standardizing API response record extraction and ensuring aggregated chart series keys match the aggregation output shape.
Changes:
- Added a centralized
extractRecords()utility in@object-ui/coreand refactored multiple data-fetching plugins to use it. - Updated
DashboardRendererto derive chart seriesdataKeyfromaggregate.fieldwhen aggregation is enabled (fallback toyFieldotherwise), with new/updated tests. - Aligned the CRM dashboard config (
yField/valueField) to match the aggregate field used for “Revenue by Account”, and documented the fix inROADMAP.md.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/utils/extract-records.ts | Introduces shared record extraction for common response shapes (records/data/value/array). |
| packages/core/src/utils/tests/extract-records.test.ts | Adds unit tests for the new core extractRecords() behavior and precedence rules. |
| packages/core/src/index.ts | Exposes extractRecords() from the core package public API. |
| packages/plugin-charts/src/ObjectChart.tsx | Switches ObjectChart fetching to use core extractRecords() and re-exports it for backwards compatibility. |
| packages/plugin-charts/src/tests/ObjectChart.aggregation.test.ts | Adds tests validating extractRecords behavior via the ObjectChart re-export. |
| packages/plugin-dashboard/src/DashboardRenderer.tsx | Uses aggregate.field as effective dataKey for provider:'object' aggregated charts. |
| packages/plugin-dashboard/src/tests/DashboardRenderer.widgetData.test.tsx | Updates expectations and adds coverage for aggregate vs non-aggregate dataKey selection. |
| packages/plugin-map/src/ObjectMap.tsx | Replaces local response-shape handling with extractRecords(). |
| packages/plugin-calendar/src/ObjectCalendar.tsx | Replaces local response-shape handling with extractRecords(). |
| packages/plugin-gantt/src/ObjectGantt.tsx | Replaces local response-shape handling with extractRecords(). |
| packages/plugin-timeline/src/ObjectTimeline.tsx | Replaces local response-shape handling with extractRecords(). |
| packages/plugin-kanban/src/ObjectKanban.tsx | Replaces local response-shape handling with extractRecords(). |
| examples/crm/src/dashboards/crm.dashboard.ts | Aligns yField/valueField to amount to match aggregate.field. |
| ROADMAP.md | Marks the chart bugfix work as completed and summarizes the fix scope/tests. |
| export function extractRecords(results: unknown): any[] { | ||
| if (Array.isArray(results)) { | ||
| return results; | ||
| } | ||
| if (results && typeof results === 'object') { | ||
| if (Array.isArray((results as any).records)) { | ||
| return (results as any).records; | ||
| } | ||
| if (Array.isArray((results as any).data)) { | ||
| return (results as any).data; | ||
| } | ||
| if (Array.isArray((results as any).value)) { | ||
| return (results as any).value; | ||
| } | ||
| } |
There was a problem hiding this comment.
extractRecords is introduced in @object-ui/core but uses any[] and multiple (results as any) casts, which triggers the repo’s @typescript-eslint/no-explicit-any warnings and weakens type safety for all consumers. Consider making it generic (e.g., <T = unknown>(): T[]) and narrowing results to a typed shape like { records?: unknown; data?: unknown; value?: unknown } to avoid any while keeping the same runtime behavior.
| export function extractRecords(results: unknown): any[] { | |
| if (Array.isArray(results)) { | |
| return results; | |
| } | |
| if (results && typeof results === 'object') { | |
| if (Array.isArray((results as any).records)) { | |
| return (results as any).records; | |
| } | |
| if (Array.isArray((results as any).data)) { | |
| return (results as any).data; | |
| } | |
| if (Array.isArray((results as any).value)) { | |
| return (results as any).value; | |
| } | |
| } | |
| interface RecordsContainer { | |
| records?: unknown; | |
| data?: unknown; | |
| value?: unknown; | |
| } | |
| export function extractRecords<T = unknown>(results: unknown): T[] { | |
| if (Array.isArray(results)) { | |
| return results as T[]; | |
| } | |
| if (results && typeof results === 'object') { | |
| const container = results as RecordsContainer; | |
| if (Array.isArray(container.records)) { | |
| return container.records as T[]; | |
| } | |
| if (Array.isArray(container.data)) { | |
| return container.data as T[]; | |
| } | |
| if (Array.isArray(container.value)) { | |
| return container.value as T[]; | |
| } | |
| } |
"Revenue by Account" chart on the CRM Dashboard displays no data. Three independent bugs compound:
ObjectChartonly extracts fromresults.records(missing.data/.valueformats),DashboardRendererbuilds series withdataKey: yFieldwhich doesn't matchaggregateRecords()output keyed byaggregate.field, and the CRM config hasyField: 'total'while the aggregate field is'amount'. Additionally, the same incomplete data extraction bug exists in 5 other data-fetching components.Centralized
extractRecords()utility (@object-ui/core)Created a shared
extractRecords()inpackages/core/src/utils/extract-records.tsthat handles all API response shapes, exported from@object-ui/corefor use by all plugins:Unified data extraction across all 6 data components
Refactored all data-fetching components to use the centralized
extractRecords():.data,.value.value.records.value.value, had inconsistent pattern.recordsDashboardRenderer: auto-adapt series dataKey from aggregate config (
DashboardRenderer.tsx)aggregateis present, derivedataKeyfromaggregate.fieldinstead ofyField— this matchesaggregateRecords()output shape{ [groupBy]: key, [field]: result }yFieldwhen no aggregate is configuredCRM dashboard config alignment (
crm.dashboard.ts)yField/valueField:'total'→'amount'to matchaggregate.fieldTests
extractRecordsin@object-ui/core(all response formats, nulls, priority order)extractRecordsre-export from ObjectChart (backward compatibility)provider: 'object'test expectationOriginal prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.