Profiler & Data Quality UI feedback & improvement #7090 Part 2 (#7114)

* Profiler & Data Quality UI feedback & improvement #7090 Part 2

* addressing comment

* fixed failing unit test
This commit is contained in:
Shailesh Parmar 2022-09-01 16:38:53 +05:30 committed by GitHub
parent bcf2fc3057
commit e180cb15af
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 141 additions and 126 deletions

View File

@ -17,12 +17,11 @@ import { AxiosError } from 'axios';
import { compare } from 'fast-json-patch'; import { compare } from 'fast-json-patch';
import { EditorContentRef } from 'Models'; import { EditorContentRef } from 'Models';
import React, { useCallback, useEffect, useRef, useState } from 'react'; import React, { useCallback, useEffect, useRef, useState } from 'react';
import { Controlled as CodeMirror } from 'react-codemirror2';
import { import {
getTestDefinitionById, getTestDefinitionById,
updateTestCaseById, updateTestCaseById,
} from '../../axiosAPIs/testAPI'; } from '../../axiosAPIs/testAPI';
import { codeMirrorOption } from '../../constants/profiler.constant'; import { CSMode } from '../../enums/codemirror.enum';
import { TestCaseParameterValue } from '../../generated/tests/testCase'; import { TestCaseParameterValue } from '../../generated/tests/testCase';
import { import {
TestDataType, TestDataType,
@ -32,6 +31,7 @@ import jsonData from '../../jsons/en';
import { showErrorToast, showSuccessToast } from '../../utils/ToastUtils'; import { showErrorToast, showSuccessToast } from '../../utils/ToastUtils';
import RichTextEditor from '../common/rich-text-editor/RichTextEditor'; import RichTextEditor from '../common/rich-text-editor/RichTextEditor';
import Loader from '../Loader/Loader'; import Loader from '../Loader/Loader';
import SchemaEditor from '../schema-editor/SchemaEditor';
import { EditTestCaseModalProps } from './AddDataQualityTest.interface'; import { EditTestCaseModalProps } from './AddDataQualityTest.interface';
import ParameterForm from './components/ParameterForm'; import ParameterForm from './components/ParameterForm';
@ -63,16 +63,13 @@ const EditTestCaseModal: React.FC<EditTestCaseModalProps> = ({
<Typography.Paragraph className="tw-mb-1.5"> <Typography.Paragraph className="tw-mb-1.5">
Profile Sample Query Profile Sample Query
</Typography.Paragraph> </Typography.Paragraph>
<CodeMirror <SchemaEditor
data-testid="sql-editor" mode={{ name: CSMode.SQL }}
options={codeMirrorOption} options={{
readOnly: false,
}}
value={sqlQuery.value || ''} value={sqlQuery.value || ''}
onBeforeChange={(_Editor, _EditorChange, value) => { onChange={(value) => setSqlQuery((pre) => ({ ...pre, value }))}
setSqlQuery((pre) => ({ ...pre, value }));
}}
onChange={(_Editor, _EditorChange, value) => {
setSqlQuery((pre) => ({ ...pre, value }));
}}
/> />
</Col> </Col>
</Row> </Row>
@ -171,6 +168,12 @@ const EditTestCaseModal: React.FC<EditTestCaseModalProps> = ({
testDefinition: testCase?.testDefinition?.name, testDefinition: testCase?.testDefinition?.name,
params: getParamsValue(), params: getParamsValue(),
}); });
setSqlQuery(
testCase?.parameterValues?.[0] ?? {
name: 'sqlExpression',
value: '',
}
);
} }
}, [testCase]); }, [testCase]);
@ -190,6 +193,7 @@ const EditTestCaseModal: React.FC<EditTestCaseModalProps> = ({
<Loader /> <Loader />
) : ( ) : (
<Form <Form
className="tw-h-70vh tw-overflow-auto"
form={form} form={form}
layout="vertical" layout="vertical"
name="tableTestForm" name="tableTestForm"

View File

@ -33,13 +33,16 @@ import {
COLORS, COLORS,
PROFILER_FILTER_RANGE, PROFILER_FILTER_RANGE,
} from '../../../constants/profiler.constant'; } from '../../../constants/profiler.constant';
import { CSMode } from '../../../enums/codemirror.enum';
import { import {
TestCaseResult, TestCaseResult,
TestCaseStatus, TestCaseStatus,
} from '../../../generated/tests/tableTest'; } from '../../../generated/tests/tableTest';
import { TestCaseParameterValue } from '../../../generated/tests/testCase';
import { showErrorToast } from '../../../utils/ToastUtils'; import { showErrorToast } from '../../../utils/ToastUtils';
import RichTextEditorPreviewer from '../../common/rich-text-editor/RichTextEditorPreviewer'; import RichTextEditorPreviewer from '../../common/rich-text-editor/RichTextEditorPreviewer';
import Loader from '../../Loader/Loader'; import Loader from '../../Loader/Loader';
import SchemaEditor from '../../schema-editor/SchemaEditor';
import { TestSummaryProps } from '../profilerDashboard.interface'; import { TestSummaryProps } from '../profilerDashboard.interface';
type ChartDataType = { type ChartDataType = {
@ -146,6 +149,34 @@ const TestSummary: React.FC<TestSummaryProps> = ({ data }) => {
fetchTestResults(); fetchTestResults();
}, [selectedTimeRange]); }, [selectedTimeRange]);
const showParamsData = (param: TestCaseParameterValue) => {
const isSqlQuery = param.name === 'sqlExpression';
if (isSqlQuery) {
return (
<div key={param.name}>
<Typography.Text>{param.name}: </Typography.Text>
<SchemaEditor
className="tw-w-11/12 tw-mt-1"
editorClass="table-query-editor"
mode={{ name: CSMode.SQL }}
options={{
styleActiveLine: false,
}}
value={param.value ?? ''}
/>
</div>
);
}
return (
<div key={param.name}>
<Typography.Text>{param.name}: </Typography.Text>
<Typography.Text>{param.value}</Typography.Text>
</div>
);
};
const referenceArea = () => { const referenceArea = () => {
const yValues = data.parameterValues?.reduce((acc, curr, i) => { const yValues = data.parameterValues?.reduce((acc, curr, i) => {
return { ...acc, [`y${i + 1}`]: parseInt(curr.value || '') }; return { ...acc, [`y${i + 1}`]: parseInt(curr.value || '') };
@ -215,13 +246,14 @@ const TestSummary: React.FC<TestSummaryProps> = ({ data }) => {
<Col span={24}> <Col span={24}>
<Typography.Text type="secondary">Parameter: </Typography.Text> <Typography.Text type="secondary">Parameter: </Typography.Text>
</Col> </Col>
<Col offset={2} span={24}> <Col offset={1} span={24}>
{data.parameterValues?.map((param) => ( {data.parameterValues && data.parameterValues.length > 0 ? (
<Typography key={param.name}> data.parameterValues.map(showParamsData)
<Typography.Text>{param.name}: </Typography.Text> ) : (
<Typography.Text>{param.value}</Typography.Text> <Typography.Text type="secondary">
</Typography> No Parameter Available
))} </Typography.Text>
)}
</Col> </Col>
<Col className="tw-flex tw-gap-2" span={24}> <Col className="tw-flex tw-gap-2" span={24}>

View File

@ -27,35 +27,14 @@ import TableProfilerV1 from './TableProfilerV1';
// mock library imports // mock library imports
jest.mock('react-router-dom', () => ({ jest.mock('react-router-dom', () => ({
useHistory: jest.fn().mockImplementation(() => {
jest.fn();
}),
Link: jest Link: jest
.fn() .fn()
.mockImplementation(({ children }) => <a href="#">{children}</a>), .mockImplementation(({ children }) => <a href="#">{children}</a>),
})); }));
jest.mock('antd', () => ({
Button: jest
.fn()
.mockImplementation(({ children, ...props }) => (
<button {...props}>{children}</button>
)),
Col: jest
.fn()
.mockImplementation(({ children, ...props }) => (
<div {...props}>{children}</div>
)),
Row: jest
.fn()
.mockImplementation(({ children, ...props }) => (
<div {...props}>{children}</div>
)),
Empty: jest
.fn()
.mockImplementation(({ description }) => <div>{description}</div>),
Link: jest.fn().mockImplementation(({ children }) => <a>{children}</a>),
Tooltip: jest.fn().mockImplementation(({ children }) => <p>{children}</p>),
}));
// mock internal imports // mock internal imports
jest.mock('./Component/ProfilerSettingsModal', () => { jest.mock('./Component/ProfilerSettingsModal', () => {
return jest.fn().mockImplementation(() => { return jest.fn().mockImplementation(() => {
@ -106,21 +85,6 @@ describe('Test TableProfiler component', () => {
expect(addTableTest).toBeInTheDocument(); expect(addTableTest).toBeInTheDocument();
}); });
it('No data placeholder should be visible where there is no profiler', async () => {
render(
<TableProfilerV1
{...mockProps}
table={{ ...mockProps.table, profile: undefined }}
/>
);
const noProfiler = await screen.findByTestId(
'no-profiler-placeholder-container'
);
expect(noProfiler).toBeInTheDocument();
});
it('CTA: Add table test should work properly', async () => { it('CTA: Add table test should work properly', async () => {
render(<TableProfilerV1 {...mockProps} />); render(<TableProfilerV1 {...mockProps} />);

View File

@ -11,12 +11,20 @@
* limitations under the License. * limitations under the License.
*/ */
import { Button, Col, Empty, Row, Tooltip } from 'antd'; import {
Button,
Col,
Radio,
RadioChangeEvent,
Row,
Space,
Tooltip,
} from 'antd';
import { AxiosError } from 'axios'; import { AxiosError } from 'axios';
import classNames from 'classnames'; import classNames from 'classnames';
import { isEmpty, isUndefined } from 'lodash'; import { isEmpty } from 'lodash';
import React, { FC, useEffect, useMemo, useState } from 'react'; import React, { FC, useEffect, useMemo, useState } from 'react';
import { Link } from 'react-router-dom'; import { Link, useHistory } from 'react-router-dom';
import { getListTestCase } from '../../axiosAPIs/testAPI'; import { getListTestCase } from '../../axiosAPIs/testAPI';
import { API_RES_MAX_SIZE } from '../../constants/constants'; import { API_RES_MAX_SIZE } from '../../constants/constants';
import { NO_PERMISSION_FOR_ACTION } from '../../constants/HelperTextUtil'; import { NO_PERMISSION_FOR_ACTION } from '../../constants/HelperTextUtil';
@ -35,6 +43,7 @@ import {
import SVGIcons, { Icons } from '../../utils/SvgUtils'; import SVGIcons, { Icons } from '../../utils/SvgUtils';
import { generateEntityLink } from '../../utils/TableUtils'; import { generateEntityLink } from '../../utils/TableUtils';
import { showErrorToast } from '../../utils/ToastUtils'; import { showErrorToast } from '../../utils/ToastUtils';
import { ProfilerDashboardTab } from '../ProfilerDashboard/profilerDashboard.interface';
import ColumnProfileTable from './Component/ColumnProfileTable'; import ColumnProfileTable from './Component/ColumnProfileTable';
import ProfilerSettingsModal from './Component/ProfilerSettingsModal'; import ProfilerSettingsModal from './Component/ProfilerSettingsModal';
import { import {
@ -50,12 +59,16 @@ const TableProfilerV1: FC<TableProfilerProps> = ({
hasEditAccess, hasEditAccess,
}) => { }) => {
const { profile, columns } = table; const { profile, columns } = table;
const history = useHistory();
const [settingModalVisible, setSettingModalVisible] = useState(false); const [settingModalVisible, setSettingModalVisible] = useState(false);
const [columnTests, setColumnTests] = useState<TestCase[]>([]); const [columnTests, setColumnTests] = useState<TestCase[]>([]);
const [tableTests, setTableTests] = useState<TableTestsType>({ const [tableTests, setTableTests] = useState<TableTestsType>({
tests: [], tests: [],
results: INITIAL_TEST_RESULT_SUMMARY, results: INITIAL_TEST_RESULT_SUMMARY,
}); });
const [activeTab] = useState<ProfilerDashboardTab>(
ProfilerDashboardTab.SUMMARY
);
const handleSettingModal = (value: boolean) => { const handleSettingModal = (value: boolean) => {
setSettingModalVisible(value); setSettingModalVisible(value);
@ -92,6 +105,24 @@ const TableProfilerV1: FC<TableProfilerProps> = ({
]; ];
}, [profile, tableTests]); }, [profile, tableTests]);
const tabOptions = useMemo(() => {
return Object.values(ProfilerDashboardTab).filter(
(value) => value !== ProfilerDashboardTab.PROFILER
);
}, []);
const handleTabChange = (e: RadioChangeEvent) => {
const value = e.target.value as ProfilerDashboardTab;
if (ProfilerDashboardTab.DATA_QUALITY === value) {
history.push(
getProfilerDashboardWithFqnPath(
ProfilerDashboardType.TABLE,
table.fullyQualifiedName || ''
)
);
}
};
const fetchAllTests = async () => { const fetchAllTests = async () => {
try { try {
const { data } = await getListTestCase({ const { data } = await getListTestCase({
@ -130,67 +161,49 @@ const TableProfilerV1: FC<TableProfilerProps> = ({
fetchAllTests(); fetchAllTests();
}, [table]); }, [table]);
if (isUndefined(profile)) {
return (
<div
className=" tw-m-2 tw-flex tw-justify-center tw-font-medium tw-items-center tw-border tw-border-main tw-rounded-md tw-p-8"
data-testid="no-profiler-placeholder-container">
<Empty
description={
<p>
<span>
Data Profiler is an optional configuration in Ingestion. Please
enable the data profiler by following the documentation
</span>
<Link
className="tw-ml-1"
target="_blank"
to={{
pathname:
'https://docs.open-metadata.org/openmetadata/ingestion/workflows/profiler',
}}>
here.
</Link>
</p>
}
/>
</div>
);
}
return ( return (
<div <div
className="table-profiler-container" className="table-profiler-container"
data-testid="table-profiler-container"> data-testid="table-profiler-container">
<div className="tw-flex tw-justify-end tw-gap-4 tw-mb-4"> <Row className="tw-mb-4" justify="space-between">
<Tooltip title={hasEditAccess ? '' : NO_PERMISSION_FOR_ACTION}> <Radio.Group
<Link buttonStyle="solid"
to={ optionType="button"
hasEditAccess options={tabOptions}
? getAddDataQualityTableTestPath( value={activeTab}
ProfilerDashboardType.TABLE, onChange={handleTabChange}
table.fullyQualifiedName || '' />
)
: '#' <Space>
}> <Tooltip title={hasEditAccess ? '' : NO_PERMISSION_FOR_ACTION}>
<Button <Link
className="tw-rounded" to={
data-testid="profiler-add-table-test-btn" hasEditAccess
disabled={!hasEditAccess} ? getAddDataQualityTableTestPath(
type="primary"> ProfilerDashboardType.TABLE,
Add Test `${table.fullyQualifiedName}`
</Button> )
</Link> : '#'
</Tooltip> }>
<Button <Button
className="profiler-setting-btn tw-border tw-border-primary tw-rounded tw-text-primary" className="tw-rounded"
data-testid="profiler-setting-btn" data-testid="profiler-add-table-test-btn"
icon={<SVGIcons alt="setting" icon={Icons.SETTINGS_PRIMERY} />} disabled={!hasEditAccess}
type="default" type="primary">
onClick={() => handleSettingModal(true)}> Add Test
Settings </Button>
</Button> </Link>
</div> </Tooltip>
<Button
className="profiler-setting-btn tw-border tw-border-primary tw-rounded tw-text-primary"
data-testid="profiler-setting-btn"
icon={<SVGIcons alt="setting" icon={Icons.SETTINGS_PRIMERY} />}
type="default"
onClick={() => handleSettingModal(true)}>
Settings
</Button>
</Space>
</Row>
<Row className="tw-rounded tw-border tw-p-4 tw-mb-4"> <Row className="tw-rounded tw-border tw-p-4 tw-mb-4">
{overallSummery.map((summery) => ( {overallSummery.map((summery) => (
@ -211,15 +224,6 @@ const TableProfilerV1: FC<TableProfilerProps> = ({
</p> </p>
</Col> </Col>
))} ))}
<Col className="tw-flex tw-justify-end" span={24}>
<Link
to={getProfilerDashboardWithFqnPath(
ProfilerDashboardType.TABLE,
table.fullyQualifiedName || ''
)}>
View more detail
</Link>
</Col>
</Row> </Row>
<ColumnProfileTable <ColumnProfileTable

View File

@ -41,14 +41,17 @@ const SchemaEditor = ({
}, },
options, options,
editorClass, editorClass,
onChange,
}: { }: {
value: string; value: string;
className?: string; className?: string;
mode?: Mode; mode?: Mode;
readOnly?: boolean;
options?: { options?: {
[key: string]: string | boolean | Array<string>; [key: string]: string | boolean | Array<string>;
}; };
editorClass?: string; editorClass?: string;
onChange?: (value: string) => void;
}) => { }) => {
const defaultOptions = { const defaultOptions = {
tabSize: JSON_TAB_SIZE, tabSize: JSON_TAB_SIZE,
@ -75,6 +78,13 @@ const SchemaEditor = ({
): void => { ): void => {
setInternalValue(getSchemaEditorValue(value)); setInternalValue(getSchemaEditorValue(value));
}; };
const handleEditorInputChange = (
_editor: Editor,
_data: EditorChange,
value: string
): void => {
onChange && onChange(getSchemaEditorValue(value));
};
return ( return (
<div className={className}> <div className={className}>
@ -83,6 +93,7 @@ const SchemaEditor = ({
options={defaultOptions} options={defaultOptions}
value={internalValue} value={internalValue}
onBeforeChange={handleEditorInputBeforeChange} onBeforeChange={handleEditorInputBeforeChange}
onChange={handleEditorInputChange}
/> />
</div> </div>
); );