#16489: fix the redirect issue to new tab for tags and glossary (#16512)

* fix the redirect issue to new tab for tags and glossary

* fix the redirect on cancel icon and unit test issue

* changes as per comments
This commit is contained in:
Ashish Gupta 2024-06-05 11:34:24 +05:30 committed by GitHub
parent cdca199ec8
commit 8d312f0853
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 74 additions and 31 deletions

View File

@ -19,6 +19,11 @@ import TableDataCardBody from './TableDataCardBody';
jest.mock('../../common/RichTextEditor/RichTextEditorPreviewer', () => { jest.mock('../../common/RichTextEditor/RichTextEditorPreviewer', () => {
return jest.fn().mockReturnValue(<p>RichTextEditorPreviewer</p>); return jest.fn().mockReturnValue(<p>RichTextEditorPreviewer</p>);
}); });
jest.mock('../../Tag/TagsViewer/TagsViewer', () => {
return jest.fn().mockReturnValue(<p>TagsViewer</p>);
});
jest.mock('../../common/EntitySummaryDetails/EntitySummaryDetails', () => { jest.mock('../../common/EntitySummaryDetails/EntitySummaryDetails', () => {
return jest return jest
.fn() .fn()

View File

@ -33,7 +33,10 @@ describe('SearchIndexSummary component tests', () => {
it('Component should render properly, when loaded in the Explore page.', async () => { it('Component should render properly, when loaded in the Explore page.', async () => {
await act(async () => { await act(async () => {
render( render(
<SearchIndexSummary entityDetails={mockSearchIndexEntityDetails} /> <SearchIndexSummary entityDetails={mockSearchIndexEntityDetails} />,
{
wrapper: MemoryRouter,
}
); );
}); });
@ -115,7 +118,10 @@ describe('SearchIndexSummary component tests', () => {
}, },
], ],
}} }}
/> />,
{
wrapper: MemoryRouter,
}
); );
}); });

View File

@ -54,6 +54,12 @@ jest.mock('../SummaryList/SummaryList.component', () =>
.fn() .fn()
.mockImplementation(() => <div data-testid="SummaryList">SummaryList</div>) .mockImplementation(() => <div data-testid="SummaryList">SummaryList</div>)
); );
jest.mock(
'../../../common/SummaryTagsDescription/SummaryTagsDescription.component',
() => jest.fn().mockImplementation(() => <p>SummaryTagsDescription</p>)
);
jest.mock('react-router-dom', () => ({ jest.mock('react-router-dom', () => ({
...jest.requireActual('react-router-dom'), ...jest.requireActual('react-router-dom'),
useLocation: jest.fn().mockReturnValue({ pathname: '/table' }), useLocation: jest.fn().mockReturnValue({ pathname: '/table' }),
@ -75,7 +81,7 @@ describe('TableSummary component tests', () => {
const profilerHeader = screen.getByTestId('profiler-header'); const profilerHeader = screen.getByTestId('profiler-header');
const schemaHeader = screen.getByTestId('schema-header'); const schemaHeader = screen.getByTestId('schema-header');
const tagsHeader = screen.getByTestId('tags-header'); const summaryTagDescription = screen.getByText('SummaryTagsDescription');
const typeLabel = screen.getByTestId('label.type-label'); const typeLabel = screen.getByTestId('label.type-label');
const queriesLabel = screen.getByTestId('label.query-plural-label'); const queriesLabel = screen.getByTestId('label.query-plural-label');
const columnsLabel = screen.getByTestId('label.column-plural-label'); const columnsLabel = screen.getByTestId('label.column-plural-label');
@ -88,7 +94,7 @@ describe('TableSummary component tests', () => {
expect(profilerHeader).toBeInTheDocument(); expect(profilerHeader).toBeInTheDocument();
expect(schemaHeader).toBeInTheDocument(); expect(schemaHeader).toBeInTheDocument();
expect(tagsHeader).toBeInTheDocument(); expect(summaryTagDescription).toBeInTheDocument();
expect(typeLabel).toBeInTheDocument(); expect(typeLabel).toBeInTheDocument();
expect(queriesLabel).toBeInTheDocument(); expect(queriesLabel).toBeInTheDocument();
expect(columnsLabel).toBeInTheDocument(); expect(columnsLabel).toBeInTheDocument();

View File

@ -12,8 +12,8 @@
*/ */
import { Tag, Tooltip, Typography } from 'antd'; import { Tag, Tooltip, Typography } from 'antd';
import classNames from 'classnames'; import classNames from 'classnames';
import React, { useCallback, useMemo } from 'react'; import React, { useMemo } from 'react';
import { useHistory } from 'react-router-dom'; import { Link } from 'react-router-dom';
import { ReactComponent as IconTerm } from '../../../assets/svg/book.svg'; import { ReactComponent as IconTerm } from '../../../assets/svg/book.svg';
import { ReactComponent as PlusIcon } from '../../../assets/svg/plus-primary.svg'; import { ReactComponent as PlusIcon } from '../../../assets/svg/plus-primary.svg';
import { ReactComponent as IconTag } from '../../../assets/svg/tag.svg'; import { ReactComponent as IconTag } from '../../../assets/svg/tag.svg';
@ -40,7 +40,6 @@ const TagsV1 = ({
tagType, tagType,
size, size,
}: TagsV1Props) => { }: TagsV1Props) => {
const history = useHistory();
const color = useMemo( const color = useMemo(
() => (isVersionPage ? undefined : tag.style?.color), () => (isVersionPage ? undefined : tag.style?.color),
[tag] [tag]
@ -87,11 +86,11 @@ const TagsV1 = ({
[showOnlyName, tag.tagFQN] [showOnlyName, tag.tagFQN]
); );
const redirectLink = useCallback( const redirectLink = useMemo(
() => () =>
(tagType ?? tag.source) === TagSource.Glossary (tagType ?? tag.source) === TagSource.Glossary
? history.push(getGlossaryPath(tag.tagFQN)) ? getGlossaryPath(tag.tagFQN)
: history.push(getTagPath(Fqn.split(tag.tagFQN)[0])), : getTagPath(Fqn.split(tag.tagFQN)[0]),
[tagType, tag.source, tag.tagFQN] [tagType, tag.source, tag.tagFQN]
); );
@ -151,18 +150,23 @@ const TagsV1 = ({
? { backgroundColor: reduceColorOpacity(color, 0.05) } ? { backgroundColor: reduceColorOpacity(color, 0.05) }
: undefined : undefined
} }
onClick={redirectLink}
{...tagProps}> {...tagProps}>
{/* Wrap only content to avoid redirect on closeable icons */}
<Link
className="no-underline h-full"
data-testid="tag-redirect-link"
to={redirectLink}>
{tagContent} {tagContent}
</Link>
</Tag> </Tag>
), ),
[color, tagContent, className] [color, tagContent, redirectLink]
); );
const addTagChip = useMemo( const addTagChip = useMemo(
() => ( () => (
<Tag <Tag
className={classNames('tag-chip tag-chip-add-button')} className="tag-chip tag-chip-add-button"
icon={<PlusIcon height={16} name="plus" width={16} />}> icon={<PlusIcon height={16} name="plus" width={16} />}>
<Typography.Paragraph <Typography.Paragraph
className="m-0 text-xs font-medium text-primary" className="m-0 text-xs font-medium text-primary"
@ -181,7 +185,7 @@ const TagsV1 = ({
return ( return (
<Tooltip <Tooltip
className="cursor-pointer" className="cursor-pointer"
mouseEnterDelay={1.5} mouseEnterDelay={0.5}
placement="bottomLeft" placement="bottomLeft"
title={tooltipOverride ?? getTagTooltip(tag.tagFQN, tag.description)} title={tooltipOverride ?? getTagTooltip(tag.tagFQN, tag.description)}
trigger="hover"> trigger="hover">

View File

@ -17,12 +17,14 @@ import { TAG_CONSTANT, TAG_START_WITH } from '../../../constants/Tag.constants';
import { LabelType, State, TagSource } from '../../../generated/type/tagLabel'; import { LabelType, State, TagSource } from '../../../generated/type/tagLabel';
import TagsV1 from './TagsV1.component'; import TagsV1 from './TagsV1.component';
const mockPush = jest.fn(); const mockLinkButton = jest.fn();
jest.mock('react-router-dom', () => ({ jest.mock('react-router-dom', () => ({
useHistory: jest.fn().mockImplementation(() => ({ Link: jest.fn().mockImplementation(({ children, ...rest }) => (
push: mockPush, <a {...rest} onClick={mockLinkButton}>
})), {children}
</a>
)),
})); }));
jest.mock('../../../utils/TagsUtils', () => ({ jest.mock('../../../utils/TagsUtils', () => ({
@ -71,12 +73,11 @@ describe('Test tags Component', () => {
}} }}
/> />
); );
const tag = getByTestId(container, 'tags'); const tag = getByTestId(container, 'tag-redirect-link');
fireEvent.click(tag); fireEvent.click(tag);
expect(mockPush).toHaveBeenCalledTimes(1); expect(mockLinkButton).toHaveBeenCalledTimes(1);
expect(mockPush).toHaveBeenCalledWith('/tags/testTag');
}); });
it('Clicking on tag with source Glossary should redirect to the proper glossary term page', () => { it('Clicking on tag with source Glossary should redirect to the proper glossary term page', () => {
@ -92,12 +93,11 @@ describe('Test tags Component', () => {
}} }}
/> />
); );
const tag = getByTestId(container, 'tags'); const tag = getByTestId(container, 'tag-redirect-link');
fireEvent.click(tag); fireEvent.click(tag);
expect(mockPush).toHaveBeenCalledTimes(1); expect(mockLinkButton).toHaveBeenCalledTimes(1);
expect(mockPush).toHaveBeenCalledWith('/glossary/glossaryTag.Test1');
}); });
it('should render size based tags, for small class should contain small', () => { it('should render size based tags, for small class should contain small', () => {

View File

@ -12,6 +12,7 @@
*/ */
import { act, render, screen } from '@testing-library/react'; import { act, render, screen } from '@testing-library/react';
import React from 'react'; import React from 'react';
import { MemoryRouter } from 'react-router-dom';
import { LabelType, State, TagSource } from '../../generated/type/tagLabel'; import { LabelType, State, TagSource } from '../../generated/type/tagLabel';
import TagsInput from './TagsInput.component'; import TagsInput from './TagsInput.component';
@ -44,7 +45,10 @@ describe('TagsInput', () => {
editable={false} editable={false}
tags={tags} tags={tags}
onTagsUpdate={mockOnTagsUpdate} onTagsUpdate={mockOnTagsUpdate}
/> />,
{
wrapper: MemoryRouter,
}
); );
}); });
@ -63,7 +67,10 @@ describe('TagsInput', () => {
editable={false} editable={false}
tags={tags} tags={tags}
onTagsUpdate={mockOnTagsUpdate} onTagsUpdate={mockOnTagsUpdate}
/> />,
{
wrapper: MemoryRouter,
}
); );
}); });
@ -79,7 +86,10 @@ describe('TagsInput', () => {
it('should render edit button when no editable', async () => { it('should render edit button when no editable', async () => {
await act(async () => { await act(async () => {
render( render(
<TagsInput editable tags={tags} onTagsUpdate={mockOnTagsUpdate} /> <TagsInput editable tags={tags} onTagsUpdate={mockOnTagsUpdate} />,
{
wrapper: MemoryRouter,
}
); );
}); });
@ -93,7 +103,10 @@ describe('TagsInput', () => {
editable={false} editable={false}
tags={tags} tags={tags}
onTagsUpdate={mockOnTagsUpdate} onTagsUpdate={mockOnTagsUpdate}
/> />,
{
wrapper: MemoryRouter,
}
); );
}); });
@ -103,7 +116,14 @@ describe('TagsInput', () => {
it('should not render tags if tags is empty', async () => { it('should not render tags if tags is empty', async () => {
await act(async () => { await act(async () => {
render( render(
<TagsInput editable={false} tags={[]} onTagsUpdate={mockOnTagsUpdate} /> <TagsInput
editable={false}
tags={[]}
onTagsUpdate={mockOnTagsUpdate}
/>,
{
wrapper: MemoryRouter,
}
); );
expect(await screen.findByTestId('tags-container')).toBeInTheDocument(); expect(await screen.findByTestId('tags-container')).toBeInTheDocument();
@ -114,7 +134,9 @@ describe('TagsInput', () => {
it('should render add tags if tags is empty and has permission', async () => { it('should render add tags if tags is empty and has permission', async () => {
await act(async () => { await act(async () => {
render(<TagsInput editable tags={[]} onTagsUpdate={mockOnTagsUpdate} />); render(<TagsInput editable tags={[]} onTagsUpdate={mockOnTagsUpdate} />, {
wrapper: MemoryRouter,
});
expect(await screen.findByTestId('entity-tags')).toBeInTheDocument(); expect(await screen.findByTestId('entity-tags')).toBeInTheDocument();
expect(await screen.findByTestId('add-tag')).toBeInTheDocument(); expect(await screen.findByTestId('add-tag')).toBeInTheDocument();