fix(permissions): Allow non-orgadmin users to access Notifications Log

[InsaneZein]

Summary

• Fixes RHCLOUD-29703: Non-orgadmin users were blocked from the Notifications Log page with "You do not have access to Notifications"
• Root cause: CheckReadPermissions gated /notificationslog behind canReadNotifications (notifications:notifications:read), which controls access to notification configuration/settings — not viewing one's own notifications
• The Notifications Log page displays the same data as the notification drawer (which is accessible to all authenticated users), so it should not require this permission
• Added unit tests for CheckReadPermissions covering all permission paths

Test plan

• Verify non-orgadmin users can access /notificationslog without notifications:notifications:read permission
• Verify orgadmin users can still access /notificationslog
• Verify /eventlog still requires canReadEvents permission
• Verify other notification pages still require canReadNotifications permission
• All 259 unit tests pass, lint clean

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 424f9be4-2190-40c8-aa65-cc24281629f7

📥 Commits

Reviewing files that changed from the base of the PR and between ce29d22 and 7bd431c.

📒 Files selected for processing (1)

• src/components/__tests__/CheckReadPermissions.test.tsx

🚧 Files skipped from review as they are similar to previous changes (1)

• src/components/tests/CheckReadPermissions.test.tsx

---

Summary by CodeRabbit

• Bug Fixes

  • Allowed reading the notifications log route even when notification read permission is denied.

• Tests

  • Added tests covering read-permission behavior across routes: notifications log (always accessible), event log (permission-gated), and fallback authorization UI when unauthorized.

Walkthrough

Short-circuits notification read permission in CheckReadPermissions for the notifications log path, and adds tests covering notifications, events, and notifications-log routes under various RBAC states.

Changes

Cohort / File(s)	Summary
Notifications Permission Logic src/components/CheckReadPermissions.tsx	Adds a pathname check that returns true for the notifications log route (linkTo.notificationsLog()), bypassing the RBAC canReadNotifications check.
CheckReadPermissions Tests src/components/__tests__/CheckReadPermissions.test.tsx	New test suite (Jest + React Testing Library) that mocks useChrome, stubs featureflags endpoint, and verifies child rendering for notifications, notifications log, and event routes under different RBAC permission combinations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The pull request title clearly and concisely summarizes the main change: allowing non-orgadmin users to access the Notifications Log by fixing the permissions check.
Description check	✅ Passed	The description is substantially complete with clear summary of changes, root cause, impact, RHCLOUD link, and test plan, though it omits screenshots and before/after comparisons.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch bot/RHCLOUD-29703

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

Chromatic Build

	Link
Storybook	https://6980bce13ca67d03fef6d1f0-dehtnkrkyn.chromatic.com/
Build	https://www.chromatic.com/build?appId=6980bce13ca67d03fef6d1f0&number=150

[coderabbitai]

🧹 Nitpick comments (1)

src/components/__tests__/CheckReadPermissions.test.tsx (1)

53-68: Optionally harden negative-path assertions.

On Line 67 and Line 118, asserting only that child content is missing can pass even if nothing renders. Consider also asserting NotAuthorizedPage content is present to prevent false positives.

Also applies to: 104-119

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/__tests__/CheckReadPermissions.test.tsx` around lines 53 - 68,
The test for CheckReadPermissions currently only asserts that the childText is
not in the document, which can give false positives; update the negative-path
tests (the one using getConfiguredAppWrapper with router initialEntries '/rhel'
and rbac canReadNotifications:false, and the similar case at lines 104-119) to
also assert that the NotAuthorizedPage is rendered by checking for its visible
content/text (e.g., the NotAuthorizedPage heading or message) after rendering
the component; locate tests referencing CheckReadPermissions, childText,
mockGetApp, and Wrapper/getConfiguredAppWrapper and add an expectation that the
NotAuthorizedPage text is present in addition to the existing
not.toBeInTheDocument assertion.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/components/__tests__/CheckReadPermissions.test.tsx`:
- Around line 53-68: The test for CheckReadPermissions currently only asserts
that the childText is not in the document, which can give false positives;
update the negative-path tests (the one using getConfiguredAppWrapper with
router initialEntries '/rhel' and rbac canReadNotifications:false, and the
similar case at lines 104-119) to also assert that the NotAuthorizedPage is
rendered by checking for its visible content/text (e.g., the NotAuthorizedPage
heading or message) after rendering the component; locate tests referencing
CheckReadPermissions, childText, mockGetApp, and Wrapper/getConfiguredAppWrapper
and add an expectation that the NotAuthorizedPage text is present in addition to
the existing not.toBeInTheDocument assertion.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c1ce361b-e0ad-4f5c-bfb3-fa20b5d82e52

📥 Commits

Reviewing files that changed from the base of the PR and between 4d9049c and ce29d22.

📒 Files selected for processing (2)

• src/components/CheckReadPermissions.tsx
• src/components/__tests__/CheckReadPermissions.test.tsx

[InsaneZein]

Addressed the nitpick — commit 7bd431c adds NotAuthorizedPage assertions (via the "User Preferences" link) to both negative-path tests, preventing false positives when nothing renders.