Fixes 30200: Paginate glossary term relation settings#30201
Conversation
✅ PR checks passedThe linked issue has a description and all required Shipping project fields set. Thanks! |
Code Review 👍 Approved with suggestions 2 resolved / 3 findingsImplements paginated glossary term relation settings and introduces compact CRUD endpoints to optimize performance. Note that the deletion usage check is susceptible to race conditions with concurrent relation creation. 💡 Quality: Duplicate-name UX depends on matching English error substringThe catch block decides whether to surface a duplicate-name error on the name field by matching the raw backend string with ✅ 2 resolved✅ Edge Case: Client-side duplicate-name check only scans current page
✅ Bug: Update/delete index computed outside atomic patch snapshot
🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
Code Review ✅ Approved 3 resolved / 3 findingsImplements paginated glossary relation settings and compact CRUD endpoints to improve performance, while resolving duplicate-name validation and atomicity issues. All identified findings regarding name-clash handling and update race conditions have been addressed. ✅ 3 resolved✅ Edge Case: Client-side duplicate-name check only scans current page
✅ Bug: Update/delete index computed outside atomic patch snapshot
✅ Quality: Duplicate-name UX depends on matching English error substring
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
🔴 Playwright Results — 1 failure(s), 23 flaky✅ 4547 passed · ❌ 1 failed · 🟡 23 flaky · ⏭️ 95 skipped
Genuine Failures (failed on all attempts)❌
|
Code Review ✅ Approved 3 resolved / 3 findingsImplements paginated glossary relation settings and compact CRUD endpoints to improve performance, while resolving duplicate-name validation and atomicity issues. All identified findings regarding name-clash handling and update race conditions have been addressed. ✅ 3 resolved✅ Edge Case: Client-side duplicate-name check only scans current page
✅ Bug: Update/delete index computed outside atomic patch snapshot
✅ Quality: Duplicate-name UX depends on matching English error substring
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
|



Describe your changes:
Fixes #30200
I paginated glossary relation type settings and added compact CRUD endpoints because the page previously fetched the complete settings payload and all relation usage counts before rendering. Relation type creation now rejects case-insensitive duplicate names inside the atomic settings update, while deletion checks usage only when requested.
Type of change:
High-level design:
Tests:
Use cases covered
Unit tests
GlossaryTermRelationSettingsUtilTest.java,SystemRepositoryPatchSettingTest.java,GlossaryTermRelationSettings.test.tsx, andglossaryRelationTypesAPI.test.ts.Backend integration tests
GlossaryTermRelationSettingsIT.java.Ingestion integration tests
Playwright (UI) tests
Manual testing performed
UI screen recording / screenshots:
Not attached yet; required before marking this draft ready for review.
Checklist:
Fixes <issue-number>: <short explanation>Fixes #<issue-number>above.Greptile Summary
This PR adds paginated glossary relation-type settings and compact CRUD operations. The main changes are:
Confidence Score: 5/5
This looks safe to merge after the existing review findings are resolved.
Important Files Changed
Comments Outside Diff (1)
openmetadata-service/src/main/java/org/openmetadata/service/resources/system/SystemResource.java, line 457-466 (link)A relation can be created after the usage query returns zero but before the settings patch removes the type. The settings update then succeeds and leaves that relation referring to a relation type that no longer exists because its compare-and-set does not cover relation writes.
Context Used: CLAUDE.md (source)
Reviews (4): Last reviewed commit: "Merge branch 'main' into harshach/pagina..." | Re-trigger Greptile
Context used (3)