fix(ui): table version page shows historical columns instead of live API columns#29174
fix(ui): table version page shows historical columns instead of live API columns#29174aniketkatkar97 wants to merge 1 commit into
Conversation
…PI columns Closes open-metadata#28690 The TableVersion component was fetching live/current columns via getTableColumnsByFQN on every version page load, then overlaying changeDescription diffs on top. This caused unchanged columns to show their latest live values instead of the historical values for the selected version. Fix: use currentVersionData.columns (the historical snapshot already present in the version entity response) as the base for column diff computation. Remove the live API fetch, server-side pagination, and server-side search callback — VersionTable already handles client-side search via searchInColumns internally. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
github-actions
Bot
added
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| "test-entity": "Testa {{entity}}", | ||
| "test-level-lowercase": "testnivå", | ||
| "test-library": "Testbibliotek", | ||
| "test-login": "Test Login", |
There was a problem hiding this comment.
💡 Quality: Unrelated SSO locale strings added to sv-se.json
The change to openmetadata-ui/src/main/resources/ui/src/locale/languages/sv-se.json adds keys that are unrelated to this PR's scope (table version columns): test-login, sso-configuration-test-failed-*, sso-new-config-save-warning, and sso-test-login-*. These appear to be incidentally included (likely from an i18n key-sync or rebase) and the values are left in English rather than translated to Swedish, which is inconsistent with the rest of this localization file. Consider removing these unrelated entries from this PR (or providing proper Swedish translations if they must stay) so the changeset stays focused on the version-page fix.
Was this helpful? React with 👍 / 👎
Code Review 👍 Approved with suggestions 0 resolved / 1 findingsUpdates TableVersion component to use historical column snapshots instead of live API data, resolving the issue of incorrect column descriptions in version history. Please remove the unrelated SSO locale string changes added to sv-se.json. 💡 Quality: Unrelated SSO locale strings added to sv-se.json📄 openmetadata-ui/src/main/resources/ui/src/locale/languages/sv-se.json:2291 📄 openmetadata-ui/src/main/resources/ui/src/locale/languages/sv-se.json:3466-3477 The change to 🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| `[data-row-key$="${col0Name}"] [data-testid="viewer-container"]` | ||
| ) | ||
| ).toContainText(col0OriginalDesc); | ||
|
|
There was a problem hiding this comment.
freshTable.create(apiContext) provisions a full service → database → schema → table hierarchy, but the cleanup only deletes the table entity itself. The service, database, and schema are never removed, causing orphaned resources to accumulate in the test environment across repeated CI runs. Consider using freshTable.delete(apiContext) (if that method handles the full chain) or manually deleting in reverse order, similar to the outer tableClass teardown pattern used elsewhere in the spec.
| "test-entity": "Testa {{entity}}", | ||
| "test-level-lowercase": "testnivå", | ||
| "test-library": "Testbibliotek", | ||
| "test-login": "Test Login", |
There was a problem hiding this comment.
The newly added SSO keys (sso-configuration-test-failed-description, sso-configuration-test-success, sso-test-login-*, test-login, etc.) are all in English inside sv-se.json. Every other SSO key nearby is in Swedish (e.g. "sso-provider-not-supported": "SSO-leverantören {{provider}} stöds inte."). These also appear unrelated to the table version fix described in this PR — they likely belong to a different branch or change set.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Pull request overview
This PR fixes the Table version view to render a true historical schema snapshot by using currentVersionData.columns (from the version entity response) instead of fetching and overlaying the table’s live/current columns.
Changes:
- Update
TableVersionto base schema rendering oncurrentVersionData.columnsand remove live column fetching + paging/search plumbing. - Add unit test coverage to ensure
VersionTablereceives historical column data, and add a Playwright regression test validating v0.1 shows historical column descriptions (fix #28690). - Add new locale keys in
sv-se.jsonfor SSO “test login”/validation messaging.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| openmetadata-ui/src/main/resources/ui/src/components/Database/TableVersion/TableVersion.component.tsx | Switch schema base data to currentVersionData.columns; remove live columns API pagination/search wiring. |
| openmetadata-ui/src/main/resources/ui/src/components/Database/TableVersion/TableVersion.test.tsx | Add tests asserting historical columns are passed into VersionTable. |
| openmetadata-ui/src/main/resources/ui/playwright/e2e/VersionPages/EntityVersionPages.spec.ts | Add Playwright step asserting historical column descriptions are shown for v0.1 (not live values). |
| openmetadata-ui/src/main/resources/ui/src/locale/languages/sv-se.json | Add Swedish-locale keys for SSO config validation/test-login messaging. |
| const historicalColumnNames = (mockTableData.columns ?? []).map( | ||
| (col) => col.name | ||
| ); | ||
|
|
||
| historicalColumnNames.forEach((name) => { | ||
| expect(receivedColumns.some((col) => col.name === name)).toBe(true); | ||
| }); |
| "test-entity": "Testa {{entity}}", | ||
| "test-level-lowercase": "testnivå", | ||
| "test-library": "Testbibliotek", | ||
| "test-login": "Test Login", |
| "sso-configuration-test-failed-description": "Validation failed. Please review your configuration and try again.", | ||
| "sso-configuration-test-failed-with-count": "Validation failed with {{count}} error(s). Review the highlighted fields below and try again.", | ||
| "sso-configuration-test-success": "Your SSO configuration is valid and reachable. You can save it when you're ready.", | ||
| "sso-new-config-save-warning": "Saving a new SSO configuration will sign you out and redirect you to the login page. Test your configuration first to avoid being locked out.", |
| "sso-test-login-description": "Sign in with your identity provider using these unsaved settings. This runs in a separate window and never changes your current session.", | ||
| "sso-test-login-error": "The sign-in could not be validated due to a server error. Please try again.", | ||
| "sso-test-login-failed": "Sign-in completed, but this configuration would reject the login.", | ||
| "sso-test-login-no-token": "No token was returned from the identity provider.", | ||
| "sso-test-login-popup-failed": "The test sign-in was cancelled or could not be completed. Please try again.", | ||
| "sso-test-login-success": "You signed in successfully as {{email}}. These settings are safe to save.", | ||
| "sso-test-login-waiting": "Complete the sign-in in the popup window to continue.", |
Jest test CoverageUI tests summary
|
|
🔴 Playwright Results — 3 failure(s), 11 flaky✅ 4297 passed · ❌ 3 failed · 🟡 11 flaky · ⏭️ 88 skipped
Genuine Failures (failed on all attempts)❌
|
Summary
Fixes #28690
The table version page was fetching live/current columns via
getTableColumnsByFQNand overlayingchangeDescriptiondiffs on top. This made the version view misleading in two ways:customer_emailas"Customer contact email"from v0.3, not the original"Primary email...")Root Cause
TableVersion.component.tsxcalledgetTableColumnsByFQN(tableFqn)on every version page load, returning the table's current columns regardless of which version was selected.Fix
Use
currentVersionData.columns— the historical column snapshot already present in the version entity API response — as the base forgetColumnsDataWithVersionChanges. Remove:getTableColumnsByFQN,searchTableColumnsByFQN)usePaging,fetchPaginatedColumns,handleColumnsPageChange,paginationProps)VersionTablealready has built-in client-side search viasearchInColumnsFiles Changed
TableVersion.component.tsxcurrentVersionData.columnsas column base; remove live API fetchTableVersion.test.tsxVersionTableEntityVersionPages.spec.tsTest plan
yarn test --testPathPatterns=TableVersion.testEntityVersionPages.spec.ts— new stepshould show historical column descriptions in version viewverifies v0.1 shows the pre-patch column description and NOT the post-patch live value🤖 Generated with Claude Code
Greptile Summary
This PR fixes a bug where the table version page displayed live/current column data instead of the historical snapshot by replacing the
getTableColumnsByFQNAPI call withcurrentVersionData.columnsalready present in the version entity response.TableVersion.component.tsx: Removes ~90 lines of server-side pagination/search state and the live API fetch;columnsuseMemo now derives directly fromcurrentVersionData.columns, which is the correct historical snapshot for every selected version.TableVersion.test.tsx: Adds two focused unit tests that verifyVersionTablereceives props fromcurrentVersionDatarather than from a mocked API response.EntityVersionPages.spec.ts: Adds an E2E regression step that patches a column description, navigates to v0.1, and asserts the pre-patch description is shown; thefreshTablecleanup deletes only the table entity and leaves the service/database/schema hierarchy behind.sv-se.json: Adds SSO-related translation keys in English rather than Swedish, which appear unrelated to the table version fix.Confidence Score: 4/5
The core fix is safe and well-scoped; the only concerns are minor test cleanup and unrelated English strings in the Swedish locale file.
The component change is correct —
currentVersionData.columnsis the right source for a version history page. The removed server-side pagination/search code was actively wrong for a historical view. Unit tests cover the new behavior. The two issues are both non-blocking: orphaned test resources from incomplete Playwright cleanup, and SSO translation keys in English inside sv-se.json that appear to belong to a different change.sv-se.json warrants a second look to confirm the English-language SSO entries are intentional; EntityVersionPages.spec.ts cleanup block should be reviewed for the service/schema teardown gap.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant User participant TableVersionPage participant VersionAPI as Version API (/versions/0.1) User->>TableVersionPage: Select version v0.1 TableVersionPage->>VersionAPI: "GET /tables/{id}/versions/0.1" VersionAPI-->>TableVersionPage: "{ columns: [...historical...], changeDescription: {...} }" Note over TableVersionPage: BEFORE (buggy)<br/>pruneEmptyChildren(liveApiColumns)<br/>+ changeDescription overlay Note over TableVersionPage: AFTER (fixed)<br/>pruneEmptyChildren(currentVersionData.columns)<br/>+ changeDescription overlay TableVersionPage->>TableVersionPage: getColumnsDataWithVersionChanges(changeDescription, historicalCols) TableVersionPage-->>User: Render VersionTable with correct historical columns%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant User participant TableVersionPage participant VersionAPI as Version API (/versions/0.1) User->>TableVersionPage: Select version v0.1 TableVersionPage->>VersionAPI: "GET /tables/{id}/versions/0.1" VersionAPI-->>TableVersionPage: "{ columns: [...historical...], changeDescription: {...} }" Note over TableVersionPage: BEFORE (buggy)<br/>pruneEmptyChildren(liveApiColumns)<br/>+ changeDescription overlay Note over TableVersionPage: AFTER (fixed)<br/>pruneEmptyChildren(currentVersionData.columns)<br/>+ changeDescription overlay TableVersionPage->>TableVersionPage: getColumnsDataWithVersionChanges(changeDescription, historicalCols) TableVersionPage-->>User: Render VersionTable with correct historical columnsReviews (1): Last reviewed commit: "fix(ui): table version page uses histori..." | Re-trigger Greptile