Fixes #22399: Add persona customization for service details page#27318
Fixes #22399: Add persona customization for service details page#27318RajdeepKushwaha5 wants to merge 22 commits intoopen-metadata:mainfrom
Conversation
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
1 similar comment
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
persona.mp4 |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds persona-based UI customization support to the Service Details page, aligning it with other customizable entity detail pages (Table/Dashboard/Topic/etc.) by introducing a Service page type, default tab/layout definitions, and a new widget for listing service child entities.
Changes:
- Extends UI customization schema/types to support
PageType.Serviceand wires Service pages into customization routing and persona UI. - Introduces
ServiceDetailsClassBase+ widget-dispatch utility to provide default tabs/layout, widget heights, and dummy data for Service customization. - Adds
ServiceEntityTablewidget and updates ServiceDetailsPage to render the DETAILS tab viaGenericProvider/GenericTabwithuseCustomPages(PageType.Service).
Reviewed changes
Copilot reviewed 14 out of 16 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| openmetadata-ui/src/main/resources/ui/src/utils/ServiceDetailsPage/ServiceDetailsPage.util.tsx | Service widget rendering dispatch (ServiceEntityTable vs CommonWidgets). |
| openmetadata-ui/src/main/resources/ui/src/utils/ServiceDetailsPage/ServiceDetailsClassBase.ts | Defines Service default tabs/layout, widget list, heights, and dummy data. |
| openmetadata-ui/src/main/resources/ui/src/utils/Persona/PersonaUtils.ts | Adds Service option/card in persona customization UI (icon + menu options). |
| openmetadata-ui/src/main/resources/ui/src/utils/CustomizePage/CustomizePageUtils.ts | Adds PageType.Service handling across customization utility switches. |
| openmetadata-ui/src/main/resources/ui/src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx | Integrates GenericProvider/GenericTab + useCustomPages for Service DETAILS tab. |
| openmetadata-ui/src/main/resources/ui/src/pages/ServiceDetailsPage/ServiceDetailsPage.test.tsx | Updates unit tests to mock new customization hooks/components. |
| openmetadata-ui/src/main/resources/ui/src/pages/CustomizeDetailsPage/CustomizeDetailPage.interface.ts | Maps PageType.Service to an entity type for customization editor. |
| openmetadata-ui/src/main/resources/ui/src/pages/CustomizablePage/CustomizablePage.tsx | Routes PageType.Service to the CustomizeDetailsPage editor. |
| openmetadata-ui/src/main/resources/ui/src/generated/system/ui/uiCustomization.ts | Generated enum update to include PageType.Service. |
| openmetadata-ui/src/main/resources/ui/src/generated/system/ui/page.ts | Generated enum update to include PageType.Service. |
| openmetadata-ui/src/main/resources/ui/src/enums/CustomizeDetailPage.enum.ts | Adds new widget key SERVICE_ENTITY_TABLE. |
| openmetadata-ui/src/main/resources/ui/src/constants/ServiceDetailPage.constants.ts | Provides dummy service data for customization editor previews. |
| openmetadata-ui/src/main/resources/ui/src/constants/Customize.constants.ts | Extends CustomizeEntityType + entity→page mapping to include Service. |
| openmetadata-ui/src/main/resources/ui/src/components/Service/ServiceEntityTable/ServiceEntityTable.tsx | New widget rendering searchable/paginated child-entity table for services. |
| openmetadata-ui/src/main/resources/ui/src/components/MyData/CustomizableComponents/AddDetailsPageWidgetModal/AddDetailsPageWidgetModal.tsx | Scales widget widths for detail-page grids vs landing-page grid sizing. |
| openmetadata-spec/src/main/resources/json/schema/system/ui/page.json | Adds "Service" to PageType enum in schema. |
| <GenericProvider<ServicesType> | ||
| customizedPage={customizedPage} | ||
| data={serviceDetails} | ||
| permissions={servicePermission} | ||
| type={entityType as CustomizeEntityType} | ||
| onUpdate={saveUpdatedServiceData}> |
There was a problem hiding this comment.
GenericProvider is being given type={entityType as CustomizeEntityType}, but CustomizeEntityType currently only includes EntityType.DATABASE_SERVICE. For non-database service categories (messaging, dashboard, pipeline, drive, etc.), ENTITY_PAGE_TYPE_MAP[type] becomes undefined, and GenericProvider falls back to an empty default layout when customizedPage is null—making the DETAILS tab render blank for personas without saved customization. Expand CustomizeEntityType/ENTITY_PAGE_TYPE_MAP to include all *Service entity types used by getEntityTypeFromServiceCategory, and remove the unsafe cast so default layouts work across all service categories.
| [EntityType.DIRECTORY]: PageType.Directory, | ||
| [EntityType.FILE]: PageType.File, | ||
| [EntityType.SPREADSHEET]: PageType.Spreadsheet, | ||
| [EntityType.WORKSHEET]: PageType.Worksheet, | ||
| [EntityType.DATABASE_SERVICE]: PageType.Service, | ||
| }; |
There was a problem hiding this comment.
ENTITY_PAGE_TYPE_MAP maps only EntityType.DATABASE_SERVICE to PageType.Service. If GenericProvider is passed another service entity type (messaging, dashboard, pipeline, etc.), ENTITY_PAGE_TYPE_MAP[type] becomes undefined and default layouts won’t load. Map all service entity types that share the Service details page to PageType.Service.
| public getCommonWidgetList() { | ||
| return [ | ||
| DESCRIPTION_WIDGET, | ||
| { | ||
| fullyQualifiedName: DetailPageWidgetKeys.SERVICE_ENTITY_TABLE, | ||
| name: i18n.t('label.entity-detail-plural', { | ||
| entity: i18n.t('label.service'), | ||
| }), | ||
| data: { | ||
| gridSizes: ['large'] as GridSizes[], | ||
| }, | ||
| }, |
There was a problem hiding this comment.
The new SERVICE_ENTITY_TABLE widget key is added to the Service detail page widget list, but the Customize Details editor preview renders widgets via WIDGET_COMPONENTS (see utils/GenericWidget/GenericWidgetUtils.tsx). Since SERVICE_ENTITY_TABLE isn’t registered there, the customization editor will show the widget name only (fallback), not the actual table preview. Add a WIDGET_COMPONENTS entry for DetailPageWidgetKeys.SERVICE_ENTITY_TABLE (and render ServiceEntityTable in customization mode) so the editor preview matches the real page.
| const res = await searchQuery({ | ||
| pageNumber, | ||
| pageSize, | ||
| searchIndex, | ||
| query: searchValue ? `*${searchValue}*` : '', | ||
| queryFilter: buildSchemaQueryFilter( | ||
| 'service.fullyQualifiedName.keyword', | ||
| decodedServiceFQN, | ||
| searchValue | ||
| ), | ||
| includeDeleted: showDeleted, |
There was a problem hiding this comment.
fetchEntities passes a non-empty query (*${searchValue}*) while also applying a queryFilter built by buildSchemaQueryFilter (which already adds wildcard should-queries). Existing service search (ServiceMainTabContent.searchService) and similar table widgets use query: '' and rely on queryFilter for wildcard matching. Using both can unintentionally over-filter results and diverge from existing behavior; align this with the established pattern by keeping query: '' and relying on the query filter for search matching.
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
1 similar comment
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 17 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
openmetadata-ui/src/main/resources/ui/src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1431
- The guard in this effect now returns early when
activeTab === EntityTabs.DETAILS, which meansgetOtherDetails(...)runs on every other tab (Connection/Agents/etc.) and never runs on the DETAILS tab. This looks inverted compared to the intent (fetching child-entity lists/counts for the details tab) and can cause redundant REST calls while navigating non-details tabs. Consider either removing this legacy fetch entirely (ifServiceEntityTableowns details data) or restoring the condition so this fetch only happens for the tab that still needs it.
useEffect(() => {
// ServiceEntityTable handles its own data fetching for the DETAILS tab
// (child entities like Databases, Topics, etc.), so skip the legacy
// REST API call when activeTab is EntityTabs.DETAILS.
if (searchValue || activeTab === EntityTabs.DETAILS) {
return;
}
const { cursorType, cursorValue } = pagingInfo?.pagingCursor ?? {};
getOtherDetails({
limit: pageSize,
...(cursorType && { [cursorType]: cursorValue }),
});
if (isInitialPaginationLoadRef.current) {
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 20 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
openmetadata-ui/src/main/resources/ui/src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1482
- This effect still calls
getOtherDetails(...)for non-DETAILS tabs, but the DETAILS tab is now rendered viaGenericTab/ServiceEntityTableand the legacydata/paging state is no longer used in the UI. This results in unnecessary REST calls/state updates whenever users are on other tabs. Consider removing this legacy fetch (and the related paging/state) now that the DETAILS tab owns its own data fetching.
useEffect(() => {
// ServiceEntityTable widget handles its own data fetching for the
// DETAILS tab (the tab showing child entities: Databases, Topics,
// Pipelines, etc.), so skip the legacy REST API call on that tab.
if (searchValue || activeTab === EntityTabs.DETAILS) {
return;
}
const { cursorType, cursorValue } = pagingInfo?.pagingCursor ?? {};
getOtherDetails({
limit: pageSize,
...(cursorType && { [cursorType]: cursorValue }),
});
if (isInitialPaginationLoadRef.current) {
isInitialPaginationLoadRef.current = false;
}
}, [
showDeleted,
deleted,
pageSize,
searchValue,
pagingInfo?.pagingCursor,
activeTab,
serviceCategory,
]);
| const [serviceDetails, setServiceDetails] = useState<ServicesType>( | ||
| {} as ServicesType | ||
| ); | ||
| const [data, setData] = useState<Array<ServicePageData>>([]); | ||
| const [_data, setData] = useState<Array<ServicePageData>>([]); | ||
| const [files, setFiles] = useState<Array<File>>([]); | ||
| const [spreadsheets, setSpreadsheets] = useState<Array<Spreadsheet>>([]); | ||
| const [isLoading, setIsLoading] = useState(!isOpenMetadataService); | ||
| const [isIngestionPipelineLoading, setIsIngestionPipelineLoading] = | ||
| useState(false); | ||
| const [isServiceLoading, setIsServiceLoading] = useState(true); | ||
| const [_isServiceLoading, setIsServiceLoading] = useState(true); | ||
| const [isFilesLoading, setIsFilesLoading] = useState(true); |
There was a problem hiding this comment.
The _data and _isServiceLoading state values are never read anywhere in this component (only their setters are used). This looks like leftover state from the removed ServiceMainTabContent flow; consider removing these state variables (and any related updates) to avoid dead code and confusion.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 20 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
openmetadata-ui/src/main/resources/ui/src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1470
- The
useEffectthat callsgetOtherDetails()currently runs for every tab exceptEntityTabs.DETAILS(it returns early whenactiveTab === EntityTabs.DETAILS). Since the DETAILS tab has been migrated toServiceEntityTable/GenericTab, this legacy REST fetch looks redundant and now triggers extra network calls + state updates on tabs like INSIGHTS/CONNECTION (and potentially on metadata/security services too). Consider removing this effect entirely, or gating it sogetOtherDetails()only runs when it is still needed (e.g., only whenactiveTab === EntityTabs.DETAILSfor any remaining legacy rendering paths).
useEffect(() => {
// ServiceEntityTable widget handles its own data fetching for the
// DETAILS tab (the tab showing child entities: Databases, Topics,
// Pipelines, etc.), so skip the legacy REST API call on that tab.
if (searchValue || activeTab === EntityTabs.DETAILS) {
return;
}
const { cursorType, cursorValue } = pagingInfo?.pagingCursor ?? {};
getOtherDetails({
limit: pageSize,
...(cursorType && { [cursorType]: cursorValue }),
});
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 21 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
openmetadata-ui/src/main/resources/ui/src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:1482
- The
useEffectthat callsgetOtherDetailsnow runs on any change whenactiveTabis not DETAILS andsearchValueis empty, but the fetcheddata/paging/isServiceLoadingstate is no longer used in the UI (those variables were renamed to_data,_paging,_isServiceLoading). This causes unnecessary background REST calls and state updates. Remove this legacy prefetch path (and the now-unused state) or gate it so it only runs when some rendered tab actually consumes the results.
useEffect(() => {
// ServiceEntityTable widget handles its own data fetching for the
// DETAILS tab (the tab showing child entities: Databases, Topics,
// Pipelines, etc.), so skip the legacy REST API call on that tab.
if (searchValue || activeTab === EntityTabs.DETAILS) {
return;
}
const { cursorType, cursorValue } = pagingInfo?.pagingCursor ?? {};
getOtherDetails({
limit: pageSize,
...(cursorType && { [cursorType]: cursorValue }),
});
if (isInitialPaginationLoadRef.current) {
isInitialPaginationLoadRef.current = false;
}
}, [
showDeleted,
deleted,
pageSize,
searchValue,
pagingInfo?.pagingCursor,
activeTab,
serviceCategory,
]);
| const tabs: TabsProps['items'] = useMemo(() => { | ||
| const tabs = []; | ||
| const ownerIds = serviceDetails?.owners?.map((owner) => owner.id) ?? []; | ||
| const userOwnsService = ownerIds.includes(currentUser?.id ?? ''); | ||
| const userInOwnerTeam = Boolean( | ||
| currentUser?.teams?.some((team) => ownerIds.includes(team.id)) | ||
| ); | ||
|
|
||
| const showIngestionTab = userInOwnerTeam || userOwnsService || isAdminUser; | ||
|
|
||
| // Create extension context for plugins | ||
| const extensionContext: PluginEntityDetailsContext = { | ||
| serviceCategory: serviceCategory as ServiceCategory, | ||
| serviceDetails, | ||
| }; | ||
|
|
||
| if (!isMetadataService && !isSecurityService) { | ||
| tabs.push( | ||
| { | ||
| name: t('label.insight-plural'), | ||
| key: EntityTabs.INSIGHTS, | ||
| children: ( | ||
| <ServiceInsightsTab | ||
| collateAIagentsList={collateAgentsList} | ||
| ingestionPipelines={ingestionPipelines} | ||
| isCollateAIagentsLoading={isCollateAgentLoading} | ||
| isIngestionPipelineLoading={isIngestionPipelineLoading} | ||
| serviceDetails={serviceDetails} | ||
| workflowStatesData={workflowStatesData} | ||
| /> | ||
| ), | ||
| }, | ||
| { | ||
| name: getCountLabel(serviceCategory), | ||
| key: getCountLabel(serviceCategory).toLowerCase(), | ||
| count: paging.total, | ||
| children: ( | ||
| <ServiceMainTabContent | ||
| currentPage={currentPage} | ||
| data={data} | ||
| isServiceLoading={isServiceLoading} | ||
| paging={paging} | ||
| pagingInfo={pagingInfo} | ||
| saveUpdatedServiceData={saveUpdatedServiceData} | ||
| serviceDetails={serviceDetails} | ||
| serviceName={serviceCategory} | ||
| servicePermission={servicePermission} | ||
| setFilters={setFilters} | ||
| setIsServiceLoading={setIsServiceLoading} | ||
| showDeleted={showDeleted} | ||
| onDataProductUpdate={handleDataProductUpdate} | ||
| onDescriptionUpdate={handleDescriptionUpdate} | ||
| onShowDeletedChange={handleShowDeleted} | ||
| /> | ||
| ), | ||
| key: EntityTabs.DETAILS, | ||
| count: serviceEntityCount, | ||
| children: <GenericTab type={PageType.Service} />, | ||
| } | ||
| ); |
There was a problem hiding this comment.
customizedPage is fetched and passed into GenericProvider, but the actual Tabs items are still built entirely from local logic and never use customizedPage.tabs (or helpers like getDetailsTabWithNewLabel / getTabLabelMapFromTabs). As a result, persona customization that hides/reorders tabs or adds new tabs won’t be reflected on the Service Details page—only the widget layout for the existing DETAILS tab can change. Update the tabs construction to be driven by customizedPage.tabs (falling back to defaults when null) so new/hidden tabs show up and map to the correct tab children (e.g., custom tabs should render GenericTab too).
…ilures - Fix getDefaultLayout to return empty layout when tab param is undefined - Use exact text matching in ServiceCustomization test locators - Update Pagination tests to use search API and /details tab route
❌ Playwright Lint Check Failed — ESLint + Prettier + Organise ImportsThe following files have style issues that need to be fixed: Fix locally (fast — only for changed files in the branch): make ui-checkstyle-playwright-changedOr to fix all playwright files: make ui-checkstyle-playwright |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 22 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
openmetadata-ui/src/main/resources/ui/src/pages/ServiceDetailsPage/ServiceDetailsPage.tsx:243
pagingInfois now only destructured into_paging(unused) pluscurrentPage/handlePageChange/handlePagingChange, but the service children list is rendered byServiceEntityTablewhich manages its own paging. Consider removing the unusedpaging: _pagingdestructure (and ideally thepagingInfohook entirely if it’s no longer driving any UI) to avoid leftover state that can confuse future maintenance.
const {
paging: _paging,
currentPage,
handlePageChange,
handlePagingChange,
} = pagingInfo;
| ); | ||
| }, [ | ||
| currentUser, | ||
| currentPage, |
There was a problem hiding this comment.
The tabs useMemo dependency list includes currentPage, but currentPage no longer appears to be used in the memo body after switching the details tab content to GenericTab. Dropping unused dependencies helps avoid unnecessary recomputation and makes it clearer what actually drives tab rendering.
| currentPage, |
| const { | ||
| version: currentVersion, | ||
| deleted, | ||
| deleted: _deleted, |
There was a problem hiding this comment.
deleted: _deleted is destructured from serviceDetails but never used. It’d be cleaner to omit it from the destructuring to reduce noise (especially since the underscore-prefixed variable is only there to satisfy no-unused-vars).
| deleted: _deleted, |
| sortBy(widgetsList, 'name')?.map((widget) => { | ||
| const scaleFactor = maxGridSizeSupport / LANDING_PAGE_MAX_GRID_SIZE; | ||
| const widgetSizeOptions: Array<WidgetSizeInfo> = | ||
| widget.data.gridSizes.map((size: GridSizes) => ({ | ||
| label: ( | ||
| <span data-testid={`${size}-size-selector`}> | ||
| {getWidgetWidthLabelFromKey(toString(size))} | ||
| </span> | ||
| ), | ||
| value: WidgetWidths[size], | ||
| value: Math.min( | ||
| maxGridSizeSupport, | ||
| Math.max(1, Math.round(WidgetWidths[size] * scaleFactor)) | ||
| ), |
There was a problem hiding this comment.
scaleFactor depends only on maxGridSizeSupport (and the landing-page max grid size), but it’s recomputed inside the widget .map() loop. Consider computing it once outside the loop to reduce repeated work and make the sizing logic easier to follow.
…n AddDetailsPageWidgetModal Moved the landingPageMaxGridSize lookup inside the component so that test suites that mock customizeMyDataPageClassBase without providing this property no longer crash on module import. This fixes CustomizableLandingPageUtils, WidgetCard and CustomiseLandingPageHeader test failures observed in ui-coverage-tests.
| interface ServiceEntityTableProps { | ||
| isCustomizationPage?: boolean; | ||
| } | ||
|
|
||
| const ServiceEntityTable = ({ | ||
| isCustomizationPage = false, | ||
| }: ServiceEntityTableProps) => { | ||
| const routeParams = useParams<{ serviceCategory?: string }>(); |
There was a problem hiding this comment.
ServiceEntityTable is a new component but it doesn’t follow the repo UI file/interface conventions (component file should be *.component.tsx, and prop types/interfaces should live in a *.interface.ts file rather than being declared inline). This makes the new component inconsistent with the expected UI structure and harder to maintain as it grows.
| const fetchEntityCount = async () => { | ||
| try { | ||
| const res = await searchQuery({ | ||
| pageNumber: 1, | ||
| pageSize: 0, | ||
| searchIndex: getSearchIndexForService(serviceCategory), | ||
| query: '', | ||
| queryFilter: buildSchemaQueryFilter( | ||
| 'service.fullyQualifiedName.keyword', | ||
| decodedServiceFQN, | ||
| searchValue ? toString(searchValue) : undefined | ||
| ), | ||
| includeDeleted: showDeleted, | ||
| trackTotalHits: true, | ||
| }); | ||
| setServiceEntityCount(res.hits.total.value); | ||
| } catch { | ||
| // count fetch is best-effort | ||
| } | ||
| }; | ||
| fetchEntityCount(); |
There was a problem hiding this comment.
ServiceDetailsPage now issues an extra searchQuery request just to compute the DETAILS tab count, while ServiceEntityTable already performs a searchQuery with trackTotalHits: true and gets the same total. This duplicates backend calls on every filter change (search/show-deleted) and can be avoided by plumbing the total hit count from the table fetch back to the parent (e.g., via context/callback) or otherwise sharing the result.
Jest test CoverageUI tests summary
|
- Fix inverted guard in ServiceDetailsClassBase.getDefaultLayout so default layout is returned when tab is undefined or the primary DETAILS tab (flagged by gitar-bot) - Remove unused outer currentPage from pagingInfo destructure and from tabs useMemo deps in ServiceDetailsPage - Remove unused _deleted destructure in ServiceDetailsPage - Hoist scaleFactor computation out of .map() loop in AddDetailsPageWidgetModal - Lint/prettier clean-up across changed files and playwright specs
…ce-persona-customization
| if (key === EntityTabs.DETAILS) { | ||
| handlePageChange(INITIAL_PAGING_VALUE, { | ||
| cursorType: null, | ||
| cursorValue: undefined, |
There was a problem hiding this comment.
activeTabHandler resets pagination via the local pagingInfo.handlePageChange(...) when switching to the Details tab. Since the Details content is now rendered via GenericTab/ServiceEntityTable (which manages its own usePaging state), this reset only mutates the shared URL params and may not actually reset the table’s internal page state, leading to URL/UI desync. Consider removing this reset here and delegating it to ServiceEntityTable, or lift/persist a single paging state that both parent and widget share.
| const fetchEntityCount = async () => { | ||
| try { | ||
| const res = await searchQuery({ | ||
| pageNumber: 1, | ||
| pageSize: 0, | ||
| searchIndex: getSearchIndexForService(serviceCategory), | ||
| query: '', | ||
| queryFilter: buildSchemaQueryFilter( | ||
| 'service.fullyQualifiedName.keyword', | ||
| decodedServiceFQN, | ||
| searchValue ? toString(searchValue) : undefined | ||
| ), | ||
| includeDeleted: showDeleted, | ||
| trackTotalHits: true, |
There was a problem hiding this comment.
The fetchEntityCount effect issues an extra /search/query request (pageSize=0) on mount and whenever showDeleted/searchValue change, even though the Details tab’s ServiceEntityTable also runs a search query and already receives hits.total.value. This duplicates work and adds load. Consider deriving serviceEntityCount from the table fetch (e.g., via a callback/context update) or gating this effect to only run when the Details tab is active.
| jest.mock('../../utils/ServiceUtils', () => ({ | ||
| getCountLabel: jest.fn().mockReturnValue('Databases'), | ||
| getEntityTypeFromServiceCategory: jest.fn().mockReturnValue('database'), | ||
| getResourceEntityFromServiceCategory: jest | ||
| .fn() | ||
| .mockReturnValue('databaseService'), | ||
| getSearchIndexForService: jest.fn().mockReturnValue('database_search_index'), |
There was a problem hiding this comment.
In this test mock, getEntityTypeFromServiceCategory returns 'database', but the real implementation returns an EntityType value (e.g. 'databaseService'). Using a non-realistic value here can mask issues where the entity type is propagated into GenericProvider / customization logic. Consider updating the mock to return EntityType.DATABASE_SERVICE (or the exact string value) to match production behavior.
Code Review ✅ Approved 12 resolved / 12 findingsImplements persona customization for the service details page, addressing multiple fetch logic inefficiencies, type mapping errors, and test stability issues. No issues found. ✅ 12 resolved✅ Bug: Non-database service types not mapped in CustomizeEntityType
✅ Quality: Misleading comment about which tab skips fetching
✅ Bug: Hardcoded DATABASE_SERVICE type breaks non-database services
✅ Bug: Tab count badge removed for service entities tab
✅ Performance: useEffect dependencies may cause double-fetch on state change
...and 7 more resolved from earlier reviews OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
|
|
Describe your changes:
Fixes #22399
What changes did I make?
Added persona-based UI customization support for the Service Details Page, following the same pattern used by Table, Dashboard, Topic, Pipeline, and other entity detail pages.
Changes breakdown:
"Service"toPageTypeenum inpage.json(+ generated TS types)PageType.Servicecases to all 6 switch functions (getDefaultTabs,getDefaultWidgetForTab,getCustomizableWidgetByPage,getDummyDataByPage,getWidgetsFromKey,getWidgetHeight)EntityType.DATABASE_SERVICEtoCustomizeEntityTypeunion andENTITY_PAGE_TYPE_MAPPageType.Service → EntityType.DATABASE_SERVICEmappingPageType.Servicerouting toCustomizeDetailsPageGenericProvider,GenericTab, anduseCustomPages(PageType.Service)to consume persona customizationWidgetWidthsenum values (1/2/3) are designed for the 3-column landing page but detail pages use an 8-column grid, causing widgets added via "+Add" to render too narrow. Added a scale factor (maxGridSizeSupport / landingPageMaxGridSize) so small→3, medium→5, large→8 columnsWhy did I make them?
The Service Details Page was the only major entity page without persona customization support. This blocked users from customizing the layout/widgets shown on service pages per persona.
How did I test?
ServiceDetailsPage.test.tsxunit tests passType of change:
Checklist:
Fixes <issue-number>: <short explanation>or decision-making process is reflected in the issue.