Repository navigation
Data Insights: fetch total_data_assets once on the Data Assets tab (backport 2.0 #36452) - #36651
Conversation
…36452) * ui: fetch total_data_assets once on the Data Insights Data Assets tab Each chart card fetched the total_data_assets series on its own, so the tab sent the same request five times per load and per filter change, and each card waited on it before its own chart. The tab now fetches it once and passes it to the cards; each card fetches its own chart and absolute values in parallel. * ui: keep percentage cards loading until the shared totals arrive A percentage card computed its change from an empty totals series while the shared total_data_assets request was in flight, or after it failed, and showed 0%. It now waits for the totals and shows the empty state if they failed. * ci: map the Data Insight components to DataInsight.spec.ts * ui: assert the empty and loading states in the chart card tests * ui: list the chart card fetch effect's dependencies in full * ui: fetch the shared totals in an async function with try/catch * ui: mark the fire-and-forget fetches in the Data Insight effects as void * ui: drive the DataAssetsTab test's provider mock without importing the page * ui: move DataInsightProvider out of pages so the Data Insight components stop importing a page openmetadata-collate still imports it from pages/DataInsightPage/DataInsightProvider, so that path stays as a re-export. * ui: sort the moved Data Insight provider's imports (cherry picked from commit 7d46ff4)
ShaileshParmar11
added
UI
Code Review ✅ Approved🟡 Medium risk · Fetching shared total-assets data once changes filter-driven chart loading; a failed request could leave all five Data Insights cards empty. Backport of the Data Insights fix that fetches VerificationRules No rules evaluated Functional validation Not enabledAuto-approval Not enabledFor more verification details, comment OptionsDisplay: compact → Showing compact verification results. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
Jest test CoverageUI tests summary
|
|
GitHub Actions
Failure
Describe your changes:
Backport of #36452 to
2.0: cherry-pick of7d46ff41928(git cherry-pick -x). No linked issue; this fixes a performance problem Sentry reported in production (COLLATE-FRONTEND-2A, "N+1 API Call" on/data-insights/data-assets).On the Data Insights → Data Assets tab, every chart card fetched the
total_data_assetsseries on its own with identical parameters: 5 times per load and 5 more on every filter change, each before the card's own request.DataAssetsTabnow fetches it once and passes it to the cards; the percentage cards fetch their own chart and absolute-values chart in parallel.DataInsightProvidermoves frompages/DataInsightPage/tocomponents/DataInsight/DataInsightProvider/, with a re-export left at the old path foropenmetadata-collate. Full description and test notes are on #36452.Conflicts resolved. On
2.0the Data Insight UI still uses antd (Row/Col, antdCardwithloading) and recharts;mainuses core components. The logic was ported onto the2.0files and the2.0rendering kept:DataInsightChartCard.tsx: the2.0card, with the same changes as onmain—totalAssetsprop,getCardDataState, parallelfetchDatainuseCallback, the fullchangeInValuememo dependencies, and thevoid fetchData()effect. While the shared totals load, the antdCardshows its loading skeleton, as it did for the card's own request.DataAssetsTab.component.tsx: the2.0antd layout, plus the singletotal_data_assetsfetch with the stale-response guard, passed to the same 5 cards.AppAnalyticsTab.component.tsx,DataInsightHeader.component.tsx: kept the2.0imports; only the provider import path changes.DataInsightChartCard.test.tsxdoes not exist on2.0, and themainversion asserts on the coreLineChart. The 4 tests for this fix are rewritten against the2.0card (graphdata-testidinstead ofLineChartprops); themain-only chart rendering tests are not carried over.DataInsightProvider.tsxandDataInsight.interface.tsare identical on2.0andmainapart from import paths.Type of change:
Tests:
Unit tests
DataAssetsTab.test.tsxcherry-picked unchanged (3 tests);DataInsightChartCard.test.tsxrewritten for2.0(4 tests).2.0dependencies (yarn install --frozen-lockfile):yarn test src/components/DataInsight src/pages/DataInsightPage: 12 suites, 29 tests passed.2.0DataInsightChartCard.tsxandDataAssetsTab.component.tsxwithout the fix, 6 of the 7 new tests fail (the remaining one, no fetch while KPIs load, held before too).Backend integration tests
Playwright (UI) tests
2.0.DataInsight.spec.tspassed 14/14 for Data Insights: fetch total_data_assets once on the Data Assets tab #36452 onmain.Manual testing performed
eslint --fix,prettier --write) on the changed files leaves no diff; ESLint reports 0 errors.tsc --noEmit: no errors in the changed files. The errors it reports are in existing files this PR does not touch (e.g.TasksUtils.test.tsx).2.0.UI screen recording:
Not applicable — backport of a performance bug fix with no visual change.
Checklist:
🤖 Generated with Claude Code