diff --git a/openmetadata-ui/src/main/resources/ui/cypress/constants/tagsAddRemove.constants.js b/openmetadata-ui/src/main/resources/ui/cypress/constants/tagsAddRemove.constants.js new file mode 100644 index 00000000000..dc78c2e42ee --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/cypress/constants/tagsAddRemove.constants.js @@ -0,0 +1,56 @@ +/* + * 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. + */ + +export const TAGS_ADD_REMOVE_ENTITIES = [ + { + term: 'sales', + displayName: 'sales', + entity: 'tables', + serviceName: 'sample_data', + fieldName: 'SKU', + tags: ['PersonalData.Personal', 'PII.Sensitive'], + }, + { + term: 'address_book', + displayName: 'address_book', + entity: 'topics', + serviceName: 'sample_kafka', + fieldName: 'AddressBook', + tags: ['PersonalData.Personal', 'PII.Sensitive'], + }, + { + term: 'deck.gl Demo', + displayName: 'deck.gl Demo', + entity: 'dashboards', + insideEntity: 'charts', + serviceName: 'sample_superset', + fieldName: 'e3cfd274-44f8-4bf3-b75d-d40cf88869ba', + tags: ['PersonalData.Personal', 'PII.Sensitive'], + }, + { + term: 'dim_address_etl', + displayName: 'dim_address etl', + entity: 'pipelines', + serviceName: 'sample_airflow', + fieldName: 'dim_address_task', + tags: ['PersonalData.Personal', 'PII.Sensitive'], + }, + { + term: 'eta_predictions', + displayName: 'ETA Predictions', + entity: 'mlmodels', + serviceName: 'mlflow_svc', + fieldName: 'sales', + tags: ['PersonalData.Personal', 'PII.Sensitive'], + }, +]; diff --git a/openmetadata-ui/src/main/resources/ui/cypress/e2e/Flow/TagsAddRemove.spec.js b/openmetadata-ui/src/main/resources/ui/cypress/e2e/Flow/TagsAddRemove.spec.js new file mode 100644 index 00000000000..bb607f792bc --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/cypress/e2e/Flow/TagsAddRemove.spec.js @@ -0,0 +1,120 @@ +/* + * 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 { + interceptURL, + verifyResponseStatusCode, + visitEntityDetailsPage, +} from '../../common/common'; +import { TAGS_ADD_REMOVE_ENTITIES } from '../../constants/tagsAddRemove.constants'; + +const addTags = (tag) => { + cy.get('[data-testid="tag-selector"]').should('be.visible').click().type(tag); + + cy.get('.ant-select-item-option-content').should('be.visible').click(); + cy.get('[data-testid="tag-selector"] > .ant-select-selector').contains(tag); +}; + +const checkTags = (tag, checkForParentEntity) => { + if (checkForParentEntity) { + cy.get('[data-testid="entity-tags"] > :nth-child(2) > .ant-space') + .scrollIntoView() + .should('be.visible') + .contains(tag); + } else { + cy.get(`[data-testid="tag-${tag}"]`).should('be.visible'); + } +}; + +const removeTags = (tag, checkForParentEntity) => { + if (checkForParentEntity) { + cy.get('[data-testid="entity-tags"] [data-testid="edit-button"] ') + .scrollIntoView() + .should('be.visible') + .click(); + + cy.get('.ant-select-selection-item-remove > .anticon') + .should('be.visible') + .click(); + + cy.get('[data-testid="saveAssociatedTag"]').should('be.visible').click(); + } else { + cy.get(`[data-testid="remove-${tag}-tag"]`).should('be.visible').click(); + } + verifyResponseStatusCode('@tagsChange', 200); +}; + +describe('Check if tags addition and removal flow working properly from tables', () => { + beforeEach(() => { + cy.login(); + }); + + TAGS_ADD_REMOVE_ENTITIES.map((entityDetails) => + it(`Adding and removing tags to the ${entityDetails.entity} entity should work properly`, () => { + visitEntityDetailsPage( + entityDetails.term, + entityDetails.serviceName, + entityDetails.entity + ); + + cy.get( + '[data-testid="entity-tags"] [data-testid="tags-wrapper"] > [data-testid="tag-container"] [data-testid="tags"] > [data-testid="add-tag"] > span' + ) + .should('be.visible') + .click(); + + addTags(entityDetails.tags[0]); + + interceptURL('PATCH', `/api/v1/${entityDetails.entity}/*`, 'tagsChange'); + + cy.get('[data-testid="saveAssociatedTag"]').should('be.visible').click(); + + verifyResponseStatusCode('@tagsChange', 200); + + checkTags(entityDetails.tags[0], true); + + removeTags(entityDetails.tags[0], true); + + if (entityDetails.entity === 'mlmodels') { + cy.get( + `[data-testid="feature-card-${entityDetails.fieldName}"] [data-testid="tag-container"] [data-testid="tags"] > [data-testid="add-tag"] > span` + ) + .should('be.visible') + .click(); + } else { + cy.get( + `.ant-table-tbody [data-testid="tag-container"] [data-testid="add-tag"]>span` + ) + .eq(0) + .should('be.visible') + .click(); + } + + entityDetails.tags.map((tag) => addTags(tag)); + + interceptURL( + 'PATCH', + `/api/v1/${entityDetails.insideEntity ?? entityDetails.entity}/*`, + 'tagsChange' + ); + + cy.get('[data-testid="saveAssociatedTag"]').should('be.visible').click(); + + verifyResponseStatusCode('@tagsChange', 200); + + entityDetails.tags.map((tag) => checkTags(tag)); + + entityDetails.tags.map((tag) => removeTags(tag)); + }) + ); +}); diff --git a/openmetadata-ui/src/main/resources/ui/cypress/e2e/Pages/Glossary.spec.js b/openmetadata-ui/src/main/resources/ui/cypress/e2e/Pages/Glossary.spec.js index 0aba47ae575..c5a0f8447ba 100644 --- a/openmetadata-ui/src/main/resources/ui/cypress/e2e/Pages/Glossary.spec.js +++ b/openmetadata-ui/src/main/resources/ui/cypress/e2e/Pages/Glossary.spec.js @@ -927,6 +927,7 @@ describe('Glossary page should work properly', () => { }); it('Remove Glossary term from entity should work properly', () => { + const glossaryName = NEW_GLOSSARY_1.name; const term = NEW_GLOSSARY_1_TERMS.term_1.name; const entity = SEARCH_ENTITY_TABLE.table_3; @@ -972,7 +973,9 @@ describe('Glossary page should work properly', () => { .and('not.contain', 'Personal'); // Remove the added column tag from entity interceptURL('PATCH', '/api/v1/tables/*', 'removeSchemaTags'); - cy.get('[data-testid="remove"]').eq(0).should('be.visible').click(); + cy.get(`[data-testid="remove-${glossaryName}.${term}-tag"]`) + .should('be.visible') + .click(); verifyResponseStatusCode('@removeSchemaTags', 200); cy.get('[data-testid="tags"]') diff --git a/openmetadata-ui/src/main/resources/ui/src/components/MlModelDetail/MlModelFeaturesList.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/MlModelDetail/MlModelFeaturesList.test.tsx index 141facc3099..d27dc76be53 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/MlModelDetail/MlModelFeaturesList.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/MlModelDetail/MlModelFeaturesList.test.tsx @@ -176,9 +176,13 @@ describe('Test MlModel feature list', () => { }); const featureList = await screen.findByTestId('feature-list'); - const featureCards = await screen.findAllByTestId('feature-card'); + const salesFeatureCard = await screen.findByTestId('feature-card-sales'); + const personaFeatureCard = await screen.findByTestId( + 'feature-card-persona' + ); expect(featureList).toBeInTheDocument(); - expect(featureCards).toHaveLength(mockData['mlFeatures'].length); + expect(salesFeatureCard).toBeInTheDocument(); + expect(personaFeatureCard).toBeInTheDocument(); }); }); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/MlModelDetail/MlModelFeaturesList.tsx b/openmetadata-ui/src/main/resources/ui/src/components/MlModelDetail/MlModelFeaturesList.tsx index 22f3e175f0f..4d419488ed3 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/MlModelDetail/MlModelFeaturesList.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/MlModelDetail/MlModelFeaturesList.tsx @@ -192,7 +192,7 @@ const MlModelFeaturesList: FC = ({ diff --git a/openmetadata-ui/src/main/resources/ui/src/components/PipelineDetails/PipelineDetails.component.tsx b/openmetadata-ui/src/main/resources/ui/src/components/PipelineDetails/PipelineDetails.component.tsx index 5547e326cb3..8407da5cbbd 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/PipelineDetails/PipelineDetails.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/PipelineDetails/PipelineDetails.component.tsx @@ -15,7 +15,7 @@ import { Card, Col, Radio, Row, Space, Table, Tabs, Tooltip } from 'antd'; import { ColumnsType } from 'antd/lib/table'; import { AxiosError } from 'axios'; import { compare, Operation } from 'fast-json-patch'; -import { isEmpty } from 'lodash'; +import { isEmpty, isUndefined } from 'lodash'; import { EntityTags, ExtraInfo, TagOption } from 'Models'; import React, { RefObject, @@ -29,6 +29,7 @@ import { Link, Redirect, useHistory, useParams } from 'react-router-dom'; import { getAllFeeds, postFeedById, postThread } from 'rest/feedsAPI'; import { restorePipeline } from 'rest/pipelineAPI'; import AppState from '../../AppState'; +import { ReactComponent as ExternalLinkIcon } from '../../assets/svg/external-link.svg'; import { FQN_SEPARATOR_CHAR } from '../../constants/char.constants'; import { getPipelineDetailsPath, ROUTES } from '../../constants/constants'; import { EntityField } from '../../constants/Feeds.constants'; @@ -94,7 +95,6 @@ import { PipeLineDetailsProp } from './PipelineDetails.interface'; const PipelineDetails = ({ entityName, slashedPipelineName, - pipelineUrl, pipelineDetails, descriptionUpdateHandler, followers, @@ -102,7 +102,6 @@ const PipelineDetails = ({ unfollowPipelineHandler, tagUpdateHandler, settingsUpdateHandler, - tasks, taskUpdateHandler, versionHandler, pipelineFQN, @@ -183,8 +182,11 @@ const PipelineDetails = ({ const { getEntityPermission } = usePermissionProvider(); const tasksInternal = useMemo( - () => tasks.map((t) => ({ ...t, tags: t.tags ?? [] })), - [tasks] + () => + pipelineDetails.tasks + ? pipelineDetails.tasks.map((t) => ({ ...t, tags: t.tags ?? [] })) + : [], + [pipelineDetails.tasks] ); const fetchResourcePermission = useCallback(async () => { @@ -232,13 +234,17 @@ const PipelineDetails = ({ key: EntityInfo.TIER, value: tier?.tagFQN ? tier.tagFQN.split(FQN_SEPARATOR_CHAR)[1] : '', }, - { - key: `${serviceType} ${EntityInfo.URL}`, - value: pipelineUrl, - placeholderText: entityName, - isLink: true, - openInNewTab: true, - }, + ...(pipelineDetails.pipelineUrl + ? [ + { + key: `${serviceType} ${EntityInfo.URL}`, + value: pipelineDetails.pipelineUrl, + placeholderText: entityName, + isLink: true, + openInNewTab: true, + }, + ] + : []), ]; const onTaskUpdate = async (taskDescription: string) => { @@ -463,40 +469,45 @@ const PipelineDetails = ({ setEditTaskTags({ task: { ...task, tags: [] }, index }); }; - const handleTableTagSelection = (selectedTags?: Array) => { - if (selectedTags && editTask) { - const prevTags = editTask.task.tags?.filter((tag) => - selectedTags.some((selectedTag) => selectedTag.tagFQN === tag.tagFQN) - ); - - const newTags = selectedTags - .filter( - (selectedTag) => - !editTask.task.tags?.some( - (tag) => tag.tagFQN === selectedTag.tagFQN - ) - ) - .map((tag) => ({ - labelType: 'Manual', - state: 'Confirmed', - source: tag.source, - tagFQN: tag.tagFQN, - })); - - const updatedTasks: Task[] = [...(pipelineDetails.tasks || [])]; - - const updatedTask = { - ...editTask.task, - tags: [...(prevTags as TagLabel[]), ...newTags], - } as Task; - - updatedTasks[editTask.index] = updatedTask; - - const updatedPipeline = { ...pipelineDetails, tasks: updatedTasks }; - const jsonPatch = compare(pipelineDetails, updatedPipeline); - - taskUpdateHandler(jsonPatch); + const handleTableTagSelection = ( + selectedTags: Array = [], + task: { + task: Task; + index: number; } + ) => { + const selectedTask = isUndefined(editTask) ? task : editTask; + const prevTags = selectedTask.task.tags?.filter((tag) => + selectedTags.some((selectedTag) => selectedTag.tagFQN === tag.tagFQN) + ); + + const newTags = selectedTags + .filter( + (selectedTag) => + !selectedTask.task.tags?.some( + (tag) => tag.tagFQN === selectedTag.tagFQN + ) + ) + .map((tag) => ({ + labelType: 'Manual', + state: 'Confirmed', + source: tag.source, + tagFQN: tag.tagFQN, + })); + + const updatedTasks: Task[] = [...(pipelineDetails.tasks || [])]; + + const updatedTask = { + ...selectedTask.task, + tags: [...(prevTags as TagLabel[]), ...newTags], + } as Task; + + updatedTasks[selectedTask.index] = updatedTask; + + const updatedPipeline = { ...pipelineDetails, tasks: updatedTasks }; + const jsonPatch = compare(pipelineDetails, updatedPipeline); + + taskUpdateHandler(jsonPatch); setEditTaskTags(undefined); }; @@ -525,10 +536,13 @@ const PipelineDetails = ({ tagList={tagList ?? []} type="label" onCancel={() => { - handleTableTagSelection(); + setEditTask(undefined); }} onSelectionChange={(tags) => { - handleTableTagSelection(tags); + handleTableTagSelection(tags, { + task: record, + index: index, + }); }} /> )} @@ -549,17 +563,18 @@ const PipelineDetails = ({ key: t('label.name'), dataIndex: 'name', title: t('label.name'), - render: (_, record) => ( - + render: (_, record) => + isEmpty(record.taskUrl) ? ( {getEntityName(record)} - - - ), + ) : ( + + {getEntityName(record)} + + + ), }, { key: t('label.type'), @@ -724,7 +739,7 @@ const PipelineDetails = ({ useEffect(() => { getEntityFeedCount(); - }, [pipelineFQN, description, pipelineDetails, tasks]); + }, [pipelineFQN, description, pipelineDetails]); return ( @@ -842,14 +857,15 @@ const PipelineDetails = ({ rowKey="name" size="small" /> - ) : !isEmpty(tasks) ? ( + ) : !isEmpty(pipelineDetails.tasks) && + !isUndefined(pipelineDetails.tasks) ? (
@@ -911,7 +927,10 @@ const PipelineDetails = ({ {t('label.execution-plural')} }> - + ; slashedPipelineName: TitleBreadcrumbProps['titleLinks']; - tasks: Task[]; paging: Paging; followPipelineHandler: () => void; unfollowPipelineHandler: () => void; diff --git a/openmetadata-ui/src/main/resources/ui/src/components/PipelineDetails/PipelineDetails.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/PipelineDetails/PipelineDetails.test.tsx index 3dd2aacf5ed..d71b8e7523b 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/PipelineDetails/PipelineDetails.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/PipelineDetails/PipelineDetails.test.tsx @@ -19,6 +19,7 @@ import { render, screen, } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import React from 'react'; import { MemoryRouter } from 'react-router-dom'; @@ -81,12 +82,24 @@ const mockTasks = [ }, ]; +const mockTags = [ + { + tagFQN: 'PII.Sensitive', + source: 'Tag', + }, + { + tagFQN: 'PersonalData.Personal', + source: 'Tag', + }, +]; + +const mockTaskUpdateHandler = jest.fn(); + const PipelineDetailsProps = { pipelineUrl: '', - tasks: mockTasks, serviceType: '', users: [], - pipelineDetails: {} as Pipeline, + pipelineDetails: { tasks: mockTasks } as Pipeline, entityLineage: {} as EntityLineage, entityName: '', activeTab: 1, @@ -96,7 +109,7 @@ const PipelineDetailsProps = { followers: [], pipelineTags: [], slashedPipelineName: [], - taskUpdateHandler: jest.fn(), + taskUpdateHandler: mockTaskUpdateHandler, setActiveTabHandler: jest.fn(), followPipelineHandler: jest.fn(), unfollowPipelineHandler: jest.fn(), @@ -198,6 +211,18 @@ jest.mock('../Execution/Execution.component', () => { return jest.fn().mockImplementation(() =>

Executions

); }); +jest.mock('../Tag/TagsContainer/tags-container', () => + jest.fn().mockImplementation(({ onSelectionChange }) => ( +
+
onSelectionChange(mockTags)}> + onSelectionChange +
+
+ )) +); + describe('Test PipelineDetails component', () => { it('Checks if the PipelineDetails component has all the proper components rendered', async () => { const { container } = render( @@ -239,9 +264,15 @@ describe('Test PipelineDetails component', () => { }); it('Should render no tasks data placeholder is tasks list is empty', async () => { - render(, { - wrapper: MemoryRouter, - }); + render( + , + { + wrapper: MemoryRouter, + } + ); const switchContainer = screen.getByTestId('pipeline-task-switch'); @@ -359,4 +390,22 @@ describe('Test PipelineDetails component', () => { expect(obServerElement).toBeInTheDocument(); }); + + it('taskUpdateHandler should be called after the tags are added or removed to a task', async () => { + render(, { + wrapper: MemoryRouter, + }); + + const tagsContainer = screen.getAllByTestId('tags-container'); + + expect(tagsContainer).toHaveLength(2); + + const onSelectionChange = screen.getAllByTestId('onSelectionChange'); + + expect(onSelectionChange).toHaveLength(2); + + await act(async () => userEvent.click(onSelectionChange[0])); + + expect(mockTaskUpdateHandler).toHaveBeenCalledTimes(1); + }); }); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Tag/Tags/tags.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Tag/Tags/tags.test.tsx index 98b3e11d6dd..cd88a1ffe0f 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Tag/Tags/tags.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Tag/Tags/tags.test.tsx @@ -11,7 +11,12 @@ * limitations under the License. */ -import { fireEvent, getByTestId, render } from '@testing-library/react'; +import { + fireEvent, + getByTestId, + queryByTestId, + render, +} from '@testing-library/react'; import { LabelType, State, TagSource } from 'generated/type/tagLabel'; import React from 'react'; import Tags from './tags'; @@ -35,7 +40,7 @@ describe('Test tags Component', () => { ); const tags = getByTestId(container, 'tags'); - const remove = getByTestId(container, 'remove'); + const remove = getByTestId(container, 'remove-test-tag'); expect(tags).toBeInTheDocument(); expect(remove).toBeInTheDocument(); @@ -43,13 +48,13 @@ describe('Test tags Component', () => { it('Component should render properly for add tag button', () => { const { container } = render( - + ); const tags = getByTestId(container, 'tags'); - const remove = getByTestId(container, 'remove'); + const remove = queryByTestId(container, 'remove-test-tag'); expect(tags).toBeInTheDocument(); - expect(remove).toBeInTheDocument(); + expect(remove).toBeNull(); }); it('onClick of X callback function should call', () => { @@ -62,7 +67,7 @@ describe('Test tags Component', () => { tag="test" /> ); - const remove = getByTestId(container, 'remove'); + const remove = getByTestId(container, 'remove-test-tag'); fireEvent.click( remove, new MouseEvent('click', { diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Tag/Tags/tags.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Tag/Tags/tags.tsx index e2aeb5cddcb..7043d3de1f9 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Tag/Tags/tags.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Tag/Tags/tags.tsx @@ -86,7 +86,7 @@ const Tags: FunctionComponent = ({ {editable && isRemovable && ( ) => { e.preventDefault(); e.stopPropagation(); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/TopicDetails/TopicSchema/TopicSchema.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/TopicDetails/TopicSchema/TopicSchema.test.tsx index 11330511140..e65748236e8 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/TopicDetails/TopicSchema/TopicSchema.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/TopicDetails/TopicSchema/TopicSchema.test.tsx @@ -36,6 +36,17 @@ const mockProps: TopicSchemaFieldsProps = { hasTagEditAccess: true, }; +const mockTags = [ + { + tagFQN: 'PII.Sensitive', + source: 'Tag', + }, + { + tagFQN: 'PersonalData.Personal', + source: 'Tag', + }, +]; + jest.mock('../../../utils/TagsUtils', () => ({ fetchTagsAndGlossaryTerms: jest.fn().mockReturnValue([]), })); @@ -63,9 +74,16 @@ jest.mock( ); jest.mock('components/Tag/TagsContainer/tags-container', () => - jest - .fn() - .mockReturnValue(
Tag Container
) + jest.fn().mockImplementation(({ onSelectionChange }) => ( +
+ Tag Container +
onSelectionChange(mockTags)}> + onSelectionChange +
+
+ )) ); jest.mock('components/Tag/TagsViewer/tags-viewer', () => @@ -155,4 +173,20 @@ describe('Topic Schema', () => { expect(editDescriptionButton).toBeNull(); }); + + it('onUpdate should be called after the tags are added or removed to a task', async () => { + render(); + + const tagsContainer = await screen.findAllByTestId('tag-container'); + + expect(tagsContainer).toHaveLength(9); + + const onSelectionChange = await screen.findAllByTestId('onSelectionChange'); + + expect(onSelectionChange).toHaveLength(9); + + await act(async () => userEvent.click(onSelectionChange[0])); + + expect(mockOnUpdate).toHaveBeenCalledTimes(1); + }); }); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/TopicDetails/TopicSchema/TopicSchema.tsx b/openmetadata-ui/src/main/resources/ui/src/components/TopicDetails/TopicSchema/TopicSchema.tsx index 280be4b051c..2f28d812dac 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/TopicDetails/TopicSchema/TopicSchema.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/TopicDetails/TopicSchema/TopicSchema.tsx @@ -86,8 +86,9 @@ const TopicSchemaFields: FC = ({ const handleFieldTagsChange = async ( selectedTags: EntityTags[] = [], - selectedField: Field + field: Field ) => { + const selectedField = isUndefined(editFieldTags) ? field : editFieldTags; const newSelectedTags: TagOption[] = selectedTags.map((tag) => ({ fqn: tag.tagFQN, source: tag.source, @@ -95,11 +96,7 @@ const TopicSchemaFields: FC = ({ const schema = cloneDeep(messageSchema); - updateFieldTags( - schema?.schemaFields, - editFieldTags?.name ?? selectedField.name, - newSelectedTags - ); + updateFieldTags(schema?.schemaFields, selectedField?.name, newSelectedTags); await onUpdate(schema); setEditFieldTags(undefined); diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/PipelineDetails/PipelineDetailsPage.component.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/PipelineDetails/PipelineDetailsPage.component.tsx index fd8b2fe64cd..65dbcf9d3d8 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/PipelineDetails/PipelineDetailsPage.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/PipelineDetails/PipelineDetailsPage.component.tsx @@ -29,7 +29,6 @@ import { patchPipelineDetails, removeFollower, } from 'rest/pipelineAPI'; -import { getServiceByFQN } from 'rest/serviceAPI'; import { getServiceDetailsPath, getVersionPath, @@ -37,8 +36,7 @@ import { import { NO_PERMISSION_TO_VIEW } from '../../constants/HelperTextUtil'; import { EntityType } from '../../enums/entity.enum'; import { ServiceCategory } from '../../enums/service.enum'; -import { Pipeline, Task } from '../../generated/entity/data/pipeline'; -import { Connection } from '../../generated/entity/services/dashboardService'; +import { Pipeline } from '../../generated/entity/data/pipeline'; import { EntityReference } from '../../generated/type/entityReference'; import { Paging } from '../../generated/type/paging'; import jsonData from '../../jsons/en'; @@ -49,7 +47,10 @@ import { } from '../../utils/CommonUtils'; import { getEntityName } from '../../utils/EntityUtils'; import { DEFAULT_ENTITY_PERMISSION } from '../../utils/PermissionsUtils'; -import { defaultFields } from '../../utils/PipelineDetailsUtils'; +import { + defaultFields, + getFormattedPipelineDetails, +} from '../../utils/PipelineDetailsUtils'; import { serviceTypeLogo } from '../../utils/ServiceUtils'; import { showErrorToast } from '../../utils/ToastUtils'; @@ -65,8 +66,6 @@ const PipelineDetailsPage = () => { const [isLoading, setLoading] = useState(true); const [followers, setFollowers] = useState>([]); - const [tasks, setTasks] = useState([]); - const [pipelineUrl, setPipelineUrl] = useState(''); const [displayName, setDisplayName] = useState(''); const [slashedPipelineName, setSlashedPipelineName] = useState< TitleBreadcrumbProps['titleLinks'] @@ -115,28 +114,6 @@ const PipelineDetailsPage = () => { return patchPipelineDetails(pipelineId, jsonPatch); }; - const fetchServiceDetails = (type: string, fqn: string) => { - return new Promise((resolve, reject) => { - getServiceByFQN(type + 's', fqn, ['owner']) - .then((resService) => { - if (resService) { - const hostPort = - (resService.connection?.config as Connection)?.hostPort || ''; - resolve(hostPort); - } else { - throw null; - } - }) - .catch((err: AxiosError) => { - showErrorToast( - err, - jsonData['api-error-messages']['fetch-pipeline-details-error'] - ); - reject(err); - }); - }); - }; - const fetchPipelineDetail = (pipelineFQN: string) => { setLoading(true); getPipelineByFqn(pipelineFQN, defaultFields) @@ -149,8 +126,6 @@ const PipelineDetailsPage = () => { serviceType, displayName, name, - tasks, - pipelineUrl = '', } = res; setDisplayName(displayName || name); setPipelineDetails(res); @@ -181,20 +156,6 @@ const PipelineDetailsPage = () => { timestamp: 0, id: id, }); - - fetchServiceDetails(service.type, service.name ?? '') - .then((hostPort: string) => { - setPipelineUrl(hostPort + pipelineUrl); - const updatedTasks = ((tasks || []) as Task[]).map((task) => ({ - ...task, - taskUrl: hostPort + task.taskUrl, - })); - setTasks(updatedTasks); - setLoading(false); - }) - .catch((err: AxiosError) => { - throw err; - }); } else { setIsError(true); @@ -313,7 +274,8 @@ const PipelineDetailsPage = () => { const response = await patchPipelineDetails(pipelineId, jsonPatch); if (response) { - setTasks(response.tasks || []); + const formattedPipelineDetails = getFormattedPipelineDetails(response); + setPipelineDetails(formattedPipelineDetails); } else { throw jsonData['api-error-messages']['unexpected-server-response']; } @@ -374,12 +336,10 @@ const PipelineDetailsPage = () => { paging={paging} pipelineDetails={pipelineDetails} pipelineFQN={pipelineFQN} - pipelineUrl={pipelineUrl} settingsUpdateHandler={settingsUpdateHandler} slashedPipelineName={slashedPipelineName} tagUpdateHandler={onTagUpdate} taskUpdateHandler={onTaskUpdate} - tasks={tasks} unfollowPipelineHandler={unfollowPipeline} versionHandler={versionHandler} onExtensionUpdate={handleExtensionUpdate} diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/TopicDetails/TopicDetailsPage.component.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/TopicDetails/TopicDetailsPage.component.tsx index aa42fd8c7a7..cd04b66eefb 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/TopicDetails/TopicDetailsPage.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/TopicDetails/TopicDetailsPage.component.tsx @@ -66,6 +66,7 @@ import { getTagsWithoutTier, getTierTags } from '../../utils/TableUtils'; import { showErrorToast } from '../../utils/ToastUtils'; import { getCurrentTopicTab, + getFormattedTopicDetails, topicDetailsTabs, } from '../../utils/TopicDetailsUtils'; @@ -407,7 +408,8 @@ const TopicDetailsPage: FunctionComponent = () => { saveUpdatedTopicData(updatedTopic) .then((res) => { if (res) { - setTopicDetails({ ...res, tags: res.tags ?? [] }); + const formattedTopicDetails = getFormattedTopicDetails(res); + setTopicDetails(formattedTopicDetails); setCurrentVersion(res.version?.toString()); setOwner(res.owner); setTier(getTierTags((res.tags ?? []) as EntityTags[])); diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/PipelineDetailsUtils.test.ts b/openmetadata-ui/src/main/resources/ui/src/utils/PipelineDetailsUtils.test.ts new file mode 100644 index 00000000000..41d2ad4871f --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/src/utils/PipelineDetailsUtils.test.ts @@ -0,0 +1,58 @@ +/* + * 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 { + mockPipelineDetails, + mockPipelineDetailsWithoutTaskTags, + mockSortedPipelineDetails, +} from './mocks/PipelineDetailsUtils.mock'; +import { getFormattedPipelineDetails } from './PipelineDetailsUtils'; + +describe('PipelineDetailsUtils test', () => { + it('getFormattedPipelineDetails should return pipeline details with sorted tags for tasks', () => { + const results = getFormattedPipelineDetails(mockPipelineDetails); + + expect(results).toEqual(mockSortedPipelineDetails); + }); + + it('getFormattedPipelineDetails should return pipeline details without any changes in case no tasks are present in it', () => { + const modifiedPipelineDetails = { + ...mockPipelineDetails, + tasks: undefined, + }; + + const results = getFormattedPipelineDetails(modifiedPipelineDetails); + + expect(results).toEqual(modifiedPipelineDetails); + }); + + it('getFormattedPipelineDetails should return pipeline details without any changes in case no tags are present for the tasks', () => { + const results = getFormattedPipelineDetails( + mockPipelineDetailsWithoutTaskTags + ); + + expect(results).toEqual(mockPipelineDetailsWithoutTaskTags); + }); + + it('getFormattedPipelineDetails should return pipeline details without any changes if empty array is present for tasks field', () => { + const results = getFormattedPipelineDetails({ + ...mockPipelineDetails, + tasks: [], + }); + + expect(results).toEqual({ + ...mockPipelineDetails, + tasks: [], + }); + }); +}); diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/PipelineDetailsUtils.ts b/openmetadata-ui/src/main/resources/ui/src/utils/PipelineDetailsUtils.ts index 85b908bc102..15c3e5dc3d5 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/PipelineDetailsUtils.ts +++ b/openmetadata-ui/src/main/resources/ui/src/utils/PipelineDetailsUtils.ts @@ -12,12 +12,14 @@ */ import { t } from 'i18next'; +import { isUndefined } from 'lodash'; import { TabSpecificField } from '../enums/entity.enum'; import { Pipeline, StatusType, TaskStatus, } from '../generated/entity/data/pipeline'; +import { sortTagsCaseInsensitive } from './CommonUtils'; import { Icons } from './SvgUtils'; export const defaultFields = `${TabSpecificField.FOLLOWERS}, ${TabSpecificField.TAGS}, ${TabSpecificField.OWNER}, @@ -135,3 +137,23 @@ export const getStatusBadgeIcon = (status?: StatusType) => { return ''; } }; + +export const getFormattedPipelineDetails = ( + pipelineDetails: Pipeline +): Pipeline => { + if (pipelineDetails.tasks) { + const updatedTasks = pipelineDetails.tasks.map((task) => ({ + ...task, + // Sorting tags as the response of PATCH request does not return the sorted order + // of tags, but is stored in sorted manner in the database + // which leads to wrong PATCH payload sent after further tags removal + tags: isUndefined(task.tags) + ? undefined + : sortTagsCaseInsensitive(task.tags), + })); + + return { ...pipelineDetails, tasks: updatedTasks }; + } else { + return pipelineDetails; + } +}; diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/TopicDetailsUtils.test.ts b/openmetadata-ui/src/main/resources/ui/src/utils/TopicDetailsUtils.test.ts new file mode 100644 index 00000000000..b999c028549 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/src/utils/TopicDetailsUtils.test.ts @@ -0,0 +1,65 @@ +/* + * 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 { + mockSortedTopicDetails, + mockTopicDetails, +} from './mocks/TopicDetailsUtils.mock'; +import { getFormattedTopicDetails } from './TopicDetailsUtils'; + +describe('TopicDetailsUtils test', () => { + it('getFormattedTopicDetails should return topic details with sorted tags for schema fields', () => { + const results = getFormattedTopicDetails(mockTopicDetails); + + expect(results).toEqual(mockSortedTopicDetails); + }); + + it('getFormattedTopicDetails should return expected results in case no messageSchema present', () => { + const modifiedTopicDetails = { + ...mockTopicDetails, + messageSchema: undefined, + }; + + const results = getFormattedTopicDetails(modifiedTopicDetails); + + expect(results).toEqual({ ...modifiedTopicDetails, tags: [] }); + }); + + it('getFormattedTopicDetails should return expected results in case no schemaFields present', () => { + const modifiedTopicDetails = { + ...mockTopicDetails, + messageSchema: { + ...mockTopicDetails.messageSchema, + schemaFields: undefined, + }, + }; + + const results = getFormattedTopicDetails(modifiedTopicDetails); + + expect(results).toEqual({ ...modifiedTopicDetails, tags: [] }); + }); + + it('getFormattedTopicDetails should return expected results in case schemaFields is an empty array', () => { + const modifiedTopicDetails = { + ...mockTopicDetails, + messageSchema: { + ...mockTopicDetails.messageSchema, + schemaFields: [], + }, + }; + + const results = getFormattedTopicDetails(modifiedTopicDetails); + + expect(results).toEqual({ ...modifiedTopicDetails, tags: [] }); + }); +}); diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/TopicDetailsUtils.ts b/openmetadata-ui/src/main/resources/ui/src/utils/TopicDetailsUtils.ts index cda0f31375b..4dfd5d07003 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/TopicDetailsUtils.ts +++ b/openmetadata-ui/src/main/resources/ui/src/utils/TopicDetailsUtils.ts @@ -13,8 +13,10 @@ import { TopicConfigObjectInterface } from 'components/TopicDetails/TopicDetails.interface'; import { t } from 'i18next'; +import { isUndefined } from 'lodash'; import { TabSpecificField } from '../enums/entity.enum'; import { Topic } from '../generated/entity/data/topic'; +import { sortTagsCaseInsensitive } from './CommonUtils'; export const topicDetailsTabs = [ { @@ -92,3 +94,28 @@ export const getConfigObject = ( 'Schema Type': topicDetails.messageSchema?.schemaType, }; }; + +export const getFormattedTopicDetails = (topicDetails: Topic): Topic => { + if ( + !isUndefined(topicDetails.messageSchema) && + !isUndefined(topicDetails.messageSchema?.schemaFields) + ) { + // Sorting tags as the response of PATCH request does not return the sorted order + // of tags, but is stored in sorted manner in the database + // which leads to wrong PATCH payload sent after further tags removal + const schemaFields = topicDetails.messageSchema.schemaFields.map( + (schemaField) => + isUndefined(schemaField.tags) + ? schemaField + : { ...schemaField, tags: sortTagsCaseInsensitive(schemaField.tags) } + ); + + return { + ...topicDetails, + tags: topicDetails.tags ?? [], + messageSchema: { ...topicDetails.messageSchema, schemaFields }, + }; + } else { + return { ...topicDetails, tags: topicDetails.tags ?? [] }; + } +}; diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/mocks/PipelineDetailsUtils.mock.ts b/openmetadata-ui/src/main/resources/ui/src/utils/mocks/PipelineDetailsUtils.mock.ts new file mode 100644 index 00000000000..7547b86a437 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/src/utils/mocks/PipelineDetailsUtils.mock.ts @@ -0,0 +1,134 @@ +/* + * 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 { + LabelType, + Pipeline, + State, + TagSource, +} from 'generated/entity/data/pipeline'; + +export const mockPipelineDetails: Pipeline = { + id: '411e4e5e-b6d0-4fc9-bd82-ebe479f68249', + name: 'dim_address_etl', + displayName: 'dim_address etl', + fullyQualifiedName: 'sample_airflow.dim_address_etl', + tasks: [ + { + name: 'dim_address_task', + displayName: 'dim_address Task', + tags: [ + { + tagFQN: 'PII.Sensitive', + source: TagSource.Classification, + labelType: LabelType.Manual, + state: State.Confirmed, + }, + { + tagFQN: 'TagClass.tag1', + source: TagSource.Classification, + labelType: LabelType.Manual, + state: State.Confirmed, + }, + { + tagFQN: 'PersonalData.Personal', + source: TagSource.Classification, + labelType: LabelType.Manual, + state: State.Confirmed, + }, + ], + }, + { + name: 'assert_table_exists', + displayName: 'Assert Table Exists', + }, + ], + service: { + id: 'cbdc2874-0984-42fb-9469-bfa4e6a3d4e8', + type: 'pipelineService', + name: 'sample_airflow', + fullyQualifiedName: 'sample_airflow', + deleted: false, + href: 'http://localhost:8585/api/v1/services/pipelineServices/cbdc2874-0984-42fb-9469-bfa4e6a3d4e8', + }, +}; + +export const mockSortedPipelineDetails: Pipeline = { + id: '411e4e5e-b6d0-4fc9-bd82-ebe479f68249', + name: 'dim_address_etl', + displayName: 'dim_address etl', + fullyQualifiedName: 'sample_airflow.dim_address_etl', + tasks: [ + { + name: 'dim_address_task', + displayName: 'dim_address Task', + tags: [ + { + tagFQN: 'PersonalData.Personal', + source: TagSource.Classification, + labelType: LabelType.Manual, + state: State.Confirmed, + }, + { + tagFQN: 'PII.Sensitive', + source: TagSource.Classification, + labelType: LabelType.Manual, + state: State.Confirmed, + }, + { + tagFQN: 'TagClass.tag1', + source: TagSource.Classification, + labelType: LabelType.Manual, + state: State.Confirmed, + }, + ], + }, + { + name: 'assert_table_exists', + displayName: 'Assert Table Exists', + }, + ], + service: { + id: 'cbdc2874-0984-42fb-9469-bfa4e6a3d4e8', + type: 'pipelineService', + name: 'sample_airflow', + fullyQualifiedName: 'sample_airflow', + deleted: false, + href: 'http://localhost:8585/api/v1/services/pipelineServices/cbdc2874-0984-42fb-9469-bfa4e6a3d4e8', + }, +}; + +export const mockPipelineDetailsWithoutTaskTags: Pipeline = { + id: '411e4e5e-b6d0-4fc9-bd82-ebe479f68249', + name: 'dim_address_etl', + displayName: 'dim_address etl', + fullyQualifiedName: 'sample_airflow.dim_address_etl', + tasks: [ + { + name: 'dim_address_task', + displayName: 'dim_address Task', + }, + { + name: 'assert_table_exists', + displayName: 'Assert Table Exists', + }, + ], + service: { + id: 'cbdc2874-0984-42fb-9469-bfa4e6a3d4e8', + type: 'pipelineService', + name: 'sample_airflow', + fullyQualifiedName: 'sample_airflow', + deleted: false, + href: 'http://localhost:8585/api/v1/services/pipelineServices/cbdc2874-0984-42fb-9469-bfa4e6a3d4e8', + }, +}; diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/mocks/TopicDetailsUtils.mock.ts b/openmetadata-ui/src/main/resources/ui/src/utils/mocks/TopicDetailsUtils.mock.ts new file mode 100644 index 00000000000..c696316b920 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/src/utils/mocks/TopicDetailsUtils.mock.ts @@ -0,0 +1,131 @@ +/* + * 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 { + DataTypeTopic, + LabelType, + MessagingServiceType, + SchemaType, + State, + TagSource, + Topic, +} from 'generated/entity/data/topic'; + +export const mockTopicDetails: Topic = { + id: 'be3e1a5d-40fa-48ba-a420-86b8427c9d6d', + name: 'customer_events', + fullyQualifiedName: 'sample_kafka.customer_events', + partitions: 56, + description: + 'Kafka topic to capture the customer events such as location updates or profile updates', + service: { + id: '00f4486e-2b54-4317-af71-716ec0150ab2', + type: 'messagingService', + name: 'sample_kafka', + fullyQualifiedName: 'sample_kafka', + deleted: false, + href: 'http://localhost:8585/api/v1/services/messagingServices/00f4486e-2b54-4317-af71-716ec0150ab2', + }, + serviceType: MessagingServiceType.Kafka, + messageSchema: { + schemaText: '', + schemaType: SchemaType.Avro, + schemaFields: [ + { + name: 'Customer', + dataType: DataTypeTopic.Record, + fullyQualifiedName: 'sample_kafka.customer_events.Customer', + tags: [ + { + tagFQN: 'TagClass.tag1', + description: '', + source: TagSource.Classification, + labelType: LabelType.Manual, + state: State.Confirmed, + }, + { + tagFQN: 'PersonalData.Personal', + description: + 'Data that can be used to directly or indirectly identify a person.', + source: TagSource.Classification, + labelType: LabelType.Manual, + state: State.Confirmed, + }, + { + tagFQN: 'PII.NonSensitive', + description: + 'PII which is easily accessible from public sources and can include zip code, race, gender, and date of birth.', + source: TagSource.Classification, + labelType: LabelType.Manual, + state: State.Confirmed, + }, + ], + }, + ], + }, +}; + +export const mockSortedTopicDetails: Topic = { + id: 'be3e1a5d-40fa-48ba-a420-86b8427c9d6d', + name: 'customer_events', + fullyQualifiedName: 'sample_kafka.customer_events', + partitions: 56, + description: + 'Kafka topic to capture the customer events such as location updates or profile updates', + service: { + id: '00f4486e-2b54-4317-af71-716ec0150ab2', + type: 'messagingService', + name: 'sample_kafka', + fullyQualifiedName: 'sample_kafka', + deleted: false, + href: 'http://localhost:8585/api/v1/services/messagingServices/00f4486e-2b54-4317-af71-716ec0150ab2', + }, + tags: [], + serviceType: MessagingServiceType.Kafka, + messageSchema: { + schemaText: '', + schemaType: SchemaType.Avro, + schemaFields: [ + { + name: 'Customer', + dataType: DataTypeTopic.Record, + fullyQualifiedName: 'sample_kafka.customer_events.Customer', + tags: [ + { + tagFQN: 'PersonalData.Personal', + description: + 'Data that can be used to directly or indirectly identify a person.', + source: TagSource.Classification, + labelType: LabelType.Manual, + state: State.Confirmed, + }, + { + tagFQN: 'PII.NonSensitive', + description: + 'PII which is easily accessible from public sources and can include zip code, race, gender, and date of birth.', + source: TagSource.Classification, + labelType: LabelType.Manual, + state: State.Confirmed, + }, + { + tagFQN: 'TagClass.tag1', + description: '', + source: TagSource.Classification, + labelType: LabelType.Manual, + state: State.Confirmed, + }, + ], + }, + ], + }, +};