fix: respect entity-level permissions for import/export in GlossaryHeader#27507
fix: respect entity-level permissions for import/export in GlossaryHeader#27507himanshu748 wants to merge 2 commits intoopen-metadata:mainfrom
Conversation
…ader When a Data Producer role has conditions (e.g. isOwner(), hasDomain(), matchTeam()) on its EditAll permission, the global permissions check returns false because conditions are not evaluated at the global level. This fix adds a fallback to the entity-level permissions.EditAll, which evaluates policy conditions against the specific entity. This ensures import/export buttons are shown when the user satisfies the conditions. Fixes open-metadata#27487
|
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! |
There was a problem hiding this comment.
Pull request overview
Fixes Glossary header import/export visibility so users with conditional EditAll permissions (e.g., isOwner(), hasDomain(), matchTeam()) still see the actions when the condition evaluates true for the current entity.
Changes:
- Update
GlossaryHeaderimport/export permission logic to OR in entity-levelpermissions.EditAll. - Add a unit test covering the scenario where global permissions deny
All/EditAllbut entity-levelEditAllis granted.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| openmetadata-ui/src/main/resources/ui/src/components/Glossary/GlossaryHeader/GlossaryHeader.component.tsx | Adds entity-level permissions.EditAll fallback for import/export visibility. |
| openmetadata-ui/src/main/resources/ui/src/components/Glossary/GlossaryHeader/GlossaryHeader.test.tsx | Adds regression test for conditional-permission behavior. |
| it('should render import and export when entity-level EditAll is true despite no global permission', async () => { | ||
| // Simulate a role with conditional EditAll (e.g. isOwner()) | ||
| // Global permissions do not grant All/EditAll | ||
| mockGlossaryTermPermission.All = false; | ||
| mockGlossaryTermPermission.EditAll = false; | ||
| // Entity-level permissions evaluate conditions and grant EditAll | ||
| mockContext.permissions = { ...DEFAULT_ENTITY_PERMISSION, EditAll: true }; | ||
| mockContext.type = EntityType.GLOSSARY; | ||
| render( |
There was a problem hiding this comment.
This new test mutates shared module-level mocks (mockGlossaryTermPermission and mockContext) without resetting them afterwards. Since other tests in this file also rely on the same shared objects (and don’t use a beforeEach reset), the suite becomes order-dependent and can be flaky when tests are re-ordered or run in isolation. Suggest resetting mockGlossaryTermPermission/mockContext to defaults in a beforeEach, or cloning fresh objects per test to avoid cross-test state leakage.
…to prevent state leakage
|
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! |
Code Review ✅ ApprovedGlossaryHeader import/export functionality now correctly respects entity-level permissions. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
Summary
When a Data Producer role has conditions (e.g.
isOwner(),hasDomain(),matchTeam()) on itsEditAllpermission, the import/export buttons in the Glossary header are incorrectly hidden.Root Cause
importExportPermissionsinGlossaryHeader.component.tsxonly checksglobalPermissionsviacheckPermission(), which evaluates static role-level operations without considering policy conditions. When conditions are added, these global checks returnfalse.Fix
Added a fallback to the entity-level
permissions.EditAll(fromuseGenericContext), which evaluates policy conditions against the actual entity. This ensures import/export buttons are shown when the user satisfies the conditions on their role's policy.Changes
GlossaryHeader.component.tsx: Addedpermissions.EditAllas an OR fallback in theimportExportPermissionsmemo, and addedpermissionsto the dependency array.GlossaryHeader.test.tsx: Added test case verifying import/export buttons render when entity-levelEditAllis true but global permissions lackAll/EditAll.Testing
should not render import and export if no permission) still passes sinceDEFAULT_ENTITY_PERMISSIONhasEditAll: false.Fixes #27487
AI Disclosure
This contribution was developed with the assistance of Antigravity, an AI coding assistant by Google DeepMind.