Skip to content

Commit 3df57da

Browse files
siddhant1SiddhantclaudeCopilot
authored and
OpenMetadata Release Bot
committed
Fix(UI): Send a single, minimal PATCH when updating a Data Product's domain (#27598)
* Fix(UI): Send a single, minimal PATCH when updating a Data Product's domain DataProductDomainWidget now delegates to the page-level handleDataProductUpdate via onUpdate, matching the DomainLabelV2 convention, so one PATCH with a single replace /domains op is sent instead of two. The second PATCH previously diffed stale context against the server response and emitted operations on /impersonatedBy, /changeDescription, /version — the /impersonatedBy op failed inside fge JsonPatch with "Non-existing name/value pair in the object for key impersonatedBy". Domain refs in the patch are trimmed to {id, type, name, fullyQualifiedName}; the backend resolves the full EntityReference via getValidatedDomains. Adds jest coverage for the widget and a Playwright regression test that asserts exactly one PATCH per domain change and validates the minimal payload shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Drop flaky UI single-PATCH Playwright case from DataProductDomainMigration The new "Changing data product domain via UI fires a single minimal PATCH" Playwright case timed out across all retries on CI. The selectors borrowed from assignDomain don't match the DataProductDomainWidget's mounted variant of DomainLabelV2 + DomainSelectableList, and the rest of this spec already drives state via API patches rather than the picker UI. The widget-level jest test (DataProductDomainWidget.test.tsx) already proves the regression: it asserts patchDataProduct is never called and that onUpdate receives a DataProduct with minimal {id, type, name, fullyQualifiedName} domain refs. That coverage is sufficient for the patch-shape regression. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: rethrow errors in performDomainUpdate to prevent optimistic UI updates on failure Agent-Logs-Url: https://github.com/open-metadata/OpenMetadata/sessions/6c1731fa-0f99-423e-828c-afbff35bd885 Co-authored-by: siddhant1 <30566406+siddhant1@users.noreply.github.com> --------- Co-authored-by: Siddhant <siddhant@MacBook-Pro-621.local> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> (cherry picked from commit 3940447)
1 parent c56f2aa commit 3df57da

2 files changed

Lines changed: 159 additions & 20 deletions

File tree

‎openmetadata-ui/src/main/resources/ui/src/components/DataProducts/DataProductDomainWidget/DataProductDomainWidget.test.tsx‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
/*
2+
* Copyright 2026 Collate.
3+
* Licensed under the Apache License, Version 2.0 (the "License");
4+
* you may not use this file except in compliance with the License.
5+
* You may obtain a copy of the License at
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
* Unless required by applicable law or agreed to in writing, software
8+
* distributed under the License is distributed on an "AS IS" BASIS,
9+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
10+
* See the License for the specific language governing permissions and
11+
* limitations under the License.
12+
*/
13+
14+
import { act, render, waitFor } from '@testing-library/react';
15+
import { DataProduct } from '../../../generated/entity/domains/dataProduct';
16+
import { EntityReference } from '../../../generated/entity/type';
17+
import { DataProductDomainWidget } from './DataProductDomainWidget';
18+
19+
const mockOnUpdate = jest.fn().mockResolvedValue(undefined);
20+
const mockPatchDataProduct = jest.fn();
21+
22+
let capturedOnUpdate:
23+
| ((d: EntityReference | EntityReference[]) => Promise<void> | void)
24+
| undefined;
25+
26+
const mockDataProduct: DataProduct = {
27+
id: 'dp-1',
28+
name: 'dp_one',
29+
displayName: 'DP One',
30+
fullyQualifiedName: 'dp_one',
31+
description: 'test',
32+
version: 0.1,
33+
updatedAt: 1,
34+
updatedBy: 'tester',
35+
domains: [
36+
{
37+
id: 'dom-source',
38+
type: 'domain',
39+
name: 'source',
40+
fullyQualifiedName: 'source',
41+
},
42+
],
43+
};
44+
45+
jest.mock('../../../rest/dataProductAPI', () => ({
46+
patchDataProduct: (...args: unknown[]) => mockPatchDataProduct(...args),
47+
}));
48+
49+
jest.mock('../../../rest/searchAPI', () => ({
50+
searchQuery: jest.fn().mockResolvedValue({ hits: { total: { value: 0 } } }),
51+
}));
52+
53+
jest.mock('../../Customization/GenericProvider/GenericProvider', () => ({
54+
useGenericContext: () => ({
55+
data: mockDataProduct,
56+
onUpdate: mockOnUpdate,
57+
}),
58+
}));
59+
60+
jest.mock('../../DataAssets/DomainLabelV2/DomainLabelV2', () => ({
61+
DomainLabelV2: jest.fn().mockImplementation((props) => {
62+
capturedOnUpdate = props.onUpdate;
63+
64+
return <div data-testid="mock-domain-label" />;
65+
}),
66+
}));
67+
68+
describe('DataProductDomainWidget', () => {
69+
beforeEach(() => {
70+
mockOnUpdate.mockClear();
71+
mockPatchDataProduct.mockClear();
72+
capturedOnUpdate = undefined;
73+
});
74+
75+
it('delegates domain change to onUpdate without self-patching', async () => {
76+
render(<DataProductDomainWidget />);
77+
78+
await waitFor(() => expect(capturedOnUpdate).toBeDefined());
79+
80+
const newDomain: EntityReference = {
81+
id: 'dom-target',
82+
type: 'domain',
83+
name: 'target',
84+
fullyQualifiedName: 'target',
85+
displayName: 'Target',
86+
description: 'should be stripped',
87+
href: 'http://ignored',
88+
deleted: false,
89+
inherited: false,
90+
};
91+
92+
await act(async () => {
93+
await capturedOnUpdate?.(newDomain);
94+
});
95+
96+
expect(mockPatchDataProduct).not.toHaveBeenCalled();
97+
expect(mockOnUpdate).toHaveBeenCalledTimes(1);
98+
99+
const payload = mockOnUpdate.mock.calls[0][0] as DataProduct;
100+
101+
expect(payload.id).toBe(mockDataProduct.id);
102+
expect(payload.domains).toEqual([
103+
{
104+
id: 'dom-target',
105+
type: 'domain',
106+
name: 'target',
107+
fullyQualifiedName: 'target',
108+
},
109+
]);
110+
});
111+
112+
it('passes empty domains array when selection is cleared', async () => {
113+
render(<DataProductDomainWidget />);
114+
115+
await waitFor(() => expect(capturedOnUpdate).toBeDefined());
116+
117+
await act(async () => {
118+
await capturedOnUpdate?.([]);
119+
});
120+
121+
expect(mockPatchDataProduct).not.toHaveBeenCalled();
122+
expect(mockOnUpdate).toHaveBeenCalledTimes(1);
123+
124+
const payload = mockOnUpdate.mock.calls[0][0] as DataProduct;
125+
126+
expect(payload.domains).toEqual([]);
127+
});
128+
129+
it('rethrows errors from onUpdate so callers can prevent optimistic UI updates', async () => {
130+
const updateError = new Error('PATCH failed');
131+
mockOnUpdate.mockRejectedValueOnce(updateError);
132+
133+
render(<DataProductDomainWidget />);
134+
135+
await waitFor(() => expect(capturedOnUpdate).toBeDefined());
136+
137+
const newDomain: EntityReference = {
138+
id: 'dom-target',
139+
type: 'domain',
140+
name: 'target',
141+
fullyQualifiedName: 'target',
142+
};
143+
144+
await expect(
145+
act(async () => {
146+
await capturedOnUpdate?.(newDomain);
147+
})
148+
).rejects.toThrow('PATCH failed');
149+
});
150+
});

‎openmetadata-ui/src/main/resources/ui/src/components/DataProducts/DataProductDomainWidget/DataProductDomainWidget.tsx‎

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,14 @@
1212
*/
1313

1414
import { Modal } from 'antd';
15-
import { AxiosError } from 'axios';
16-
import { compare } from 'fast-json-patch';
1715
import { isEmpty } from 'lodash';
1816
import { useCallback, useEffect, useState } from 'react';
1917
import { useTranslation } from 'react-i18next';
2018
import { SearchIndex } from '../../../enums/search.enum';
2119
import { DataProduct } from '../../../generated/entity/domains/dataProduct';
2220
import { EntityReference } from '../../../generated/entity/type';
23-
import { patchDataProduct } from '../../../rest/dataProductAPI';
2421
import { searchQuery } from '../../../rest/searchAPI';
2522
import { getTermQuery } from '../../../utils/SearchUtils';
26-
import { showErrorToast } from '../../../utils/ToastUtils';
2723
import { useGenericContext } from '../../Customization/GenericProvider/GenericProvider';
2824
import { DomainLabelV2 } from '../../DataAssets/DomainLabelV2/DomainLabelV2';
2925

@@ -64,33 +60,26 @@ export const DataProductDomainWidget = () => {
6460

6561
const performDomainUpdate = useCallback(
6662
async (selectedDomain: EntityReference | EntityReference[]) => {
67-
if (!dataProduct) {
63+
if (!dataProduct || !onUpdate) {
6864
return;
6965
}
7066

7167
setIsLoading(true);
7268
try {
73-
const domains = Array.isArray(selectedDomain)
69+
const rawDomains = Array.isArray(selectedDomain)
7470
? selectedDomain
7571
: isEmpty(selectedDomain)
7672
? []
7773
: [selectedDomain];
7874

79-
const jsonPatch = compare(dataProduct, {
80-
...dataProduct,
81-
domains,
82-
});
75+
const domains: EntityReference[] = rawDomains.map((d) => ({
76+
id: d.id,
77+
type: d.type,
78+
name: d.name,
79+
fullyQualifiedName: d.fullyQualifiedName,
80+
}));
8381

84-
const updatedDataProduct = await patchDataProduct(
85-
dataProduct.id,
86-
jsonPatch
87-
);
88-
89-
if (onUpdate) {
90-
onUpdate(updatedDataProduct);
91-
}
92-
} catch (err) {
93-
showErrorToast(err as AxiosError);
82+
await onUpdate({ ...dataProduct, domains });
9483
} finally {
9584
setIsLoading(false);
9685
setIsConfirmModalOpen(false);

0 commit comments

Comments
 (0)