Skip to content

fix(AF-578): render and route access-request bell notifications#584

Merged
babltiga merged 1 commit into
mainfrom
fix/AF-578-access-notification-bell
Jul 3, 2026
Merged

fix(AF-578): render and route access-request bell notifications#584
babltiga merged 1 commit into
mainfrom
fix/AF-578-access-notification-bell

Conversation

@babltiga

@babltiga babltiga commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Closes #578

What

In-app (bell) notifications for the five JIT-access events — ACCESS_REQUEST_SUBMITTED, ACCESS_REQUEST_APPROVED, ACCESS_REQUEST_REJECTED, ACCESS_GRANT_EXPIRED, ACCESS_GRANT_REVOKED — rendered the generic "New notification" fallback and clicking them navigated nowhere. The backend (AccessNotificationListener) already records these with a rich payload; the frontend bell was never taught about them. Follows the same pattern as PR #568 (API_REQUEST_* events).

Changes

  • frontend/src/types/api.ts — added the five ACCESS_* values to UserNotificationEventType; added access_request_id, requester, requested_duration, status, expires_at to UserNotificationPayload.
  • frontend/src/components/common/NotificationBell.tsx
    • renderMessage(): specific localized messages per ACCESS_* event, with a _no_requester variant for ACCESS_REQUEST_SUBMITTED (mirrors the QUERY_SUBMITTED pattern).
    • routeForNotification(): ACCESS_REQUEST_SUBMITTED (reviewer-targeted) → /admin/access-requests; the four requester-targeted events → /access-requests.
  • Locales — added the six notifications.events.ACCESS_* keys to all 7 locale files (en, de, es, fr, hy, ru, zh-CN); parity gate green.
  • Tests — 6 new cases in NotificationBell.test.tsx covering message rendering and click-navigation for every event, including the no-requester variant.

Docs / website

No updates needed: no endpoint, route, env var, or user-facing pitch change. docs/08-notifications.md already documents the ACCESS_* in-app inbox recipients; the frontend docs don't enumerate per-event bell message mappings.

E2E

No spec added: no existing Playwright spec covers the notification bell, and this is a pure message/route mapping fix — exercising it end-to-end would require seeding a full JIT access-request round-trip. The mapping branches are fully covered by component tests (render + navigation per event).

Verification

  • npm run lint — 0 errors (warnings pre-existing)
  • npm run typecheck — green
  • npm run test:coverage — 1139/1139 tests pass incl. locales.parity.test.ts; coverage 94.18% lines / 83.69% branches
  • npm run build — green

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Frontend Test Results

1 139 tests   1 139 ✅  2m 46s ⏱️
  153 suites      0 💤
    1 files        0 ❌

Results for commit 97b511e.

@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Coverage Report for Frontend Coverage (frontend)

Status Category Percentage Covered / Total
🔵 Lines 94.18% (🎯 90%) 1994 / 2117
🔵 Statements 92.12% (🎯 90%) 2222 / 2412
🔵 Functions 91.47% (🎯 90%) 601 / 657
🔵 Branches 83.69% (🎯 80%) 1247 / 1490
File CoverageNo changed files found.
Generated in workflow #733 for commit 97b511e by the Vitest Coverage Report Action

@babltiga babltiga merged commit fcef41c into main Jul 3, 2026
14 checks passed
@babltiga babltiga deleted the fix/AF-578-access-notification-bell branch July 3, 2026 13:03
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.

Access-request in-app notifications render as generic "New notification" and don't navigate anywhere

1 participant