open-metadata · chirag-madlani · Dec 21, 2023 · Dec 21, 2023 · Dec 21, 2023 · Dec 21, 2023
@@ -263,7 +263,6 @@ const PIPELINE_DETAILS_FOR_VERSION_TEST = {
   tasks: [
     {
       name: 'cypress_task_1',
-      displayName: 'cypress_task_1',
       fullyQualifiedName: `sample_airflow.${PIPELINE_NAME}.cypress_task_1`,
       sourceUrl:
         'http://localhost:8080/taskinstance/list/?flt1_dag_id_equals=assert_table_exists',
@@ -273,7 +272,6 @@ const PIPELINE_DETAILS_FOR_VERSION_TEST = {
     },
     {
       name: 'cypress_task_2',
-      displayName: 'cypress_task_2',
       fullyQualifiedName: `sample_airflow.${PIPELINE_NAME}.cypress_task_2`,
       description: 'Description for task cypress_task_2',
       sourceUrl:
@@ -746,6 +744,8 @@ export const ENTITY_DETAILS_FOR_VERSION_TEST = {
     entityCreationDetails: TABLE_DETAILS_FOR_VERSION_TEST,
     entityPatchPayload: TABLE_PATCH_PAYLOAD,
     isChildrenExist: true,
+    childFieldNameToCheck: 'shop_id',
+    columnDisplayNameToUpdate: 'Shop Id',
     childSelector: 'data-row-key',
     entityAddedDescription: `Description for ${TABLE_NAME}`,
     updatedTagEntityChildName: 'user_id',
@@ -759,6 +759,7 @@ export const ENTITY_DETAILS_FOR_VERSION_TEST = {
     entityCreationDetails: TOPIC_DETAILS_FOR_VERSION_TEST,
     entityPatchPayload: TOPIC_PATCH_PAYLOAD,
     isChildrenExist: true,
+    childFieldNameToCheck: 'first_name',
     childSelector: 'data-row-key',
     entityAddedDescription: `Description for ${TOPIC_NAME}`,
     updatedTagEntityChildName: 'default',
@@ -781,6 +782,7 @@ export const ENTITY_DETAILS_FOR_VERSION_TEST = {
     entityCreationDetails: PIPELINE_DETAILS_FOR_VERSION_TEST,
     entityPatchPayload: PIPELINE_PATCH_PAYLOAD,
     isChildrenExist: true,
+    childFieldNameToCheck: 'cypress_task_2',
     childSelector: 'data-row-key',
     entityAddedDescription: `Description for ${PIPELINE_NAME}`,
     updatedTagEntityChildName: 'cypress_task_1',
@@ -807,6 +809,7 @@ export const ENTITY_DETAILS_FOR_VERSION_TEST = {
     entityCreationDetails: CONTAINER_DETAILS_FOR_VERSION_TEST,
     entityPatchPayload: CONTAINER_PATCH_PAYLOAD,
     isChildrenExist: true,
+    childFieldNameToCheck: 'column_2',
     childSelector: 'data-row-key',
     entityAddedDescription: `Description for ${CONTAINER_NAME}`,
     updatedTagEntityChildName: 'column_1',
@@ -820,6 +823,7 @@ export const ENTITY_DETAILS_FOR_VERSION_TEST = {
     entityCreationDetails: SEARCH_INDEX_DETAILS_FOR_VERSION_TEST,
     entityPatchPayload: SEARCH_INDEX_PATCH_PAYLOAD,
     isChildrenExist: true,
+    childFieldNameToCheck: 'description',
     childSelector: 'data-row-key',
     entityAddedDescription: `Description for ${SEARCH_INDEX_NAME}`,
     updatedTagEntityChildName: 'name',

@@ -14,6 +14,7 @@
 // eslint-disable-next-line spaced-comment
 /// <reference types="Cypress" />
 
+import { isEmpty } from 'lodash';
 import {
   addOwner,
   addTier,
@@ -110,7 +111,7 @@ describe('Version page tests for data assets', () => {
           cy.login();
         });
 
-        it(`${entityType} version page should show description and tag changes properly`, () => {
+        it(`${entityType} version page should show description, tag and child field name changes properly`, () => {
           visitEntityDetailsVersionPage(
             entityDetails,
             entityId,
@@ -134,6 +135,13 @@ describe('Version page tests for data assets', () => {
             .scrollIntoView()
             .should('be.visible');
 
+          // Check if child field names are displayed properly on version page
+          if (!isEmpty(entityDetails.childFieldNameToCheck)) {
+            cy.get(
+              `[${entityDetails.childSelector}="${entityDetails.childFieldNameToCheck}"]`
+            ).should('contain', entityDetails.childFieldNameToCheck);
+          }
+
           if (entityDetails.isChildrenExist) {
             cy.get(
               `[${entityDetails.childSelector}="${entityDetails.updatedTagEntityChildName}"] .diff-added [data-testid="tag-PersonalData.Personal"]`
@@ -159,6 +167,60 @@ describe('Version page tests for data assets', () => {
           }
         });
 
+        if (entityType === 'Table') {
+          it(`${entityType} version page should show column display name changes properly`, () => {
+            visitEntityDetailsPage({
+              term: entityDetails.name,
+              serviceName: entityDetails.serviceName,
+              entity: entityDetails.entity,
+            });
+
+            cy.get('[data-testid="version-button"]').as('versionButton');
+
+            cy.get('@versionButton').contains('0.2');
+
+            cy.get(
+              `[data-row-key$="${entityDetails.childFieldNameToCheck}"] [data-testid="edit-displayName-button"]`
+            ).click({ waitForAnimations: true });
+
+            cy.get('#displayName')
+              .clear()
+              .type(entityDetails.columnDisplayNameToUpdate);
+
+            interceptURL('PATCH', `/api/v1/tables/*`, `updateColumnName`);
+
+            cy.get('.ant-modal-footer [data-testid="save-button"]').click();
+
+            verifyResponseStatusCode(`@updateColumnName`, 200);
+
+            interceptURL(
+              'GET',
+              `/api/v1/${entityDetails.entity}/name/${entityFQN}?*include=all`,
+              `get${entityType}Details`
+            );
+            interceptURL(
+              'GET',
+              `/api/v1/${entityDetails.entity}/${entityId}/versions`,
+              'getVersionsList'
+            );
+            interceptURL(
+              'GET',
+              `/api/v1/${entityDetails.entity}/${entityId}/versions/0.2`,
+              'getSelectedVersionDetails'
+            );
+
+            cy.get('@versionButton').contains('0.2').click();
+
+            verifyResponseStatusCode(`@get${entityType}Details`, 200);
+            verifyResponseStatusCode('@getVersionsList', 200);
+            verifyResponseStatusCode('@getSelectedVersionDetails', 200);
+
+            cy.get(
+              `[data-row-key$="${entityDetails.childFieldNameToCheck}"] [data-testid="diff-added"]`
+            ).should('contain', entityDetails.columnDisplayNameToUpdate);
+          });
+        }
+
         it(`${entityType} version page should show owner changes properly`, () => {
           visitEntityDetailsPage({
             term: entityDetails.name,

@@ -32,6 +32,7 @@ import { TABLE_SCROLL_VALUE } from '../../constants/Table.constants';
 import { EntityTabs, EntityType } from '../../enums/entity.enum';
 import { ChangeDescription, Task } from '../../generated/entity/data/pipeline';
 import { TagSource } from '../../generated/type/schema';
+import { getEntityName } from '../../utils/EntityUtils';
 import {
   getCommonExtraInfoForVersionDetails,
   getEntityVersionByField,
@@ -104,11 +105,11 @@ const PipelineVersion: FC<PipelineVersionProp> = ({
         title: t('label.task-entity', {
           entity: t('label.column-plural'),
         }),
-        dataIndex: 'displayName',
-        key: 'displayName',
+        dataIndex: 'name',
+        key: 'name',
         width: 250,
-        render: (displayName) => (
-          <RichTextEditorPreviewer markdown={displayName} />
+        render: (_, record) => (
+          <RichTextEditorPreviewer markdown={getEntityName(record)} />
         ),
       },
       {

@@ -38,7 +38,12 @@ jest.mock('../../components/Tag/TagsContainerV2/TagsContainerV2', () =>
 
 jest.mock(
   '../../components/common/RichTextEditor/RichTextEditorPreviewer',
-  () => jest.fn().mockImplementation(() => <div>RichTextEditorPreviewer</div>)
+  () =>
+    jest
+      .fn()
+      .mockImplementation(({ markdown }) => (
+        <div data-testid="rich-text-editor-previewer">{markdown}</div>
+      ))
 );
 
 jest.mock('../../components/Tag/TagsViewer/TagsViewer', () =>
@@ -155,4 +160,16 @@ describe('PipelineVersion tests', () => {
       '/pipeline/sample_airflow.snowflake_etl/versions/0.3/custom_properties'
     );
   });
+
+  it('Should show task name if no displayName is present', async () => {
+    await act(async () => {
+      render(<PipelineVersion {...pipelineVersionMockProps} />, {
+        wrapper: MemoryRouter,
+      });
+    });
+
+    const taskWithoutDisplayName = screen.getByText('snowflake_task');
+
+    expect(taskWithoutDisplayName).toBeInTheDocument();
+  });
 });
@@ -11,8 +11,7 @@
  *  limitations under the License.
  */
 
-import Icon from '@ant-design/icons';
-import { Tooltip, Typography } from 'antd';
+import { Button, Tooltip, Typography } from 'antd';
 import { ColumnsType } from 'antd/lib/table';
 import { ExpandableConfig } from 'antd/lib/table/interface';
 import {
@@ -36,7 +35,10 @@ import { EntityName } from '../../components/Modals/EntityNameModal/EntityNameMo
 import { ColumnFilter } from '../../components/Table/ColumnFilter/ColumnFilter.component';
 import TableDescription from '../../components/TableDescription/TableDescription.component';
 import TableTags from '../../components/TableTags/TableTags.component';
-import { NO_DATA_PLACEHOLDER } from '../../constants/constants';
+import {
+  DE_ACTIVE_COLOR,
+  NO_DATA_PLACEHOLDER,
+} from '../../constants/constants';
 import { TABLE_SCROLL_VALUE } from '../../constants/Table.constants';
 import { EntityType } from '../../enums/entity.enum';
 import { Column } from '../../generated/entity/data/table';
@@ -366,11 +368,18 @@ const SchemaTable = ({
               ) : null}
               {(tablePermissions?.EditAll ||
                 tablePermissions?.EditDisplayName) && (
-                <Icon
-                  className="hover-cell-icon text-left m-t-xss"
-                  component={IconEdit}
-                  onClick={() => handleEditDisplayNameClick(record)}
-                />
+                <Button
+                  className="cursor-pointer hover-cell-icon w-fit-content"
+                  data-testid="edit-displayName-button"
+                  style={{
+                    color: DE_ACTIVE_COLOR,
+                    padding: 0,
+                    border: 'none',
+                    background: 'transparent',
+                  }}
+                  onClick={() => handleEditDisplayNameClick(record)}>
+                  <IconEdit />
+                </Button>
               )}
             </div>
           );

@@ -11,9 +11,9 @@
  *  limitations under the License.
  */
 
-import { Col, Row, Space, Table, Tooltip } from 'antd';
+import { Col, Row, Table, Tooltip } from 'antd';
 import { ColumnsType } from 'antd/lib/table';
-import { isUndefined } from 'lodash';
+import { isEmpty, isUndefined } from 'lodash';
 import React, { useCallback, useEffect, useMemo, useState } from 'react';
 import { useTranslation } from 'react-i18next';
 import FilterTablePlaceHolder from '../../components/common/ErrorWithPlaceholder/FilterTablePlaceHolder';
@@ -46,7 +46,7 @@ function VersionTable<T extends Column | SearchIndexField>({
   deletedColumnConstraintDiffs,
   addedTableConstraintDiffs,
   deletedTableConstraintDiffs,
-}: VersionTableProps<T>) {
+}: Readonly<VersionTableProps<T>>) {
   const [searchedColumns, setSearchedColumns] = useState<Array<T>>([]);
   const { t } = useTranslation();
 
@@ -115,14 +115,16 @@ function VersionTable<T extends Column | SearchIndexField>({
       });
 
       return (
-        <Space
-          align="start"
-          className="w-max-90 vertical-align-inherit"
-          size={2}>
-          {deletedConstraintIcon}
-          {addedConstraintIcon}
-          <RichTextEditorPreviewer markdown={name} />
-        </Space>
+        <div className="d-inline-flex flex-column hover-icon-group w-full">
+          <div className="d-inline-flex">
+            {deletedConstraintIcon}
+            {addedConstraintIcon}
+            <RichTextEditorPreviewer markdown={name} />
+          </div>
+          {!isEmpty(record.displayName) ? (
+            <RichTextEditorPreviewer markdown={record.displayName ?? ''} />
+          ) : null}
+        </div>
       );
     },
     [
@@ -184,7 +186,7 @@ function VersionTable<T extends Column | SearchIndexField>({
               <RichTextEditorPreviewer markdown={description} />
               {getFrequentlyJoinedColumns(
                 columnName,
-                joins,
+                joins ?? [],
                 t('label.frequently-joined-column-plural')
               )}
             </>

@@ -22,7 +22,7 @@ import {
 export interface VersionTableProps<T extends Column | SearchIndexField> {
   columnName: string;
   columns: T[];
-  joins: Array<ColumnJoins>;
+  joins?: Array<ColumnJoins>;
   addedColumnConstraintDiffs?: FieldChange[];
   deletedColumnConstraintDiffs?: FieldChange[];
   addedTableConstraintDiffs?: FieldChange[];

@@ -0,0 +1,52 @@
+/*
+ *  Copyright 2023 Collate.
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+import { render, screen } from '@testing-library/react';
+import React from 'react';
+import { mockVersionTableProps } from '../../mocks/VersionTable.mock';
+import VersionTable from './VersionTable.component';
+
+jest.mock(
+  '../../components/common/ErrorWithPlaceholder/FilterTablePlaceHolder',
+  () => jest.fn().mockImplementation(() => <div>FilterTablePlaceHolder</div>)
+);
+
+jest.mock('../common/SearchBarComponent/SearchBar.component', () =>
+  jest.fn().mockImplementation(() => <div>SearchBar</div>)
+);
+
+jest.mock('../Tag/TagsViewer/TagsViewer', () =>
+  jest.fn().mockImplementation(() => <div>TagsViewer</div>)
+);
+
+jest.mock('../common/RichTextEditor/RichTextEditorPreviewer', () =>
+  jest
+    .fn()
+    .mockImplementation(({ markdown }) => (
+      <div data-testid="rich-text-editor-previewer">{markdown}</div>
+    ))
+);
+
+describe('VersionTable component', () => {
+  it('VersionTable should show column display names along with name if present', () => {
+    render(<VersionTable {...mockVersionTableProps} />);
+
+    // Check Names
+    expect(screen.getByText('address_id')).toBeInTheDocument();
+    expect(screen.getByText('shop_id')).toBeInTheDocument();
+    expect(screen.getByText('first_name')).toBeInTheDocument();
+
+    // Check Display Name
+    expect(screen.getByText('Address Id')).toBeInTheDocument();
+  });
+});
@@ -78,7 +78,6 @@ export const mockPipelineData = {
   tasks: [
     {
       name: 'snowflake_task',
-      displayName: 'Snowflake Task',
       fullyQualifiedName: 'sample_airflow.snowflake_etl.snowflake_task',
       sourceUrl:
         'http://localhost:8080/taskinstance/list/?flt1_dag_id_equals=assert_table_exists',