fix the suggestion not visible for nested columns (#20786)

* fix the suggestion not visible for nested columns

* fix the playwright test
This commit is contained in:
Ashish Gupta 2025-04-13 10:48:11 +05:30 committed by GitHub
parent 54bb3e80b8
commit aca52df600
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 276 additions and 26 deletions

View File

@ -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],
];

View File

@ -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]

View File

@ -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(

View File

@ -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,

View File

@ -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;

View File

@ -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 = {

View File

@ -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>
): 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;
}
});
};