Fix dashboard to not display hardcoded data on server errors#1152
Fix dashboard to not display hardcoded data on server errors#1152
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Agent-Logs-Url: https://github.com/objectstack-ai/objectui/sessions/27c667be-572a-4498-9b2d-2e9a96ab381a Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
|
@copilot 继续 |
- Fix DashboardRenderer.widgetData tests to verify rendered output instead of relying on getRenderedSchemas (object-metric is registered, so it renders directly rather than falling through to <pre> JSON dump) - Remove debug_metric.test.tsx that was accidentally committed - Update CHANGELOG.md documenting all dashboard error state improvements Agent-Logs-Url: https://github.com/objectstack-ai/objectui/sessions/2c052023-f69e-4edb-a3a1-c4f4394c4b70 Co-authored-by: xuyushun441-sys <255036401+xuyushun441-sys@users.noreply.github.com>
All checklist items are now complete in commit Changes made:
All 209 tests pass across 15 test files. |
|
@copilot 13:10:38.160 �[31merror during build: |
The ObjectChart error state UI uses AlertCircle from lucide-react, but the package was not listed as a dependency. This caused the Vercel build to fail with: "Rolldown failed to resolve import lucide-react". Agent-Logs-Url: https://github.com/objectstack-ai/objectui/sessions/049d7802-348f-46aa-8587-3d6e7f67f8ae Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Fixed in |
There was a problem hiding this comment.
Pull request overview
This PR ensures dashboard widgets stop masking server/API failures by introducing consistent three-state rendering (Loading → Error → Data/Fallback) for data-bound charts and metrics, so dashboards no longer display misleading hardcoded/static values when live queries fail.
Changes:
- Added async, data-bound
ObjectMetricWidgetand routed object-bound metric widgets to it fromDashboardRenderer. - Extended
MetricWidgetandMetricCardwithloading/errorstates and updatedObjectChartto surface fetch errors with explicit UI. - Added/updated tests and documentation notes; updated dependencies and changelog.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Locks lucide-react addition for workspace dependency resolution. |
| packages/plugin-dashboard/src/ObjectMetricWidget.tsx | New data-bound metric widget with loading/error/fallback behavior. |
| packages/plugin-dashboard/src/MetricWidget.tsx | Adds loading + error rendering paths (spinner / alert) for metrics. |
| packages/plugin-dashboard/src/MetricCard.tsx | Adds loading + error rendering paths for metric cards. |
| packages/plugin-dashboard/src/index.tsx | Exports and registers object-metric in the component registry. |
| packages/plugin-dashboard/src/DashboardRenderer.tsx | Routes object-bound metric widgets to object-metric and passes fallback props. |
| packages/plugin-dashboard/src/tests/ObjectMetricWidget.test.tsx | New unit tests for ObjectMetricWidget loading/error/fallback/aggregate behavior. |
| packages/plugin-dashboard/src/tests/MetricCard.test.tsx | Adds tests for MetricCard loading/error props behavior. |
| packages/plugin-dashboard/src/tests/DashboardRenderer.widgetData.test.tsx | Adds tests around metric routing behavior for object-bound vs static metrics. |
| packages/plugin-charts/src/ObjectChart.tsx | Adds chart fetch error state + test ids; stops swallowing fetch errors. |
| packages/plugin-charts/package.json | Adds lucide-react dependency for chart error icon usage. |
| examples/crm/src/dashboards/crm.dashboard.ts | Documents that metric options.value is fallback/demo-only when no dataSource. |
| CHANGELOG.md | Documents dashboard error-state improvements and new widget behavior. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| groupBy: widget.categoryField || widgetData.aggregate.groupBy, | ||
| } | ||
| : widget.aggregate ? { | ||
| field: widget.valueField || 'value', | ||
| function: widget.aggregate, | ||
| groupBy: widget.categoryField || 'name', | ||
| } : undefined; |
There was a problem hiding this comment.
For object-bound metric widgets using the widget.aggregate shorthand, groupBy defaults to 'name'. That will typically return multiple groups, but ObjectMetricWidget displays only the first row for non-count aggregates, yielding incorrect metrics. Consider defaulting metric aggregations to a single group (e.g., '_all') unless the user explicitly provides categoryField/groupBy.
| groupBy: widget.categoryField || widgetData.aggregate.groupBy, | |
| } | |
| : widget.aggregate ? { | |
| field: widget.valueField || 'value', | |
| function: widget.aggregate, | |
| groupBy: widget.categoryField || 'name', | |
| } : undefined; | |
| // Prefer explicit categoryField or aggregate.groupBy; otherwise, default to a single bucket. | |
| groupBy: widget.categoryField ?? widgetData.aggregate.groupBy ?? '_all', | |
| } | |
| : widget.aggregate | |
| ? { | |
| field: widget.valueField || 'value', | |
| function: widget.aggregate, | |
| // Default to a single group unless the user explicitly configures a categoryField. | |
| groupBy: widget.categoryField || '_all', | |
| } | |
| : undefined; |
| const displayValue = fetchedValue !== null | ||
| ? fetchedValue | ||
| : (!dataSource ? (fallbackValue ?? '—') : '—'); |
There was a problem hiding this comment.
If dataSource becomes unavailable (or changes) after a successful fetch, fetchedValue stays non-null and displayValue will keep showing the stale server value instead of falling back to fallbackValue/placeholder. Consider resetting fetchedValue (and possibly error) when dataSource is falsy or changes.
| it('should route metric widgets with object binding to object-metric (renders fallback without dataSource)', () => { | ||
| const schema = { | ||
| type: 'dashboard' as const, | ||
| name: 'test', | ||
| title: 'Test', |
There was a problem hiding this comment.
This test only asserts label/value rendering without a dataSource, which would also pass with the legacy metric rendering (pre-change), so it doesn’t verify routing to object-metric or the new error behavior. Consider adding an assertion using a failing dataSource (via SchemaRendererProvider) and checking for the error state (e.g., data-testid="metric-error") and absence of the fallback value.
|
@copilot fix all |
1. Default metric aggregation groupBy to '_all' instead of 'name' for single-bucket results in DashboardRenderer. 2. Reset fetchedValue/error in ObjectMetricWidget when dataSource becomes unavailable, preventing stale server data from masking fallback values. 3. Add test with failing dataSource to verify error state routing in DashboardRenderer object-metric path. Agent-Logs-Url: https://github.com/objectstack-ai/objectui/sessions/7a05b366-541b-4772-8f1f-91bcb9a25ff7 Co-authored-by: xuyushun441-sys <255036401+xuyushun441-sys@users.noreply.github.com>
All 3 review comments addressed in
All 210 tests pass, CodeQL clean. |
When the analytics API (
/api/v1/analytics/query) returns errors (e.g. "Cube name is required"), dashboard widgets previously continued displaying hardcoded static values, masking real data failures. This PR implements proper three-state rendering (Loading → Error → Data/Fallback) across all dashboard widget types.Changes Made
ObjectChart (
@object-ui/plugin-charts)errorstate tracking viauseState. WhendataSource.aggregate()ordataSource.find()fails, the chart now renders a prominent error alert with a redAlertCircleicon and the error message, instead of silently swallowing errors.data-testidattributes for loading, error, and no-datasource states.lucide-reactas a dependency for theAlertCircleicon.MetricWidget / MetricCard (
@object-ui/plugin-dashboard)loadinganderrorprops to both components. Whenloadingis true, a spinner is shown. Whenerroris set, a destructive-colored error message is displayed instead of the metric value. The label/title remains visible in all states.ObjectMetricWidget (new component,
@object-ui/plugin-dashboard)dataSource.aggregate()ordataSource.find().fallbackValueonly when nodataSourceis available (demo/storybook mode).fetchedValueanderrorstate whendataSourcebecomes unavailable or changes, preventing stale server data from masking fallback values.object-metricin the ComponentRegistry.DashboardRenderer (
@object-ui/plugin-dashboard)widget.objectbinding are now routed toObjectMetricWidget(object-metrictype) for async data loading with proper error/loading states, instead of always rendering static hardcoded values.objectbinding) continue to work as before.groupBydefaults to_all(single bucket) instead ofname, so shorthand aggregate configs return correct single-value results for metric widgets.CRM Dashboard Example
options.valuefields in metric widgets are demo/fallback values that only display when nodataSourceis connected.CHANGELOG
CHANGELOG.mdwith all dashboard error state improvements.Testing
@object-ui/plugin-dashboard