Extract persona customize-page categories into PersonaClassBase#29706
Conversation
Moves getCustomizePageCategories/getCustomizePageOptions out of PersonaUtils into an overridable PersonaClassBase, following the same ClassBase pattern used elsewhere in the UI. Lets downstream products extend the persona customize-page category list without forking PersonaUtils.
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
github-actions
Bot
added
safe to test
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| { | ||
| key: 'governance', | ||
| label: i18n.t('label.governance'), | ||
| isBeta: false, | ||
| description: | ||
| 'Customize the Govern pages with widget of your preference', | ||
| icon: ENTITY_ICONS['govern'], | ||
| }, | ||
| { | ||
| key: 'data-assets', | ||
| label: i18n.t('label.data-asset-plural'), | ||
| isBeta: false, | ||
| description: | ||
| 'Customize the entity detail page with widget of your preference', | ||
| icon: ENTITY_ICONS['dataAssets'], | ||
| }, | ||
| ]; | ||
| } | ||
|
|
||
| public getCustomizePageOptions(category: string): SettingMenuItem[] { | ||
| const list = map(PageType); | ||
|
|
||
| switch (category) { | ||
| case 'governance': | ||
| return list.reduce((acc, item) => { | ||
| if ( | ||
| [ | ||
| PageType.Glossary, | ||
| PageType.GlossaryTerm, | ||
| PageType.Domain, | ||
| PageType.DataProduct, | ||
| ].includes(item) | ||
| ) { | ||
| acc.push(this.generateSettingItems(item)); | ||
| } | ||
|
|
||
| return acc; | ||
| }, [] as SettingMenuItem[]); | ||
| case 'data-assets': | ||
| return list.reduce((acc, item) => { | ||
| if ( | ||
| ![ | ||
| PageType.Glossary, | ||
| PageType.GlossaryTerm, | ||
| PageType.Domain, | ||
| PageType.DataProduct, | ||
| PageType.LandingPage, | ||
| PageType.Tag, | ||
| PageType.Classification, | ||
| PageType.DataMarketplace, | ||
| ].includes(item) | ||
| ) { | ||
| acc.push(this.generateSettingItems(item)); | ||
| } | ||
|
|
||
| return acc; | ||
| }, [] as SettingMenuItem[]); | ||
| default: | ||
| return []; | ||
| } | ||
| } | ||
|
|
||
| protected generateSettingItems(pageType: PageType): SettingMenuItem { | ||
| return { | ||
| key: pageType, | ||
| label: startCase(pageType), | ||
| description: i18n.t('message.entity-customize-description', { | ||
| entity: startCase(pageType), | ||
| }), | ||
| icon: ENTITY_ICONS[pageType], | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| const personaClassBase = new PersonaClassBase(); | ||
|
|
||
| export default personaClassBase; | ||
| export { PersonaClassBase }; |
There was a problem hiding this comment.
Every other ClassBase utility in src/utils/ has a dedicated *.test.ts alongside it (e.g. LeftSidebarClassBase.test.ts, GlobalSettingsClassBase.test.ts, DocumentationLinksClassBase.test.ts, etc.). PersonaClassBase.ts is the only ClassBase file in the directory without one. While PersonaUtils.test.ts still exercises the delegated functions through the singleton, it does not cover getEntityIcons(), the protected generateSettingItems() contract, or any subclass-override scenarios — which is precisely the extension point this PR is introducing.
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 refactors persona customize-page configuration by extracting the customize-page category/option generation (and entity icon mapping) from PersonaUtils.ts into a new PersonaClassBase, aligning persona customization with the existing *ClassBase override pattern used elsewhere in the UI.
Changes:
- Added
PersonaClassBaseto encapsulate persona customize-page categories/options and the icon map behind an overridable class. - Updated
PersonaUtils.tsto delegategetCustomizePageCategories()/getCustomizePageOptions()to apersonaClassBasesingleton. - Re-exported
CustomizeIconKeysfrom the new class base to preserve the public type surface.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| openmetadata-ui/src/main/resources/ui/src/utils/PersonaClassBase.ts | Introduces the new overridable class base containing the category/option generation logic and icon mapping. |
| openmetadata-ui/src/main/resources/ui/src/utils/Persona/PersonaUtils.ts | Converts the previous inline logic into thin delegating wrappers to the personaClassBase singleton. |
| key: 'navigation', | ||
| label: i18n.t('label.navigation'), | ||
| isBeta: false, | ||
| description: 'Customize left sidebar ', | ||
| icon: ENTITY_ICONS['navigation'], | ||
| }, | ||
| { | ||
| key: PageType.LandingPage, | ||
| label: i18n.t('label.home-page'), | ||
| description: | ||
| 'Customize the My data page with widget of your preference', | ||
| icon: ENTITY_ICONS[PageType.LandingPage], |
| key: 'governance', | ||
| label: i18n.t('label.governance'), | ||
| isBeta: false, | ||
| description: | ||
| 'Customize the Govern pages with widget of your preference', | ||
| icon: ENTITY_ICONS['govern'], | ||
| }, | ||
| { | ||
| key: 'data-assets', | ||
| label: i18n.t('label.data-asset-plural'), | ||
| isBeta: false, | ||
| description: | ||
| 'Customize the entity detail page with widget of your preference', | ||
| icon: ENTITY_ICONS['dataAssets'], |
| label: i18n.t('label.navigation'), | ||
| isBeta: false, | ||
| description: 'Customize left sidebar ', | ||
| icon: ENTITY_ICONS['navigation'], |
| key: pageType, | ||
| label: startCase(pageType), | ||
| description: i18n.t('message.entity-customize-description', { | ||
| entity: startCase(pageType), | ||
| }), |
Jest test CoverageUI tests summary
|
🔴 Playwright Results — 1 failure(s), 25 flaky✅ 4477 passed · ❌ 1 failed · 🟡 25 flaky · ⏭️ 38 skipped
Genuine Failures (failed on all attempts)❌
|
- Route category/option icons through this.getEntityIcons() instead of the module-level ENTITY_ICONS constant, so subclass overrides of getEntityIcons() actually take effect (was previously bypassed). - Localize the four category descriptions that were hardcoded English strings (one with a trailing space), reusing existing message.entity-customize-description and message.customize-your-navigation-subheader keys. - generateSettingItems now derives PageType labels/descriptions via i18n.t(label.<kebab-case>) instead of lodash startCase, matching every label key that already exists for these PageTypes. - Add PersonaClassBase.test.ts, following the *ClassBase.test.ts convention used elsewhere in src/utils/. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
DataMarketplace previously reused DataProductIcon; add its own colored SVG.
Revert the customize-page category descriptions to their original hardcoded copy (navigation, home page, governance, data assets). The i18n-driven descriptions changed the rendered text and broke the Governance customize Playwright assertion; restoring the previous strings keeps behavior unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
Revert generateSettingItems option labels from i18n.t('label.<kebab>') back
to startCase(pageType) to preserve the original output (e.g. "Ml Model", not
"ML Model"). The i18n labels broke customize-page Playwright clicks
(CustomizeDetailPage, CustomizeCollate) and contradicted the PR's
no-behavior-change claim. Update the affected unit expectations accordingly.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Code Review ✅ Approved 1 resolved / 1 findingsRefactors persona customization logic into an overridable ✅ 1 resolved✅ Quality: Category descriptions hardcoded in English, not localized
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
Summary
getCustomizePageCategories/getCustomizePageOptions(and the entity-icon map) out ofPersonaUtils.tsinto a newPersonaClassBaseclass, following the same overridable-classbase pattern already used forLeftSidebarClassBase,CustomizeMyDataPageClassBase, etc.PersonaUtils.tsnow delegates to apersonaClassBasesingleton instead of holding the logic inline.getCustomizePageCategories()/getCustomizePageOptions()return identical output to before.This lets downstream products extend the persona customize-page category list (e.g. adding a product-specific customization entry) via a
PersonaClassCollate-style override instead of forkingPersonaUtils.ts.Test plan
getCustomizePageCategories/getCustomizePageOptionsbehavior unchanged (pure refactor, same return values)PersonaUtils.test.ts/CustomizeUIconsumers unaffected (same public exports)Greptile Summary
This PR extracts
getCustomizePageCategories,getCustomizePageOptions, and theENTITY_ICONSmap fromPersonaUtils.tsinto a newPersonaClassBaseclass, following the overridable ClassBase pattern already established forLeftSidebarClassBase,CustomizeMyDataPageClassBase, etc.PersonaUtils.tsis now a thin delegation layer preserving the existing public API.getEntityIcons(),getCustomizePageCategories(),getCustomizePageOptions(), and aprotected generateSettingItems()hook; exported as both a named class and a singleton default.CustomizeIconKeysand forward the two public functions to the singleton, keeping all callers unaffected.PageType.DataMarketplacepreviously reusedDataProductIconand now correctly maps to its own icon (a minor undocumented visual improvement alongside the refactor).Confidence Score: 5/5
Safe to merge — pure structural refactoring with no change to the existing public API and comprehensive test coverage of all three methods including subclass-override scenarios.
All logic is moved verbatim into PersonaClassBase; PersonaUtils.ts delegates faithfully; the new test file covers getEntityIcons, both public category methods, override propagation, and the singleton contract. The only observable difference from the original code is that PageType.DataMarketplace now uses its own dedicated SVG instead of reusing DataProductIcon — an intentional improvement signalled by the new SVG file being part of this commit.
No files require special attention.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Consumer as Consumer(CustomizeUI, etc.) participant PU as PersonaUtils.ts participant PCB as personaClassBase(PersonaClassBase singleton) participant ICONS as ENTITY_ICONS(module constant) Consumer->>PU: getCustomizePageCategories() PU->>PCB: personaClassBase.getCustomizePageCategories() PCB->>ICONS: this.getEntityIcons() ICONS-->>PCB: Record CustomizeIconKeys SvgComponent PCB-->>PU: SettingMenuItem[] PU-->>Consumer: SettingMenuItem[] Consumer->>PU: getCustomizePageOptions(category) PU->>PCB: personaClassBase.getCustomizePageOptions(category) PCB->>PCB: this.generateSettingItems(pageType) PCB->>ICONS: this.getEntityIcons()[pageType] ICONS-->>PCB: SvgComponent PCB-->>PU: SettingMenuItem[] PU-->>Consumer: SettingMenuItem[]%%{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 Consumer as Consumer(CustomizeUI, etc.) participant PU as PersonaUtils.ts participant PCB as personaClassBase(PersonaClassBase singleton) participant ICONS as ENTITY_ICONS(module constant) Consumer->>PU: getCustomizePageCategories() PU->>PCB: personaClassBase.getCustomizePageCategories() PCB->>ICONS: this.getEntityIcons() ICONS-->>PCB: Record CustomizeIconKeys SvgComponent PCB-->>PU: SettingMenuItem[] PU-->>Consumer: SettingMenuItem[] Consumer->>PU: getCustomizePageOptions(category) PU->>PCB: personaClassBase.getCustomizePageOptions(category) PCB->>PCB: this.generateSettingItems(pageType) PCB->>ICONS: this.getEntityIcons()[pageType] ICONS-->>PCB: SvgComponent PCB-->>PU: SettingMenuItem[] PU-->>Consumer: SettingMenuItem[]Reviews (5): Last reviewed commit: "fix: keep startCase option labels in cus..." | Re-trigger Greptile