Skip to content

fix(RHCLOUD-50619): Consolidate nav to single block, drop permission checks - #1055

Merged
aferd merged 1 commit into
RedHatInsights:masterfrom
aferd:RHCLOUD-50619
Aug 24, 2026
Merged

fix(RHCLOUD-50619): Consolidate nav to single block, drop permission checks#1055
aferd merged 1 commit into
RedHatInsights:masterfrom
aferd:RHCLOUD-50619

Conversation

@aferd

@aferd aferd commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Consolidate the dual v1/v2 nav blocks in frontend.yaml into a single ungated block
  • Remove all loosePermissions and loosePermissionsKessel route-level permission checks
  • Remove featureFlag/isKesselEnabled section-level gates

Why

Chrome's loosePermissionsKessel visibility function checks Kessel permissions against resourceType: 'tenant', but RBAC places role bindings on the default workspace. This causes all loosePermissionsKessel checks to return ALLOWED_FALSE, hiding Configure Events and Event Log for all users on stage (where platform.chrome.kessel is true).

The root cause is in Chrome's VisibilitySingleton.ts — tracked in RHCLOUD-50619. Until that's resolved at the API/Chrome level, we drop the nav permission checks entirely and rely on the app's existing page-level unauthorized screens (Configure Events and Event Log already show a "You do not have access" forbidden state when the API returns non-200).

Test plan

  • v2 org admin on stage: all four nav items visible (Overview, Configure Events, Event Log, Notification Preferences)
  • v2 org non-admin without permissions on stage: nav items visible, forbidden state shown when clicking Configure Events or Event Log
  • v1 org admin on prod: nav items visible, pages load normally
  • FedRAMP: nav items visible, pages load normally

🤖 Generated with Claude Code

…checks

Chrome's loosePermissionsKessel checks permissions against resourceType
'tenant' but RBAC binds roles to the default workspace, causing all
checks to return ALLOWED_FALSE. This hides Configure Events and Event
Log for all users on stage where platform.chrome.kessel is true.

Replace the dual v1/v2 nav blocks with a single ungated block with no
permission checks. The app already handles unauthorized access at the
page level — Configure Events and Event Log show a forbidden state
when the API returns non-200.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aferd
aferd requested a review from a team as a code owner August 21, 2026 15:08
@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 51e1cb89-6481-4a02-83c4-81ac55eba094

📥 Commits

Reviewing files that changed from the base of the PR and between 7ce1ee9 and 7811278.

📒 Files selected for processing (1)
  • .rhcicd/frontend.yaml

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


Summary by CodeRabbit

  • Changes
    • Consolidated separate Notifications navigation entries into one expandable menu.
    • Simplified access to notification-related pages by removing feature and permission-based navigation restrictions.
    • Standardized notification route identifiers.

Walkthrough

The frontend navigation replaces separate V1 and V2 Notifications entries with one expandable entry. It includes four normalized routes and removes feature-flag, Kessel, and route-level permission conditions.

Changes

Notifications navigation

Layer / File(s) Summary
Consolidate notification routes
.rhcicd/frontend.yaml
Defines one notifications entry with Overview, Configure Events, Event Log, and user-preferences routes. Removes feature-flag, Kessel, and loose-permission gating.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: 🔵 Low · up to 78112

The navigation changes are mergeable with explicit owner follow-up to confirm that Notification Preferences still enforces authorization for direct navigation; otherwise, unauthorized users could potentially access that page.

Suggested reviewers: apinkert

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the navigation consolidation and removal of permission checks, which are the main changes.
Description check ✅ Passed The description explains the changes, root cause, tracked issue, affected behavior, and test plan; screenshots and checklist details are not included.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
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.

@github-actions

Copy link
Copy Markdown

@aferd

aferd commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

E2E test failures — expected, self-resolving on merge

The 4 failing tests in notifications-ui.spec.ts (Sidebar Navigation) are caused by the bug this PR fixes.

The e2e tests run against stage Chrome, which serves the deployed nav config. After PR #1052 was merged and deployed, the v2 nav block became active on stage (isKesselEnabled(true) is satisfied), but loosePermissionsKessel returns ALLOWED_FALSE for Configure Events and Event Log due to the tenant-vs-workspace bug (RHCLOUD-50619).

The tests correctly expect all 4 nav items to be visible — they're failing because the deployed config hides 2 of them. Once this PR is merged and deployed, the nav permission checks are removed and all 4 items will be visible again, resolving the test failures.

@aferd
aferd merged commit d7e8283 into RedHatInsights:master Aug 24, 2026
9 of 11 checks passed
@aferd
aferd deleted the RHCLOUD-50619 branch August 24, 2026 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants