Conversation
- Add AccessControlPanel to profile nav config with breadcrumb override support - Extend ProfileContentHeader to accept custom breadcrumbs and onBreadcrumbAction - Add headerOverride state in ProfilePage so panels can control header title/breadcrumbs - Clear override on nav-item switch; adjust content area layout for access-control tab - Fix AuditLogList container width (tw:w-full) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
|
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! |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
…n index Keep both access-control (administration group) and custom-properties (workspace group) nav items. Adopt selfContainedLayout pattern from main for both panels. Merge ProfileContentHeader to support titleInput, titleSuffix (access-control rename UX) and iconNode (custom-properties dynamic icon) props simultaneously. Restore AuditLogs icon export in ui-core-components index. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
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! |
🔄 Playwright impact map auto-refreshedThis PR touched specs or UI source that changed the source→spec routing map. I regenerated What is this file? It is the auto-generated half of Playwright's PR planner. It routes "if source X changes, run specs Y" by walking spec imports and cross-referencing What if I want to regenerate locally instead? Run this before pushing your next change to skip the bot commit: python3 .github/scripts/generate_playwright_impact_map.py
git add .github/playwright/impact-map.generated.json
git commit --amend --no-edit # or a separate commit |
|
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! |
Rohit0301
added
the
safe to test
|
| Count | Rule |
|---|---|
| 4 | react-hooks/exhaustive-deps |
| 4 | openmetadata-imports/review-sequential-api-calls |
| 1 | openmetadata-imports/no-internal-barrel-imports |
All findings
| Location | Rule | Message | |
|---|---|---|---|
| 🟡 | src/components/discovery/personal-space/Profile/tabs/access-control/AccessControlAuditLogFilters.tsx:65:5 |
react-hooks/exhaustive-deps |
React Hook useMemo has an unnecessary dependency: 't'. Either exclude it or remove the dependency array. |
| 🟡 | src/components/discovery/personal-space/Profile/tabs/access-control/AccessControlDebuggerPanel.tsx:57:23 |
react-hooks/exhaustive-deps |
React Hook useCallback received a function whose dependencies are unknown. Pass an inline function instead. |
| 🟡 | src/components/discovery/personal-space/Profile/tabs/access-control/AccessControlPolicyDetail.tsx:607:9 |
openmetadata-imports/review-sequential-api-calls |
Review these sequential API requests. If they are independent, start them together with Promise.all/Promise.allSettled; keep sequencing only when data-dependent |
| 🟡 | src/components/discovery/personal-space/Profile/tabs/access-control/AccessControlPolicyDetail.tsx:645:9 |
openmetadata-imports/review-sequential-api-calls |
Review these sequential API requests. If they are independent, start them together with Promise.all/Promise.allSettled; keep sequencing only when data-dependent |
| 🟡 | src/components/discovery/personal-space/Profile/tabs/access-control/AccessControlPolicyDetail.tsx:808:6 |
react-hooks/exhaustive-deps |
React Hook useCallback has a missing dependency: 'onRename'. Either include it or remove the dependency array. If 'onRename' changes too often, find the parent |
| 🟡 | src/components/discovery/personal-space/Profile/tabs/access-control/AccessControlRoleDetail.tsx:479:6 |
react-hooks/exhaustive-deps |
React Hook useCallback has a missing dependency: 'onRename'. Either include it or remove the dependency array. If 'onRename' changes too often, find the parent |
| 🟡 | src/components/discovery/personal-space/Profile/tabs/access-control/AccessControlRoleDetail.tsx:645:9 |
openmetadata-imports/review-sequential-api-calls |
Review these sequential API requests. If they are independent, start them together with Promise.all/Promise.allSettled; keep sequencing only when data-dependent |
| 🟡 | src/components/discovery/personal-space/Profile/tabs/access-control/AccessControlRoleDetail.tsx:681:9 |
openmetadata-imports/review-sequential-api-calls |
Review these sequential API requests. If they are independent, start them together with Promise.all/Promise.allSettled; keep sequencing only when data-dependent |
| 🟡 | src/pages/AuditLogsPage/AuditLogsPage.tsx:30:1 |
openmetadata-imports/no-internal-barrel-imports |
Import the internal module directly instead of its index barrel so unrelated siblings do not enter the bundle graph. |
Fix locally (fast - only checks files changed in this branch):
make ui-checkstyle-changed
Code Review ✅ Approved 8 resolved / 8 findingsComprehensive Access Control settings panel adding unified CRUD for roles, policies, and permissions with audit log viewer and debugger. Seven issues were resolved: rule condition validation, condition options filtering, React key usage, header icon rendering, header-override consistency, debug logging, and silent element-not-found handling in pagination. No open findings. ✅ 8 resolved✅ Performance: Rule condition validated via API on every keystroke
✅ Bug: Condition options list narrows permanently while typing
✅ Quality: uniqueId() used as React key during render
✅ Bug: Custom Properties header icon (iconNode) never rendered
✅ Quality: Redundant/inconsistent header-override reset on tab switch
...and 3 more resolved from earlier reviews OptionsDisplay: compact → Counting what did not apply, without listing it. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
|
1 participant
Describe your changes:
Fixes #
I worked on adding a full Access Control settings panel to the user Profile page because the platform lacked a unified UI for managing roles, policies, and permissions. This includes role/policy CRUD, an audit log viewer, and a permission debugger.
Type of change:
High-level design:
New components under
Profile/tabs/access-control/:AccessControlPanel— top-level router/nav (breadcrumbs, header actions) driven by a discriminated-unionAccessControlViewtypeAccessControlRolesPanel/AccessControlPoliciesPanel— list views with search, create, deleteAccessControlRoleDetail/AccessControlPolicyDetail— detail views with inline rename, description editing, rule management, and role/team/user association tablesAccessControlAddRoleForm/AccessControlAddPolicyForm— creation formsAccessControlAuditLogsPanel/AccessControlAuditLogFilters— refactored audit log viewer integrated into the settings panelAccessControlDebuggerPanel— permission evaluation debugger (evaluate any user+resource+operation)AccessControlUserPermissions— per-user permission breakdown (direct roles, team permissions, inherited)AccessControlRuleForm— rule editor with resource/operation/condition fieldsAccessControlLanding— entry-point landing card gridSupporting additions:
audit-logs,permission-debugger,policy,role) + React wrappersAccessControl.types.ts,.constants.ts,.utils.ts— shared types, constants, link utilitiesAuditLogListfully rewritten using@openmetadata/ui-core-components(removes legacy Less styles)AccessControlSettings.spec.ts, 1484 lines) covering roles, policies, audit logs, and permissions debuggerTests:
Use cases covered
Unit tests
AccessControlAddPolicyForm.test.tsxAccessControlAddRoleForm.test.tsxAccessControlDebuggerPanel.test.tsxAccessControlPolicyDetail.test.tsxAccessControlRoleDetail.test.tsxAuditLogList.test.tsx(updated)profileNavConfig.test.tsx(updated)Backend integration tests
Ingestion integration tests
Playwright (UI) tests
playwright/e2e/Features/AccessControlSettings.spec.ts— full CRUD and navigation coverageManual testing performed
N/A — covered by Playwright E2E suite.
UI screen recording / screenshots:
Not applicable.
Checklist:
Fixes <issue-number>: <short explanation>Fixes #<issue-number>above.