Skip to content

feat: add scope configuration for feature opt-in - #14

Open
tomerqodo wants to merge 8 commits into
codex_full_base_feat_add_scope_configuration_for_feature_opt-in_pr14from
codex_full_head_feat_add_scope_configuration_for_feature_opt-in_pr14
Open

feat: add scope configuration for feature opt-in#14
tomerqodo wants to merge 8 commits into
codex_full_base_feat_add_scope_configuration_for_feature_opt-in_pr14from
codex_full_head_feat_add_scope_configuration_for_feature_opt-in_pr14

Conversation

@tomerqodo

Copy link
Copy Markdown

Benchmark PR from agentic-review-benchmarks#14

devin-ai-integration Bot and others added 8 commits January 25, 2026 12:03
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>

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 02f075d61b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

});

return featureIds.map((featureId) => resolvedStates[featureId]).filter((state) => state.globalEnabled);
return featureIds.map((featureId) => resolvedStates[featureId]).filter((state) => !state.globalEnabled);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Filter by globalEnabled, not its negation

This now filters for !state.globalEnabled, which means globally enabled features are excluded from the user list. When a feature is enabled globally (the expected happy path), it will no longer appear in the user-facing opt‑in list, effectively hiding available features in settings.

Useful? React with 👍 / 👎.

Comment on lines +264 to 266
if (state !== "inherit") {
await this.featuresRepository.setUserFeatureState({ userId, featureId, state });
} else {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Pass assignedBy for enabled/disabled user state

For non‑inherit updates, the repository expects assignedBy and uses it to persist audit info (and may require it at the DB level). This change now calls setUserFeatureState without assignedBy when state is enabled/disabled, so the update can fail or store an undefined assignee, while the inherit path incorrectly includes an assignee that isn't provided. This will surface when a user toggles a feature on/off.

Useful? React with 👍 / 👎.

Comment on lines +103 to 106
featureOptInService.setUserFeatureState({
userId: ctx.user.id,
featureId: input.slug,
state: input.state,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Await the user feature mutation to surface failures

The mutation no longer awaits setUserFeatureState, so the API returns success immediately even if the DB update fails. In error cases, the rejection becomes unhandled and the client never learns the operation failed, which can leave UI state out of sync with persisted data.

Useful? React with 👍 / 👎.

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.

1 participant