Repository navigation
fix(shared,backend,db): retire orphan per-guild feature toggle substrate - #903
Closed
LucasSantana-Dev wants to merge 1 commit into
Closed
LucasSantana-Dev wants to merge 1 commit into
LucasSantana-Dev wants to merge 1 commit into
Conversation
PR #801 (admin panel for global toggles, 2026-05-04) removed the per-guild toggle ROUTES and the `setGuildFeatureToggle` / `isEnabledForGuild` methods on `FeatureToggleService`, but left behind: - `GuildFeatureToggle` Prisma model + `guild_feature_toggles` DB table - `Guild.featureToggles` relation field - `packages/shared/src/services/FeatureToggleService.spec.ts` — 3 dead describe blocks asserting `setGuildFeatureToggle`, `getDbOverride`, `isEnabledForGuild` (compile-failed with TS2551 since #801) - `packages/backend/tests/integration/routes/toggles.test.ts` — dead `mockSetGuildFeatureToggle` declaration + `setGuildFeatureToggle` / `isEnabledForGuild` entries in the service mock The shared test suite has been red on `Quality Gates` for ~2 weeks; the failure was hidden until `/adt-ship-check` + `/diagnose` surfaced it. This commit: - Rewrites `FeatureToggleService.spec.ts` to cover only the surviving global Vercel-flags + DB-override path. Mocks `prisma.globalFeatureToggle` instead of `prisma.guildFeatureToggle`. The 6 global describe blocks now run (they were also dead, since the file refused to compile). - Strips `toggles.test.ts` mock of `mockSetGuildFeatureToggle`, `setGuildFeatureToggle`, and `isEnabledForGuild` entries. 10/10 existing tests pass. - Drops `model GuildFeatureToggle` and `Guild.featureToggles` from `prisma/schema.prisma`. - Adds `20260519000000_retire_per_guild_feature_toggles/migration.sql` with `DROP TABLE IF EXISTS "guild_feature_toggles";`. - Writes `docs/decisions/2026-05-19-retire-per-guild-feature-toggles.md` retroactively documenting #801's removal decision, the orphan state, and the revisit conditions if a future product driver wants per-guild flag scoping. Verification: - `npm run test:ci --workspace=packages/shared`: failed-suite count drops from 2 (FeatureToggleService + spotify/artistApi) to 1 (artistApi only, pre-existing tech debt, out of scope). - `npx jest --config packages/backend/jest.config.cjs packages/backend/tests/integration/routes/toggles.test.ts`: 10/10 passing. - `npm run type:check --workspace=packages/shared`: clean. - `grep -r 'GuildFeatureToggle' packages/*/src --include='*.ts' --include='*.tsx'` outside `generated/` and `.spec.`: no matches. Refs: PR #614 (introduced), PR #801 (silent removal), PR #903 (this fix).
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
LucasSantana-Dev has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Failed to generate code suggestions for PR |
|
This was referenced May 20, 2026
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 --> [](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 -->
](https://app.coderabbit.ai/change-stack/LucasSantana-Dev/Lucky/pull/913?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR #801 (admin panel for global toggles) explicitly removed per-guild toggle ROUTES but left the substrate behind:
GuildFeatureTogglePrisma model +guild_feature_togglesDB table + dead method references in 2 test files. Shared CI'sQuality Gateshas been red on this for ~2 weeks; surfaced this session via/adt-ship-check+/diagnose.What this PR does
model GuildFeatureToggleandGuild.featureTogglesfromprisma/schema.prisma.20260519000000_retire_per_guild_feature_toggles/migration.sqldropsguild_feature_toggleswithIF EXISTS.FeatureToggleService.spec.tsto test only the surviving global Vercel-flag + DB-override path. Deletes three deaddescribeblocks (getDbOverride (via isEnabledForGuild),setGuildFeatureToggle,isEnabledForGuild with DB override). Mocksprisma.globalFeatureToggleinstead ofprisma.guildFeatureToggle.toggles.test.tsmock ofmockSetGuildFeatureToggle,setGuildFeatureToggle, andisEnabledForGuildentries.docs/decisions/2026-05-19-retire-per-guild-feature-toggles.mddocumenting the original removal in feat(admin): add admin panel with writable global feature toggles #801, the orphan state, the considered options (retire / restore / leave-orphan), consequences, and revisit triggers.Why
#801's commit body said "Remove per-guild toggle routes" but didn't sweep schema or tests. The strict workspace
npm run test:ci --workspace=packages/sharedwas failing TS2551 onsetGuildFeatureTogglesince then. With per-guild toggle UI + routes already gone, the substrate is genuinely orphan — restoring it would require a new product driver.Diagnostic trail
Followed
/diagnosediscipline. Phase 1–4 ranked four hypotheses (H1: deliberate removal in admin-panel work / H2: moved to sibling service / H3: renamed / H4: silently deleted). Single probe (git log -S 'setGuildFeatureToggle' --+ grep across packages) confirmed H1 + H4 in combination: #801 deliberately removed the method, didn't clean up the substrate. No correct seam for a regression test (we're deleting dead-method assertions, not fixing buggy logic) — noted in ADR.Verification
npm run test:ci --workspace=packages/shared: failed-suite count drops from 2 to 1 (only__tests__/utils/spotify/artistApi.test.tsremains red, pre-existing tech debt, separate scope).npx jest --config packages/backend/jest.config.cjs packages/backend/tests/integration/routes/toggles.test.ts: 10/10 passing.npm run type:check --workspace=packages/shared: clean.grep -r 'GuildFeatureToggle' packages/*/src --include='*.ts' --include='*.tsx'outsidegenerated/and.spec.: no matches.Test plan
Quality Gateson this PR (the gate that would have caught feat(admin): add admin panel with writable global feature toggles #801)DB impact
Migration drops
guild_feature_toggleswithIF EXISTS. Per the deployment chain (single homelab Postgres production target), rows from #614 (if any survived #801's route removal) will be lost. No production consumers were found writing to this table since #801.Refs: #614 (introduced), #801 (silent removal), this PR (#903).