yujunjun/OpenMetadata
1
0
Fork 0
mirror of https://github.com/open-metadata/OpenMetadata.git synced 2025-10-01 11:52:12 +00:00
Code Issues Packages Projects Releases Wiki Activity

change contract update to patch call from put (#23229)

Browse Source 
* change contract update to patch call from put

* collapse the card when data is not availbale instead of hiding

* minor fix

* fix the sonar issue and addressed comments

* disabled save button while edit, if not changes detected and added playwright for it

* fix the playwright test due to recent pagination merge
This commit is contained in:
Ashish Gupta 2025-09-06 00:41:28 +05:30 committed by GitHub
parent cb5bcdbb9c
commit 1f65e92c58
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 407 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
openmetadata-ui/src/main/resources/ui/playwright/constant/dataContracts.ts
View File

@ -15,6 +15,8 @@ import { uuid } from '../utils/common';
export const DATA_CONTRACT_DETAILS = {
name: `data_contract_${uuid()}`,
description: 'new data contract description',
displayName: `Data Contract_${uuid()}`,
description2: 'Modified Data Contract Description',
};
export const DATA_CONTRACT_SEMANTICS1 = {

92
openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/DataContracts.spec.ts
View File

@ -39,7 +39,7 @@ import {
validateDataContractInsideBundleTestSuites,
waitForDataContractExecution,
} from '../../utils/dataContracts';
import { addOwner } from '../../utils/entity';
import { addOwner, addOwnerWithoutValidation } from '../../utils/entity';
import { settingClick } from '../../utils/sidebar';
const adminUser = new UserClass();
@ -314,6 +314,11 @@ test.describe('Data Contracts', () => {
await page.reload();
await page.waitForLoadState('networkidle');
await page.waitForSelector('[data-testid="loader"]', {
state: 'detached',
});
await expect(
page.getByTestId('contract-status-card-item-Semantics-status')
).toContainText('Passed');
@ -517,6 +522,85 @@ test.describe('Data Contracts', () => {
await download.saveAs('downloads/' + download.suggestedFilename());
});
await test.step('Edit and Validate Contract data', async () => {
await page.getByTestId('contract-edit-button').click();
await expect(page.getByTestId('save-contract-btn')).toBeDisabled();
// Change the Contract Details
await page
.getByTestId('contract-name')
.fill(DATA_CONTRACT_DETAILS.displayName);
await page.click('.om-block-editor[contenteditable="true"]');
await page.keyboard.press('Control+A');
await page.keyboard.type(DATA_CONTRACT_DETAILS.description2);
await addOwnerWithoutValidation({
page,
owner: 'admin',
type: 'Users',
initiatorId: 'select-owners',
});
await expect(
page.getByTestId('user-tag').getByText('admin')
).toBeVisible();
// Move to Schema Tab
await page.getByRole('button', { name: 'Schema' }).click();
// TODO: will enable this once nested column is fixed
// await page.waitForSelector('[data-testid="loader"]', {
// state: 'detached',
// });
// await page.getByRole('checkbox', { name: 'Select all' }).click();
// await expect(
// page.getByRole('checkbox', { name: 'Select all' })
// ).not.toBeChecked();
// Move to Semantic Tab
await page.getByRole('button', { name: 'Semantics' }).click();
await page.getByTestId('delete-condition-button').last().click();
await expect(
page.getByTestId('query-builder-form-field').getByText('Description')
).not.toBeVisible();
await expect(page.getByTestId('save-contract-btn')).not.toBeDisabled();
const saveContractResponse = page.waitForResponse(
'/api/v1/dataContracts/*'
);
await page.getByTestId('save-contract-btn').click();
await saveContractResponse;
await page.waitForLoadState('networkidle');
await page.waitForSelector('[data-testid="loader"]', {
state: 'detached',
});
// Validate the Updated Values
await expect(page.getByTestId('contract-title')).toContainText(
DATA_CONTRACT_DETAILS.displayName
);
await expect(
page.getByTestId('contract-owner-card').getByTestId('admin')
).toBeVisible();
await expect(
page.locator(
'[data-testid="viewer-container"] [data-testid="markdown-parser"]'
)
).toContainText(DATA_CONTRACT_DETAILS.description2);
// TODO: will enable this once nested column is fixed
// await expect(page.getByTestId('schema-table-card')).not.toBeVisible();
});
await test.step('Delete contract', async () => {
const deleteContractResponse = page.waitForResponse(
'api/v1/dataContracts/*?hardDelete=true&recursive=true'
@ -893,7 +977,7 @@ test.describe('Data Contracts', () => {
await test.step('Save contract and validate for schema', async () => {
const saveContractResponse = page.waitForResponse(
'/api/v1/dataContracts'
'/api/v1/dataContracts/*'
);
await page.getByTestId('save-contract-btn').click();
@ -937,7 +1021,7 @@ test.describe('Data Contracts', () => {
).not.toBeChecked();
const saveContractResponse = page.waitForResponse(
'/api/v1/dataContracts'
'/api/v1/dataContracts/*'
);
await page.getByTestId('save-contract-btn').click();
@ -979,7 +1063,7 @@ test.describe('Data Contracts', () => {
}
const saveContractResponse = page.waitForResponse(
'/api/v1/dataContracts'
'/api/v1/dataContracts/*'
);
await page.getByTestId('save-contract-btn').click();

7
openmetadata-ui/src/main/resources/ui/playwright/utils/dataContracts.ts
View File

@ -19,7 +19,7 @@ export const saveAndTriggerDataContractValidation = async (
page: Page,
isContractStatusNotVisible?: boolean
): Promise<string | undefined> => {
const saveContractResponse = page.waitForResponse('/api/v1/dataContracts');
const saveContractResponse = page.waitForResponse('/api/v1/dataContracts/*');
await page.getByTestId('save-contract-btn').click();
const response = await saveContractResponse;
const responseData = await response.json();
@ -40,6 +40,11 @@ export const saveAndTriggerDataContractValidation = async (
await page.reload();
await page.waitForLoadState('networkidle');
await page.waitForSelector('[data-testid="loader"]', {
state: 'detached',
});
return responseData;
};

45
openmetadata-ui/src/main/resources/ui/playwright/utils/entity.ts
View File

@ -164,6 +164,51 @@ export const addOwner = async ({
).toBeVisible();
};
export const addOwnerWithoutValidation = async ({
page,
owner,
type = 'Users',
initiatorId = 'edit-owner',
}: {
page: Page;
owner: string;
type?: 'Teams' | 'Users';
initiatorId?: string;
}) => {
await page.getByTestId(initiatorId).click();
if (type === 'Users') {
const userListResponse = page.waitForResponse(
'/api/v1/search/query?q=*isBot:false*index=user_search_index*'
);
await page.getByRole('tab', { name: type }).click();
await userListResponse;
}
await page.waitForSelector('[data-testid="loader"]', { state: 'detached' });
const ownerSearchBar = await page
.getByTestId(`owner-select-${lowerCase(type)}-search-bar`)
.isVisible();
if (!ownerSearchBar) {
await page.getByRole('tab', { name: type }).click();
}
const searchUser = page.waitForResponse(
`/api/v1/search/query?q=*${encodeURIComponent(owner)}*`
);
await page
.getByTestId(`owner-select-${lowerCase(type)}-search-bar`)
.fill(owner);
await searchUser;
if (type === 'Teams') {
await page.getByRole('listitem', { name: owner, exact: true }).click();
} else {
await page.getByRole('listitem', { name: owner, exact: true }).click();
await page.getByTestId('selectable-list-update-btn').click();
}
};
export const updateOwner = async ({
page,
owner,

207
openmetadata-ui/src/main/resources/ui/src/components/DataContract/AddDataContract/AddDataContract.test.tsx
View File

@ -12,6 +12,7 @@
*/
import '@testing-library/jest-dom';
import { act, fireEvent, render, screen } from '@testing-library/react';
import { AxiosError } from 'axios';
import { EntityType } from '../../../enums/entity.enum';
import {
DataContract,
@ -73,7 +74,9 @@ jest.mock('../ContractDetailFormTab/ContractDetailFormTab', () => ({
.mockImplementation(({ onChange, onNext }) => (
<div>
<h2>Contract Details</h2>
<button onClick={onChange}>Change</button>
<button onClick={() => onChange({ name: 'Test Contract Change' })}>
Change
</button>
<button onClick={onNext}>Next</button>
</div>
)),
@ -84,7 +87,9 @@ jest.mock('../ContractQualityFormTab/ContractQualityFormTab', () => ({
.mockImplementation(({ onChange, onNext }) => (
<div>
<h2>Contract Quality</h2>
<button onClick={onChange}>Change</button>
<button onClick={() => onChange({ qualityExpectations: [] })}>
Change
</button>
<button onClick={onNext}>Next</button>
</div>
)),
@ -96,7 +101,7 @@ jest.mock('../ContractSchemaFormTab/ContractScehmaFormTab', () => ({
<div>
<h2>Contract Schema</h2>
<button onClick={onPrev}>Previous</button>
<button onClick={onChange}>Change</button>
<button onClick={() => onChange({ schema: [] })}>Change</button>
<button onClick={onNext}>Next</button>
</div>
)),
@ -109,7 +114,7 @@ jest.mock('../ContractSemanticFormTab/ContractSemanticFormTab', () => ({
<div>
<h2>Contract Semantics</h2>
<button onClick={onPrev}>Previous</button>
<button onClick={onChange}>Change</button>
<button onClick={() => onChange({ semantics: [] })}>Change</button>
<button onClick={onNext}>Next</button>
</div>
)),
@ -202,9 +207,19 @@ describe('AddDataContract', () => {
});
describe('Save Functionality', () => {
it('should call createContract for new contract', async () => {
beforeEach(() => {
jest.clearAllMocks();
});
it('should call createContract for new contract with correct parameters', async () => {
render(<AddDataContract onCancel={mockOnCancel} onSave={mockOnSave} />);
// First trigger a form change to enable the save button
const changeButton = screen.getByText('Change');
await act(async () => {
fireEvent.click(changeButton);
});
const saveButton = screen.getByTestId('save-contract-btn');
await act(async () => {
@ -213,12 +228,13 @@ describe('AddDataContract', () => {
expect(createContract).toHaveBeenCalledWith(
expect.objectContaining({
displayName: 'Test Contract Change', // Now formValues.name is set from mock
entity: {
id: 'table-id',
type: EntityType.TABLE,
},
semantics: undefined, // validSemantics - undefined when no semantics provided
entityStatus: EntityStatus.Approved,
semantics: undefined,
})
);
expect(showSuccessToast).toHaveBeenCalledWith(
@ -227,7 +243,39 @@ describe('AddDataContract', () => {
expect(mockOnSave).toHaveBeenCalled();
});
it('should call updateContract for existing contract', async () => {
it('should call createContract with form changes applied', async () => {
render(<AddDataContract onCancel={mockOnCancel} onSave={mockOnSave} />);
// Trigger a form change to enable the save button and set form values
const changeButton = screen.getByText('Change');
await act(async () => {
fireEvent.click(changeButton);
});
const saveButton = screen.getByTestId('save-contract-btn');
await act(async () => {
fireEvent.click(saveButton);
});
expect(createContract).toHaveBeenCalledWith(
expect.objectContaining({
displayName: 'Test Contract Change', // formValues.name from mock onChange
entity: {
id: 'table-id',
type: EntityType.TABLE,
},
semantics: undefined, // validSemantics - undefined when no semantics provided
entityStatus: EntityStatus.Approved,
})
);
expect(showSuccessToast).toHaveBeenCalledWith(
'message.data-contract-saved-successfully'
);
expect(mockOnSave).toHaveBeenCalled();
});
it('should call updateContract for existing contract with JSON patch', async () => {
render(
<AddDataContract
contract={mockContract}
@ -236,13 +284,22 @@ describe('AddDataContract', () => {
/>
);
// Trigger a form change to enable the save button for existing contracts
const changeButton = screen.getByText('Change');
await act(async () => {
fireEvent.click(changeButton);
});
const saveButton = screen.getByTestId('save-contract-btn');
await act(async () => {
fireEvent.click(saveButton);
});
expect(updateContract).toHaveBeenCalled();
expect(updateContract).toHaveBeenCalledWith(
'contract-1',
expect.any(Array) // JSON patch array from fast-json-patch compare
);
expect(showSuccessToast).toHaveBeenCalledWith(
'message.data-contract-saved-successfully'
);
@ -255,6 +312,12 @@ describe('AddDataContract', () => {
render(<AddDataContract onCancel={mockOnCancel} onSave={mockOnSave} />);
// Trigger form change to enable save button
const changeButton = screen.getByText('Change');
await act(async () => {
fireEvent.click(changeButton);
});
const saveButton = screen.getByTestId('save-contract-btn');
await act(async () => {
@ -262,6 +325,35 @@ describe('AddDataContract', () => {
});
expect(showErrorToast).toHaveBeenCalledWith(mockError);
expect(mockOnSave).not.toHaveBeenCalled(); // Should not call onSave on error
});
it('should handle update contract errors gracefully', async () => {
const mockError = new AxiosError('Update failed');
(updateContract as jest.Mock).mockRejectedValue(mockError);
render(
<AddDataContract
contract={mockContract}
onCancel={mockOnCancel}
onSave={mockOnSave}
/>
);
// Trigger form change to enable save button
const changeButton = screen.getByText('Change');
await act(async () => {
fireEvent.click(changeButton);
});
const saveButton = screen.getByTestId('save-contract-btn');
await act(async () => {
fireEvent.click(saveButton);
});
expect(showErrorToast).toHaveBeenCalledWith(mockError);
expect(mockOnSave).not.toHaveBeenCalled(); // Should not call onSave on error
});
it('should filter out empty semantics before saving', async () => {
@ -269,7 +361,9 @@ describe('AddDataContract', () => {
...mockContract,
semantics: [
{ name: 'Valid Semantic', rule: 'valid rule' },
{ name: '', rule: '' },
{ name: '', rule: '' }, // Should be filtered out
{ name: 'Valid Name', rule: '' }, // Should be filtered out (empty rule)
{ name: '', rule: 'valid rule' }, // Should be filtered out (empty name)
{ name: 'Another Valid', rule: 'another rule' },
] as SemanticsRule[],
};
@ -288,15 +382,102 @@ describe('AddDataContract', () => {
fireEvent.click(saveButton);
});
// Should call updateContract with only valid semantics
expect(updateContract).toHaveBeenCalledWith(
'contract-1',
expect.any(Array) // JSON patch comparing with filtered semantics
);
});
it('should set displayName from formValues.name in create mode', async () => {
render(<AddDataContract onCancel={mockOnCancel} onSave={mockOnSave} />);
// Trigger form change first
const changeButton = screen.getByText('Change');
await act(async () => {
fireEvent.click(changeButton);
});
const saveButton = screen.getByTestId('save-contract-btn');
await act(async () => {
fireEvent.click(saveButton);
});
expect(createContract).toHaveBeenCalledWith(
expect.objectContaining({
semantics: [
{ name: 'Valid Semantic', rule: 'valid rule' },
{ name: 'Another Valid', rule: 'another rule' },
],
displayName: 'Test Contract Change', // formValues.name from mock
})
);
});
it('should include displayName in update patch for edit mode', async () => {
render(
<AddDataContract
contract={mockContract}
onCancel={mockOnCancel}
onSave={mockOnSave}
/>
);
// Trigger form change to enable save button
const changeButton = screen.getByText('Change');
await act(async () => {
fireEvent.click(changeButton);
});
const saveButton = screen.getByTestId('save-contract-btn');
await act(async () => {
fireEvent.click(saveButton);
});
// For update, displayName should be set to formValues.name in the patch comparison
expect(updateContract).toHaveBeenCalledWith(
'contract-1',
expect.any(Array)
);
});
it('should disable save button when no changes are detected', async () => {
render(
<AddDataContract
contract={mockContract}
onCancel={mockOnCancel}
onSave={mockOnSave}
/>
);
const saveButton = screen.getByTestId('save-contract-btn');
// Button should be disabled when no changes are made (isSaveDisabled logic)
// This happens when the JSON patch comparison results in an empty array
expect(saveButton).toBeInTheDocument();
});
it('should show loading state during save operation', async () => {
// Mock a delayed response to test loading state
(createContract as jest.Mock).mockImplementation(
() => new Promise((resolve) => setTimeout(resolve, 100))
);
render(<AddDataContract onCancel={mockOnCancel} onSave={mockOnSave} />);
// Trigger form change to enable save button first
const changeButton = screen.getByText('Change');
await act(async () => {
fireEvent.click(changeButton);
});
const saveButton = screen.getByTestId('save-contract-btn');
act(() => {
fireEvent.click(saveButton);
});
// Should show loading state (Ant Design Button shows loading via classes)
expect(saveButton).toHaveClass('ant-btn-loading');
});
});
describe('Cancel Functionality', () => {

65
openmetadata-ui/src/main/resources/ui/src/components/DataContract/AddDataContract/AddDataContract.tsx
View File

@ -13,6 +13,7 @@
import { Button, Card, RadioChangeEvent, Tabs, Typography } from 'antd';
import { AxiosError } from 'axios';
import { compare } from 'fast-json-patch';
import { isEmpty } from 'lodash';
import React, { useCallback, useMemo, useState } from 'react';
import { useTranslation } from 'react-i18next';
@ -32,7 +33,6 @@ import {
} from '../../../generated/entity/data/dataContract';
import { Table } from '../../../generated/entity/data/table';
import { createContract, updateContract } from '../../../rest/contractAPI';
import { getUpdatedContractDetails } from '../../../utils/DataContract/DataContractUtils';
import { showErrorToast, showSuccessToast } from '../../../utils/ToastUtils';
import { useGenericContext } from '../../Customization/GenericProvider/GenericProvider';
import SchemaEditor from '../../Database/SchemaEditor/SchemaEditor';
@ -73,28 +73,49 @@ const AddDataContract: React.FC<{
setActiveTab(key);
}, []);
const handleSave = useCallback(async () => {
setIsSubmitting(true);
const { validSemantics, isSaveDisabled } = useMemo(() => {
const validSemantics = formValues.semantics?.filter(
(semantic) => !isEmpty(semantic.name) && !isEmpty(semantic.rule)
);
return {
validSemantics,
isSaveDisabled: isEmpty(
compare(contract ?? {}, {
...contract,
...formValues,
semantics: validSemantics,
})
),
};
}, [contract, formValues]);
const handleSave = useCallback(async () => {
setIsSubmitting(true);
try {
await (contract
? updateContract({
...getUpdatedContractDetails(contract, formValues),
semantics: validSemantics,
})
: createContract({
if (contract) {
await updateContract(
contract?.id,
compare(contract, {
...contract,
...formValues,
entity: {
id: table.id,
type: EntityType.TABLE,
},
semantics: validSemantics,
entityStatus: EntityStatus.Approved,
}));
displayName: formValues.name,
})
);
} else {
await createContract({
...formValues,
displayName: formValues.name,
entity: {
id: table.id,
type: EntityType.TABLE,
},
semantics: validSemantics,
entityStatus: EntityStatus.Approved,
});
}
showSuccessToast(t('message.data-contract-saved-successfully'));
onSave();
@ -103,7 +124,7 @@ const AddDataContract: React.FC<{
} finally {
setIsSubmitting(false);
}
}, [contract, formValues, table?.id]);
}, [contract, formValues, table?.id, validSemantics]);
const onFormChange = useCallback(
(data: Partial<DataContract>) => {
@ -227,6 +248,7 @@ const AddDataContract: React.FC<{
<Button
className="add-contract-save-button"
data-testid="save-contract-btn"
disabled={isSaveDisabled}
loading={isSubmitting}
type="primary"
onClick={handleSave}>
@ -235,7 +257,14 @@ const AddDataContract: React.FC<{
</div>
</div>
);
}, [mode, handleModeChange, onCancel, handleSave, isSubmitting]);
}, [
mode,
isSubmitting,
isSaveDisabled,
handleModeChange,
onCancel,
handleSave,
]);
const cardContent = useMemo(() => {
if (mode === DataContractMode.YAML) {

3
openmetadata-ui/src/main/resources/ui/src/components/DataContract/ContractDetailFormTab/ContractDetailFormTab.tsx
View File

@ -17,6 +17,7 @@ import { useTranslation } from 'react-i18next';
import { ReactComponent as RightIcon } from '../../../assets/svg/right-arrow.svg';
import { DataContract } from '../../../generated/entity/data/dataContract';
import { FieldProp, FieldTypes } from '../../../interface/FormUtils.interface';
import { getEntityName } from '../../../utils/EntityUtils';
import { generateFormFields } from '../../../utils/formUtils';
import './contract-detail-form-tab.less';
@ -72,7 +73,7 @@ export const ContractDetailFormTab: React.FC<{
useEffect(() => {
if (initialValues) {
form.setFieldsValue({
name: initialValues.name,
name: getEntityName(initialValues),
description: initialValues.description,
owners: initialValues.owners,
});

17
openmetadata-ui/src/main/resources/ui/src/components/DataContract/ContractDetailTab/ContractDetail.tsx
View File

@ -286,11 +286,15 @@ const ContractDetail: React.FC<{
return (
<Row align="middle" justify="space-between">
<Col flex="auto">
<Typography.Text className="contract-title">
<Typography.Text
className="contract-title"
data-testid="contract-title">
{getEntityName(contract)}
</Typography.Text>
<Typography.Text className="contract-time">
<Typography.Text
className="contract-time"
data-testid="contract-last-updated-time">
{t('message.modified-time-ago-by', {
time: getRelativeTime(contract.updatedAt),
by: contract.updatedBy,
@ -316,7 +320,9 @@ const ContractDetail: React.FC<{
<Col>
<div className="contract-action-container">
{!isEmpty(contract.owners) && (
<div className="contract-owner-label-container">
<div
className="contract-owner-label-container"
data-testid="contract-owner-card">
<Typography.Text>{t('label.owner-plural')}</Typography.Text>
<OwnerLabel
avatarSize={24}
@ -475,7 +481,10 @@ const ContractDetail: React.FC<{
</Typography.Text>
</div>
),
}}>
}}
dataTestId="schema-table-card"
defaultExpanded={!isEmpty(schemaDetail)}
isExpandDisabled={isEmpty(schemaDetail)}>
<Table
columns={schemaColumns}
dataSource={schemaDetail}

6
openmetadata-ui/src/main/resources/ui/src/components/DataContract/ContractSemanticFormTab/ContractSemanticFormTab.tsx
View File

@ -17,7 +17,7 @@ import { Button, Col, Form, Input, Row, Switch, Typography } from 'antd';
import Card from 'antd/lib/card/Card';
import TextArea from 'antd/lib/input/TextArea';
import classNames from 'classnames';
import { isNull, isNumber } from 'lodash';
import { isEmpty, isNull, isNumber } from 'lodash';
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
import { useTranslation } from 'react-i18next';
import { ReactComponent as DeleteIcon } from '../../../assets/svg/ic-trash.svg';
@ -84,9 +84,9 @@ export const ContractSemanticFormTab: React.FC<{
}, [queryBuilderAddRule]);
useEffect(() => {
if (initialValues?.semantics) {
if (!isEmpty(initialValues?.semantics)) {
form.setFieldsValue({
semantics: initialValues.semantics,
semantics: initialValues?.semantics,
});
} else {
form.setFieldsValue({

12
openmetadata-ui/src/main/resources/ui/src/rest/contractAPI.ts
View File

@ -10,6 +10,8 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { AxiosResponse } from 'axios';
import { Operation } from 'fast-json-patch';
import {
ContractAllResult,
ContractResultFilter,
@ -62,11 +64,11 @@ export const createContract = async (contract: CreateDataContract) => {
return response.data;
};
export const updateContract = async (contract: CreateDataContract) => {
const response = await APIClient.put<CreateDataContract>(
`/dataContracts`,
contract
);
export const updateContract = async (id: string, data: Operation[]) => {
const response = await APIClient.patch<
Operation[],
AxiosResponse<CreateDataContract>
>(`/dataContracts/${id}`, data);
return response.data;
};