Fix(UI): Send a single, minimal PATCH when updating a Data Product's domain#27598
Merged
Fix(UI): Send a single, minimal PATCH when updating a Data Product's domain#27598
Conversation
…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>
Contributor
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adjusts the Data Product domain editor to delegate updates to the page-level update handler so the UI emits a single minimal PATCH /dataProducts/{id} per domain change (avoiding stale diff-based extra ops that caused backend JsonPatch exceptions).
Changes:
- Remove direct
patchDataProduct(compare(...))flow fromDataProductDomainWidgetand instead call the contextonUpdatewith trimmed domain references. - Add Jest coverage ensuring the widget delegates via
onUpdateand strips domain ref fields. - Add a Playwright regression test asserting exactly one minimal PATCH is sent when changing a Data Product’s domain.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| openmetadata-ui/src/main/resources/ui/src/components/DataProducts/DataProductDomainWidget/DataProductDomainWidget.tsx | Stops self-patching and delegates to page-level update with minimal domains refs. |
| openmetadata-ui/src/main/resources/ui/src/components/DataProducts/DataProductDomainWidget/DataProductDomainWidget.test.tsx | Adds unit tests verifying delegation and domain ref stripping. |
| openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataProductDomainMigration.spec.ts | Adds e2e regression test for single minimal PATCH behavior. |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
siddhant1
added
UI
Contributor
Jest test CoverageUI tests summary
|
Contributor
🟡 Playwright Results — all passed (12 flaky)✅ 3699 passed · ❌ 0 failed · 🟡 12 flaky · ⏭️ 89 skipped
🟡 12 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
…ation
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>
…pdates 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>
auto-merge was automatically disabled
April 22, 2026 05:52
Head branch was pushed to by a user without write access
Code Review 👍 Approved with suggestions 0 resolved / 1 findingsUpdates to the Data Product domain successfully reduce network traffic to a single PATCH request. Consider providing user feedback instead of a silent no-op when the update handler is falsy. 💡 Edge Case: Silent no-op when onUpdate is falsy (dead-code guard)The new guard 🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
karanh37
approved these changes
Apr 22, 2026
|
Contributor
|
Changes have been cherry-picked to the 1.12.6 branch. |
github-actions Bot
pushed a commit
that referenced
this pull request
Apr 22, 2026
…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)
jatinmasaram
pushed a commit
to jatinmasaram/OpenMetadata
that referenced
this pull request
May 2, 2026
…domain (open-metadata#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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DataProductDomainWidgetnow delegates to the page-levelhandleDataProductUpdateviaonUpdate, matching theDomainLabelV2.handleDomainSaveconvention — so onePATCH /api/v1/dataProducts/{id}with a singlereplace /domainsop fires per domain change, instead of two./impersonatedBy,/changeDescription,/version. fge'sJsonPatch.applythrew "Non-existing name/value pair in the object for key impersonatedBy" on the/impersonatedByop and aborted the whole request.{id, type, name, fullyQualifiedName}; the backend resolves the fullEntityReferenceviaEntityRepository.getValidatedDomains(…).Summary by Gitar
DataProductDomainWidgetto rethrow errors fromonUpdateto ensure callers can block optimistic UI updates on failure.DataProductDomainWidget.test.tsxto verify thatonUpdatefailures are correctly rethrown.mockShowErrorToastdependencies in the test suite.This will update automatically on new commits.