Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { cx } from '@/utils/cx';
import type { ReactNode } from 'react';
import { forwardRef } from 'react';
import type {
ButtonProps as AriaButtonProps,
TooltipProps as AriaTooltipProps,
TooltipTriggerComponentProps as AriaTooltipTriggerComponentProps,
} from 'react-aria-components';
import {
Button as AriaButton,
Focusable as AriaFocusable,
OverlayArrow as AriaOverlayArrow,
Tooltip as AriaTooltip,
TooltipTrigger as AriaTooltipTrigger,
Expand Down Expand Up @@ -126,7 +128,7 @@ export const Tooltip = ({
isExiting &&
'tw:ease-in tw:animate-out tw:fade-out tw:zoom-out-95 tw:in-placement-left:slide-out-to-right-0.5 tw:in-placement-right:slide-out-to-left-0.5 tw:in-placement-top:slide-out-to-bottom-0.5 tw:in-placement-bottom:slide-out-to-top-0.5'
)}>
<span className="tw:text-xs tw:font-semibold tw:text-white">
<span className="tw:break-all tw:text-xs tw:font-semibold tw:text-white">
{title}
</span>

Expand All @@ -145,21 +147,29 @@ export const Tooltip = ({

type TooltipTriggerProps = AriaButtonProps;

export const TooltipTrigger = ({
children,
className,
...buttonProps
}: TooltipTriggerProps) => {
// AriaTooltipTrigger passes its hover/focus-open handlers down through
// FocusableContext, not through cloned props or a plain ref - AriaButton never
// reads that context (it only reads ButtonContext), so wrapping it directly
// silently drops the tooltip's open/close wiring even though the ref itself
// gets through. AriaFocusable is the react-aria primitive that actually reads
// FocusableContext and clones the merged handlers onto its single child.
export const TooltipTrigger = forwardRef<
HTMLButtonElement,
TooltipTriggerProps
>(function TooltipTrigger({ children, className, ...buttonProps }, ref) {
return (
<AriaButton
{...buttonProps}
className={(values) =>
cx(
'tw:h-max tw:w-max tw:outline-hidden',
typeof className === 'function' ? className(values) : className
)
}>
{children}
</AriaButton>
<AriaFocusable>
<AriaButton
ref={ref}
{...buttonProps}
className={(values) =>
cx(
'tw:h-max tw:w-max tw:outline-hidden',
typeof className === 'function' ? className(values) : className
)
}>
{children}
</AriaButton>
</AriaFocusable>
);
};
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,15 @@
fireEvent,
render,
screen,
within,
} from '@testing-library/react';
import React, { act } from 'react';
import { Link } from 'react-router-dom';
import { TestCase, TestCaseStatus } from '../../../../generated/tests/testCase';
import { MOCK_PERMISSIONS } from '../../../../mocks/Glossary.mock';
import { MOCK_TEST_CASE } from '../../../../mocks/TestSuite.mock';
import { getEntityName } from '../../../../utils/EntityNameUtils';
import observabilityRouterClassBase from '../../../../utils/ObservabilityRouterClassBase';
import TestCaseIncidentManagerStatus from '../../../DataQuality/IncidentManager/TestCaseStatus/TestCaseIncidentManagerStatus.component';
import { DataQualityTabProps } from '../ProfilerDashboard/profilerDashboard.interface';
import DataQualityTab from './DataQualityTab';
Expand Down Expand Up @@ -49,7 +53,7 @@
isOpen?: boolean;
onOpenChange?: (isOpen: boolean) => void;
}>) => (
<DropdownContext.Provider value={{ isOpen, onOpenChange }}>

Check warning on line 56 in openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/DataQualityTab/DataQualityTab.test.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

The object passed as the value prop to the Context provider (at line 56) changes every render. To fix this consider wrapping it in a useMemo hook
{children}
</DropdownContext.Provider>
);
Expand Down Expand Up @@ -149,7 +153,7 @@
sortDescriptor?: { column?: string; direction?: string };
[key: string]: unknown;
}>) => (
<SortContext.Provider value={{ sortDescriptor, onSortChange }}>

Check warning on line 156 in openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/DataQualityTab/DataQualityTab.test.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

The object passed as the value prop to the Context provider (at line 156) changes every render. To fix this consider wrapping it in a useMemo hook
<table data-testid={testId}>{children}</table>
</SortContext.Provider>
);
Expand Down Expand Up @@ -244,9 +248,24 @@
children,
title,
}: React.PropsWithChildren<{ title?: string }>) => (
<div title={title}>{children}</div>
<div data-testid="tooltip" title={String(title)}>
{children}
</div>
),
TooltipTrigger: ({
children,
className,
onPress,
'data-testid': testId,
}: React.PropsWithChildren<{
className?: string;
onPress?: () => void;
'data-testid'?: string;
}>) => (
<button className={className} data-testid={testId} onClick={onPress}>
{children}
</button>
),
TooltipTrigger: ({ children }: React.PropsWithChildren) => <>{children}</>,
Typography: ({
children,
className,
Expand All @@ -265,6 +284,14 @@
};
});

jest.mock('react-aria-components', () => ({
...jest.requireActual('react-aria-components'),
// Focusable is a transparent ref/context-wiring wrapper in real usage; for
// DOM-structure assertions in tests it's equivalent to rendering its child
// directly.
Focusable: ({ children }: React.PropsWithChildren) => <>{children}</>,
}));

jest.mock('../../../../rest/testAPI', () => ({
removeTestCaseFromTestSuite: jest.fn().mockResolvedValue({}),
}));
Expand Down Expand Up @@ -541,6 +568,45 @@
expect(deleteButton).toBeInTheDocument();
});

it('Should show a styled Tooltip with the full entity name for the Name cell, not a native title attribute', async () => {
const firstRowData = MOCK_TEST_CASE[0];
await act(async () => {
render(<DataQualityTab {...mockProps} />);
});

const nameCellWrapper = await screen.findByTestId(firstRowData.name);
const trigger = within(nameCellWrapper).getByText(
getEntityName(firstRowData)
);

// The trigger is a real Link (wrapped in Focusable, not TooltipTrigger),
// so it keeps native link semantics, while still not relying on a
// native title attribute for the full name.
expect(trigger).not.toHaveAttribute('title');

const tooltip = within(nameCellWrapper).getByTestId('tooltip');

expect(tooltip).toHaveAttribute('title', getEntityName(firstRowData));
});

it('Should link the Name cell trigger to the test case detail page', async () => {
const firstRowData = MOCK_TEST_CASE[0];
await act(async () => {
render(<DataQualityTab {...mockProps} />);
});

const nameLinkCall = (Link as unknown as jest.Mock).mock.calls.find(
([props]) =>
props.to?.pathname ===
observabilityRouterClassBase.getTestCaseDetailPagePath(
firstRowData.fullyQualifiedName ?? ''
)
);

expect(nameLinkCall).toBeDefined();
expect(nameLinkCall?.[0].state).toEqual({ breadcrumbData: undefined });
});

it('Should keep action dropdowns aligned when dimensions are present', async () => {
const dimensionalTestCase: TestCase = {
...MOCK_TEST_CASE[0],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import { isUndefined, sortBy, toLower } from 'lodash';
import { useEffect, useMemo, useRef, useState } from 'react';
import type { Selection, SortDescriptor } from 'react-aria-components';
import { Focusable } from 'react-aria-components';
import { useTranslation } from 'react-i18next';
import { Link, useNavigate } from 'react-router-dom';
import { ReactComponent as DimensionIcon } from '../../../../assets/svg/data-observability/dimension.svg';
Expand All @@ -43,7 +44,7 @@
} from '../../../../generated/tests/testCase';
import { TestCaseResolutionStatus } from '../../../../generated/tests/testCaseResolutionStatus';
import { TestSuite } from '../../../../generated/tests/testSuite';
import { TestCasePageTabs } from '../../../../pages/IncidentManager/IncidentManager.interface';

Check warning on line 47 in openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/DataQualityTab/DataQualityTab.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

Pages are route-level composition modules. Move the shared implementation/type to a lower layer instead of importing a page from here
import { deleteEntity } from '../../../../rest/miscAPI';
import { removeTestCaseFromTestSuite } from '../../../../rest/testAPI';
import { getDefaultTestCaseFormVariant } from '../../../../utils/DataQuality/TestCaseFormVariantUtils';
Expand Down Expand Up @@ -126,7 +127,7 @@
editVariant = getDefaultTestCaseFormVariant(),
hasActiveFilters = false,
emptyStateAction,
}: DataQualityTabProps) => {

Check warning on line 130 in openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/DataQualityTab/DataQualityTab.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

{"message":"Function has a complexity of 13 which is greater than 10 authorized.","cost":3,"secondaryLocations":[{"line":130,"column":24,"endLine":130,"endColumn":26,"message":"+1"},{"line":732,"column":19,"endLine":732,"endColumn":21,"message":"+1"},{"line":735,"column":41,"endLine":735,"endColumn":43,"message":"+1"},{"line":735,"column":25,"endLine":735,"endColumn":27,"message":"+1"},{"line":800,"column":38,"endLine":800,"endColumn":40,"message":"+1"},{"line":802,"column":37,"endLine":802,"endColumn":39,"message":"+1"},{"line":808,"column":43,"endLine":808,"endColumn":44,"message":"+1"},{"line":838,"column":29,"endLine":838,"endColumn":30,"message":"+1"},{"line":879,"column":36,"endLine":879,"endColumn":38,"message":"+1"},{"line":879,"column":18,"endLine":879,"endColumn":20,"message":"+1"},{"line":880,"column":25,"endLine":880,"endColumn":27,"message":"+1"},{"line":897,"column":45,"endLine":897,"endColumn":47,"message":"+1"},{"line":907,"column":27,"endLine":907,"endColumn":28,"message":"+1"}]}
const { t } = useTranslation();
const navigate = useNavigate();
const { getEntityPermissionByFqn } = usePermissionProvider();
Expand Down Expand Up @@ -286,7 +287,7 @@
});
};

const handleSortChange = (descriptor: SortDescriptor) => {

Check warning on line 290 in openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/DataQualityTab/DataQualityTab.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

{"message":"Function has a complexity of 11 which is greater than 10 authorized.","cost":1,"secondaryLocations":[{"line":290,"column":56,"endLine":290,"endColumn":58,"message":"+1"},{"line":292,"column":50,"endLine":292,"endColumn":52,"message":"+1"},{"line":295,"column":4,"endLine":295,"endColumn":6,"message":"+1"},{"line":297,"column":6,"endLine":297,"endColumn":8,"message":"+1"},{"line":307,"column":4,"endLine":307,"endColumn":6,"message":"+1"},{"line":307,"column":40,"endLine":307,"endColumn":42,"message":"+1"},{"line":312,"column":12,"endLine":312,"endColumn":13,"message":"+1"},{"line":316,"column":12,"endLine":316,"endColumn":13,"message":"+1"},{"line":319,"column":11,"endLine":319,"endColumn":13,"message":"+1"},{"line":320,"column":36,"endLine":320,"endColumn":38,"message":"+1"},{"line":323,"column":6,"endLine":323,"endColumn":8,"message":"+1"}]}
const isSameSort =
descriptor.column === sortDescriptor.column &&
descriptor.direction === sortDescriptor.direction;
Expand Down Expand Up @@ -315,7 +316,7 @@
? SORT_ORDER.ASC
: SORT_ORDER.DESC,
});
} else if (

Check warning on line 319 in openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/DataQualityTab/DataQualityTab.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

Merge this if statement with the nested one
descriptor.column === 'table' ||
descriptor.column === 'column'
) {
Expand Down Expand Up @@ -409,7 +410,7 @@
} else {
setIsStatusLoading(false);
}
}, [testCases]);

Check warning on line 413 in openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/DataQualityTab/DataQualityTab.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

React Hook useEffect has missing dependencies: 'collectInlineIncidentStatuses' and 'fetchTestCasePermissions'. Either include them or remove the dependency array

const handleOpenBundleSuiteForm = (cases: TestCase[]) => {
setBundleSuiteFormInitialCases(cases);
Expand Down Expand Up @@ -512,7 +513,7 @@
);
};

const renderActionsCell = (record: TestCase) => {

Check warning on line 516 in openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/DataQualityTab/DataQualityTab.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

{"message":"Function has a complexity of 13 which is greater than 10 authorized.","cost":3,"secondaryLocations":[{"line":516,"column":47,"endLine":516,"endColumn":49,"message":"+1"},{"line":517,"column":4,"endLine":517,"endColumn":6,"message":"+1"},{"line":521,"column":47,"endLine":521,"endColumn":49,"message":"+1"},{"line":528,"column":49,"endLine":528,"endColumn":51,"message":"+1"},{"line":530,"column":37,"endLine":530,"endColumn":39,"message":"+1"},{"line":533,"column":6,"endLine":533,"endColumn":7,"message":"+1"},{"line":536,"column":52,"endLine":536,"endColumn":54,"message":"+1"},{"line":547,"column":32,"endLine":547,"endColumn":33,"message":"+1"},{"line":552,"column":10,"endLine":552,"endColumn":11,"message":"+1"},{"line":559,"column":31,"endLine":559,"endColumn":33,"message":"+1"},{"line":564,"column":16,"endLine":564,"endColumn":17,"message":"+1"},{"line":572,"column":42,"endLine":572,"endColumn":44,"message":"+1"},{"line":587,"column":48,"endLine":587,"endColumn":50,"message":"+1"}]}
if (isPermissionLoading) {
return <Skeleton height={30} width={30} />;
}
Expand Down Expand Up @@ -651,18 +652,21 @@
data-testid={record.name}
onClick={(e) => e.stopPropagation()}
onPointerDown={(e) => e.stopPropagation()}>
<Link
className="tw:block tw:min-w-0 tw:truncate"
state={{ breadcrumbData }}
title={getEntityName(record)}
to={{
pathname:
observabilityRouterClassBase.getTestCaseDetailPagePath(
record.fullyQualifiedName ?? ''
),
}}>
{getEntityName(record)}
</Link>
<Tooltip placement="top" title={getEntityName(record)}>
<Focusable>
<Link
className="tw:block tw:min-w-0 tw:truncate"
state={{ breadcrumbData }}
to={{
pathname:
observabilityRouterClassBase.getTestCaseDetailPagePath(
record.fullyQualifiedName ?? ''
),
}}>
{getEntityName(record)}
</Link>
</Focusable>
</Tooltip>
</Box>
</Table.Cell>
{showTableColumn && (
Expand Down Expand Up @@ -839,7 +843,7 @@
<Skeleton
className="tw:mb-2"
height={40}
key={i}

Check warning on line 846 in openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/DataQualityTab/DataQualityTab.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

Do not use Array index in keys
width="100%"
/>
))}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { render, screen } from '@testing-library/react';
import React from 'react';
import { Link } from 'react-router-dom';
import { TestCaseResolutionStatus } from '../../generated/tests/testCaseResolutionStatus';
import { getNameFromFQN } from '../../utils/FqnUtils';
import { NextPreviousProps } from '../common/NextPrevious/NextPrevious.interface';
import { TestCasePermission } from '../Database/Profiler/ProfilerDashboard/profilerDashboard.interface';
import IncidentManagerTable, {
Expand Down Expand Up @@ -88,9 +89,22 @@ jest.mock('@openmetadata/ui-core-components', () => {
.fn()
.mockImplementation(() => <div data-testid="skeleton" />),
Table: TableMock,
Tooltip: jest.fn().mockImplementation(({ children, title }) => (
<div data-testid="tooltip" title={String(title)}>
{children}
</div>
)),
};
});

jest.mock('react-aria-components', () => ({
...jest.requireActual('react-aria-components'),
// Focusable is a transparent ref/context-wiring wrapper in real usage; for
// DOM-structure assertions in tests it's equivalent to rendering its child
// directly.
Focusable: ({ children }: React.PropsWithChildren) => <>{children}</>,
}));

jest.mock('../common/NextPrevious/NextPrevious', () => {
return jest
.fn()
Expand Down Expand Up @@ -292,12 +306,47 @@ describe('IncidentManagerTable', () => {
expect(nameLinkCall?.[0].state).toEqual({ breadcrumbData });
});

it('should truncate the table link and expose the full name via title', () => {
it('should wrap the truncated table link in a Tooltip showing the table FQN', () => {
renderTable();

const tooltip = screen.getAllByTestId('tooltip')[0];
const tableLink = screen.getAllByTestId('table-link')[0];

expect(tableLink).toHaveAttribute('title', 'NameFromFQN');
// The trigger is a real anchor (wrapped in Focusable, not TooltipTrigger),
// so it keeps native link semantics - Ctrl/Cmd+click, middle-click,
// right-click "copy link address", and screen readers reading it as a
// link - while still not relying on a native title attribute.
expect(tableLink.tagName).toBe('A');
expect(tableLink).not.toHaveAttribute('title');
expect(tableLink).toHaveClass('tw:truncate');

// Tooltip shows the table's own FQN (Service.Database.Schema.Table, as
// returned by getPartialNameFromTableFQN), not the test case's FQN -
// the test case reference's fullyQualifiedName has the test case name
// appended and must not leak into the tooltip.
expect(tooltip).toHaveAttribute('title', 'PartialName');
});

it('should fall back to fullyQualifiedName when getNameFromFQN returns an empty string', () => {
(getNameFromFQN as jest.Mock).mockReturnValueOnce('');

renderTable();

const tableLink = screen.getAllByTestId('table-link')[0];

// Should not render blank - falls back to ref.fullyQualifiedName via `||`
expect(tableLink).toHaveTextContent(
mockRecords[0].testCaseReference?.fullyQualifiedName ?? ''
);
});

it('should link the table trigger to the table profiler page', () => {
renderTable();

const tableLinkCall = (Link as unknown as jest.Mock).mock.calls.find(
([props]) => props['data-testid'] === 'table-link'
);

expect(tableLinkCall?.[0].to).toEqual('entity-details-path');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@
EmptyPlaceholder,
Skeleton,
Table,
Tooltip,
} from '@openmetadata/ui-core-components';
import { ShieldTick } from '@untitledui/icons';
import { useMemo } from 'react';
import { Focusable } from 'react-aria-components';
import { useTranslation } from 'react-i18next';
import { Link } from 'react-router-dom';
import { EntityTabs, EntityType, FqnPart } from '../../enums/entity.enum';
Expand All @@ -28,7 +30,7 @@
Severities,
TestCaseResolutionStatus,
} from '../../generated/tests/testCaseResolutionStatus';
import { TestCaseIncidentStatusData } from '../../pages/IncidentManager/IncidentManager.interface';

Check warning on line 33 in openmetadata-ui/src/main/resources/ui/src/components/IncidentManager/IncidentManagerTable.component.tsx

View workflow job for this annotation

GitHub Actions / checkstyle

Pages are route-level composition modules. Move the shared implementation/type to a lower layer instead of importing a page from here
import { getEntityName } from '../../utils/EntityNameUtils';
import {
getNameFromFQN,
Expand Down Expand Up @@ -180,19 +182,22 @@
</Table.Cell>
{isIncidentPage && (
<Table.Cell>
<Link
className="tw:inline-block tw:max-w-52 tw:truncate tw:align-middle"
data-testid="table-link"
title={getNameFromFQN(tableFqn) ?? ref?.fullyQualifiedName}
to={getEntityDetailsPath(
EntityType.TABLE,
tableFqn,
EntityTabs.PROFILER,
ProfilerTabPath.DATA_QUALITY
)}
onClick={(e) => e.stopPropagation()}>
{getNameFromFQN(tableFqn) ?? ref?.fullyQualifiedName}
</Link>
<Tooltip placement="top" title={tableFqn}>
<Focusable>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we need to introduce focusable, we should be using Tooltip trigger, Tooltip just like we have done in other places.

<Link

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we not untitled button component with link variant here ?

className="tw:inline-block tw:max-w-52 tw:truncate tw:align-middle"
data-testid="table-link"
to={getEntityDetailsPath(
EntityType.TABLE,
tableFqn,
EntityTabs.PROFILER,
ProfilerTabPath.DATA_QUALITY
)}
onClick={(e) => e.stopPropagation()}>
{getNameFromFQN(tableFqn) || ref?.fullyQualifiedName}
</Link>
</Focusable>
</Tooltip>
</Table.Cell>
)}
<Table.Cell className="tw:whitespace-nowrap">
Expand Down
Loading