feat: add scope configuration for feature opt-in#109
Conversation
Add scope field to OptInFeatureConfig that allows features to be scoped to specific levels (org, team, user). This enables features to be shown only at certain settings pages rather than all three. Changes: - Add OptInFeatureScope type with values 'org', 'team', 'user' - Add optional scope field to OptInFeatureConfig interface - Add getOptInFeaturesForScope helper function to filter features by scope - Update FeatureOptInService to filter features based on scope - Update tRPC router to pass scope parameter for org/team endpoints Features without a scope field default to all scopes for backward compatibility. Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
…tate - Add isFeatureAllowedForScope helper function to check if a feature is allowed for a scope - Update setUserFeatureState to reject if feature is not scoped to 'user' - Update setTeamFeatureState to accept scope parameter and reject if feature is not allowed - Update tRPC router to pass scope parameter for team and org endpoints - Fix unit test mock to include new config exports Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
- Replace raw Error with ErrorWithCode using ErrorCode.BadRequest - Add comprehensive tests for setUserFeatureState scope validation - Add comprehensive tests for setTeamFeatureState scope validation - Test both enabled/disabled and inherit state scenarios - Test error messages include feature ID and scope name Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
…on tests Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
Code Review by Qodo
1. Missing await setUserFeatureState
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+103
to
106
| featureOptInService.setUserFeatureState({ | ||
| userId: ctx.user.id, | ||
| featureId: input.slug, | ||
| state: input.state, |
There was a problem hiding this comment.
1. Missing await setuserfeaturestate 📘 Rule violation ⛯ Reliability
• The TRPC setUserState mutation calls featureOptInService.setUserFeatureState(...) without
await, so the handler can return { success: true } before the write completes.
• This can silently drop/ignore async errors (e.g., repository failures), making failures harder to
detect and debug and potentially leaving the system in an inconsistent state.
Agent prompt
## Issue description
The TRPC mutation `setUserState` calls an async service method without `await`, allowing the endpoint to return success before the operation completes and potentially swallowing errors.
## Issue Context
Errors from `featureOptInService.setUserFeatureState(...)` should propagate through TRPC so clients receive an error response and the server can log/monitor failures.
## Fix Focus Areas
- packages/trpc/server/routers/viewer/featureOptIn/_router.ts[95-111]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| }); | ||
|
|
||
| return featureIds.map((featureId) => resolvedStates[featureId]).filter((state) => state.globalEnabled); | ||
| return featureIds.map((featureId) => resolvedStates[featureId]).filter((state) => !state.globalEnabled); |
There was a problem hiding this comment.
2. Wrong globalenabled filter 🐞 Bug ✓ Correctness
• listFeaturesForUser filters for globally disabled features via !state.globalEnabled, contradicting its own docstring and likely making the user opt-in settings show the wrong/empty list. • Team/org listing correctly filters result.globalEnabled, so the user and team/org pages will behave inconsistently.
Agent prompt
### Issue description
`FeatureOptInService.listFeaturesForUser` currently filters *out* globally enabled features (`!state.globalEnabled`), contradicting the method contract and making user feature opt-in settings incorrect/empty.
### Issue Context
Team/org listing uses `result.globalEnabled` filtering; user listing should align.
### Fix Focus Areas
- packages/features/feature-opt-in/services/FeatureOptInService.ts[187-208]
- packages/features/feature-opt-in/services/FeatureOptInService.ts[210-243]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+257
to
267
| if (!isFeatureAllowedForScope(featureId, "user")) { | ||
| throw new ErrorWithCode( | ||
| ErrorCode.BadRequest, | ||
| `Feature "${featureId}" is not available at the user scope` | ||
| ); | ||
| } | ||
|
|
||
| if (state !== "inherit") { | ||
| await this.featuresRepository.setUserFeatureState({ userId, featureId, state }); | ||
| } else { | ||
| const { assignedBy } = input; |
There was a problem hiding this comment.
3. Broken user state update 🐞 Bug ✓ Correctness
• FeatureOptInService.setUserFeatureState has inverted branching: it calls FeaturesRepository.setUserFeatureState without assignedBy for enabled/disabled states (required), which can break writes or fail type-checking. • The inherit branch incorrectly reads/passes assignedBy, but the repository’s inherit path is defined to delete rows and does not take assignedBy.
Agent prompt
### Issue description
`FeatureOptInService.setUserFeatureState` is currently inverted: it forwards enabled/disabled updates without `assignedBy` (required by repository contract) and tries to read/pass `assignedBy` for `inherit` (where it should delete rows).
### Issue Context
`FeaturesRepository.setUserFeatureState` requires `assignedBy` only when state is enabled/disabled.
### Fix Focus Areas
- packages/features/feature-opt-in/services/FeatureOptInService.ts[245-275]
- packages/features/flags/features.repository.interface.ts[14-18]
- packages/features/flags/features.repository.ts[350-384]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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.
Benchmark PR from agentic-review-benchmarks#14