Skip to content

fix: enable DI for FeatureOptInService#124

Open
tomerqodo wants to merge 2 commits into
qodo_claude_vs_qodo_base_fix_enable_di_for_featureoptinservice_pr13from
qodo_claude_vs_qodo_head_fix_enable_di_for_featureoptinservice_pr13
Open

fix: enable DI for FeatureOptInService#124
tomerqodo wants to merge 2 commits into
qodo_claude_vs_qodo_base_fix_enable_di_for_featureoptinservice_pr13from
qodo_claude_vs_qodo_head_fix_enable_di_for_featureoptinservice_pr13

Conversation

@tomerqodo

Copy link
Copy Markdown

Benchmark PR from agentic-review-benchmarks#13

@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Enable dependency injection for FeatureOptInService and FeaturesRepository

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Enable dependency injection for FeatureOptInService and FeaturesRepository
• Create DI containers and modules for feature-opt-in and flags services
• Extract ResolvedFeatureState type into IFeatureOptInService interface
• Update imports across codebase to use DI containers instead of direct instantiation
Diagram
flowchart LR
  A["Features Module<br/>Features.ts"] -->|"refactored to"| B["FeaturesRepository Module<br/>FeaturesRepository.ts"]
  C["FeatureOptInService<br/>Direct instantiation"] -->|"refactored to"| D["FeatureOptInService Module<br/>FeatureOptInService.ts"]
  B -->|"dependency"| D
  D -->|"exposed via"| E["DI Container<br/>FeatureOptInService.ts"]
  B -->|"exposed via"| F["DI Container<br/>FeaturesRepository.ts"]
  G["IFeatureOptInService<br/>Interface"] -->|"implemented by"| H["FeatureOptInService<br/>Class"]
  I["TRPC Router<br/>_router.ts"] -->|"uses"| E
  J["Integration Tests<br/>integration-test.ts"] -->|"uses"| E
  J -->|"uses"| F
Loading

Grey Divider

File Changes

1. packages/features/di/modules/FeaturesRepository.ts ✨ Enhancement +24/-0

Create FeaturesRepository DI module with new structure

packages/features/di/modules/FeaturesRepository.ts


2. packages/features/di/modules/Features.ts ✨ Enhancement +0/-16

Remove old Features module file completely

packages/features/di/modules/Features.ts


3. packages/features/di/modules/FeatureOptInService.ts ✨ Enhancement +24/-0

Create FeatureOptInService DI module with dependencies

packages/features/di/modules/FeatureOptInService.ts


View more (12)
4. packages/features/di/containers/FeaturesRepository.ts ✨ Enhancement +8/-0

Create FeaturesRepository DI container factory function

packages/features/di/containers/FeaturesRepository.ts


5. packages/features/di/containers/FeatureOptInService.ts ✨ Enhancement +10/-0

Create FeatureOptInService DI container factory function

packages/features/di/containers/FeatureOptInService.ts


6. packages/features/feature-opt-in/services/IFeatureOptInService.ts ✨ Enhancement +39/-0

Create interface for FeatureOptInService with type definitions

packages/features/feature-opt-in/services/IFeatureOptInService.ts


7. packages/features/feature-opt-in/services/FeatureOptInService.ts ✨ Enhancement +2/-14

Implement IFeatureOptInService interface and extract types

packages/features/feature-opt-in/services/FeatureOptInService.ts


8. packages/features/flags/di/tokens.ts ✨ Enhancement +4/-0

Create FLAGS_DI_TOKENS for features repository symbols

packages/features/flags/di/tokens.ts


9. packages/features/feature-opt-in/di/tokens.ts ✨ Enhancement +4/-0

Create FEATURE_OPT_IN_DI_TOKENS for service symbols

packages/features/feature-opt-in/di/tokens.ts


10. packages/features/di/tokens.ts ✨ Enhancement +4/-2

Update DI tokens to use new FLAGS and FEATURE_OPT_IN tokens

packages/features/di/tokens.ts


11. packages/features/booking-audit/di/BookingAuditTaskConsumer.module.ts ✨ Enhancement +1/-2

Update import path from Features to FeaturesRepository

packages/features/booking-audit/di/BookingAuditTaskConsumer.module.ts


12. packages/features/bookings/di/RegularBookingService.module.ts ✨ Enhancement +1/-1

Update import path from Features to FeaturesRepository

packages/features/bookings/di/RegularBookingService.module.ts


13. packages/features/di/containers/AvailableSlots.ts ✨ Enhancement +1/-1

Update import path from Features to FeaturesRepository

packages/features/di/containers/AvailableSlots.ts


14. packages/features/feature-opt-in/services/FeatureOptInService.integration-test.ts 🧪 Tests +7/-5

Update tests to use DI containers instead of direct instantiation

packages/features/feature-opt-in/services/FeatureOptInService.integration-test.ts


15. packages/trpc/server/routers/viewer/featureOptIn/_router.ts ✨ Enhancement +2/-5

Update TRPC router to use DI container for service

packages/trpc/server/routers/viewer/featureOptIn/_router.ts


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Mar 10, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1) 📎 Requirement gaps (0)

Grey Divider


Action required

1. IFeatureOptInService formatting violations 📘 Rule violation ✓ Correctness
Description
The newly added interface file contains trailing whitespace and several overly long lines that do
not conform to the repository’s Biome formatting rules. This reduces consistency and can cause
formatter/linter failures in CI.
Code

packages/features/feature-opt-in/services/IFeatureOptInService.ts[R11-28]

+  orgAutoOptIn: boolean;   
+  teamAutoOptIns: boolean[];
+  userAutoOptIn: boolean;
+};
+
+export interface IFeatureOptInService {
+  resolveFeatureStatesAcrossTeams(input: {
+    userId: number;
+    orgId: number | null;
+    teamIds: number[];
+    featureIds: FeatureId[];
+  }): Promise<Record<string, ResolvedFeatureState>>;
+  listFeaturesForUser(input: { userId: number; orgId: number | null; teamIds: number[] }): Promise<
+    ResolvedFeatureState[]
+  >;
+  listFeaturesForTeam(
+    input: { teamId: number }
+  ): Promise<{ featureId: FeatureId; globalEnabled: boolean; teamState: FeatureState }[]>;
Evidence
PR Compliance ID 3 requires adhering to Biome formatting standards; the added code includes trailing
spaces and very long single lines that Biome would reformat (or flag).

AGENTS.md
packages/features/feature-opt-in/services/IFeatureOptInService.ts[11-11]
packages/features/feature-opt-in/services/IFeatureOptInService.ts[23-28]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new `IFeatureOptInService.ts` file contains Biome formatting violations (trailing whitespace and long lines) that may fail formatting checks and reduces consistency.

## Issue Context
Compliance requires TypeScript files to adhere to the repo’s Biome formatting rules (line width, whitespace, etc.).

## Fix Focus Areas
- packages/features/feature-opt-in/services/IFeatureOptInService.ts[11-28]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Wrong DI token exported 🐞 Bug ✓ Correctness
Description
packages/features/di/modules/FeaturesRepository.ts exports moduleLoader.token as the module token
instead of the binding token, so bindModuleToClassOnToken injects an unbound token and
container.get() is called with a token that was never bound. This causes runtime DI resolution
failures for services that depend on FeaturesRepository (e.g., RegularBookingService,
BookingAuditTaskConsumer, FeatureOptInService) and for getFeaturesRepository().
Code

packages/features/di/modules/FeaturesRepository.ts[R19-22]

+export const moduleLoader: ModuleLoader = {
+  token: moduleToken,
+  loadModule,
+};
Evidence
FeaturesRepository binds the class on token but exports moduleLoader.token as moduleToken;
bindModuleToClassOnToken uses dep.token for constructor injection and consumers (and the new
container) use moduleLoader.token for resolution, so they will request
FEATURES_REPOSITORY_MODULE instead of FEATURES_REPOSITORY and fail at runtime.

packages/features/di/modules/FeaturesRepository.ts[7-22]
packages/features/di/di.ts[94-106]
packages/features/bookings/di/RegularBookingService.module.ts[15-34]
packages/features/di/containers/FeaturesRepository.ts[4-8]
packages/features/users/di/MembershipRepository.module.ts[6-20]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`packages/features/di/modules/FeaturesRepository.ts` binds `FeaturesRepository` on `FLAGS_DI_TOKENS.FEATURES_REPOSITORY` but exports `moduleLoader.token` as `FLAGS_DI_TOKENS.FEATURES_REPOSITORY_MODULE`. The DI helper `bindModuleToClassOnToken` uses `dep.token` for constructor injection, and containers use `moduleLoader.token` for `container.get()`, so consumers try to resolve an unbound token at runtime.

### Issue Context
Other modules’ loaders export the *binding* token (e.g., MembershipRepository). `moduleToken` is only meant for `container.load(moduleToken, module)`.

### Fix Focus Areas
- packages/features/di/modules/FeaturesRepository.ts[7-22]
- packages/features/di/containers/FeaturesRepository.ts[4-8]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Comment on lines +11 to +28
orgAutoOptIn: boolean;
teamAutoOptIns: boolean[];
userAutoOptIn: boolean;
};

export interface IFeatureOptInService {
resolveFeatureStatesAcrossTeams(input: {
userId: number;
orgId: number | null;
teamIds: number[];
featureIds: FeatureId[];
}): Promise<Record<string, ResolvedFeatureState>>;
listFeaturesForUser(input: { userId: number; orgId: number | null; teamIds: number[] }): Promise<
ResolvedFeatureState[]
>;
listFeaturesForTeam(
input: { teamId: number }
): Promise<{ featureId: FeatureId; globalEnabled: boolean; teamState: FeatureState }[]>;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. ifeatureoptinservice formatting violations 📘 Rule violation ✓ Correctness

The newly added interface file contains trailing whitespace and several overly long lines that do
not conform to the repository’s Biome formatting rules. This reduces consistency and can cause
formatter/linter failures in CI.
Agent Prompt
## Issue description
The new `IFeatureOptInService.ts` file contains Biome formatting violations (trailing whitespace and long lines) that may fail formatting checks and reduces consistency.

## Issue Context
Compliance requires TypeScript files to adhere to the repo’s Biome formatting rules (line width, whitespace, etc.).

## Fix Focus Areas
- packages/features/feature-opt-in/services/IFeatureOptInService.ts[11-28]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +19 to +22
export const moduleLoader: ModuleLoader = {
token: moduleToken,
loadModule,
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

2. Wrong di token exported 🐞 Bug ✓ Correctness

packages/features/di/modules/FeaturesRepository.ts exports moduleLoader.token as the module token
instead of the binding token, so bindModuleToClassOnToken injects an unbound token and
container.get() is called with a token that was never bound. This causes runtime DI resolution
failures for services that depend on FeaturesRepository (e.g., RegularBookingService,
BookingAuditTaskConsumer, FeatureOptInService) and for getFeaturesRepository().
Agent Prompt
### Issue description
`packages/features/di/modules/FeaturesRepository.ts` binds `FeaturesRepository` on `FLAGS_DI_TOKENS.FEATURES_REPOSITORY` but exports `moduleLoader.token` as `FLAGS_DI_TOKENS.FEATURES_REPOSITORY_MODULE`. The DI helper `bindModuleToClassOnToken` uses `dep.token` for constructor injection, and containers use `moduleLoader.token` for `container.get()`, so consumers try to resolve an unbound token at runtime.

### Issue Context
Other modules’ loaders export the *binding* token (e.g., MembershipRepository). `moduleToken` is only meant for `container.load(moduleToken, module)`.

### Fix Focus Areas
- packages/features/di/modules/FeaturesRepository.ts[7-22]
- packages/features/di/containers/FeaturesRepository.ts[4-8]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

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