Skip to content

Migrate legacy CAN() permission checks to Sistent useHasPermission hook (Pattern C) - #20860

Merged
jamieplu merged 10 commits into
meshery:masterfrom
rishiraj38:fix-patternC
Jul 24, 2026
Merged

jamieplu merged 10 commits into
meshery:masterfrom
rishiraj38:fix-patternC

Conversation

@rishiraj38

@rishiraj38 rishiraj38 commented Jul 21, 2026 •

Copy link
Copy Markdown
Contributor

Description

This PR completes the Pattern C migration across Meshery UI components. Legacy CAN() evaluation functions from @/utils/can have been replaced with Sistent's reactive useHasPermission hook for layout/section gating and conditional variable evaluation.

Key Changes

  • Connections & Wizards:

    • ui/components/connections/ConnectionTable.columns.tsx: Migrated permission checks in column definitions to useHasPermission.
    • ui/components/connections/ConnectionWizardLauncher.tsx: Replaced direct CAN() evaluations with useHasPermission hook calls for Keys.LifecycleManagementAddCluster and Keys.MesherySystemConnectMetrics.
    • ui/components/connections/index.tsx: Updated workspace view permissions check to use useHasPermission.
    • ui/components/connections/wizard/genericSteps.tsx: Introduced useKindPermission hook replacing legacy kindPermission utility function.
  • Workspaces:

    • ui/components/workspaces/WorkspaceActionList.tsx: Replaced inline CAN() calls with canEditWorkspace and canDeleteWorkspace hooks.
    • ui/components/workspaces/WorkspaceFormModalNav.tsx: Refactored getNavItem helper to accept resolved permission props from the parent component.
    • ui/components/workspaces/index.tsx: Replaced 7 inline CAN() calls with top-level permission hooks.
  • Environments:

    • ui/components/environments/index.tsx: Migrated 7 inline CAN() calls to top-level permission hook variables.
    • ui/components/environments/environment-card.tsx: Cleaned up legacy commented CAN() code.
  • Designs:

    • ui/components/designs/lifecycle/DeployConfirmationModal.tsx: Migrated design validate/deploy/undeploy checks to useHasPermission.
    • ui/components/designs/patterns/design-permissions.ts: Refactored canEditDesign to a pure function accepting the permission boolean to adhere to React Rule of Hooks.
    • ui/components/designs/patterns/MesheryPatterns.tsx: Migrated design catalog permission checks to top-level hooks.
    • ui/components/designs/patterns/MesheryPatterns.columns.tsx: Refactored buildPatternActions and buildPatternColumns to accept a permissions object.
    • ui/components/designs/patterns/MesheryPatternCard.tsx: Updated canEditDesign call signature with hasEditPermission.
  • Filters & Settings:

    • ui/components/filters/Filters.tsx: Migrated filter catalog view, import, publish, and details checks to useHasPermission.
    • ui/components/settings/MesherySettings.tsx: Migrated system settings and adapters view checks to useHasPermission.
  • Layout, Extensions & Shared Components:

    • ui/components/layout/AppShell/AppComponents.tsx: Migrated KubernetesSubscription permissions to useHasPermission.
    • ui/components/layout/Navigator/NavigatorExtension.tsx: Exposed useHasPermission hook under hooks inside remote component injection props.
    • ui/components/shared/Modal/Information/LegacyInfoModal.tsx: Replaced CAN() with canPublishDesign hook.
    • ui/components/dashboard/widgets/getting-started/index.tsx: Updated canAssignUserRoles check.
  • Test Suite Updates:

    • Updated vitest test mocks across 11 test files to properly stub useHasPermission and preserve dynamic permission assertions.

Verification

  • Unit Tests: All unit tests (npm run test) pass cleanly (380 test files, 2608 tests).
  • Build & Linting: Production Next.js build (make ui-build) compiles and exports static pages with zero errors.

Signed commits

  • Yes, I signed my commits.

Summary by CodeRabbit

  • Refactor / Authorization

    • Switched permission gating across connections, designs, environments, workspaces, settings, dashboards, catalog, extensions, filters, and performance to use hook-based checks.
    • UI actions are now consistently enabled/disabled based on permissions, including connections (environment selection and status changes) and design publishing/validation flows.
    • Updated the connection wizard to respect kind-level permissions and prevent progression when not allowed.
  • Tests

    • Updated and expanded mocks to support useHasPermission, plus added/adjusted coverage for permission-driven rendering and link/button enablement/disablement.

Signed-off-by: Rishi Raj <[email protected]>
Copilot AI review requested due to automatic review settings July 21, 2026 10:20
@rishiraj38
rishiraj38 marked this pull request as draft July 21, 2026 10:20
@coderabbitai

coderabbitai Bot commented Jul 21, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR replaces legacy CAN(...) checks with useHasPermission across connection, dashboard, catalog, design, environment, workspace, settings, navigation, and performance components. Tests now mock or control hook-based permission results, and connection wizard progression enforces kind-specific permissions.

Changes

Permission migration

Layer / File(s) Summary
Connection authorization and wizard progression
ui/components/connections/*
Connection visibility, creation, assignment, state changes, and kind selection use hook-derived permissions.
Dashboard, shell, catalog, and design authorization
ui/components/dashboard/*, ui/components/layout/*, ui/components/performance/*, ui/components/settings/*, ui/components/designs/*, ui/components/filters/*, ui/pages/*
Links, pages, tabs, modals, lifecycle actions, and design actions use explicit permission booleans.
Workspace and environment authorization
ui/components/environments/*, ui/components/workspaces/*
Workspace actions, transfers, navigation, assignments, and move/import controls use hook-derived permissions or permissionKey props.
Permission-focused test coverage
ui/components/**/*.test.tsx, ui/components/designs/patterns/*.test.ts
Mocks and assertions cover denied permissions, disabled controls, gated content, and wizard progression.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

  • meshery/meshery#20697: Related connection and component authorization changes using Sistent permission handling.
  • meshery/meshery#20878: Related connection-wizard deep-link behavior covered by the new kind-permission gating.

Suggested reviewers: leecalcote, aabidsofi19

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: migrating legacy CAN() permission checks to Sistent's useHasPermission hook.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@rishiraj38
rishiraj38 removed the request for review from Copilot July 21, 2026 10:21
@github-actions github-actions Bot added the component/ui User Interface label Jul 21, 2026
@hortison

Copy link
Copy Markdown
Contributor

Still draft, @rishiraj38?

@rishiraj38

Copy link
Copy Markdown
Contributor Author

Still draft, @rishiraj38?

Yep, working on it right now.

@rishiraj38
rishiraj38 requested review from Copilot and removed request for Copilot July 21, 2026 15:33
@rishiraj38
rishiraj38 marked this pull request as ready for review July 21, 2026 15:37
Copilot AI review requested due to automatic review settings July 21, 2026 15:37
@rishiraj38
rishiraj38 removed the request for review from Copilot July 21, 2026 15:37
@rishiraj38

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@rishiraj38

Copy link
Copy Markdown
Contributor Author

@coderabbitai help

@coderabbitai

coderabbitai Bot commented Jul 21, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

This comment was marked as off-topic.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (2)
ui/components/connections/ConnectionTable.test.tsx (1)

1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Blanket useHasPermission: () => true mocks skip permission-denied coverage. All three test files mock the hook to always return true, so none exercise the disabled/hidden UI states when permission is absent (env-assignment select and status select in ConnectionTable, "Get Token" menu item in HeaderMenu, "Publish to Catalog" button in LegacyInfoModal).

  • ui/components/connections/ConnectionTable.test.tsx#L83-83: consider a variant test with useHasPermission: () => false to assert MultiSelectWrapper/ConnectionStatusSelect render disabled.
  • ui/components/layout/Header/HeaderMenu.test.tsx#L69-69: consider asserting the "Get Token" item is hidden when hasDownloadTokenPermission is false.
  • ui/components/shared/Modal/Information/InfoModal.test.tsx#L207-207: consider asserting the "Publish to Catalog" button is disabled when canPublishDesign is false.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ui/components/connections/ConnectionTable.test.tsx` at line 1, The tests only
cover permitted states because useHasPermission is always mocked to return true.
Add denied-permission variants in ConnectionTable tests to verify
MultiSelectWrapper and ConnectionStatusSelect are disabled, in HeaderMenu tests
to verify “Get Token” is hidden when hasDownloadTokenPermission is false, and in
InfoModal tests to verify “Publish to Catalog” is disabled when canPublishDesign
is false.
ui/components/dashboard/charts/ConnectionCharts.test.tsx (1)

82-82: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Make the permission mocks key-aware.

These mocks control permission outcomes but do not validate which permission key production requests, leaving authorization wiring regressions undetected.

  • ui/components/dashboard/charts/ConnectionCharts.test.tsx#L82-L82: forward useHasPermission to canSpy and assert Keys.WorkspaceManagementViewConnections.
  • ui/components/dashboard/charts/DashboardMeshModelGraph.test.tsx#L67-L67: replace the unconditional true mock with a spy and assert Keys.MesherySystemViewRegistry.
  • ui/components/settings/MesherySettings.test.tsx#L67-L67: replace the shared boolean with a key-aware mapping or spy covering the settings, infrastructure, and registry keys.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ui/components/dashboard/charts/ConnectionCharts.test.tsx` at line 82, Make
all permission mocks key-aware: in
ui/components/dashboard/charts/ConnectionCharts.test.tsx at lines 82-82, forward
the requested key from useHasPermission to canSpy and assert
Keys.WorkspaceManagementViewConnections; in
ui/components/dashboard/charts/DashboardMeshModelGraph.test.tsx at lines 67-67,
replace the unconditional true mock with a spy and assert
Keys.MesherySystemViewRegistry; in
ui/components/settings/MesherySettings.test.tsx at lines 67-67, replace the
shared boolean with a key-aware mapping or spy covering the settings,
infrastructure, and registry permission keys.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ui/components/connections/wizard/genericSteps.tsx`:
- Around line 86-90: The selectStep canProceed guard must enforce kind
permissions for preset/deep-linked connections, not only check that kindConfig
exists. Update selectStep.canProceed to require
kindPermission(ctx.data.kindConfig) alongside the existing presence check,
preserving normal selection behavior while rejecting unauthorized kinds.

In `@ui/components/designs/patterns/MesheryPatterns.tsx`:
- Around line 72-83: Update the permission gate in MesheryPatterns so
catalog-only users authorized by Keys.CatalogManagementViewCatalog are not
rejected by the separate Keys.CatalogManagementViewDesigns check. Pass the
page-specific permission into the shared component or condition the gate on the
active route, while preserving the existing design permission behavior for
design routes.

In `@ui/components/settings/MesherySettings.tsx`:
- Line 255: Gate the registry content rendering, not only its tab control:
update the REGISTRY branch identified by tabVal and MeshModelComponent to
require canViewRegistry, or enforce the equivalent check inside
MeshModelComponent, so direct Registry routes cannot render without permission.

In `@ui/components/shared/Modal/Information/LegacyInfoModal.tsx`:
- Around line 496-501: Update the disabled expression for the “Publish to
Catalog” button in LegacyInfoModal so unpublished designs are enabled only when
canPublishDesign is true and the current user owns the selected resource, while
published designs remain disabled. Remove the swapped ternary branches and
redundant isPublished condition so the permission and ownership checks affect
the result.

In `@ui/components/workspaces/WorkspaceFormModalNav.tsx`:
- Line 50: Update WorkspaceContentWrapper so its getNavItem call passes the
required canViewViews argument, propagating the existing permission value or
retrieving it within the wrapper before building navigation items. Ensure all
getNavItem invocations satisfy its updated signature.

---

Nitpick comments:
In `@ui/components/connections/ConnectionTable.test.tsx`:
- Line 1: The tests only cover permitted states because useHasPermission is
always mocked to return true. Add denied-permission variants in ConnectionTable
tests to verify MultiSelectWrapper and ConnectionStatusSelect are disabled, in
HeaderMenu tests to verify “Get Token” is hidden when hasDownloadTokenPermission
is false, and in InfoModal tests to verify “Publish to Catalog” is disabled when
canPublishDesign is false.

In `@ui/components/dashboard/charts/ConnectionCharts.test.tsx`:
- Line 82: Make all permission mocks key-aware: in
ui/components/dashboard/charts/ConnectionCharts.test.tsx at lines 82-82, forward
the requested key from useHasPermission to canSpy and assert
Keys.WorkspaceManagementViewConnections; in
ui/components/dashboard/charts/DashboardMeshModelGraph.test.tsx at lines 67-67,
replace the unconditional true mock with a spy and assert
Keys.MesherySystemViewRegistry; in
ui/components/settings/MesherySettings.test.tsx at lines 67-67, replace the
shared boolean with a key-aware mapping or spy covering the settings,
infrastructure, and registry permission keys.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 47e9eb4a-e213-4516-9b6a-0af37cc90375

📥 Commits

Reviewing files that changed from the base of the PR and between 91f51f5 and 8735935.

📒 Files selected for processing (55)
  • ui/components/connections/ConnectionTable.columns.tsx
  • ui/components/connections/ConnectionTable.test.tsx
  • ui/components/connections/ConnectionWizardLauncher.tsx
  • ui/components/connections/index.test.tsx
  • ui/components/connections/index.tsx
  • ui/components/connections/wizard/genericSteps.tsx
  • ui/components/dashboard/charts/ConnectionCharts.test.tsx
  • ui/components/dashboard/charts/ConnectionCharts.tsx
  • ui/components/dashboard/charts/DashboardMeshModelGraph.test.tsx
  • ui/components/dashboard/charts/DashboardMeshModelGraph.tsx
  • ui/components/dashboard/charts/KubernetesConnectionChart.test.tsx
  • ui/components/dashboard/charts/KubernetesConnectionChart.tsx
  • ui/components/dashboard/charts/MesheryConfigurationCharts.test.tsx
  • ui/components/dashboard/charts/MesheryConfigurationCharts.tsx
  • ui/components/dashboard/charts/WorkloadChart.test.tsx
  • ui/components/dashboard/charts/WorkloadChart.tsx
  • ui/components/dashboard/widgets/getting-started/index.test.tsx
  • ui/components/dashboard/widgets/getting-started/index.tsx
  • ui/components/designs/lifecycle/DeployConfirmationModal.tsx
  • ui/components/designs/patterns/MesheryPatternCard.tsx
  • ui/components/designs/patterns/MesheryPatterns.columns.test.tsx
  • ui/components/designs/patterns/MesheryPatterns.columns.tsx
  • ui/components/designs/patterns/MesheryPatterns.tsx
  • ui/components/designs/patterns/design-permissions.test.ts
  • ui/components/designs/patterns/design-permissions.ts
  • ui/components/environments/environment-card.tsx
  • ui/components/environments/index.tsx
  • ui/components/filters/Filters.tsx
  • ui/components/layout/AppShell/AppComponents.test.tsx
  • ui/components/layout/AppShell/AppComponents.tsx
  • ui/components/layout/Header/HeaderMenu.test.tsx
  • ui/components/layout/Header/HeaderMenu.tsx
  • ui/components/layout/Navigator/NavigatorExtension.tsx
  • ui/components/performance/index.tsx
  • ui/components/settings/MesherySettings.test.tsx
  • ui/components/settings/MesherySettings.tsx
  • ui/components/shared/Modal/Information/InfoModal.test.tsx
  • ui/components/shared/Modal/Information/LegacyInfoModal.tsx
  • ui/components/workspaces/MesheryWorkspaceCard.tsx
  • ui/components/workspaces/SpacesSwitcher/MainDesignsContent.tsx
  • ui/components/workspaces/SpacesSwitcher/MainViewsContent.tsx
  • ui/components/workspaces/SpacesSwitcher/RecentContent.tsx
  • ui/components/workspaces/SpacesSwitcher/SharedContent.tsx
  • ui/components/workspaces/SpacesSwitcher/WorkspaceContent.tsx
  • ui/components/workspaces/SpacesSwitcher/WorkspaceSwitcher.tsx
  • ui/components/workspaces/SpacesSwitcher/components.tsx
  • ui/components/workspaces/WorkspaceActionList.test.tsx
  • ui/components/workspaces/WorkspaceActionList.tsx
  • ui/components/workspaces/WorkspaceDataTable.tsx
  • ui/components/workspaces/WorkspaceFormModalNav.tsx
  • ui/components/workspaces/WorkspaceGridView.test.tsx
  • ui/components/workspaces/WorkspaceGridView.tsx
  • ui/components/workspaces/index.tsx
  • ui/pages/configuration/catalog.tsx
  • ui/pages/extensions.tsx

@rishiraj38
rishiraj38 requested review from aabidsofi19, Copilot and leecalcote and removed request for Copilot July 21, 2026 17:54

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ui/components/connections/wizard/useConnectionWizard.tsx`:
- Around line 145-150: Update the preset-handling effect around
appliedPresetRef, data.kindConfig, and kindPermission so permission is confirmed
before consuming the preset or changing wizard state. When permission is denied,
leave the preset unapplied and do not select or advance it; when permission
becomes available later, allow the same preset to be processed and advance to
the kind-details step. Add coverage for denied and delayed-permission scenarios.

In `@ui/components/workspaces/WorkspaceFormModalNav.tsx`:
- Line 100: Update the canViewViews permission checks in WorkspaceContentWrapper
and the related Navigation/getNavItem callers to consistently use
Keys.KanvasViewViews, matching the existing navigation authorization contract
and eliminating use of Keys.CatalogManagementViewViews for My-Views.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7115e567-cdf0-4565-9410-6d2b55c8c629

📥 Commits

Reviewing files that changed from the base of the PR and between 8735935 and 1e3b060.

📒 Files selected for processing (5)
  • ui/components/connections/wizard/useConnectionWizard.tsx
  • ui/components/designs/patterns/MesheryPatterns.tsx
  • ui/components/settings/MesherySettings.tsx
  • ui/components/shared/Modal/Information/LegacyInfoModal.tsx
  • ui/components/workspaces/WorkspaceFormModalNav.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
  • ui/components/settings/MesherySettings.tsx
  • ui/components/designs/patterns/MesheryPatterns.tsx

@yi-nuo426

Copy link
Copy Markdown
Contributor

@rishiraj38 conflict 👁️

@rishiraj38
rishiraj38 requested review from Copilot and removed request for Copilot July 22, 2026 17:40
…ration

Addresses the outstanding review feedback on the CAN() -> useHasPermission
migration.

WorkspaceFormModalNav: the two `getNavItem` callers read different keys for
the same "My Views" item - the drawer used `KanvasViewViews` (as master did)
while the new content wrapper used `CatalogManagementViewViews`, so navigation
and content could authorize differently. Replaced the helper with a single
`useNavConfig` hook that reads `KanvasViewViews` once, which removes the class
of defect rather than the one instance, and made the wrapper honour
`enabled !== false` so a stale selection cannot render a gated item.

useConnectionWizard: the preset deep-link recorded `appliedPresetRef` before
evaluating `kindPermission`. Permissions resolve asynchronously, so a preset
seen while access was still denied latched in and the effect short-circuited
once permission arrived, never advancing an authorized user. Permission is now
resolved first; a denied kind is left unapplied so the kind chooser (which
already disables unusable kinds) stays in charge.

Also hardened the params-sync effects, which wrote a fresh state object on
every run. A caller passing a non-memoized `availableKinds` would re-render,
mint new params, and re-run the effect without end; they now compare by value
and return the current state unchanged so React bails out.

Extracted `useKindPermission` into its own module - the wizard hook was
importing the entire step UI just to read one permission rule.

Tests: new `useConnectionWizard` coverage for the granted, denied and
delayed-permission preset cases plus the selected-kind guard; new
`ConnectionTable.columns` coverage asserting each row control is gated on its
own key in both directions; and the blanket `useHasPermission: () => true`
mocks in the charts, settings, header and info-modal suites are now key-aware
so a cross-wired key fails the suite instead of passing silently.

Signed-off-by: Arjun Mehta <[email protected]>
@arjunmehta-git

Copy link
Copy Markdown
Contributor

Picking up the two review nitpicks, both accepted and addressed in cfce4dc.

Key-aware permission mocks. A blanket useHasPermission: () => true cannot distinguish a gate that was dropped from one wired to the wrong key - which is exactly the defect found in WorkspaceFormModalNav on this PR, and it passed every existing test. The mocks in ConnectionCharts, DashboardMeshModelGraph, MesherySettings, HeaderMenu and InfoModal now forward the requested key to a spy, and the suites assert the specific key each gate reads. MesherySettings uses a per-key override map so Registry and Adapters can be denied individually, pinning the direct ?settingsCategory= gating.

Permission-denied coverage. Added for all three sites, with one correction: for HeaderMenu, Sistent's NavigationNavbar renders a permission: false item disabled (permission ?? true), it does not hide it - so the test asserts disabled, and the mock was made faithful to that contract rather than asserting a behaviour the component does not have.

The ConnectionTable cases moved into a new ConnectionTable.columns.test.tsx exercising useConnectionColumns directly: the gates live in that hook, the file needs far fewer mocks, and it keeps ConnectionTable.test.tsx under the 600-line max-lines threshold it was already sitting on. Each row control is asserted in both directions and against its own key.

Also removed the now-dead vi.mock('@/utils/can', ...) stubs left behind in the migrated suites.

Locally: make ui-lint clean, tsc --noEmit clean, full unit suite 2621 passed / 18 skipped.

@arjunmehta-git

Copy link
Copy Markdown
Contributor

@yi-nuo426 the conflict was resolved by the master merge in 66f4ba5 - the branch reports mergeable: true and is up to date with master as of the latest push.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ui/components/workspaces/WorkspaceFormModalNav.tsx`:
- Line 151: Update the navigation state around navConfig and selectedId so that
when the currently selected item is no longer present because its permission is
denied, selectedId is reset to the allowed default view (such as Recents).
Preserve the existing selection when the selected navigation item remains
enabled, and ensure the header and displayed content use the reconciled
selection.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 38398844-08c6-4702-b0aa-4613340ae649

📥 Commits

Reviewing files that changed from the base of the PR and between 66f4ba5 and cfce4dc.

📒 Files selected for processing (11)
  • ui/components/connections/ConnectionTable.columns.test.tsx
  • ui/components/connections/wizard/genericSteps.tsx
  • ui/components/connections/wizard/useConnectionWizard.test.tsx
  • ui/components/connections/wizard/useConnectionWizard.tsx
  • ui/components/connections/wizard/useKindPermission.ts
  • ui/components/dashboard/charts/ConnectionCharts.test.tsx
  • ui/components/dashboard/charts/DashboardMeshModelGraph.test.tsx
  • ui/components/layout/Header/HeaderMenu.test.tsx
  • ui/components/settings/MesherySettings.test.tsx
  • ui/components/shared/Modal/Information/InfoModal.test.tsx
  • ui/components/workspaces/WorkspaceFormModalNav.tsx

…ow tests

`ui/components/workspaces/index.test.tsx` and
`ui/components/environments/index.test.tsx` landed on master after this
branch's last merge. Both stub `@sistent/sistent` with an explicit export list
that predates this PR's migration of the components they render, so once
`Workspaces` and `Environments` read `useHasPermission` the mock resolves it to
undefined and the suites fail on render.

A semantic conflict rather than a textual one: neither side is wrong on its
own, so it only surfaces once the branch is merged with current master.

Both suites exercise the create flow rather than authorization, so the mock
grants every capability and the permission gates stay out of the way of the
behaviour under test.

Signed-off-by: Arjun Mehta <[email protected]>
@arjunmehta-git

Copy link
Copy Markdown
Contributor

CI's ui-unit-tests went red on the previous push - reporting it here since the cause is not visible in the diff.

ui/components/workspaces/index.test.tsx and ui/components/environments/index.test.tsx landed on master (e95b536) after this branch's last merge. Both stub @sistent/sistent with an explicit export list written before this PR migrated the components they render, so once Workspaces and Environments call useHasPermission the mock resolves it to undefined and the suites fail on render. It is a semantic conflict, not a textual one - neither side is wrong alone, and git reports the branch as cleanly mergeable, so it only appears once CI builds the merge.

Merged current master and added useHasPermission to both mocks in 7dd3b5e. Both suites exercise the create flow rather than authorization, so the mock grants every capability and the gates stay out of the way of the behaviour under test.

Full local run on the merged tree: 2650 passed / 18 skipped, make ui-lint clean, tsc --noEmit clean.

Neither InfoModal nor LegacyInfoModal reads `@/utils/can` after the migration,
so the stub is inert. The equivalent stubs were already removed from the charts,
settings and header suites in this branch; this leaves no odd one out.

Signed-off-by: Arjun Mehta <[email protected]>
When My Views is selected and its permission is later denied, the drawer drops
the item and the content wrapper falls back to Recents, but `selectedId` kept
its value - so `updateHeaderInfo` found no matching item, never updated, and
the header went on naming a view whose content was no longer rendered.

Reconcile the selection to the default when the selected nav item loses its
gate. The guard is a boolean derived from the unfiltered item list, so it
cannot loop and never touches a selected workspace id (those are not nav ids).

Adds the first test coverage for this component: the permission-gated item is
listed or omitted as expected, the selected item's content renders, and the
revocation path is asserted end to end (content falls back *and* the header
follows). Verified the last case fails without the fix.

Signed-off-by: Arjun Mehta <[email protected]>
@github-actions

Copy link
Copy Markdown
Contributor

Commit SHA: 95231126b2385b293e50b148562803492f06c66d

END-TO-END TESTS

  • Testing started at: July 24th 2026, 7:36:19 am

📦 Test Result Summary

  • ✅ 63 passed
  • ❌ 0 failed
  • ⚠️ 0 flaked
  • ⏩ 7 skipped

⌛ Duration: 3 minutes and 8 seconds

Overall Result: 👍 All tests passed.

[Show/Hide] Test Result Details
Test Provider Browser Test Case Tags Result
1 Local chromium-local-provider Aggregation Charts are displayed ✅
2 Local chromium-local-provider alias resolution relationship ✅
3 Local chromium-local-provider alias resolution relationship ✅
4 Local chromium-local-provider alias resolution relationship ✅
5 Local chromium-local-provider alias resolution relationship ✅
6 Local chromium-local-provider alias resolution relationship ✅
7 Local chromium-local-provider alias resolution relationship ✅
8 Local chromium-local-provider alias resolution relationship ✅
9 Local chromium-local-provider alias resolution relationship ✅
10 Local chromium-local-provider alias resolution relationship ✅
11 Local chromium-local-provider Charts (Grafana) page loads ✅
12 Local chromium-local-provider config patching correctness relationship ✅
13 Local chromium-local-provider config patching correctness relationship ✅
14 Local chromium-local-provider config patching correctness relationship ✅
15 Local chromium-local-provider config patching correctness relationship ✅
16 Local chromium-local-provider config patching correctness relationship ✅
17 Local chromium-local-provider config patching correctness relationship ✅
18 Local chromium-local-provider config patching correctness relationship ✅
19 Local chromium-local-provider config patching correctness relationship ✅
20 Local chromium-local-provider config patching correctness relationship ✅
21 Local chromium-local-provider Configure Existing Istio adapter through Mesh Adapter URL from Management page unstable ⚠️
22 Local chromium-local-provider Connect to Meshery Istio Adapter and configure it ➖
23 Local chromium-local-provider Create a Model ✅
24 Local chromium-local-provider Delete Kubernetes cluster connections ➖
25 Local chromium-local-provider evaluation idempotency relationship ✅
26 Local chromium-local-provider evaluation idempotency relationship ✅
27 Local chromium-local-provider evaluation idempotency relationship ✅
28 Local chromium-local-provider evaluation idempotency relationship ✅
29 Local chromium-local-provider evaluation idempotency relationship ✅
30 Local chromium-local-provider evaluation idempotency relationship ✅
31 Local chromium-local-provider evaluation idempotency relationship ✅
32 Local chromium-local-provider evaluation idempotency relationship ✅
33 Local chromium-local-provider evaluation idempotency relationship ✅
34 Local chromium-local-provider Import a Model via CSV Import ✅
35 Local chromium-local-provider Import a Model via File Import ✅
36 Local chromium-local-provider Import a Model via Url Import ✅
37 Local chromium-local-provider Logout from current user session ✅
38 Local chromium-local-provider Metrics (Prometheus) page loads ✅
39 Local chromium-local-provider Performance dashboard controls ✅
40 Local chromium-local-provider Ping Istio Adapter unstable ⚠️
41 Local chromium-local-provider Search a Model and Export it ✅
42 Local chromium-local-provider should edit design in Design Configurator ✅
43 Local chromium-local-provider should identify relationships for All Relationships relationship ✅
44 Local chromium-local-provider should identify relationships for Container-Hierarchical-Parent-Alias-Relationship relationship ✅
45 Local chromium-local-provider should identify relationships for deployment-configmap-reference-relationship relationship ✅
46 Local chromium-local-provider should identify relationships for Hierarchical-Parent-Namespace-Relationship relationship ✅
47 Local chromium-local-provider should identify relationships for meshery-design relationship ✅
48 Local chromium-local-provider should identify relationships for pv-pvc-edge-non-binding-reference-relationship relationship ✅
49 Local chromium-local-provider should identify relationships for Service-To-Deployment-Network relationship ✅
50 Local chromium-local-provider should verify Design Configurator page elements ✅
51 Local chromium-local-provider structural integrity relationship ✅
52 Local chromium-local-provider structural integrity relationship ✅
53 Local chromium-local-provider structural integrity relationship ✅
54 Local chromium-local-provider structural integrity relationship ✅
55 Local chromium-local-provider structural integrity relationship ✅
56 Local chromium-local-provider structural integrity relationship ✅
57 Local chromium-local-provider structural integrity relationship ✅
58 Local chromium-local-provider structural integrity relationship ✅
59 Local chromium-local-provider structural integrity relationship ✅
60 Local chromium-local-provider Test if Left Navigation Panel is displayed ✅
61 Local chromium-local-provider Test if Notification button is displayed ✅
62 Local chromium-local-provider Test if Profile button is displayed ✅
63 Local chromium-local-provider Toggle "Send Anonymous Performance Results" ✅
64 Local chromium-local-provider Toggle "Send Anonymous Usage Statistics" ✅
65 Local chromium-local-provider Verify extension nav items use top-level layout ➖
66 Local chromium-local-provider Verify Meshery Adapter for Istio Section ✅
67 Local chromium-local-provider Verify Meshery Catalog Section Details ➖
68 Local chromium-local-provider Verify Meshery Design Embed Details ➖
69 Local chromium-local-provider Verify Meshery Docker Extension Details ➖
70 Local chromium-local-provider Verify Performance Analysis Details ➖
71 Local chromium-local-provider Verify that UI components are displayed ✅
72 Local local-setup authenticate as Local provider ✅

🔗 Relationship Tests

[Show/Hide] Relationship Test Details (18 tests)
Kind Type SubType From To Model Design Name Status
edge binding permission ClusterRole ServiceAccount kubernetes Understanding Relationships ❌
edge binding permission Role ServiceAccount kubernetes Understanding Relationships ❌
edge non-binding network Service Deployment kubernetes service-to-deployment-network ✅
edge non-binding network Service Deployment kubernetes meshery-design-fixture.json ✅
edge non-binding reference ClusterRoleBinding ClusterRole kubernetes meshery-design-fixture.json ❌
hierarchical parent alias Container Deployment kubernetes container-hierarchical-parent-alias-relationship ✅
hierarchical parent alias Container Deployment kubernetes deployment-configmap-reference-relationship ✅
hierarchical parent alias Container Deployment kubernetes service-to-deployment-network ✅
hierarchical parent alias Container Deployment kubernetes meshery-design-fixture.json ✅
hierarchical parent alias Container Deployment kubernetes Understanding Relationships ✅
hierarchical parent alias Container Pod kubernetes container-hierarchical-parent-alias-relationship ✅
hierarchical parent inventory * Namespace kubernetes hierarchical-parent-namespace-relationship ✅
hierarchical parent inventory * Namespace kubernetes service-to-deployment-network ✅
hierarchical parent inventory * Namespace kubernetes pv-pvc-edge-non-binding-reference-relationship ✅
hierarchical parent inventory * Namespace kubernetes Understanding Relationships ✅
hierarchical sibling matchlabels ClusterRole ClusterRole kubernetes meshery-design-fixture.json ✅
hierarchical sibling matchlabels Deployment Deployment kubernetes meshery-design-fixture.json ✅
hierarchical sibling matchlabels Service Service kubernetes meshery-design-fixture.json ✅

@jamieplu
jamieplu merged commit 64c9433 into meshery:master Jul 24, 2026
9 checks passed
@rishiraj38

Copy link
Copy Markdown
Contributor Author

Thanks, @arjunmehta-git.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/ui User Interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[UI] Refactor component authorizations to Sistent permissionKey and useHasPermission

5 participants