Skip to content

feat(admin): add admin panel with writable global feature toggles - #801

Merged
LucasSantana-Dev merged 12 commits into
mainfrom
feature/admin-panel-global-toggles
May 4, 2026
Merged

LucasSantana-Dev merged 12 commits into
mainfrom
feature/admin-panel-global-toggles

Conversation

@LucasSantana-Dev

@LucasSantana-Dev LucasSantana-Dev commented May 4, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • New /admin page gated behind isDeveloper — accessible only to Discord users listed in DEVELOPER_USER_IDS
  • Admin sidebar link visible only to developers
  • Global feature toggles are now writable: DB-backed overrides (new global_feature_toggles table) take priority over Vercel Flags and env defaults
  • Per-guild toggle routes removed from backend; Features page simplified to a read-only list
  • GlobalFeatureToggleProvider extended with 'database' variant

Migration

Adds global_feature_toggles table (id, name UNIQUE, enabled).

How it works

Toggle resolution priority (highest → lowest):

  1. DB override (global_feature_toggles)
  2. Vercel Flags
  3. Env var (FEATURE_<NAME>)
  4. Code default (all true)

To disable a feature from the Admin Panel: toggle it off → writes to DB → takes effect immediately, no restart needed.

Test plan

  • TS compiles clean across all packages
  • /admin route accessible after signing in with Discord (ID 282294772570521600)
  • Non-developer accounts see "Access Denied"
  • Toggling a feature off in admin panel persists and disables it globally
  • Features page shows read-only list with current toggle states
  • Admin link appears in sidebar only for developer accounts

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Admin panel added (admin-only) to view bot servers and statistics and manage global feature toggles (now editable when allowed).
    • Admin-only global toggle endpoint and UI integrated; global toggles can be persisted to the database.
  • User-Facing Changes

    • Feature page now shows an "Available Features" read-only view; server-level toggle management removed from the UI.
  • Documentation

    • README updated with admin panel setup and usage instructions.

@vercel

vercel Bot commented May 4, 2026 •

Copy link
Copy Markdown
Contributor

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

Project Deployment Actions Updated (UTC)
lucky Ready Ready Preview, Comment May 4, 2026 6:36pm

Request Review

@coderabbitai

coderabbitai Bot commented May 4, 2026 •

Copy link
Copy Markdown

Warning

Rate limit exceeded

@LucasSantana-Dev has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 27 minutes and 7 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f9e38b4e-fcab-4bdf-9c71-427a28d84489

📥 Commits

Reviewing files that changed from the base of the PR and between 4ed6ce0 and 8921435.

📒 Files selected for processing (5)
  • README.md
  • packages/backend/tests/unit/services/GuildService.test.ts
  • packages/frontend/src/components/Features/FeatureErrorBanner.tsx
  • packages/frontend/src/pages/Admin.tsx
  • packages/frontend/src/pages/Features.tsx
📝 Walkthrough

Walkthrough

Adds an admin panel: backend admin middleware and routes, DB-backed global feature toggles, guild-summary endpoint, frontend admin UI and navigation, and removes server-scoped toggle plumbing. Tests and docs updated; Prisma migration adds a global_feature_toggles table.

Changes

Admin panel + global toggles

Layer / File(s) Summary
Database Schema
prisma/schema.prisma, prisma/migrations/20260504000000_add_global_feature_toggles/migration.sql
New GlobalFeatureToggle model/table with unique name and boolean enabled.
Types
packages/shared/src/types/featureToggle.ts, packages/frontend/src/types/feature.ts
Add 'database' provider; add ARTIST_COMMAND and ALBUM_COMMAND toggle names.
Service / Persistence
packages/shared/src/services/FeatureToggleService.ts
New DB helper getDbGlobalOverride(), new setGlobalFeatureToggle(); global-status now prefers DB override and reports provider/writability accordingly; removed guild-scoped toggle methods.
Backend Guild Data
packages/backend/src/services/GuildService.ts
Added getAllBotGuilds() to enumerate cached bot guilds, compute icon URL and metrics, return normalized guild summaries.
Backend Auth Middleware
packages/backend/src/middleware/requireAdmin.ts
New requireAdmin middleware: 401 if no req.userId, 403 if isDeveloperUser false, otherwise next().
Backend Routes / Wiring
packages/backend/src/routes/admin.ts, packages/backend/src/routes/toggles.ts, packages/backend/src/routes/index.ts
New setupAdminRoutes() with GET /api/admin/guilds (requires auth+admin); moved route-level guards so /api/admin and /api/toggles/global are behind requireAuth+requireAdmin (and writeLimiter for writes); POST /api/toggles/global/:name now persists via setGlobalFeatureToggle() and returns success.
Frontend API
packages/frontend/src/services/api.ts
Added api.admin.getGuilds(); removed server-scoped getServerToggles/updateServerToggle.
Frontend state & hooks
packages/frontend/src/stores/featuresStore.ts, packages/frontend/src/hooks/useFeatures.ts
Removed server/guild toggle state/actions; store/hook now focus on global toggles only; narrowed error scope to `catalog
Frontend UI & Routing
packages/frontend/src/components/Layout/Sidebar.tsx, packages/frontend/src/pages/Admin.tsx, packages/frontend/src/App.tsx
Sidebar shows Admin link when isDeveloper; added lazy AdminPage route; AdminPage handles auth/authorization views, error/retry, and composes global toggles + bot guilds sections.
Frontend Components
packages/frontend/src/components/Admin/BotGuildsSection.tsx, packages/frontend/src/components/Features/GlobalTogglesSection.tsx, packages/frontend/src/pages/Features.tsx
New BotGuildsSection fetches/displays guild summaries; GlobalTogglesSection uses providerLabel and writable messaging; FeaturesPage now shows read-only feature cards.
Removed UI
packages/frontend/src/components/Features/ServerTogglesSection.tsx
Deleted per-server toggle management UI and related client/server toggle helpers.
Tests
packages/backend/tests/unit/middleware/requireAdmin.test.ts, packages/backend/tests/integration/routes/admin.test.ts, packages/backend/tests/integration/routes/toggles.test.ts, packages/backend/tests/unit/routes/index.test.ts, packages/frontend/...
Added unit/integration tests for admin middleware and admin routes; updated toggle-route tests to expect DB-backed success; frontend tests updated/added for admin page, bot guilds, global toggles; store/hook tests adjusted to global-only shape.
Docs / Spec
README.md, docs/specs/2026-05-03-backlog-map/admin-panel-top10.md
Documented admin panel setup (DEVELOPER_USER_IDS env, run Prisma migration, restart backend) and added implementation backlog/spec.

Sequence Diagram(s)

sequenceDiagram
    participant User as Admin User
    participant Frontend as Frontend App
    participant Backend as Backend API
    participant GuildSvc as Guild Service
    participant FeatureSvc as Feature Toggle Service
    participant DB as Database

    User->>Frontend: Open /admin
    Frontend->>Backend: GET /api/admin/guilds
    activate Backend
    Backend->>Backend: requireAuth + requireAdmin
    Backend->>GuildSvc: getAllBotGuilds()
    GuildSvc->>GuildSvc: enumerate cached guilds, fetch metrics
    GuildSvc-->>Backend: guild list
    Backend-->>Frontend: 200 { guilds: [...] }
    deactivate Backend

    User->>Frontend: Toggle global feature
    Frontend->>Backend: POST /api/toggles/global/:name
    activate Backend
    Backend->>Backend: requireAuth + requireAdmin + writeLimiter
    Backend->>FeatureSvc: setGlobalFeatureToggle(name, enabled)
    FeatureSvc->>DB: upsert global_feature_toggles
    DB-->>FeatureSvc: ok
    FeatureSvc-->>Backend: ok
    Backend-->>Frontend: 200 { success: true, name, enabled }
    deactivate Backend
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

size/l

🚥 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 'feat(admin): add admin panel with writable global feature toggles' clearly and accurately summarizes the main change: adding an admin panel with the ability to write/modify global feature toggles.
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 unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/admin-panel-global-toggles

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 27 minutes and 7 seconds.

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

@github-actions

github-actions Bot commented May 4, 2026 •

Copy link
Copy Markdown

Size Change: +2.01 kB (+0.55%)

Total Size: 367 kB

📦 View Changed
Filename Size Change
packages/frontend/dist/assets/ActionPanel-BKwxz_N1.js 0 B -403 B (removed) 🏆
packages/frontend/dist/assets/ActionPanel-dTlZUgqe.js 402 B +402 B (new file) 🆕
packages/frontend/dist/assets/Admin-DnWz1MOA.js 1.97 kB +1.97 kB (new file) 🆕
packages/frontend/dist/assets/AutoMessages-k2Z-ca93.js 0 B -2.65 kB (removed) 🏆
packages/frontend/dist/assets/AutoMessages-vRB62tJd.js 2.66 kB +2.66 kB (new file) 🆕
packages/frontend/dist/assets/AutoMod-DL7YT_Sx.js 0 B -4.05 kB (removed) 🏆
packages/frontend/dist/assets/AutoMod-K_viJkXQ.js 4.07 kB +4.07 kB (new file) 🆕
packages/frontend/dist/assets/badge-8qbHQXXg.js 504 B +504 B (new file) 🆕
packages/frontend/dist/assets/badge-CaMg9CmL.js 0 B -501 B (removed) 🏆
packages/frontend/dist/assets/Card-FDwyOZj9.js 0 B -506 B (removed) 🏆
packages/frontend/dist/assets/Card-PloSFCea.js 506 B +506 B (new file) 🆕
packages/frontend/dist/assets/CommandsConfig-BOgWJ-yw.js 0 B -1.45 kB (removed) 🏆
packages/frontend/dist/assets/CommandsConfig-CESed-fz.js 1.44 kB +1.44 kB (new file) 🆕
packages/frontend/dist/assets/Config-By5IL35T.js 1.64 kB +1.64 kB (new file) 🆕
packages/frontend/dist/assets/Config-CmLvmhMV.js 0 B -1.64 kB (removed) 🏆
packages/frontend/dist/assets/CustomCommands-CXfqmyJo.js 2.14 kB +2.14 kB (new file) 🆕
packages/frontend/dist/assets/CustomCommands-DC3vnoFk.js 0 B -2.13 kB (removed) 🏆
packages/frontend/dist/assets/DashboardOverview-CkqVDfb8.js 0 B -3.35 kB (removed) 🏆
packages/frontend/dist/assets/DashboardOverview-CL541_VM.js 3.36 kB +3.36 kB (new file) 🆕
packages/frontend/dist/assets/dialog-BCLK2JMT.js 952 B +952 B (new file) 🆕
packages/frontend/dist/assets/dialog-Dk800Uh4.js 0 B -948 B (removed) 🏆
packages/frontend/dist/assets/EmbedBuilder-BdtZhzFK.js 3.31 kB +3.31 kB (new file) 🆕
packages/frontend/dist/assets/EmbedBuilder-DKwdVfJc.js 0 B -3.31 kB (removed) 🏆
packages/frontend/dist/assets/Features-ByYF18ea.js 756 B +756 B (new file) 🆕
packages/frontend/dist/assets/Features-CvsIoaaR.js 0 B -3.08 kB (removed) 🏆
packages/frontend/dist/assets/GuildAutomation-Dc7me2lV.js 2.9 kB +2.9 kB (new file) 🆕
packages/frontend/dist/assets/GuildAutomation-DEJDFtSZ.js 0 B -2.9 kB (removed) 🏆
packages/frontend/dist/assets/index-Cs6dR9m-.js 0 B -55.9 kB (removed) 🏆
packages/frontend/dist/assets/index-DDLMHTiW.css 17.1 kB +17.1 kB (new file) 🆕
packages/frontend/dist/assets/index-DyCTxeYW.js 56 kB +56 kB (new file) 🆕
packages/frontend/dist/assets/index-e8hwoCgB.css 0 B -17.1 kB (removed) 🏆
packages/frontend/dist/assets/input-cOH1PQVi.js 467 B +467 B (new file) 🆕
packages/frontend/dist/assets/input-Dyjw0IB8.js 0 B -464 B (removed) 🏆
packages/frontend/dist/assets/label-Bj8ZrfyL.js 0 B -477 B (removed) 🏆
packages/frontend/dist/assets/label-Dh1uOFi2.js 478 B +478 B (new file) 🆕
packages/frontend/dist/assets/Landing-CD9QzN7X.js 3.71 kB +3.71 kB (new file) 🆕
packages/frontend/dist/assets/Landing-qkJJQG6c.js 0 B -3.71 kB (removed) 🏆
packages/frontend/dist/assets/LastFm-blvMf2ay.js 1.74 kB +1.74 kB (new file) 🆕
packages/frontend/dist/assets/LastFm-CYtPFJ1z.js 0 B -1.74 kB (removed) 🏆
packages/frontend/dist/assets/Levels-B96wzwSM.js 2.61 kB +2.61 kB (new file) 🆕
packages/frontend/dist/assets/Levels-BKEZJ6VC.js 0 B -2.6 kB (removed) 🏆
packages/frontend/dist/assets/Login-h8kdY0j2.js 0 B -2.48 kB (removed) 🏆
packages/frontend/dist/assets/Login-tEr-uYLk.js 2.48 kB +2.48 kB (new file) 🆕
packages/frontend/dist/assets/Lyrics-C85UT26x.js 0 B -1.32 kB (removed) 🏆
packages/frontend/dist/assets/Lyrics-Dvgco87j.js 1.32 kB +1.32 kB (new file) 🆕
packages/frontend/dist/assets/Moderation-Bqlfkaz_.js 3.81 kB +3.81 kB (new file) 🆕
packages/frontend/dist/assets/Moderation-DO6jN13L.js 0 B -3.81 kB (removed) 🏆
packages/frontend/dist/assets/Music-Dqx9eZ6w.js 7.11 kB +7.11 kB (new file) 🆕
packages/frontend/dist/assets/Music-DvDvMj2o.js 0 B -7.09 kB (removed) 🏆
packages/frontend/dist/assets/MusicConfig-BqStVf8i.js 0 B -1.59 kB (removed) 🏆
packages/frontend/dist/assets/MusicConfig-C1cDkQXP.js 1.59 kB +1.59 kB (new file) 🆕
packages/frontend/dist/assets/PreferredArtists-CcbRnFlR.js 3.67 kB +3.67 kB (new file) 🆕
packages/frontend/dist/assets/PreferredArtists-Dv3EJaYE.js 0 B -3.68 kB (removed) 🏆
packages/frontend/dist/assets/PrivacyPolicy-BmZtXGg8.js 1.38 kB +1.38 kB (new file) 🆕
packages/frontend/dist/assets/PrivacyPolicy-BwU-Y3kI.js 0 B -1.38 kB (removed) 🏆
packages/frontend/dist/assets/ReactionRoles-Djsyh-ZX.js 0 B -1.86 kB (removed) 🏆
packages/frontend/dist/assets/ReactionRoles-Dm64nzHC.js 1.86 kB +1.86 kB (new file) 🆕
packages/frontend/dist/assets/SectionHeader-CkR_OuP_.js 0 B -895 B (removed) 🏆
packages/frontend/dist/assets/SectionHeader-CtzacEAY.js 895 B +895 B (new file) 🆕
packages/frontend/dist/assets/select-BHGZCNx0.js 0 B -1.23 kB (removed) 🏆
packages/frontend/dist/assets/select-FOgY7B8V.js 1.23 kB +1.23 kB (new file) 🆕
packages/frontend/dist/assets/ServerLogs-B2ey5G7h.js 0 B -2.87 kB (removed) 🏆
packages/frontend/dist/assets/ServerLogs-QqqQW9cg.js 2.87 kB +2.87 kB (new file) 🆕
packages/frontend/dist/assets/ServerSettings-sAnZa1vj.js 0 B -4.19 kB (removed) 🏆
packages/frontend/dist/assets/ServerSettings-sBfBpHL0.js 4.19 kB +4.19 kB (new file) 🆕
packages/frontend/dist/assets/ServersPage-CuWU6B6g.js 2.87 kB +2.87 kB (new file) 🆕
packages/frontend/dist/assets/ServersPage-DIRZ8muX.js 0 B -2.87 kB (removed) 🏆
packages/frontend/dist/assets/Skeleton-BKxDqjuF.js 237 B +237 B (new file) 🆕
packages/frontend/dist/assets/Skeleton-DxhI0m5Q.js 0 B -237 B (removed) 🏆
packages/frontend/dist/assets/Spotify-ChhuqXVt.js 1.75 kB +1.75 kB (new file) 🆕
packages/frontend/dist/assets/Spotify-vUMO2PnY.js 0 B -1.75 kB (removed) 🏆
packages/frontend/dist/assets/Starboard-CReFRkuG.js 2.05 kB +2.05 kB (new file) 🆕
packages/frontend/dist/assets/Starboard-RSBfLZ3H.js 0 B -2.05 kB (removed) 🏆
packages/frontend/dist/assets/StatTile-B31RaVZ2.js 638 B +638 B (new file) 🆕
packages/frontend/dist/assets/StatTile-BNKjuOsO.js 0 B -636 B (removed) 🏆
packages/frontend/dist/assets/switch-aBPsIdW6.js 0 B -541 B (removed) 🏆
packages/frontend/dist/assets/switch-BE_uObSf.js 543 B +543 B (new file) 🆕
packages/frontend/dist/assets/TermsOfService-BfcDuN7w.js 0 B -1.37 kB (removed) 🏆
packages/frontend/dist/assets/TermsOfService-BolVjdlH.js 1.37 kB +1.37 kB (new file) 🆕
packages/frontend/dist/assets/TrackHistory-DbwnXH5f.js 0 B -2.15 kB (removed) 🏆
packages/frontend/dist/assets/TrackHistory-Dv_7dEQE.js 2.15 kB +2.15 kB (new file) 🆕
packages/frontend/dist/assets/TwitchNotifications-C1pSdK-W.js 0 B -2.43 kB (removed) 🏆
packages/frontend/dist/assets/TwitchNotifications-NRZsAuz4.js 2.43 kB +2.43 kB (new file) 🆕
packages/frontend/dist/assets/useFeatures-BSC8EiID.js 1.99 kB +1.99 kB (new file) 🆕
packages/frontend/dist/assets/usePageMetadata-a_bzyoiC.js 0 B -328 B (removed) 🏆
packages/frontend/dist/assets/usePageMetadata-T1T1Fi3u.js 329 B +329 B (new file) 🆕
packages/frontend/dist/assets/vendor-ui-CbZ_GPii.js 64.7 kB +64.7 kB (new file) 🆕
packages/frontend/dist/assets/vendor-ui-o-aTEl4v.js 0 B -64.6 kB (removed) 🏆
ℹ️ View Unchanged
Filename Size
packages/frontend/dist/assets/rolldown-runtime-BYbx6iT9.js 471 B
packages/frontend/dist/assets/vendor-forms-DNkRbYRs.js 25.6 kB
packages/frontend/dist/assets/vendor-radix-hBYIp2YC.js 38.9 kB
packages/frontend/dist/assets/vendor-react-CFDOOXi7.js 55.7 kB
packages/frontend/dist/assets/vendor-state-BDM_FkZa.js 24.1 kB

compressed-size-action

coderabbitai[bot]
coderabbitai Bot previously requested changes May 4, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
packages/frontend/src/types/feature.ts (1)

7-25: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Frontend FeatureToggleName is missing the new toggle names.

The shared type packages/shared/src/types/featureToggle.ts now includes 'ARTIST_COMMAND' and 'ALBUM_COMMAND', but this frontend type does not. This mismatch could cause type errors or runtime issues when the API returns these toggles.

🔧 Proposed fix
 export type FeatureToggleName =
     | 'DOWNLOAD_VIDEO'
     | 'DOWNLOAD_AUDIO'
     | 'MUSIC_RECOMMENDATIONS'
     | 'AUTOPLAY'
     | 'LYRICS'
     | 'QUEUE_MANAGEMENT'
     | 'REACTION_ROLES'
     | 'ROLE_MANAGEMENT'
     | 'MODERATION'
     | 'AUTOMOD'
     | 'CUSTOM_COMMANDS'
     | 'AUTO_MESSAGES'
     | 'SERVER_LOGS'
     | 'WEBAPP'
     | 'TWITCH_NOTIFICATIONS'
     | 'LASTFM_INTEGRATION'
     | 'WELCOME_MESSAGES'
+    | 'ARTIST_COMMAND'
+    | 'ALBUM_COMMAND'

Consider importing the type from @lucky/shared to keep these in sync automatically.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/frontend/src/types/feature.ts` around lines 7 - 25, The frontend
FeatureToggleName union (symbol: FeatureToggleName) is missing the new
'ARTIST_COMMAND' and 'ALBUM_COMMAND' values which must match the shared
definition; update FeatureToggleName to include those two literal types or,
better, replace the local union with an import of the shared type from
`@lucky/shared` (import the type exported from
packages/shared/src/types/featureToggle.ts) so the frontend stays in sync with
the shared FeatureToggleName.
packages/shared/src/services/FeatureToggleService.ts (1)

138-143: ⚠️ Potential issue | 🟠 Major

Remove mocked methods from test files.

The isEnabledForGuild and setGuildFeatureToggle methods no longer exist on FeatureToggleService. Update test mocks in the following files to remove these references:

  • packages/backend/tests/setup.ts:239
  • packages/backend/tests/integration/api.test.ts:52
  • packages/backend/tests/integration/routes/moderation.test.ts:35
  • packages/backend/tests/integration/routes/toggles.test.ts:25–26, 346
  • packages/backend/tests/integration/routes/lastfm.test.ts:43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/shared/src/services/FeatureToggleService.ts` around lines 138 - 143,
Tests are still mocking removed methods isEnabledForGuild and
setGuildFeatureToggle on FeatureToggleService; update the test mocks to remove
those references and instead mock the existing methods (e.g., isEnabled(name, {
guildId, userId }) and setFeatureToggle or isEnabledGlobal if applicable). Find
test mocks that set isEnabledForGuild or setGuildFeatureToggle and either delete
those mock entries or replace them with mock implementations for isEnabled
(accepting the context object) or the correct public methods on
FeatureToggleService; ensure any assertions or stubs that expected the old
method names are updated to call/assert against isEnabled with the proper
context or the current method names.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/bot/src/functions/music/commands/album.spec.ts`:
- Around line 64-68: Add a test that covers the feature-toggle-disabled path by
setting featureToggleIsEnabledMock.mockResolvedValue(false) (mirror the
equivalent test in artist.spec.ts); invoke the album command handler used in
album.spec.ts, then assert that deferReply was NOT called and that reply was
called with the warning embed (and that the warning embed content matches the
disabled-toggle message), and also assert featureToggleIsEnabledMock was invoked
with the expected arguments; update or add this test near the existing
toggle-enabled setup so both true and false paths are covered.

In `@packages/frontend/src/components/Admin/BotGuildsSection.tsx`:
- Around line 71-82: The useEffect in BotGuildsSection.tsx can update state
after unmount; fix by guarding state setters with a mounted flag: inside the
effect set let mounted = true; call api.admin.getGuilds() as before, but in the
.then/.catch/.finally only call setGuilds, setError, and setLoading if mounted
is still true; return a cleanup function that sets mounted = false.
Alternatively, if api.admin.getGuilds supports AbortController, create an
AbortController, pass its signal to the request, and call controller.abort() in
the cleanup before relying on setGuilds/setError/setLoading.

---

Outside diff comments:
In `@packages/frontend/src/types/feature.ts`:
- Around line 7-25: The frontend FeatureToggleName union (symbol:
FeatureToggleName) is missing the new 'ARTIST_COMMAND' and 'ALBUM_COMMAND'
values which must match the shared definition; update FeatureToggleName to
include those two literal types or, better, replace the local union with an
import of the shared type from `@lucky/shared` (import the type exported from
packages/shared/src/types/featureToggle.ts) so the frontend stays in sync with
the shared FeatureToggleName.

In `@packages/shared/src/services/FeatureToggleService.ts`:
- Around line 138-143: Tests are still mocking removed methods isEnabledForGuild
and setGuildFeatureToggle on FeatureToggleService; update the test mocks to
remove those references and instead mock the existing methods (e.g.,
isEnabled(name, { guildId, userId }) and setFeatureToggle or isEnabledGlobal if
applicable). Find test mocks that set isEnabledForGuild or setGuildFeatureToggle
and either delete those mock entries or replace them with mock implementations
for isEnabled (accepting the context object) or the correct public methods on
FeatureToggleService; ensure any assertions or stubs that expected the old
method names are updated to call/assert against isEnabled with the proper
context or the current method names.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4a05cbf2-7fea-4031-b9ef-8f8454acd010

📥 Commits

Reviewing files that changed from the base of the PR and between 90727de and 9321370.

📒 Files selected for processing (22)
  • packages/backend/src/middleware/requireAdmin.ts
  • packages/backend/src/routes/admin.ts
  • packages/backend/src/routes/index.ts
  • packages/backend/src/routes/toggles.ts
  • packages/backend/src/services/GuildService.ts
  • packages/bot/src/functions/music/commands/album.spec.ts
  • packages/bot/src/functions/music/commands/album.ts
  • packages/bot/src/functions/music/commands/artist.spec.ts
  • packages/bot/src/functions/music/commands/artist.ts
  • packages/frontend/src/App.tsx
  • packages/frontend/src/components/Admin/BotGuildsSection.tsx
  • packages/frontend/src/components/Features/GlobalTogglesSection.tsx
  • packages/frontend/src/components/Layout/Sidebar.tsx
  • packages/frontend/src/pages/Admin.tsx
  • packages/frontend/src/pages/Features.tsx
  • packages/frontend/src/services/api.ts
  • packages/frontend/src/types/feature.ts
  • packages/shared/src/config/featureToggles.ts
  • packages/shared/src/services/FeatureToggleService.ts
  • packages/shared/src/types/featureToggle.ts
  • prisma/migrations/20260504000000_add_global_feature_toggles/migration.sql
  • prisma/schema.prisma
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Quality Gates
  • GitHub Check: compressed-size
  • GitHub Check: SonarCloud Scan
🔇 Additional comments (22)
packages/backend/src/middleware/requireAdmin.ts (1)

1-21: LGTM!

Clean middleware implementation with proper separation of concerns:

  • 401 for missing authentication (no userId)
  • 403 for insufficient authorization (not a developer)
  • Correctly delegates developer check to the isDeveloperUser utility
packages/frontend/src/pages/Features.tsx (1)

75-95: LGTM!

The refactor to a read-only features display is clean:

  • Default fallback globalToggles[feature.name] ?? true correctly aligns with the documented toggle resolution priority (code default is true)
  • readOnly and isGlobal props properly signal intent
  • Empty onToggle handler is appropriate for read-only cards
packages/shared/src/types/featureToggle.ts (1)

19-20: LGTM!

Type definitions correctly extended:

  • ARTIST_COMMAND and ALBUM_COMMAND toggle names added to support the new feature gates in music commands
  • 'database' provider added to GlobalFeatureToggleProvider to support the new DB-backed toggle storage

Also applies to: 32-32

packages/frontend/src/types/feature.ts (1)

28-28: LGTM!

The 'database' provider addition correctly mirrors the shared type update.

packages/backend/src/services/GuildService.ts (1)

725-761: LGTM!

Clean implementation of getAllBotGuilds():

  • Properly returns empty array when bot client is unavailable
  • Uses Promise.all for parallel metric fetching (leveraging the existing cache in getGuildMetrics)
  • Icon URL follows the correct Discord CDN pattern
  • Return type aligns with both the AdminGuildSummary interface and frontend API contract
packages/backend/src/routes/admin.ts (1)

1-27: LGTM!

Well-structured admin route setup:

  • Proper middleware ordering: requireAuth → requireAdmin → handler
  • AdminGuildSummary interface correctly matches the service return type
  • Clean delegation to guildService.getAllBotGuilds()
packages/bot/src/functions/music/commands/album.ts (1)

69-86: LGTM!

Feature toggle gate properly implemented:

  • Check positioned after guildId validation (required for toggle context) but before expensive operations
  • Uses ephemeral warning embed for clean UX when feature is disabled
  • Follows the same pattern as the /artist command for consistency
packages/shared/src/config/featureToggles.ts (1)

95-104: LGTM!

New feature toggle configurations are properly defined:

  • Follow the established pattern with name, enabled, and description
  • Default to enabled: true allowing the commands to work out-of-the-box
  • Toggle names align with the type definitions and command usage (ARTIST_COMMAND, ALBUM_COMMAND)
packages/frontend/src/components/Features/GlobalTogglesSection.tsx (1)

18-55: LGTM – providerLabel is exhaustive and the writable conditional is correct.

All three GlobalFeatureToggleProvider values are mapped, the Record type ensures compile-time exhaustiveness, and the readOnly={!writable} prop correctly propagates to each FeatureCard.

prisma/schema.prisma (1)

85-91: LGTM – minimal model is consistent with the existing GuildFeatureToggle pattern.

@unique on name creates an implicit index covering the expected findUnique({ where: { name } }) lookup, and omitting createdAt/updatedAt matches the GuildFeatureToggle precedent.

packages/frontend/src/components/Layout/Sidebar.tsx (1)

430-474: LGTM – admin nav section renders correctly and is correctly gated by isDeveloper.

The backend requireAdmin middleware provides the real authorization boundary; this is appropriate UI-layer gating on top.

prisma/migrations/20260504000000_add_global_feature_toggles/migration.sql (1)

1-9: LGTM – DDL matches the Prisma model exactly and the unique index covers the expected lookup pattern.

packages/bot/src/functions/music/commands/artist.ts (1)

65-82: LGTM – feature gate is correctly placed and the guildId non-null invariant holds at this point.

The check executes after the !interaction.guildId early-return (line 52) so interaction.guildId is guaranteed non-null at line 66, and placing the gate before deferReply (line 93) means no interaction state needs to be cleaned up when the feature is disabled.

packages/frontend/src/services/api.ts (1)

317-330: LGTM – api.admin.getGuilds() is correctly scoped and the response shape matches the backend contract.

iconUrl (pre-constructed URL from the admin endpoint) is intentionally distinct from BackendGuild.icon (raw hash), and the global 401 interceptor already handles unauthenticated access to this method.

packages/backend/src/routes/index.ts (1)

89-90: This concern is invalid; fetchGlobalToggles() is already gated to developers only.

The useFeatures hook guards fetchGlobalToggles() with an isDeveloper check at lines 45–46 and 87–88. Non-developer users will never call getGlobalToggles() on the frontend, so the backend's requireAdmin middleware will not impact them. The Features page displays global toggle states with a fallback to true for missing values, and error handling displays a user-friendly message if the fetch fails. The backend and frontend protections are aligned.

			> Likely an incorrect or invalid review comment.
packages/frontend/src/App.tsx (1)

27-27: LGTM!

The lazy-loaded AdminPage route follows the existing pattern. The route is appropriately placed inside AuthenticatedRoutes for authentication, while authorization (developer check) is correctly delegated to the AdminPage component itself rather than using guardedRoute, since admin access is based on developer status rather than module-level RBAC.

Also applies to: 132-132

packages/bot/src/functions/music/commands/artist.spec.ts (1)

21-26: LGTM! Mock setup correctly aligns with the feature-gated implementation.

The mocks for featureToggleService.isEnabled and createWarningEmbed match the command's behavior. Defaulting featureToggleIsEnabledMock.mockResolvedValue(true) ensures existing tests pass while allowing the feature gate to be tested.

Consider adding a test case for the disabled toggle path (e.g., verifying the warning embed is returned when featureToggleIsEnabledMock.mockResolvedValue(false)), but this can be deferred.

Also applies to: 60-64, 71-72, 137-144

packages/backend/src/routes/toggles.ts (2)

85-90: LGTM!

The POST endpoint now correctly writes to the database via featureToggleService.setGlobalFeatureToggle() and returns the expected response shape { success: true, name, enabled } that matches the frontend API contract (per packages/frontend/src/services/api.ts:282-298).


10-31: Middleware guard is correctly applied. The /api/toggles/global routes are properly pre-guarded by requireAdmin (and requireAuth) middleware in index.ts at line 90. This is a valid pattern for centralizing authentication and authorization logic at the router level.

packages/frontend/src/pages/Admin.tsx (1)

11-121: LGTM!

The component implements proper authorization flow: authentication check first, then developer status check, then loading state, and finally content rendering. Error handling includes both retry functionality and conditional re-authentication for auth/forbidden errors. The defensive rendering of loadError after the loading check ensures errors are displayed only when loading is complete.

packages/frontend/src/components/Admin/BotGuildsSection.tsx (1)

16-64: LGTM!

The component correctly handles all UI states (loading, error, empty, populated) and the GuildRow subcomponent properly renders guild information with conditional metric display. The interface aligns with the API response shape from packages/frontend/src/services/api.ts:318-329.

Also applies to: 84-124

packages/shared/src/services/FeatureToggleService.ts (1)

39-51: LGTM!

The toggle resolution priority is correctly implemented: (1) DB override → (2) Vercel Flags → (3) environment fallback. The writable property accurately reflects which sources can be modified via the admin panel. The Prisma operations use the correct patterns for read (findUnique) and write (upsert).

Also applies to: 53-62, 103-131

Comment thread packages/bot/src/functions/music/commands/album.spec.ts
Comment thread packages/frontend/src/components/Admin/BotGuildsSection.tsx
coderabbitai[bot]
coderabbitai Bot previously requested changes May 4, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/frontend/src/hooks/useFeatures.test.ts (1)

19-19: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

globalToggleProvider is missing the 'database' variant introduced in this PR.

The PR extends GlobalFeatureToggleProvider with a 'database' value, but the test-local FeaturesState type only lists 'vercel' | 'environment'. Because the mock is force-cast with as typeof useFeaturesStore (lines 71–73), TypeScript won't surface the mismatch, leaving the 'database' provider path untested and making this type silently out-of-sync with the real store.

🛠 Proposed fix
-    globalToggleProvider: 'vercel' | 'environment'
+    globalToggleProvider: 'vercel' | 'environment' | 'database'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/frontend/src/hooks/useFeatures.test.ts` at line 19, The test-local
FeaturesState type in useFeatures.test.ts is out-of-sync with the real
GlobalFeatureToggleProvider: update the FeaturesState declaration so
globalToggleProvider includes the new 'database' variant (i.e., 'vercel' |
'environment' | 'database') and adjust any test mock values used when casting
the mock to "as typeof useFeaturesStore" so the mocked store covers the
'database' path as the real store does.
🧹 Nitpick comments (1)
packages/frontend/src/pages/Features.test.tsx (1)

69-82: ⚡ Quick win

renders available features list only asserts the section heading, not the list content.

The test sets up a feature (MUSIC_RECOMMENDATIONS / 'Music recs') and an explicit toggle state, but the sole assertion is that 'Available Features' heading appears. A regression where the feature items themselves fail to render would pass this test undetected. Adding an assertion on the feature's display text would give this test real signal.

♻️ Proposed improvement
     expect(screen.getByText('Available Features')).toBeInTheDocument()
+    expect(screen.getByText('Music recs')).toBeInTheDocument()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/frontend/src/pages/Features.test.tsx` around lines 69 - 82, The test
"renders available features list" only checks the heading; update it to assert
the actual feature item renders by verifying the feature's display text or
description appears (e.g., check for 'MUSIC_RECOMMENDATIONS' or 'Music recs')
after calling mockFeaturesStore and mockFeatures and rendering <FeaturesPage />;
use screen.getByText or screen.findByText to assert the feature entry is present
so the test fails if feature items are not rendered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/specs/2026-05-03-backlog-map/admin-panel-top10.md`:
- Around line 56-102: Update the checklist statuses to reflect completed work:
mark item 5 (Apply `writeLimiter` to `POST /api/toggles/global/:name`) as Done
since `writeLimiter` was added in packages/backend/src/routes/index.ts (ensure
the change sits after the requireAdmin guard), mark item 6 (Integration tests
for admin routes) and item 7 (Unit tests for requireAdmin middleware) as Done
because new tests exist in
packages/backend/tests/integration/routes/admin.test.ts and
packages/backend/tests/unit/middleware/requireAdmin.test.ts respectively, and
mark item 10 (Document admin panel setup in README) as Done because the README
changes were added in the PR; update the statuses and any dates/notes in
docs/specs/2026-05-03-backlog-map/admin-panel-top10.md so the checklist no
longer lists these as Pending.
- Around line 9-32: Update the Markdown fenced code blocks to include a language
tag so markdownlint MD040 is suppressed: add an explicit language (e.g., "text"
or "sh") to the top of the triple-backtick fences for the changelog block ("Fix
stale 409 test → SonarCloud...") and the two command blocks containing `docker
compose restart lucky-backend` and `npx prisma migrate deploy`, ensuring each
opening ``` is changed to something like ```sh or ```text to tag the block
appropriately.

In `@packages/backend/tests/integration/routes/admin.test.ts`:
- Around line 61-63: Remove the redundant pre-guard that double-wraps
authorization: do not call app.use('/api/admin', requireAuth, requireAdmin) in
this test file since setupAdminRoutes(app) already mounts requireAuth and
requireAdmin; edit the test setup to only call setupSessionMiddleware(app) and
setupAdminRoutes(app) (remove or comment out the app.use('/api/admin',
requireAuth, requireAdmin) line) so routes are exercised with a single, correct
auth chain.

In `@README.md`:
- Around line 137-140: The fenced example showing environment variables (the
block containing "DEVELOPER_USER_IDS=your_discord_user_id") is missing a
language tag and triggers markdownlint MD040; fix it by tagging the fence with
an appropriate language such as env (change ``` to ```env) so the block becomes
```env ... ```; update the README.md fenced block that contains
DEVELOPER_USER_IDS accordingly to silence the linter.

---

Outside diff comments:
In `@packages/frontend/src/hooks/useFeatures.test.ts`:
- Line 19: The test-local FeaturesState type in useFeatures.test.ts is
out-of-sync with the real GlobalFeatureToggleProvider: update the FeaturesState
declaration so globalToggleProvider includes the new 'database' variant (i.e.,
'vercel' | 'environment' | 'database') and adjust any test mock values used when
casting the mock to "as typeof useFeaturesStore" so the mocked store covers the
'database' path as the real store does.

---

Nitpick comments:
In `@packages/frontend/src/pages/Features.test.tsx`:
- Around line 69-82: The test "renders available features list" only checks the
heading; update it to assert the actual feature item renders by verifying the
feature's display text or description appears (e.g., check for
'MUSIC_RECOMMENDATIONS' or 'Music recs') after calling mockFeaturesStore and
mockFeatures and rendering <FeaturesPage />; use screen.getByText or
screen.findByText to assert the feature entry is present so the test fails if
feature items are not rendered.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: dc69a6a8-d0bd-4491-9e73-946a2fcf1166

📥 Commits

Reviewing files that changed from the base of the PR and between 9321370 and 40e3c49.

📒 Files selected for processing (14)
  • README.md
  • docs/specs/2026-05-03-backlog-map/admin-panel-top10.md
  • packages/backend/src/routes/index.ts
  • packages/backend/tests/integration/routes/admin.test.ts
  • packages/backend/tests/integration/routes/toggles.test.ts
  • packages/backend/tests/unit/middleware/requireAdmin.test.ts
  • packages/frontend/src/components/Features/ServerTogglesSection.tsx
  • packages/frontend/src/hooks/useFeatures.test.ts
  • packages/frontend/src/hooks/useFeatures.ts
  • packages/frontend/src/pages/Features.test.tsx
  • packages/frontend/src/services/api.test.ts
  • packages/frontend/src/services/api.ts
  • packages/frontend/src/stores/featuresStore.test.ts
  • packages/frontend/src/stores/featuresStore.ts
💤 Files with no reviewable changes (3)
  • packages/frontend/src/services/api.test.ts
  • packages/frontend/src/components/Features/ServerTogglesSection.tsx
  • packages/frontend/src/hooks/useFeatures.ts
✅ Files skipped from review due to trivial changes (1)
  • packages/backend/src/routes/index.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: SonarCloud Scan
  • GitHub Check: Quality Gates
  • GitHub Check: compressed-size
🧰 Additional context used
🪛 markdownlint-cli2 (0.22.1)
README.md

[warning] 138-138: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

docs/specs/2026-05-03-backlog-map/admin-panel-top10.md

[warning] 9-9: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


[warning] 21-21: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


[warning] 30-30: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

Comment thread docs/specs/2026-05-03-backlog-map/admin-panel-top10.md
Comment thread docs/specs/2026-05-03-backlog-map/admin-panel-top10.md
Comment thread packages/backend/tests/integration/routes/admin.test.ts
Comment thread README.md
- Add GlobalFeatureToggle DB table (Prisma migration + schema)
- FeatureToggleService: read DB global overrides first, then Vercel,
  then env defaults. Add setGlobalFeatureToggle() for DB writes.
- Backend: POST /api/toggles/global/:name now writes to DB instead
  of returning 409. Remove per-guild toggle routes.
- Frontend: new /admin page gated behind isDeveloper with writable
  global toggle controls and Discord login prompt
- Frontend: Features page simplified to read-only list (no per-server
  or per-guild toggles)
- Sidebar: Admin Panel link visible only to developers
- GlobalFeatureToggleProvider extended with 'database' variant
- New requireAdmin middleware (auth + developer check) applied at
  router level to /api/admin/* and /api/toggles/global/* paths
- Remove per-handler requireDeveloperUser calls — single enforcement
  point in index.ts prevents bypass
- GET /api/admin/guilds: returns all guilds the bot is in with
  member/channel/role counts (bot client cache + Discord API)
- BotGuildsSection component: sorted guild list with icons,
  member count, text/voice channels, role count
- Admin page now shows Bot Servers section below global toggles
…ggle dead code

- Apply writeLimiter to POST /api/toggles/global/:name (was only covered by apiLimiter)
- Fix stale 409 test in toggles.test.ts — route now writes to DB and returns 200
- Add requireAuth/requireAdmin middleware to toggles integration test app setup
- Add integration tests for GET /api/admin/guilds (auth/403/200/empty-bot cases)
- Add unit tests for requireAdmin middleware (401/403/next cases)
- Remove orphaned server-toggle code: featuresStore, useFeatures, api.ts, ServerTogglesSection.tsx
- Update Features.test.tsx to match simplified read-only page
- Document admin panel setup (DEVELOPER_USER_IDS, migration, restart) in README
…oval

Server-toggle store methods and API endpoints were deleted as part of the
admin-panel global-toggles feature; align test files to match.
… to index.test.ts

setupAdminRoutes calls app.get directly; without the mock the unit test
app stub (which only has .use) throws TypeError.
Use proper FeatureToggleName/FeatureToggleState casts and as-any for
AxiosResponse mocks to pass tsc strict mode on CI.
Bare 'as any' cast is sufficient; unused-directive warnings were
breaking CI with --max-warnings 0.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (5)
packages/frontend/src/components/Admin/BotGuildsSection.test.tsx (2)

64-83: ⚡ Quick win

Missing complementary null-stats negative assertion.

This test confirms non-null stats are rendered, but no test verifies that null stats are not rendered — which is the other half of the "rendered only when non-null" contract stated in the PR summary. The makeGuild defaults conveniently provide all-null counts, making the addition trivial.

✅ Suggested additional test
+    test('does not display stat nodes when counts are null', async () => {
+        vi.mocked(api.admin.getGuilds).mockResolvedValue({
+            data: { guilds: [makeGuild()] }, // all counts default to null
+        } as any)
+        render(<BotGuildsSection />)
+        await waitFor(() => expect(screen.getByText('Test Guild')).toBeInTheDocument())
+        expect(screen.queryByLabelText(/members/i)).not.toBeInTheDocument()
+        expect(screen.queryByLabelText(/text channels/i)).not.toBeInTheDocument()
+        expect(screen.queryByLabelText(/voice channels/i)).not.toBeInTheDocument()
+        expect(screen.queryByLabelText(/roles/i)).not.toBeInTheDocument()
+    })

Adjust the queryBy* selectors to match whatever accessible labels or test-ids the component uses for stat nodes.

🤖 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 `@packages/frontend/src/components/Admin/BotGuildsSection.test.tsx` around
lines 64 - 83, Add a complementary negative assertion to the BotGuildsSection
test suite: after mocking api.admin.getGuilds with a guild whose numeric stats
are null (you can use makeGuild() defaults or explicitly set
memberCount/textChannelCount/voiceChannelCount/roleCount to null) and rendering
<BotGuildsSection />, assert that the stat elements are not rendered by using
queryByText/queryBy* (e.g., expect(screen.queryByText('250')).toBeNull() or more
generally expect(screen.queryByText(/^\d+$/)).toBeNull()) so the test verifies
stats are only shown when non-null.

30-34: 💤 Low value

Consider a more semantic loading-state selector.

container.querySelectorAll('.animate-pulse') couples the test to a Tailwind utility class name. A data-testid="skeleton-row" attribute or role="status" on each skeleton row would survive CSS refactors and convey intent more clearly.

♻️ Suggested approach

In the component, add data-testid="skeleton-row" to each skeleton element, then in the test:

-        expect(container.querySelectorAll('.animate-pulse').length).toBe(5)
+        expect(screen.getAllByTestId('skeleton-row').length).toBe(5)
🤖 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 `@packages/frontend/src/components/Admin/BotGuildsSection.test.tsx` around
lines 30 - 34, The test 'shows loading skeletons while fetching' for component
BotGuildsSection is coupling to Tailwind ('.animate-pulse'); update the
component to mark each skeleton row with a stable selector (e.g., add
data-testid="skeleton-row" or role="status" on the skeleton element), then
change the test (the one that uses
vi.mocked(api.admin.getGuilds).mockImplementation(() => new Promise(() => {})))
to assert container.querySelectorAll('[data-testid="skeleton-row"]').length ===
5 (or use getAllByRole('status') if using role) instead of querying the CSS
class.
packages/frontend/src/components/Features/GlobalTogglesSection.test.tsx (2)

9-25: 💤 Low value

onToggle stub type is narrower than the real FeatureCard prop — add a brief comment.

The stub declares onToggle: (v: boolean) => void, which is intentionally the internal adapter signature (the lambda (v) => parentOnToggle(feature.name, v) that GlobalTogglesSection creates). This is correct and the interaction test at line 140 passes as a result. However, the mismatch is silent: if the component ever passes parentOnToggle directly to FeatureCard (removing the adapter), the mock would still call it with one boolean argument, and expect(onToggle).toHaveBeenCalledWith('AUTOPLAY', true) would silently fail for the wrong reason.

A one-line comment explaining the indirection keeps this safe for future maintainers:

📝 Proposed clarifying comment
 vi.mock('@/components/Features/FeatureCard', () => ({
     default: ({ feature, enabled, onToggle, readOnly }: {
         feature: { name: string }
         enabled: boolean
+        // Receives the per-card adapter lambda from GlobalTogglesSection:
+        // (newEnabled) => parentOnToggle(feature.name, newEnabled)
         onToggle: (v: boolean) => void
         readOnly: boolean
     }) => (
🤖 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 `@packages/frontend/src/components/Features/GlobalTogglesSection.test.tsx`
around lines 9 - 25, Add a one-line clarifying comment inside the FeatureCard
mock (the vi.mock for '@/components/Features/FeatureCard') next to the onToggle
prop in the stub explaining that the stub's onToggle signature is the internal
adapter (the lambda the parent creates: (v) => parentOnToggle(feature.name, v))
and not the parentOnToggle signature, so tests relying on the adapter will pass
and to warn future maintainers not to remove the adapter without updating the
mock.

9-25: 💤 Low value

onToggle prop type in mock stub is narrower than the real component signature.

The stub declares onToggle: (v: boolean) => void, but GlobalTogglesSection's actual onToggle prop is (name: FeatureToggleName, enabled: boolean) => void. The component passes a wrapping lambda (v) => onToggle(feature.name, v) to FeatureCard, so the test for line 140 passes correctly today. However, if the component's internal wiring ever changes (e.g., the component passes onToggle directly), the stub's single-argument call would silently stop exercising the two-argument contract, giving a false green.

Consider aligning the stub's type with the actual FeatureCard prop contract (if known) or adding an explicit comment explaining the indirection:

📝 Clarifying comment proposal
 vi.mock('@/components/Features/FeatureCard', () => ({
     default: ({ feature, enabled, onToggle, readOnly }: {
         feature: { name: string }
         enabled: boolean
-        onToggle: (v: boolean) => void
+        // onToggle here is the per-card adapter lambda from GlobalTogglesSection,
+        // i.e. (newEnabled) => parentOnToggle(feature.name, newEnabled)
+        onToggle: (v: boolean) => void
         readOnly: boolean
     }) => (
🤖 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 `@packages/frontend/src/components/Features/GlobalTogglesSection.test.tsx`
around lines 9 - 25, The mock for FeatureCard uses onToggle: (v: boolean) =>
void which is narrower than the real component signature; update the stub so
onToggle matches the actual contract (e.g., onToggle: (name: FeatureToggleName,
enabled: boolean) => void) or add an explicit comment documenting the
intentional indirection; adjust the mock implementation to call
onToggle(feature.name, !enabled) so tests exercise the two-argument form
(referencing FeatureCard, GlobalTogglesSection, and the onToggle prop).
packages/frontend/src/pages/Admin.test.tsx (1)

122-142: ⚡ Quick win

retryLoad is never exercised — the Retry button's behavior goes untested.

retryLoad is declared as a vi.fn() and wired into mockFeatures, signalling intent to verify the click handler. However, the test only asserts the button's presence and never clicks it, leaving the most important behaviour (that clicking "Retry" actually calls retryLoad) completely unverified.

♻️ Proposed fix — make the test async and assert the click
-    test('shows error banner with retry button on load failure', () => {
-        const retryLoad = vi.fn()
+    test('shows error banner with retry button on load failure', async () => {
+        const user = userEvent.setup()
+        const retryLoad = vi.fn()
         mockAuthStore({ isAuthenticated: true, isDeveloper: true })
         mockFeatures({
             loadError: {
                 kind: 'upstream',
                 message: 'Discord API error',
                 scope: 'catalog',
                 status: 502,
             },
             retryLoad,
         })
         render(
             <MemoryRouter>
                 <AdminPage />
             </MemoryRouter>,
         )
         expect(screen.getByText('Unable to load feature data')).toBeInTheDocument()
         expect(screen.getByText('Discord API error')).toBeInTheDocument()
-        expect(screen.getByRole('button', { name: 'Retry' })).toBeInTheDocument()
+        const retryButton = screen.getByRole('button', { name: 'Retry' })
+        expect(retryButton).toBeInTheDocument()
+        await user.click(retryButton)
+        expect(retryLoad).toHaveBeenCalledTimes(1)
     })
🤖 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 `@packages/frontend/src/pages/Admin.test.tsx` around lines 122 - 142, The test
"shows error banner with retry button on load failure" declares retryLoad but
never exercises it; update the test to be async, use user-event or fireEvent to
click the Retry button (located via getByRole({ name: 'Retry' })), and assert
that the mock function retryLoad was called (e.g.
expect(retryLoad).toHaveBeenCalled()). Keep mockAuthStore, mockFeatures,
AdminPage and MemoryRouter setup as-is and reference retryLoad and the button
role to locate where to add the click and assertion.
🤖 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.

Nitpick comments:
In `@packages/frontend/src/components/Admin/BotGuildsSection.test.tsx`:
- Around line 64-83: Add a complementary negative assertion to the
BotGuildsSection test suite: after mocking api.admin.getGuilds with a guild
whose numeric stats are null (you can use makeGuild() defaults or explicitly set
memberCount/textChannelCount/voiceChannelCount/roleCount to null) and rendering
<BotGuildsSection />, assert that the stat elements are not rendered by using
queryByText/queryBy* (e.g., expect(screen.queryByText('250')).toBeNull() or more
generally expect(screen.queryByText(/^\d+$/)).toBeNull()) so the test verifies
stats are only shown when non-null.
- Around line 30-34: The test 'shows loading skeletons while fetching' for
component BotGuildsSection is coupling to Tailwind ('.animate-pulse'); update
the component to mark each skeleton row with a stable selector (e.g., add
data-testid="skeleton-row" or role="status" on the skeleton element), then
change the test (the one that uses
vi.mocked(api.admin.getGuilds).mockImplementation(() => new Promise(() => {})))
to assert container.querySelectorAll('[data-testid="skeleton-row"]').length ===
5 (or use getAllByRole('status') if using role) instead of querying the CSS
class.

In `@packages/frontend/src/components/Features/GlobalTogglesSection.test.tsx`:
- Around line 9-25: Add a one-line clarifying comment inside the FeatureCard
mock (the vi.mock for '@/components/Features/FeatureCard') next to the onToggle
prop in the stub explaining that the stub's onToggle signature is the internal
adapter (the lambda the parent creates: (v) => parentOnToggle(feature.name, v))
and not the parentOnToggle signature, so tests relying on the adapter will pass
and to warn future maintainers not to remove the adapter without updating the
mock.
- Around line 9-25: The mock for FeatureCard uses onToggle: (v: boolean) => void
which is narrower than the real component signature; update the stub so onToggle
matches the actual contract (e.g., onToggle: (name: FeatureToggleName, enabled:
boolean) => void) or add an explicit comment documenting the intentional
indirection; adjust the mock implementation to call onToggle(feature.name,
!enabled) so tests exercise the two-argument form (referencing FeatureCard,
GlobalTogglesSection, and the onToggle prop).

In `@packages/frontend/src/pages/Admin.test.tsx`:
- Around line 122-142: The test "shows error banner with retry button on load
failure" declares retryLoad but never exercises it; update the test to be async,
use user-event or fireEvent to click the Retry button (located via getByRole({
name: 'Retry' })), and assert that the mock function retryLoad was called (e.g.
expect(retryLoad).toHaveBeenCalled()). Keep mockAuthStore, mockFeatures,
AdminPage and MemoryRouter setup as-is and reference retryLoad and the button
role to locate where to add the click and assertion.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5b099c9e-a2e8-418e-9e57-133b25fb8a6f

📥 Commits

Reviewing files that changed from the base of the PR and between 13dfcd5 and 4ed6ce0.

📒 Files selected for processing (3)
  • packages/frontend/src/components/Admin/BotGuildsSection.test.tsx
  • packages/frontend/src/components/Features/GlobalTogglesSection.test.tsx
  • packages/frontend/src/pages/Admin.test.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Quality Gates
  • GitHub Check: SonarCloud Scan
  • GitHub Check: compressed-size
🔇 Additional comments (8)
packages/frontend/src/components/Admin/BotGuildsSection.test.tsx (1)

1-1: No action required.

Vitest version 4.0.18 is a valid, stable patch release. The imported APIs (describe, test, expect, vi, beforeEach) are core, stable exports that have not changed in this release and are fully compatible.

packages/frontend/src/components/Features/GlobalTogglesSection.test.tsx (6)

127-141: LGTM — interaction test correctly isolates its own onToggle mock.

Creating a fresh vi.fn() at line 129 and overriding baseProps.onToggle via prop ensures the assertion on line 140 is unambiguous, even though vi.clearAllMocks() would have reset the shared mock anyway.


1-5: Library version confirmed — vitest 4.x is valid.

Vitest 4 was announced on October 22, 2025. The latest version on npm is 4.1.5. 4.0.18 falls within the valid 4.0.x patch series — no issue here.


27-44: mockStore + beforeEach setup is correct.

The selector-fallback pattern (typeof selector === 'function' ? selector(state) : state) correctly handles both Zustand selector and non-selector call forms. The module-scope baseProps.onToggle vi.fn() is safely reset by vi.clearAllMocks() in beforeEach, so no cross-test pollution.


58-66: test.each provider label mapping covers all defined variants.

All three provider values ('vercel', 'database', 'environment') specified by the GlobalFeatureToggleProvider union are exercised. The as const assertion on the tuple array correctly narrows the literal types for the provider prop spread.


88-141: Feature-card rendering, prop mapping, and interaction tests are well-structured.

  • Lines 88–125: Card-per-feature rendering, enabled state resolution (including the missing-key default-to-false case at lines 109–113), and readOnly mirroring of writable are all individually tested.
  • Lines 127–141: The interaction test correctly creates a fresh vi.fn() and overrides baseProps.onToggle to isolate the assertion. The click chain (stub calls onToggle(!enabled) → adapter → parentOnToggle('AUTOPLAY', true)) is fully exercised.

1-1: The test file structure and mock setup are sound. Vitest version 4.0.18 is valid as of 2025 and poses no concern. The module-scope baseProps with shared vi.fn() references is acceptable since vi.clearAllMocks() resets call counts between tests. Double casting patterns (as unknown as FeatureToggleState) and pragmatic typing in test utilities are standard test practices. The FeatureCard mock correctly adapts a single-argument callback for card-level toggle behavior, while the parent-level onToggle receives the full (name, enabled) signature. Test coverage for provider labels and toggle states is adequate for the component's scope.

packages/frontend/src/pages/Admin.test.tsx (1)

144-178: LGTM — auth vs. non-auth error bifurcation is correctly tested.

Using getByText for the positive case and queryByText(...).not.toBeInTheDocument() for the negative case is the idiomatic RTL pattern here.

…getAllBotGuilds unit tests

Resolves two SonarCloud quality gate failures on PR #801:
- Duplication (4.6% > 3%): identical loadError banner in Admin.tsx and
  Features.tsx extracted into FeatureErrorBanner component
- Coverage (79.6% < 80%): four unit tests added for GuildService.getAllBotGuilds
  covering null client, empty guild cache, icon URL construction, and null icon
@LucasSantana-Dev
LucasSantana-Dev dismissed stale reviews from coderabbitai[bot], coderabbitai[bot], and coderabbitai[bot] May 4, 2026 18:35

Issues addressed: coverage/duplication fixed, README fence tagged

@sonarqubecloud

sonarqubecloud Bot commented May 4, 2026

Copy link
Copy Markdown

@LucasSantana-Dev
LucasSantana-Dev merged commit be295a6 into main May 4, 2026
13 checks passed
LucasSantana-Dev added a commit that referenced this pull request May 4, 2026
Roadmap updated with admin panel PR #801 status, v2.8.0 shipped items
(artist/album commands, Vercel feature flags), deferred Phase 3, and
next sprint refactor targets.

Adds PRISMA_MIGRATION_GUIDE.md documenting the GlobalFeatureToggle
table migration (20260504000000) and homelab apply procedure.
LucasSantana-Dev added a commit that referenced this pull request May 13, 2026
* feat(admin): add admin panel with writable global feature toggles

- Add GlobalFeatureToggle DB table (Prisma migration + schema)
- FeatureToggleService: read DB global overrides first, then Vercel,
  then env defaults. Add setGlobalFeatureToggle() for DB writes.
- Backend: POST /api/toggles/global/:name now writes to DB instead
  of returning 409. Remove per-guild toggle routes.
- Frontend: new /admin page gated behind isDeveloper with writable
  global toggle controls and Discord login prompt
- Frontend: Features page simplified to read-only list (no per-server
  or per-guild toggles)
- Sidebar: Admin Panel link visible only to developers
- GlobalFeatureToggleProvider extended with 'database' variant

* feat(admin): add bot server list and harden admin security

- New requireAdmin middleware (auth + developer check) applied at
  router level to /api/admin/* and /api/toggles/global/* paths
- Remove per-handler requireDeveloperUser calls — single enforcement
  point in index.ts prevents bypass
- GET /api/admin/guilds: returns all guilds the bot is in with
  member/channel/role counts (bot client cache + Discord API)
- BotGuildsSection component: sorted guild list with icons,
  member count, text/voice channels, role count
- Admin page now shows Bot Servers section below global toggles

* fix(admin): harden admin panel security, tests and clean up server-toggle dead code

- Apply writeLimiter to POST /api/toggles/global/:name (was only covered by apiLimiter)
- Fix stale 409 test in toggles.test.ts — route now writes to DB and returns 200
- Add requireAuth/requireAdmin middleware to toggles integration test app setup
- Add integration tests for GET /api/admin/guilds (auth/403/200/empty-bot cases)
- Add unit tests for requireAdmin middleware (401/403/next cases)
- Remove orphaned server-toggle code: featuresStore, useFeatures, api.ts, ServerTogglesSection.tsx
- Update Features.test.tsx to match simplified read-only page
- Document admin panel setup (DEVELOPER_USER_IDS, migration, restart) in README

* test(frontend): remove server-toggle tests after per-guild toggle removal

Server-toggle store methods and API endpoints were deleted as part of the
admin-panel global-toggles feature; align test files to match.

* test(backend): add setupAdminRoutes, writeLimiter, requireAdmin mocks to index.test.ts

setupAdminRoutes calls app.get directly; without the mock the unit test
app stub (which only has .use) throws TypeError.

* chore: trigger CI for admin panel test fixes

* test(backend): assert setupSpotifyRoutes is called in route registry

* test(frontend): add coverage for Admin, BotGuildsSection, GlobalTogglesSection

27 new tests across three files to satisfy SonarCloud quality gate on PR #801.

* fix(tests): resolve TypeScript type errors in admin panel test files

Use proper FeatureToggleName/FeatureToggleState casts and as-any for
AxiosResponse mocks to pass tsc strict mode on CI.

* fix(tests): remove invalid eslint-disable directives from guild test

Bare 'as any' cast is sufficient; unused-directive warnings were
breaking CI with --max-warnings 0.

* fix: extract FeatureErrorBanner to eliminate CPD duplication and add getAllBotGuilds unit tests

Resolves two SonarCloud quality gate failures on PR #801:
- Duplication (4.6% > 3%): identical loadError banner in Admin.tsx and
  Features.tsx extracted into FeatureErrorBanner component
- Coverage (79.6% < 80%): four unit tests added for GuildService.getAllBotGuilds
  covering null client, empty guild cache, icon URL construction, and null icon

* docs: add language tag to env code fence in README (MD040)
LucasSantana-Dev added a commit that referenced this pull request May 13, 2026
Roadmap updated with admin panel PR #801 status, v2.8.0 shipped items
(artist/album commands, Vercel feature flags), deferred Phase 3, and
next sprint refactor targets.

Adds PRISMA_MIGRATION_GUIDE.md documenting the GlobalFeatureToggle
table migration (20260504000000) and homelab apply procedure.
@LucasSantana-Dev
LucasSantana-Dev deleted the feature/admin-panel-global-toggles branch May 14, 2026 02:02
LucasSantana-Dev added a commit that referenced this pull request May 21, 2026
## Goal
Prevent orphan code (Prisma models, broken test files, dead types) from
being left behind after feature/route removal PRs. PR #903 had to clean
up 2-week-old orphans from PR #801.

## Changes
1. **PR Template**: Added `.github/PULL_REQUEST_TEMPLATE.md` with a new
"Feature-removal sweep" checklist section (skip-friendly) covering:
   - Orphan Prisma models removed
   - Broken/stale test files removed
   - Unused type aliases removed
   - Imports/exports cleaned up
   - ADRs and CONTEXT.md cross-references updated

2. **Dangerfile Guard**: Added rule #9 to `dangerfile.ts` that detects
commit messages matching removal patterns
(`remove|delete|retire|drop|deprecate` +
`route|handler|endpoint|model|toggle|feature`) and warns if the PR body
doesn't contain the sweep checklist text.

## Acceptance
- [x] PR template has "Feature-removal sweep" section  
- [x] Dangerfile rule flags qualifying commit bodies  
- Ready for validation against next 3 feature-removal PRs

See docs/decisions/2026-05-19-retire-per-guild-feature-toggles.md for
context.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Chores**
* Enhanced pull request template with contributor guidelines, including
reminders for testing and changelog updates.
* Added automated CI check to ensure feature removal pull requests
include proper code cleanup verification.

<!-- review_stack_entry_start -->

[![Review Change
Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/LucasSantana-Dev/Lucky/pull/913?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)

<!-- review_stack_entry_end -->

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

This branch was successfully deployed

1 active deployment
Preview — 8921435b Deployed May 4, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant