Fixes #31980: handle null persona customization pages - #31981
Conversation
github-actions
Bot
added
backend
safe to test
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR fixes a UI crash when persona UI customization documents migrated from 1.13.3 → 2.0.0 contain invalid legacy pages entries (notably null). It centralizes runtime validation/normalization at the shared React Query boundary and updates key consumers/writers to safely read and update persona pages without persisting new invalid entries.
Changes:
- Added shared persona-page utilities to (a) safely lookup pages and (b) normalize documents by filtering invalid page entries.
- Normalized persona documents in the shared DocStore React Query function so invalid
pagesentries never enter the cache. - Updated Data Marketplace, My Data, customization store, and hook consumers to use the defensive page lookup, plus added targeted unit/component tests for migrated legacy payloads and reset behavior.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| openmetadata-ui/src/main/resources/ui/src/utils/CustomizePage/PersonaPage.utils.ts | Introduces safe page lookup (getPersonaPage), normalization (normalizePersonaDocument), and safe page updates (updatePersonaDocumentPage). |
| openmetadata-ui/src/main/resources/ui/src/utils/CustomizePage/PersonaPage.utils.test.ts | Adds focused unit tests covering invalid legacy entries, immutability, and page add/replace/remove behavior. |
| openmetadata-ui/src/main/resources/ui/src/rest/queries/docStoreQuery.ts | Normalizes persona documents at the shared React Query fetch boundary to keep cache entries clean. |
| openmetadata-ui/src/main/resources/ui/src/pages/MyDataPage/MyDataPage.component.tsx | Uses defensive page lookup to avoid null dereferences for LandingPage customization reads. |
| openmetadata-ui/src/main/resources/ui/src/pages/MyDataPage/MyDataPage.test.tsx | Adds regression test ensuring My Data renders with a legacy null page entry present. |
| openmetadata-ui/src/main/resources/ui/src/pages/DataMarketplacePage/DataMarketplacePage.component.tsx | Uses defensive page lookup to avoid null dereferences for Data Marketplace customization reads. |
| openmetadata-ui/src/main/resources/ui/src/pages/DataMarketplacePage/DataMarketplacePage.component.test.tsx | Adds regression test ensuring default layout renders when legacy null page exists. |
| openmetadata-ui/src/main/resources/ui/src/pages/CustomizablePage/CustomizeStore.ts | Normalizes documents entering the store and uses shared helpers for reads/writes to avoid invalid pages. |
| openmetadata-ui/src/main/resources/ui/src/pages/CustomizablePage/CustomizablePage.tsx | Uses shared update + normalization helpers to prevent persisting undefined/null pages and to sync normalized responses into cache/store. |
| openmetadata-ui/src/main/resources/ui/src/pages/CustomizablePage/CustomizablePage.test.tsx | Adds regression test ensuring “reset unsaved layout” does not persist an undefined/null page. |
| openmetadata-ui/src/main/resources/ui/src/hooks/useCustomPages.ts | Switches to shared defensive page lookup for consistent null-safe reads. |
| openmetadata-ui/src/main/resources/ui/src/hooks/useCustomPages.test.ts | Adds regression validating that cached persona documents are normalized (null pages removed). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Jest test CoverageUI tests summary
|
Code Review ✅ ApprovedAdds defensive validation and normalization for persona customization pages to gracefully handle null legacy entries from migrations. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
| const isPersonaPage = (value: unknown): value is Page => | ||
| typeof value === 'object' && | ||
| value !== null && | ||
| typeof (value as { pageType?: unknown }).pageType === 'string'; |
There was a problem hiding this comment.
If a legacy LandingPage entry has a string pageType but a truthy non-array layout, isPersonaPage retains it and the customization store passes it to the landing-page layout utilities, where layout.filter throws and crashes the persona customization editor.
Knowledge Base Used:
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
openmetadata-ui/src/main/resources/ui/src/pages/CustomizablePage/CustomizablePage.tsx:110
syncSavedDocumentwrites to React Query usingdocStoreQueryKey(document?.fullyQualifiedName ?? ''), which can be stale (closure) and can produce a cache entry for an empty FQN ifdocumentis ever null/undefined at call time. Since the server response includes the authoritativefullyQualifiedName, key the cache update offnormalizedResponse.fullyQualifiedNameinstead.
const syncSavedDocument = (response: Document) => {
const normalizedResponse = normalizePersonaDocument(response);
setDocument(normalizedResponse);
queryClient.setQueryData(
docStoreQueryKey(document?.fullyQualifiedName ?? ''),
normalizedResponse
);
openmetadata-ui/src/main/resources/ui/src/pages/CustomizablePage/CustomizeStore.ts:55
setDocumentcan be called whencurrentPageTypeis stillnull(e.g., CustomizablePage initializes the store withsetDocument(pageData)beforesetCurrentPageType(...)). The current implementation then forces({ pageType: currentPageType } as Page), which can put an invalidpageType: nullinto state and cause downstream code to assume a validPageTypestring. Guard the update socurrentPageis only set whencurrentPageTypeis available.
setDocument: (document: Document) => {
const { updateCurrentPage, currentPageType } = get();
const normalizedDocument = normalizePersonaDocument(document);
const newPage = getPersonaPage(normalizedDocument, currentPageType);
updateCurrentPage(newPage ?? ({ pageType: currentPageType } as Page));
set({ document: normalizedDocument });
|
✅ Playwright Results — workflow succeededValidated commit ✅ 553 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky PerformanceBlocking targets: ✅ met · Optimization targets: 🟡 in progress Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting. 🕒 Full workflow signal wall (to summary) 58m 53s ⏱️ Max setup 5m 14s · max shard execution 16m 21s · max shard-job elapsed before upload 20m 58s · reporting 5s 🌐 215.84 requests/attempt · 2.83 app boots/UI scenario · 41.90% common-shard skew Optimization targets still in progress:
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
Describe your changes:
Fixes #31980
I made persona customization reads, caches, stores, and writes tolerant of invalid legacy page entries because documents migrated from 1.13.3 to 2.0.0 can contain
null, crashing Data Marketplace and My Data or allowing new null entries to be persisted.Type of change:
High-level design:
useCustomPages, and the customization storeTests:
Use cases covered
Unit tests
PersonaPage.utils.test.ts,useCustomPages.test.ts,DataMarketplacePage.component.test.tsx,MyDataPage.test.tsx, andCustomizablePage.test.tsxBackend integration tests
Ingestion integration tests
Playwright (UI) tests
Manual testing performed
UI screen recording / screenshots:
Not attached; the before-upgrade failure and reproducible payload are documented in #31980, and the corrected behavior is covered by component tests.
Checklist:
Fixes <issue-number>: <short explanation>Fixes #<issue-number>above.Greptile Summary
The PR normalizes legacy persona customization documents before caching or storing them and adds defensive page lookup and update helpers. One malformed-layout path remains in the persona customization editor.
Confidence Score: 4/5
The PR is not yet safe to merge because a malformed legacy landing-page layout can still crash the persona customization editor.
Central normalization retains pages whose layout is a truthy non-array value, allowing the customization editor to pass that value into layout utilities that invoke filter on it.
Files Needing Attention: openmetadata-ui/src/main/resources/ui/src/utils/CustomizePage/PersonaPage.utils.ts, openmetadata-ui/src/main/resources/ui/src/pages/CustomizablePage/CustomizeStore.ts
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Fetch persona document] --> B[normalizePersonaDocument] B --> C[React Query cache] B --> D[CustomizeStore] C --> E[My Data and Marketplace] D --> F[CustomizeMyData editor] F --> G[Landing-page layout utilities]Reviews (2): Last reviewed commit: "fix(ui): validate My Data customized lay..." | Re-trigger Greptile
Context used: