Skip to content

feat: put notification bell behind a WAC flag#2835

Merged
chmanie merged 1 commit into
developfrom
feat/notifications-wallet-access
May 4, 2026
Merged

feat: put notification bell behind a WAC flag#2835
chmanie merged 1 commit into
developfrom
feat/notifications-wallet-access

Conversation

@chmanie
Copy link
Copy Markdown
Contributor

@chmanie chmanie commented Apr 30, 2026

NOTE this is in draft as it's based on https://github.com/jumperexchange/strapi-cms/pull/107

Which Jira task belongs to this PR?

https://linear.app/lifi-linear/issue/JUM-722/strapi-based-feature-flag-to-activate-notifications-for-a-subset-of

Why did I implement it this way?

The gatekeeper was already used for Earn so I could just re-use the existing logic.

Checklist before requesting a review

  • I have performed a self-review of my code
  • This pull request is as small as possible and only tackles one problem
  • I have added tests that cover the functionality / test the bug
  • If this changed the API, I have updated the documentation
  • I have provided QA instructions for the feature / fix implemented in this PR (if applicable)
  • I have provided instructions for any environment / deployment changes that this PR needs when merged

Summary by CodeRabbit

  • New Features

    • Backend now returns two feature flags (earn and notifications), enabling explicit notification gating.
    • Notification bell visibility is now driven by the new notifications flag.
  • Chores

    • Migrated notification visibility from the beta check to flag-based access control across desktop and mobile navigation layouts.

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 30, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
jumper-exchange Ready Ready Preview, Comment May 4, 2026 1:52pm
jumper-exchange-storybook Ready Ready Preview, Comment May 4, 2026 1:52pm

Request Review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 30, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 162158ec-8d86-499f-94fc-42324de9bcaf

📥 Commits

Reviewing files that changed from the base of the PR and between 9c9ae71 and 7f57d79.

📒 Files selected for processing (3)
  • src/app/api/profile/[walletAddress]/flags/route.tsx
  • src/components/Navbar/layout/DesktopLayout.tsx
  • src/components/Navbar/layout/MobileLayout.tsx
✅ Files skipped from review due to trivial changes (1)
  • src/app/api/profile/[walletAddress]/flags/route.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/components/Navbar/layout/MobileLayout.tsx
  • src/components/Navbar/layout/DesktopLayout.tsx

Walkthrough

The PR adds a hasNotifications flag to the profile flags API response and switches DesktopLayout and MobileLayout to gate NotificationBell rendering using useGatekeeperStatus('hasNotifications') (checking status === GatekeeperStatus.SUCCESS) instead of isBeta().

Changes

Notification Feature Gating

Layer / File(s) Summary
API Response Shape
src/app/api/profile/[walletAddress]/flags/route.tsx
GET now returns both hasEarn and hasNotifications. Fallback when no data: { hasEarn: false, hasNotifications: false }.
Imports & Hook Use
src/components/Navbar/layout/DesktopLayout.tsx, src/components/Navbar/layout/MobileLayout.tsx
Removed isBeta import; added GatekeeperStatus and useGatekeeperStatus.
Component Wiring
src/components/Navbar/layout/DesktopLayout.tsx, src/components/Navbar/layout/MobileLayout.tsx
Both layouts call useGatekeeperStatus('hasNotifications') and render NotificationBell only when status === GatekeeperStatus.SUCCESS.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Suggested reviewers

  • mmioana
  • vinzenzLIFI

Poem

🐰 I hopped through flags both near and far,
A bell now peeks where keepers are.
API whispers both earn and ring,
Gatekeeper nods — the bell will sing. 🔔

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.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 and concisely describes the main change: using a WAC (Wallet Access Control) flag to control the notification bell visibility across the codebase.
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 feat/notifications-wallet-access

Review rate limit: 4/5 reviews remaining, refill in 12 minutes.

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

@chmanie chmanie requested review from laurentsenta and mmioana April 30, 2026 09:36
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 30, 2026

✅ All snapshot tests passed

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 30, 2026

Playwright test results

passed  53 passed
flaky  3 flaky
skipped  1 skipped

Details

stats  57 tests across 11 suites
duration  5 minutes, 33 seconds
commit  7f57d79

Flaky tests

chromium › landingPage.spec.ts › Landing page and navigation › Should navigate to the homepage and change tabs (Qase ID: 2)
chromium › mobileView.spec.ts › Verify essential mobile flows › Verify welcome page elements are visible in mobile view (Qase ID: 5)
chromium › swapActions.spec.ts › On chain swaps [Viewport: Mobile] › Hyperliquid chain swap pairs (Qase ID: 25)

Skipped tests

chromium › themeManipulation.spec.ts › Switch between dark and light theme and check the background color › Partner theme should appear in theme menu and apply background color (Qase ID: 49)

📋 View Detailed Qase Report

@chmanie chmanie force-pushed the feat/notifications-wallet-access branch from 9c9ae71 to 7f57d79 Compare May 4, 2026 13:42
@chmanie chmanie merged commit 54f3e65 into develop May 4, 2026
18 checks passed
@chmanie chmanie deleted the feat/notifications-wallet-access branch May 4, 2026 16:14
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