From aca52df6004765be21199f3703593e7450086143 Mon Sep 17 00:00:00 2001 From: Ashish Gupta Date: Sun, 13 Apr 2025 10:48:11 +0530 Subject: [PATCH] fix the suggestion not visible for nested columns (#20786) * fix the suggestion not visible for nested columns * fix the playwright test --- .../playwright/support/entity/TableClass.ts | 4 +- .../TableDescription.component.tsx | 2 +- .../Tag/TagsContainerV2/TagsContainerV2.tsx | 2 +- .../TableDetailsPageV1/TableDetailsPageV1.tsx | 39 ++-- .../main/resources/ui/src/utils/EntityLink.ts | 7 +- .../ui/src/utils/TableUtils.test.tsx | 166 ++++++++++++++++++ .../resources/ui/src/utils/TableUtils.tsx | 82 +++++++++ 7 files changed, 276 insertions(+), 26 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/playwright/support/entity/TableClass.ts b/openmetadata-ui/src/main/resources/ui/playwright/support/entity/TableClass.ts index 8c1632aa5e4..b80e907a32b 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/support/entity/TableClass.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/support/entity/TableClass.ts @@ -66,8 +66,8 @@ export class TableClass extends EntityClass { this.columnsName[0], this.columnsName[1], this.columnsName[2], - `"${this.columnsName[2]}.${this.columnsName[3]}"`, - `"${this.columnsName[2]}.${this.columnsName[4]}"`, + `${this.columnsName[2]}.${this.columnsName[3]}`, + `${this.columnsName[2]}.${this.columnsName[4]}`, this.columnsName[5], ]; diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Database/TableDescription/TableDescription.component.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Database/TableDescription/TableDescription.component.tsx index 6419556fccd..e8583c2e5d1 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Database/TableDescription/TableDescription.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Database/TableDescription/TableDescription.component.tsx @@ -44,7 +44,7 @@ const TableDescription = ({ entityType === EntityType.TABLE ? EntityLink.getTableEntityLink( entityFqn, - EntityLink.getTableColumnNameFromColumnFqn(columnData.fqn) + EntityLink.getTableColumnNameFromColumnFqn(columnData.fqn, false) ) : getEntityFeedLink(entityType, columnData.fqn), [entityType, entityFqn] diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Tag/TagsContainerV2/TagsContainerV2.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Tag/TagsContainerV2/TagsContainerV2.tsx index 3f189ff0b42..d638df131d1 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Tag/TagsContainerV2/TagsContainerV2.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Tag/TagsContainerV2/TagsContainerV2.tsx @@ -393,7 +393,7 @@ const TagsContainerV2 = ({ if (!isGlossaryType && entityType === EntityType.TABLE) { const entityLink = EntityLink.getTableEntityLink( entityFqn ?? '', - EntityLink.getTableColumnNameFromColumnFqn(columnData?.fqn ?? '') + EntityLink.getTableColumnNameFromColumnFqn(columnData?.fqn ?? '', false) ); const activeSuggestion = selectedUserSuggestions?.tags.find( diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/TableDetailsPageV1/TableDetailsPageV1.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/TableDetailsPageV1/TableDetailsPageV1.tsx index be3dfb5687d..65dda9dfee4 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/TableDetailsPageV1/TableDetailsPageV1.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/TableDetailsPageV1/TableDetailsPageV1.tsx @@ -86,16 +86,17 @@ import { getTabLabelMapFromTabs, } from '../../utils/CustomizePage/CustomizePageUtils'; import { defaultFields } from '../../utils/DatasetDetailsUtils'; -import EntityLink from '../../utils/EntityLink'; import entityUtilClassBase from '../../utils/EntityUtilClassBase'; import { getEntityName } from '../../utils/EntityUtils'; import { DEFAULT_ENTITY_PERMISSION } from '../../utils/PermissionsUtils'; import { getEntityDetailsPath, getVersionPath } from '../../utils/RouterUtils'; import tableClassBase from '../../utils/TableClassBase'; import { + findColumnByEntityLink, getJoinsFromTableJoins, getTagsWithoutTier, getTierTags, + updateColumnInNestedStructure, } from '../../utils/TableUtils'; import { updateTierTag } from '../../utils/TagsUtils'; import { showErrorToast, showSuccessToast } from '../../utils/ToastUtils'; @@ -684,14 +685,11 @@ const TableDetailsPageV1: React.FC = () => { return; } - const activeCol = prev?.columns.find((column) => { - return ( - EntityLink.getTableEntityLink( - prev.fullyQualifiedName ?? '', - column.name ?? '' - ) === suggestion.entityLink - ); - }); + const activeCol = findColumnByEntityLink( + prev.fullyQualifiedName ?? '', + prev.columns, + suggestion.entityLink + ); if (!activeCol) { return { @@ -701,18 +699,17 @@ const TableDetailsPageV1: React.FC = () => { : { tags: suggestion.tagLabels }), }; } else { - const updatedColumns = prev.columns.map((column) => { - if (column.fullyQualifiedName === activeCol.fullyQualifiedName) { - return { - ...column, - ...(suggestion.type === SuggestionType.SuggestDescription - ? { description: suggestion.description } - : { tags: suggestion.tagLabels }), - }; - } else { - return column; - } - }); + const update = + suggestion.type === SuggestionType.SuggestDescription + ? { description: suggestion.description } + : { tags: suggestion.tagLabels }; + + // Update the column in the nested structure + const updatedColumns = updateColumnInNestedStructure( + prev.columns, + activeCol.fullyQualifiedName ?? '', + update + ); return { ...prev, diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/EntityLink.ts b/openmetadata-ui/src/main/resources/ui/src/utils/EntityLink.ts index 571a8197d4b..93b73257ae6 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/EntityLink.ts +++ b/openmetadata-ui/src/main/resources/ui/src/utils/EntityLink.ts @@ -92,13 +92,18 @@ export default class EntityLink { /** * * @param string columnFqn + * @param boolean withQuotes for the column name if nested column name is present * @returns column name for table entity */ - static getTableColumnNameFromColumnFqn(columnFqn: string) { + static getTableColumnNameFromColumnFqn(columnFqn: string, withQuotes = true) { const columnName = getPartialNameFromTableFQN(columnFqn, [ FqnPart.NestedColumn, ]); + if (!withQuotes) { + return columnName; + } + return columnName.includes(FQN_SEPARATOR_CHAR) ? `"${columnName}"` : columnName; 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 4061a55e95d..e5ad4fbb6bc 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 @@ -13,12 +13,16 @@ import React from 'react'; import { OperationPermission } from '../context/PermissionProvider/PermissionProvider.interface'; import { TagLabel } from '../generated/entity/data/container'; +import { Column } from '../generated/entity/data/table'; import { ExtraTableDropdownOptions, + findColumnByEntityLink, getEntityIcon, getTagsWithoutTier, getTierTags, + updateColumnInNestedStructure, } from '../utils/TableUtils'; +import EntityLink from './EntityLink'; jest.mock( '../components/Entity/EntityExportModalProvider/EntityExportModalProvider.component', @@ -40,6 +44,12 @@ jest.mock('react-router-dom', () => ({ useHistory: jest.fn(), })); +// Mock EntityLink methods +jest.mock('./EntityLink', () => ({ + getTableColumnNameFromColumnFqn: jest.fn(), + getTableEntityLink: jest.fn(), +})); + describe('TableUtils', () => { it('getTierTags should return the correct usage percentile', () => { const tags = [ @@ -72,6 +82,162 @@ describe('TableUtils', () => { expect(result).toBeNull(); }); + describe('findColumnByEntityLink', () => { + const mockTableFqn = 'sample_data.ecommerce_db.shopify.dim_location'; + const mockEntityLink = + '<#E::table::sample_data.ecommerce_db.shopify.dim_location::columns::column1>'; + + beforeEach(() => { + jest.clearAllMocks(); + ( + EntityLink.getTableColumnNameFromColumnFqn as jest.Mock + ).mockImplementation((fqn) => fqn.split('.').pop() || ''); + (EntityLink.getTableEntityLink as jest.Mock).mockImplementation( + (tableFqn, columnName) => + `<#E::table::${tableFqn}::columns::${columnName}>` + ); + }); + + it('should find a column by entity link in a flat structure', () => { + const columns: Column[] = [ + { + name: 'column1', + fullyQualifiedName: + 'sample_data.ecommerce_db.shopify.dim_location.column1', + } as Column, + { + name: 'column2', + fullyQualifiedName: + 'sample_data.ecommerce_db.shopify.dim_location.column2', + } as Column, + ]; + + const result = findColumnByEntityLink( + mockTableFqn, + columns, + mockEntityLink + ); + + expect(result).toEqual(columns[0]); + expect(EntityLink.getTableColumnNameFromColumnFqn).toHaveBeenCalledWith( + 'sample_data.ecommerce_db.shopify.dim_location.column1', + false + ); + expect(EntityLink.getTableEntityLink).toHaveBeenCalledWith( + mockTableFqn, + 'column1' + ); + }); + + it('should return null if no matching column is found', () => { + const columns: Column[] = [ + { + name: 'column1', + fullyQualifiedName: + 'sample_data.ecommerce_db.shopify.dim_location.column1', + } as Column, + ]; + + const nonExistentEntityLink = + '<#E::table::sample_data.ecommerce_db.shopify.dim_location::columns::nonExistentColumn>'; + + const result = findColumnByEntityLink( + mockTableFqn, + columns, + nonExistentEntityLink + ); + + expect(result).toBeNull(); + }); + }); + + describe('updateColumnInNestedStructure', () => { + it('should update a column in a flat structure', () => { + const columns: Column[] = [ + { + name: 'column1', + fullyQualifiedName: + 'sample_data.ecommerce_db.shopify.dim_location.column1', + description: 'old description', + } as Column, + { + name: 'column2', + fullyQualifiedName: + 'sample_data.ecommerce_db.shopify.dim_location.column2', + description: 'description 2', + } as Column, + ]; + + const targetFqn = 'sample_data.ecommerce_db.shopify.dim_location.column1'; + const update = { description: 'new description' }; + + const result = updateColumnInNestedStructure(columns, targetFqn, update); + + expect(result[0].description).toBe('new description'); + expect(result[1].description).toBe('description 2'); + }); + + it('should update a column in a nested structure', () => { + const columns: Column[] = [ + { + name: 'column1', + fullyQualifiedName: + 'sample_data.ecommerce_db.shopify.dim_location.column1', + description: 'description 1', + } as Column, + { + name: 'parentColumn', + fullyQualifiedName: + 'sample_data.ecommerce_db.shopify.dim_location.parentColumn', + description: 'parent description', + children: [ + { + name: 'nestedColumn', + fullyQualifiedName: + 'sample_data.ecommerce_db.shopify.dim_location.parentColumn.nestedColumn', + description: 'nested description', + } as Column, + ], + } as Column, + ]; + + const targetFqn = + 'sample_data.ecommerce_db.shopify.dim_location.parentColumn.nestedColumn'; + const update = { description: 'updated nested description' }; + + const result = updateColumnInNestedStructure(columns, targetFqn, update); + + expect(result[0].description).toBe('description 1'); + expect(result[1].description).toBe('parent description'); + expect(result[1].children?.[0].description).toBe( + 'updated nested description' + ); + }); + + it('should return the original columns if no matching column is found', () => { + const columns: Column[] = [ + { + name: 'column1', + fullyQualifiedName: + 'sample_data.ecommerce_db.shopify.dim_location.column1', + description: 'description 1', + } as Column, + ]; + + const nonExistentFqn = + 'sample_data.ecommerce_db.shopify.dim_location.nonExistentColumn'; + const update = { description: 'new description' }; + + const result = updateColumnInNestedStructure( + columns, + nonExistentFqn, + update + ); + + expect(result).toEqual(columns); + }); + }); + describe('ExtraTableDropdownOptions', () => { it('should render import button when user has editAll permission', () => { const permission = { 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 2d73b781dfd..c764334884b 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/TableUtils.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/utils/TableUtils.tsx @@ -1214,3 +1214,85 @@ export const getTableWidgetFromKey = ( ); } }; + +/** + * Helper function to find a column by entity link in a nested structure + * Recursively searches through the column hierarchy to find a matching column + * + * @param tableFqn - The fully qualified name of the table + * @param columns - Array of columns to search through + * @param entityLink - The entity link to match against + * @returns The matching column or null if not found + */ +export const findColumnByEntityLink = ( + tableFqn: string, + columns: Column[], + entityLink: string +): Column | null => { + for (const column of columns) { + const columnName = EntityLink.getTableColumnNameFromColumnFqn( + column.fullyQualifiedName ?? '', + false + ); + + // Generate the entity link for this column and compare with the target entity link + const columnEntityLink = EntityLink.getTableEntityLink( + tableFqn, + columnName + ); + if (columnEntityLink === entityLink) { + return column; + } + + // If this column has children, recursively search them + if (column.children && column.children.length > 0) { + const found = findColumnByEntityLink( + tableFqn, + column.children, + entityLink + ); + if (found) { + return found; + } + } + } + + return null; +}; + +/** + * Helper function to update a column in a nested structure + * Recursively traverses the column hierarchy to find and update the target column + * + * @param columns - Array of columns to search through + * @param targetFqn - The fully qualified name of the column to update + * @param update - The properties to update on the matching column + * @returns Updated array of columns with the target column modified + */ +export const updateColumnInNestedStructure = ( + columns: Column[], + targetFqn: string, + update: Partial +): Column[] => { + return columns.map((column: Column) => { + if (column.fullyQualifiedName === targetFqn) { + return { + ...column, + ...update, + }; + } + // If this column has children, recursively search them + else if (column.children && column.children.length > 0) { + return { + ...column, + children: updateColumnInNestedStructure( + column.children, + targetFqn, + update + ), + }; + } else { + return column; + } + }); +};