Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix: Data quality bug fixes #3272

Merged
merged 4 commits into from
Mar 8, 2022
Merged

Fix: Data quality bug fixes #3272

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8e5091a
Fixed: issue #3223, #3228, #3260, #3243,
ShaileshParmar11 Mar 8, 2022
3fe9c10
Merge branch 'main' into data-quality-fix
ShaileshParmar11 Mar 8, 2022
3e5aa97
addressing comment
ShaileshParmar11 Mar 8, 2022
6a1445c
fixed broken test
ShaileshParmar11 Mar 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions ...etadata-ui/src/main/resources/ui/src/components/AddDataQualityTest/AddDataQualityTest.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,15 @@ type Props = {
handleAddTableTestCase: (data: CreateTableTest) => void;
handleAddColumnTestCase: (data: ColumnTest) => void;
onFormCancel: () => void;
selectedColumn: string;
};

const AddDataQualityTest = ({
tableTestCase,
data,
testMode,
columnOptions = [],
selectedColumn,
handleAddTableTestCase,
handleAddColumnTestCase,
onFormCancel,
Expand All @@ -55,6 +57,7 @@ const AddDataQualityTest = ({
column={columnOptions}
data={data as ColumnTest}
handleAddColumnTestCase={handleAddColumnTestCase}
selectedColumn={selectedColumn}
onFormCancel={onFormCancel}
/>
)}
Expand Down
87 changes: 64 additions & 23 deletions ...adata-ui/src/main/resources/ui/src/components/AddDataQualityTest/Forms/ColumnTestForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,15 @@ import {
getCurrentUserId,
requiredField,
} from '../../../utils/CommonUtils';
import { isSupportedTest } from '../../../utils/EntityUtils';
import SVGIcons from '../../../utils/SvgUtils';
import { getDataTypeString } from '../../../utils/TableUtils';
import { Button } from '../../buttons/Button/Button';
import MarkdownWithPreview from '../../common/editor/MarkdownWithPreview';

type Props = {
data: ColumnTest;
selectedColumn: string;
column: ModifiedTableColumn[];
handleAddColumnTestCase: (data: ColumnTest) => void;
onFormCancel: () => void;
Expand All @@ -50,6 +52,7 @@ export const Field = ({
};

const ColumnTestForm = ({
selectedColumn,
data,
column,
handleAddColumnTestCase,
Expand Down Expand Up @@ -77,12 +80,15 @@ const ColumnTestForm = ({
data?.testCase?.config?.forbiddenValues || ['']
);
const [isShowError, setIsShowError] = useState({
testName: false,
columnName: false,
regex: false,
minOrMax: false,
missingCountValue: false,
values: false,
minMaxValue: false,
allTestAdded: false,
notSupported: false,
});

const [columnName, setColumnName] = useState(data?.columnName);
Expand Down Expand Up @@ -114,36 +120,50 @@ const ColumnTestForm = ({
setIsShowError({ ...isShowError, values: false });
};

const setAllTestOption = (defaultSelected: boolean) => {
const newTest = Object.values(ColumnTestType);
setTestTypeOptions(newTest);
setColumnTest(defaultSelected ? newTest[0] : ('' as ColumnTestType));
};

const handleTestTypeOptionChange = (name: string) => {
const selectedColumn = column.find((d) => d.name === name);
const existingTests =
selectedColumn?.columnTests?.map(
(d: ColumnTest) => d.testCase.columnTestType
) || [];
if (existingTests.length) {
const newTest = Object.values(ColumnTestType).filter(
(d) => !existingTests.includes(d)
);
setTestTypeOptions(newTest);
setColumnTest(newTest[0]);
if (!isEmpty(name)) {
const selectedColumn = column.find((d) => d.name === name);
const existingTests =
selectedColumn?.columnTests?.map(
(d: ColumnTest) => d.testCase.columnTestType
) || [];
if (existingTests.length) {
const newTest = Object.values(ColumnTestType).filter(
(d) => !existingTests.includes(d)
);
setTestTypeOptions(newTest);
setColumnTest(newTest[0]);
} else {
setAllTestOption(true);
}
} else {
const newTest = Object.values(ColumnTestType);
setTestTypeOptions(newTest);
setColumnTest(newTest[0]);
setAllTestOption(false);
}
};

useEffect(() => {
if (isUndefined(data)) {
const allOption = column.filter((value) => {
return (
value?.dataType !== 'STRUCT' &&
value.columnTests?.length !== Object.values(ColumnTestType).length
);
});
setColumnOptions(allOption);
setColumnName(allOption[0]?.name || '');
handleTestTypeOptionChange(allOption[0]?.name || '');
if (!isEmpty(selectedColumn)) {
const selectedData = column.find((d) => d.name === selectedColumn);
const allTestAdded =
selectedData?.columnTests?.length ===
Object.values(ColumnTestType).length;
const isSupported = isSupportedTest(selectedData?.dataType || '');
setIsShowError({
...isShowError,
allTestAdded,
notSupported: isSupported,
});
}
setColumnOptions(column);
setColumnName(selectedColumn || '');
handleTestTypeOptionChange(selectedColumn || '');
} else {
setColumnOptions(column);
setTestTypeOptions(Object.values(ColumnTestType));
Expand All @@ -154,6 +174,7 @@ const ColumnTestForm = ({
const validateForm = () => {
const errMsg = cloneDeep(isShowError);
errMsg.columnName = isEmpty(columnName);
errMsg.testName = isEmpty(columnTest);

switch (columnTest) {
case ColumnTestType.columnValueLengthsToBeBetween:
Expand Down Expand Up @@ -220,7 +241,13 @@ const ColumnTestForm = ({
};

case ColumnTestType.columnValuesToBeNotNull:
return {
columnValuesToBeNotNull: true,
};
case ColumnTestType.columnValuesToBeUnique:
return {
columnValuesToBeUnique: true,
};
default:
return {};
}
Expand Down Expand Up @@ -269,6 +296,7 @@ const ColumnTestForm = ({
errorMsg.missingCountValue = false;
errorMsg.values = false;
errorMsg.minMaxValue = false;
errorMsg.testName = false;

break;
}
Expand Down Expand Up @@ -303,7 +331,12 @@ const ColumnTestForm = ({
setForbiddenValues(['']);
setColumnName(value);
handleTestTypeOptionChange(value);
errorMsg.allTestAdded =
selectedColumn?.columnTests?.length ===
Object.values(ColumnTestType).length;
errorMsg.columnName = false;
errorMsg.testName = false;
errorMsg.notSupported = isSupportedTest(selectedColumn?.dataType || '');

break;
}
Expand Down Expand Up @@ -534,13 +567,18 @@ const ColumnTestForm = ({
name="columnName"
value={columnName}
onChange={handleValidation}>
<option value="">Select column name</option>
{columnOptions.map((option) => (
<option key={option.name} value={option.name}>
{option.name}
</option>
))}
</select>
{isShowError.columnName && errorMsg('Column name is required.')}
{isShowError.notSupported &&
errorMsg('Complex data type is not yet supported for test.')}
{isShowError.allTestAdded &&
errorMsg('All the tests have been added to the selected column.')}
</Field>

<Field>
Expand All @@ -556,6 +594,7 @@ const ColumnTestForm = ({
name="columTestType"
value={columnTest}
onChange={handleValidation}>
<option value="">Select column test</option>
{testTypeOptions &&
testTypeOptions.length > 0 &&
testTypeOptions.map((option) => (
Expand All @@ -564,6 +603,7 @@ const ColumnTestForm = ({
</option>
))}
</select>
{isShowError.testName && errorMsg('Column test is required.')}
</Field>

<Field>
Expand Down Expand Up @@ -611,6 +651,7 @@ const ColumnTestForm = ({
</Button>
<Button
className="tw-w-16 tw-h-10"
disabled={isShowError.allTestAdded || isShowError.notSupported}
size="regular"
theme="primary"
variant="contained"
Expand Down
17 changes: 15 additions & 2 deletions ...tadata-ui/src/main/resources/ui/src/components/AddDataQualityTest/Forms/TableTestForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ const TableTestForm = ({
}: Props) => {
const markdownRef = useRef<EditorContentRef>();
const [tableTest, setTableTest] = useState<TableTestType | undefined>(
data?.testCase?.tableTestType
data?.testCase?.tableTestType || ('' as TableTestType)
);
const [description] = useState(data?.description || '');
const [minValue, setMinValue] = useState<number | undefined>(
Expand All @@ -75,6 +75,8 @@ const TableTestForm = ({
minOrMax: false,
values: false,
minMaxValue: false,
allTestAdded: false,
tableTest: false,
});
const [testTypeOptions, setTestTypeOptions] = useState<string[]>();

Expand All @@ -86,8 +88,13 @@ const TableTestForm = ({
const newTest = Object.values(TableTestType).filter(
(d) => !existingTest.includes(d)
);
const allTestAdded =
tableTestCase.length === Object.values(TableTestType).length;
setIsShowError({
...isShowError,
allTestAdded,
});
setTestTypeOptions(newTest);
setTableTest(newTest[0]);
} else {
const testValue = Object.values(TableTestType);
setTestTypeOptions(testValue);
Expand All @@ -101,6 +108,8 @@ const TableTestForm = ({
const isTableRowCountToBeBetweenTest =
tableTest === TableTestType.TableRowCountToBeBetween;

errMsg.tableTest = isEmpty(tableTest);

if (isTableRowCountToBeBetweenTest) {
errMsg.minOrMax = isEmpty(minValue) && isEmpty(maxValue);
if (!isUndefined(minValue) && !isUndefined(maxValue)) {
Expand Down Expand Up @@ -291,6 +300,7 @@ const TableTestForm = ({
name="tableTestType"
value={tableTest}
onChange={handleValidation}>
<option value="">Select table test</option>
{testTypeOptions &&
testTypeOptions.length > 0 &&
testTypeOptions.map((option) => (
Expand All @@ -299,6 +309,8 @@ const TableTestForm = ({
</option>
))}
</select>
{isShowError.allTestAdded &&
errorMsg('All the tests have been added to the table.')}
</Field>

<Field>
Expand Down Expand Up @@ -349,6 +361,7 @@ const TableTestForm = ({
</Button>
<Button
className="tw-w-16 tw-h-10"
disabled={isShowError.allTestAdded}
size="regular"
theme="primary"
variant="contained"
Expand Down
3 changes: 3 additions & 0 deletions openmetadata-ui/src/main/resources/ui/src/components/DataQualityTab/DataQualityTab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ type Props = {
handleShowTestForm: (value: boolean) => void;
tableTestCase: TableTest[];
handleRemoveTableTest: (testType: TableTestType) => void;
selectedColumn: string;
handleRemoveColumnTest: (
columnName: string,
testType: ColumnTestType
Expand All @@ -52,6 +53,7 @@ const DataQualityTab = ({
handleRemoveColumnTest,
testMode,
tableTestCase,
selectedColumn,
}: Props) => {
const [showDropDown, setShowDropDown] = useState(false);
const [activeData, setActiveData] = useState<TableTestDataType>();
Expand Down Expand Up @@ -106,6 +108,7 @@ const DataQualityTab = ({
data={activeData}
handleAddColumnTestCase={onColumnTestSave}
handleAddTableTestCase={onTableTestSave}
selectedColumn={selectedColumn}
tableTestCase={tableTestCase}
testMode={testMode}
onFormCancel={onFormCancel}
Expand Down