From 8a030802fd2634b25564aa452fea7d249bfdf500 Mon Sep 17 00:00:00 2001 From: Chirag Madlani <12962843+chirag-madlani@users.noreply.github.com> Date: Tue, 22 Jul 2025 22:54:27 +0530 Subject: [PATCH] fix(ui): expandable icon for non-nested columns (#22469) --- .../ui/playwright/e2e/Features/Table.spec.ts | 71 ++++ .../ModelTab/ModelTab.component.tsx | 3 +- .../ColumnProfileTable/ColumnProfileTable.tsx | 3 +- .../SchemaTable/SchemaTable.component.tsx | 3 +- .../Database/SchemaTable/SchemaTable.test.tsx | 1 + .../ui/src/utils/TableUtils.test.tsx | 341 +++++++++++++++++- .../resources/ui/src/utils/TableUtils.tsx | 22 ++ 7 files changed, 440 insertions(+), 4 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/Table.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/Table.spec.ts index b74b49e4fbe..e7655420217 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/Table.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/Table.spec.ts @@ -363,4 +363,75 @@ test.describe('Table & Data Model columns table pagination', () => { page.getByTestId('data-model-column-table').getByRole('row') ).toHaveCount(26); }); + + test('expand collapse should only visible for nested columns', async ({ + page, + }) => { + await page.goto('/table/sample_data.ecommerce_db.shopify.dim_customer'); + + await page.waitForLoadState('networkidle'); + await page.waitForSelector('[data-testid="loader"]', { + state: 'detached', + }); + + // Should show expand icon for nested columns + expect( + page + .locator( + '[data-row-key="sample_data.ecommerce_db.shopify.dim_customer.shipping_address"]' + ) + .getByTestId('expand-icon') + ).toBeVisible(); + + // Should not show expand icon for non-nested columns + expect( + page + .locator( + '[data-row-key="sample_data.ecommerce_db.shopify.dim_customer.customer_id"]' + ) + .getByTestId('expand-icon') + ).not.toBeVisible(); + + // Should not show expand icon for non-nested columns + expect( + page + .locator( + '[data-row-key="sample_data.ecommerce_db.shopify.dim_customer.shop_id"]' + ) + .getByTestId('expand-icon') + ).not.toBeVisible(); + + // verify column profile table + await page.getByRole('tab', { name: 'Data Observability' }).click(); + await page.waitForSelector('[data-testid="loader"]', { + state: 'detached', + }); + + const colsResponse = page.waitForResponse( + '/api/v1/tables/name/*/columns?*' + ); + await page.getByRole('menuitem', { name: 'Column Profile' }).click(); + + await colsResponse; + await page.waitForSelector('[data-testid="loader"]', { + state: 'detached', + }); + + // Should show expand icon for nested columns + expect( + page + .locator('[data-row-key="shipping_address"]') + .getByTestId('expand-icon') + ).toBeVisible(); + + // Should not show expand icon for non-nested columns + expect( + page.locator('[data-row-key="customer_id"]').getByTestId('expand-icon') + ).not.toBeVisible(); + + // Should not show expand icon for non-nested columns + expect( + page.locator('[data-row-key="shop_id"]').getByTestId('expand-icon') + ).not.toBeVisible(); + }); }); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Dashboard/DataModel/DataModels/ModelTab/ModelTab.component.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Dashboard/DataModel/DataModels/ModelTab/ModelTab.component.tsx index 046467d1507..177dfb878be 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Dashboard/DataModel/DataModels/ModelTab/ModelTab.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Dashboard/DataModel/DataModels/ModelTab/ModelTab.component.tsx @@ -43,6 +43,7 @@ import { getAllTags, searchTagInData, } from '../../../../../utils/TableTags/TableTags.utils'; +import { pruneEmptyChildren } from '../../../../../utils/TableUtils'; import DisplayName from '../../../../common/DisplayName/DisplayName'; import { EntityAttachmentProvider } from '../../../../common/EntityDescription/EntityAttachmentProvider/EntityAttachmentProvider'; import FilterTablePlaceHolder from '../../../../common/ErrorWithPlaceholder/FilterTablePlaceHolder'; @@ -108,7 +109,7 @@ const ModelTab = () => { fields: TabSpecificField.TAGS, }); - setPaginatedColumns(response.data || []); + setPaginatedColumns(pruneEmptyChildren(response.data) || []); handlePagingChange(response.paging); } catch (error) { setPaginatedColumns([]); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/TableProfiler/ColumnProfileTable/ColumnProfileTable.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/TableProfiler/ColumnProfileTable/ColumnProfileTable.tsx index f0782240d6a..d00a94334f3 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/TableProfiler/ColumnProfileTable/ColumnProfileTable.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/TableProfiler/ColumnProfileTable/ColumnProfileTable.tsx @@ -67,6 +67,7 @@ import { getAddCustomMetricPath } from '../../../../../utils/RouterUtils'; import { generateEntityLink, getTableExpandableConfig, + pruneEmptyChildren, } from '../../../../../utils/TableUtils'; import DatePickerMenu from '../../../../common/DatePickerMenu/DatePickerMenu.component'; import ErrorPlaceHolder from '../../../../common/ErrorWithPlaceholder/ErrorPlaceHolder'; @@ -426,7 +427,7 @@ const ColumnProfileTable = () => { fields: TabSpecificField.PROFILE, }); - setData(response.data || []); + setData(pruneEmptyChildren(response.data) || []); handlePagingChange(response.paging); } catch { setData([]); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaTable/SchemaTable.component.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaTable/SchemaTable.component.tsx index d48e52da8b4..c8fed741370 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaTable/SchemaTable.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaTable/SchemaTable.component.tsx @@ -80,6 +80,7 @@ import { getAllRowKeysByKeyName, getTableExpandableConfig, prepareConstraintIcon, + pruneEmptyChildren, updateColumnInNestedStructure, } from '../../../utils/TableUtils'; import { EntityAttachmentProvider } from '../../common/EntityDescription/EntityAttachmentProvider/EntityAttachmentProvider'; @@ -212,7 +213,7 @@ const SchemaTable = () => { fields: 'tags,customMetrics', }); - setTableColumns(response.data || []); + setTableColumns(pruneEmptyChildren(response.data) || []); handlePagingChange(response.paging); } catch { // Set empty state if API fails diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaTable/SchemaTable.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaTable/SchemaTable.test.tsx index 0b31ea8bdac..7ad822f1cf7 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaTable/SchemaTable.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaTable/SchemaTable.test.tsx @@ -97,6 +97,7 @@ jest.mock('../../../utils/CommonUtils', () => ({ jest.mock('../../../utils/TableUtils', () => ({ getAllRowKeysByKeyName: jest.fn(), + pruneEmptyChildren: jest.fn().mockImplementation((value) => value), makeData: jest.fn().mockImplementation((value) => value), prepareConstraintIcon: jest.fn(), updateFieldTags: jest.fn(), diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/TableUtils.test.tsx b/openmetadata-ui/src/main/resources/ui/src/utils/TableUtils.test.tsx index 5c94cb75869..8f4f2145001 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/TableUtils.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/utils/TableUtils.test.tsx @@ -12,13 +12,14 @@ */ import { OperationPermission } from '../context/PermissionProvider/PermissionProvider.interface'; import { TagLabel } from '../generated/entity/data/container'; -import { Column } from '../generated/entity/data/table'; +import { Column, DataType } from '../generated/entity/data/table'; import { ExtraTableDropdownOptions, findColumnByEntityLink, getEntityIcon, getTagsWithoutTier, getTierTags, + pruneEmptyChildren, updateColumnInNestedStructure, } from '../utils/TableUtils'; import EntityLink from './EntityLink'; @@ -325,4 +326,342 @@ describe('TableUtils', () => { expect(result).toStrictEqual([]); }); }); + + describe('pruneEmptyChildren', () => { + it('should remove children property when children array is empty', () => { + const columns: Column[] = [ + { + name: 'column1', + dataType: DataType.String, + children: [], + } as Column, + { + name: 'column2', + dataType: DataType.Int, + children: [], + } as Column, + ]; + + const result = pruneEmptyChildren(columns); + + expect(result[0]).not.toHaveProperty('children'); + expect(result[1]).not.toHaveProperty('children'); + expect(result[0].name).toBe('column1'); + expect(result[1].name).toBe('column2'); + }); + + it('should keep children property when children array has items', () => { + const columns: Column[] = [ + { + name: 'parentColumn', + dataType: DataType.Struct, + children: [ + { + name: 'childColumn1', + dataType: DataType.String, + } as Column, + ], + } as Column, + ]; + + const result = pruneEmptyChildren(columns); + + expect(result[0]).toHaveProperty('children'); + expect(result[0].children).toHaveLength(1); + expect(result[0].children?.[0].name).toBe('childColumn1'); + }); + + it('should handle columns without children property', () => { + const columns: Column[] = [ + { + name: 'column1', + dataType: DataType.String, + } as Column, + { + name: 'column2', + dataType: DataType.Int, + } as Column, + ]; + + const result = pruneEmptyChildren(columns); + + expect(result[0]).not.toHaveProperty('children'); + expect(result[1]).not.toHaveProperty('children'); + expect(result[0].name).toBe('column1'); + expect(result[1].name).toBe('column2'); + }); + + it('should handle mixed columns with and without empty children', () => { + const columns: Column[] = [ + { + name: 'column1', + dataType: DataType.String, + children: [], + } as Column, + { + name: 'parentColumn', + dataType: DataType.Struct, + children: [ + { + name: 'childColumn1', + dataType: DataType.String, + } as Column, + ], + } as Column, + { + name: 'column3', + dataType: DataType.Int, + } as Column, + { + name: 'column4', + dataType: DataType.Boolean, + children: [], + } as Column, + ]; + + const result = pruneEmptyChildren(columns); + + expect(result[0]).not.toHaveProperty('children'); + expect(result[1]).toHaveProperty('children'); + expect(result[1].children).toHaveLength(1); + expect(result[2]).not.toHaveProperty('children'); + expect(result[3]).not.toHaveProperty('children'); + }); + + it('should handle nested empty children recursively', () => { + const columns: Column[] = [ + { + name: 'parentColumn', + dataType: DataType.Struct, + children: [ + { + name: 'childColumn1', + dataType: DataType.String, + children: [], + } as Column, + { + name: 'childColumn2', + dataType: DataType.Int, + children: [ + { + name: 'grandchildColumn', + dataType: DataType.Boolean, + children: [], + } as Column, + ], + } as Column, + ], + } as Column, + ]; + + const result = pruneEmptyChildren(columns); + + expect(result[0]).toHaveProperty('children'); + expect(result[0].children).toHaveLength(2); + expect(result[0].children?.[0]).not.toHaveProperty('children'); + expect(result[0].children?.[1]).toHaveProperty('children'); + expect(result[0].children?.[1].children?.[0]).not.toHaveProperty( + 'children' + ); + }); + + it('should return empty array when input is empty', () => { + const columns: Column[] = []; + + const result = pruneEmptyChildren(columns); + + expect(result).toEqual([]); + }); + + it('should preserve all other column properties', () => { + const columns: Column[] = [ + { + name: 'column1', + dataType: DataType.String, + description: 'Test description', + displayName: 'Test Display Name', + fullyQualifiedName: 'test.table.column1', + ordinalPosition: 1, + precision: 10, + scale: 2, + dataLength: 255, + children: [], + tags: [], + customMetrics: [], + } as Column, + ]; + + const result = pruneEmptyChildren(columns); + + expect(result[0]).not.toHaveProperty('children'); + expect(result[0].name).toBe('column1'); + expect(result[0].dataType).toBe(DataType.String); + expect(result[0].description).toBe('Test description'); + expect(result[0].displayName).toBe('Test Display Name'); + expect(result[0].fullyQualifiedName).toBe('test.table.column1'); + expect(result[0].ordinalPosition).toBe(1); + expect(result[0].precision).toBe(10); + expect(result[0].scale).toBe(2); + expect(result[0].dataLength).toBe(255); + expect(result[0].tags).toEqual([]); + expect(result[0].customMetrics).toEqual([]); + }); + + it('should handle complex nested structure with multiple empty children levels', () => { + const columns: Column[] = [ + { + name: 'level1', + dataType: DataType.Struct, + children: [ + { + name: 'level2a', + dataType: DataType.Struct, + children: [], + } as Column, + { + name: 'level2b', + dataType: DataType.Struct, + children: [ + { + name: 'level3a', + dataType: DataType.String, + children: [], + } as Column, + { + name: 'level3b', + dataType: DataType.Int, + children: [ + { + name: 'level4', + dataType: DataType.Boolean, + children: [], + } as Column, + ], + } as Column, + ], + } as Column, + ], + } as Column, + ]; + + const result = pruneEmptyChildren(columns); + + expect(result[0]).toHaveProperty('children'); + expect(result[0].children).toHaveLength(2); + + // level2a should have children removed + expect(result[0].children?.[0]).not.toHaveProperty('children'); + + // level2b should keep children + expect(result[0].children?.[1]).toHaveProperty('children'); + expect(result[0].children?.[1].children).toHaveLength(2); + + // level3a should have children removed + expect(result[0].children?.[1].children?.[0]).not.toHaveProperty( + 'children' + ); + + // level3b should keep children but level4 should have children removed + expect(result[0].children?.[1].children?.[1]).toHaveProperty('children'); + expect( + result[0].children?.[1].children?.[1].children?.[0] + ).not.toHaveProperty('children'); + }); + + it('should handle columns with undefined children property', () => { + const columns: Column[] = [ + { + name: 'column1', + dataType: DataType.String, + children: undefined, + } as Column, + { + name: 'column2', + dataType: DataType.Int, + } as Column, + ]; + + const result = pruneEmptyChildren(columns); + + expect(result[0]).not.toHaveProperty('children'); + expect(result[1]).not.toHaveProperty('children'); + expect(result[0].name).toBe('column1'); + expect(result[1].name).toBe('column2'); + }); + + it('should handle columns with null children property', () => { + const columns: Column[] = [ + { + name: 'column1', + dataType: DataType.String, + children: null as any, + } as Column, + ]; + + const result = pruneEmptyChildren(columns); + + expect(result[0]).not.toHaveProperty('children'); + expect(result[0].name).toBe('column1'); + }); + + it('should handle deeply nested structure where all children become empty after pruning', () => { + const columns: Column[] = [ + { + name: 'parent', + dataType: DataType.Struct, + children: [ + { + name: 'child1', + dataType: DataType.Struct, + children: [ + { + name: 'grandchild1', + dataType: DataType.String, + children: [], + } as Column, + { + name: 'grandchild2', + dataType: DataType.Int, + children: [], + } as Column, + ], + } as Column, + { + name: 'child2', + dataType: DataType.Struct, + children: [ + { + name: 'grandchild3', + dataType: DataType.Boolean, + children: [], + } as Column, + ], + } as Column, + ], + } as Column, + ]; + + const result = pruneEmptyChildren(columns); + + expect(result[0]).toHaveProperty('children'); + expect(result[0].children).toHaveLength(2); + + // child1 should keep children but grandchildren should have children removed + expect(result[0].children?.[0]).toHaveProperty('children'); + expect(result[0].children?.[0].children).toHaveLength(2); + expect(result[0].children?.[0].children?.[0]).not.toHaveProperty( + 'children' + ); + expect(result[0].children?.[0].children?.[1]).not.toHaveProperty( + 'children' + ); + + // child2 should keep children but grandchild should have children removed + expect(result[0].children?.[1]).toHaveProperty('children'); + expect(result[0].children?.[1].children).toHaveLength(1); + expect(result[0].children?.[1].children?.[0]).not.toHaveProperty( + 'children' + ); + }); + }); }); diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/TableUtils.tsx b/openmetadata-ui/src/main/resources/ui/src/utils/TableUtils.tsx index 59fc2d469ef..f29beb91916 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/TableUtils.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/utils/TableUtils.tsx @@ -1262,3 +1262,25 @@ export const updateColumnInNestedStructure = ( } }); }; + +export const pruneEmptyChildren = (columns: Column[]): Column[] => { + return columns.map((column) => { + // If column has no children or empty children array, remove children property + if (!column.children || column.children.length === 0) { + return omit(column, 'children'); + } + + // If column has children, recursively prune them + const prunedChildren = pruneEmptyChildren(column.children); + + // If after pruning, children array becomes empty, remove children property + if (prunedChildren.length === 0) { + return omit(column, 'children'); + } + + return { + ...column, + children: prunedChildren, + }; + }); +};