feat: ✨ add findByEmail use case #94
Conversation
WalkthroughThe pull request introduces significant changes across multiple files, primarily focusing on enhancing user retrieval capabilities and integrating GraphQL with Apollo Federation. Key modifications include the addition of new use cases for retrieving users by ID and email, the restructuring of the Changes
Possibly related issues
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (5)libs/users/application/src/lib/users.service.spec.ts (5)
The necessary use cases are imported and variables are correctly declared. Line range hint The test setup properly includes and mocks both use cases, addressing previous configuration issues.
The test cases properly cover both success and failure scenarios with appropriate assertions.
All previous issues have been addressed:
While the basic success and failure scenarios are well covered, consider adding tests for the following edge cases:
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (8)
libs/users/domain/src/lib/users.repository.ts (1)
5-5: Consider adding JSDoc commentsThe new
findByEmailmethod is well-designed and consistent with the existing pattern. Consider adding JSDoc comments to document the behavior, especially regarding null returns and email matching criteria (case sensitivity, etc.).+ /** + * Finds a user by their email address + * @param email The email address to search for + * @returns Promise resolving to User if found, null otherwise + */ findByEmail(email: string): Promise<User | null>;libs/users/application/src/lib/use-cases/get-user-by-id.use-case.ts (1)
11-13: Consider adding input validationThe execute method could benefit from input validation to handle invalid ID formats or empty strings before querying the repository.
async execute(id: string): Promise<User | null> { + if (!id?.trim()) { + throw new Error('Invalid user ID provided'); + } return await this.usersRepository.findById(id); }libs/users/application/src/lib/use-cases/get-user-by-email.use-case.ts (1)
1-14: LGTM! Clean implementation following best practices.The implementation follows clean architecture principles with proper dependency injection and clear single responsibility.
Consider adding input validation for the email parameter:
async execute(email: string): Promise<User | null> { + if (!email || !email.includes('@')) { + throw new Error('Invalid email format'); + } return this.usersRepository.findByEmail(email); }libs/users/interface-adapters/src/lib/users.module.ts (1)
27-29: Consider using a more type-safe MongoDB configuration.The current configuration could benefit from stronger typing.
Consider using a type-safe configuration:
MongooseModule.forFeature([ - { name: UserDocument.name, schema: UserSchema }, + { + name: UserDocument.name, + schema: UserSchema, + collection: 'users' as const + }, ]),libs/users/infrastructure/mongoose/src/lib/mongoose-users.repository.ts (1)
29-41: Consider case-insensitive email search and input validationThe implementation works but could be improved in a few ways:
- Make email search case-insensitive for better user experience
- Add email parameter validation
- Extract repeated user mapping logic to reduce duplication
Consider this implementation:
async findByEmail(email: string): Promise<User | null> { + if (!email || typeof email !== 'string') { + throw new Error('Invalid email parameter'); + } + - const userDocument = await this.userModel.findOne({ email }).exec(); + const userDocument = await this.userModel + .findOne({ email: { $regex: new RegExp(`^${email}$`, 'i') } }) + .exec(); if (!userDocument) { return null; } - return new User( - userDocument.id, - userDocument.email, - userDocument.firstName, - userDocument.lastName, - ); + return this.mapToUser(userDocument); } + private mapToUser(doc: UserDocument): User { + return new User( + doc.id, + doc.email, + doc.firstName, + doc.lastName + ); + }libs/users/application/src/lib/use-cases/get-user-by-id.use-case.spec.ts (1)
4-13: Consider adding error handling test casesThe test suite could benefit from additional test cases to verify:
- Invalid ID handling (e.g., malformed ObjectId)
- Repository error handling
Example additional test cases:
it('should throw error for invalid ID format', async () => { await expect(getUserByIdUseCase.execute('invalid-id')) .rejects .toThrow('Invalid ID format'); }); it('should handle repository errors', async () => { (usersRepository.findById as jest.Mock) .mockRejectedValue(new Error('DB error')); await expect(getUserByIdUseCase.execute('1')) .rejects .toThrow('Failed to fetch user'); });libs/users/domain/src/lib/users.repository.spec.ts (1)
46-59: Consider adding edge case testsThe current tests cover the basic success and failure cases. Consider adding tests for:
- Case-sensitivity handling
- Email with leading/trailing spaces
- Special characters in email
test('findByEmail should handle email case-insensitively', async () => { const user = await usersRepository.findByEmail('[email protected]'); expect(user).not.toBeNull(); }); test('findByEmail should handle whitespace', async () => { const user = await usersRepository.findByEmail(' [email protected] '); expect(user).not.toBeNull(); });libs/users/application/src/lib/users.service.spec.ts (1)
Line range hint
13-21: Missing GetUserByEmailUseCase provider in test configurationThe test module configuration is missing the GetUserByEmailUseCase provider, which is required for the findByEmail tests. Currently, the tests are incorrectly using GetUserByIdUseCase for email-based lookups.
Add the missing provider:
providers: [ UsersService, { provide: GetUserByIdUseCase, useValue: { execute: jest.fn(), }, }, + { + provide: GetUserByEmailUseCase, + useValue: { + execute: jest.fn(), + }, + }, ],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (13)
apps/users/src/app/app.module.ts(2 hunks)libs/auth/interface-adapters/src/lib/auth.module.ts(2 hunks)libs/users/application/src/index.ts(1 hunks)libs/users/application/src/lib/use-cases/get-user-by-email.use-case.spec.ts(1 hunks)libs/users/application/src/lib/use-cases/get-user-by-email.use-case.ts(1 hunks)libs/users/application/src/lib/use-cases/get-user-by-id.use-case.spec.ts(3 hunks)libs/users/application/src/lib/use-cases/get-user-by-id.use-case.ts(1 hunks)libs/users/application/src/lib/users.service.spec.ts(3 hunks)libs/users/application/src/lib/users.service.ts(1 hunks)libs/users/domain/src/lib/users.repository.spec.ts(2 hunks)libs/users/domain/src/lib/users.repository.ts(1 hunks)libs/users/infrastructure/mongoose/src/lib/mongoose-users.repository.ts(1 hunks)libs/users/interface-adapters/src/lib/users.module.ts(2 hunks)
🔇 Additional comments (9)
libs/users/application/src/index.ts (1)
1-2: LGTM! Clean and focused exports
The exports are well-structured and follow a clear naming convention, properly exposing both ID and email-based user retrieval functionality.
libs/users/application/src/lib/use-cases/get-user-by-id.use-case.ts (1)
5-9: LGTM! Clean implementation following SOLID principles
The class is well-structured with proper dependency injection and clear responsibility.
libs/users/application/src/lib/users.service.ts (1)
4-5: LGTM! Good separation of concerns.
The refactoring improves the code by using specific use cases for each retrieval method, making the code more maintainable and following the single responsibility principle.
Also applies to: 9-12, 15-15, 19-19
libs/users/interface-adapters/src/lib/users.module.ts (1)
3-3: LGTM! Module configuration properly updated.
The module is correctly configured with the new use cases and maintains proper dependency injection.
Also applies to: 18-19
apps/users/src/app/app.module.ts (2)
27-40: LGTM: GraphQL configuration follows best practices
The configuration:
- ✅ Disables playground in production
- ✅ Enables schema sorting for consistency
- ✅ Includes trace plugin for debugging
29-36: Verify federation version compatibility with existing services
The explicit federation version 2 setting is added to address a known issue (nestjs/graphql#2646). Ensure this version is compatible with other federated services in your architecture.
✅ Verification successful
Let me search for other GraphQL module configurations to better understand the federation setup across the codebase.
Federation v2 configuration is correctly aligned across the architecture
The codebase shows a consistent GraphQL Federation setup:
- Users service is explicitly configured with Federation v2
- Gateway service uses
IntrospectAndComposewhich is compatible with Federation v2 - No conflicting federation versions were found in other services
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other federation version configurations
rg -l "federation.*[12]" --type ts
Length of output: 69
Script:
#!/bin/bash
# Search for GraphQL module configurations and federation settings in the codebase
rg -A 10 "GraphQLModule\.forRoot" --type ts
Length of output: 1628
libs/auth/interface-adapters/src/lib/auth.module.ts (1)
8-8: Verify all GetUserUseCase consumers are updated
The replacement of GetUserUseCase with GetUserByIdUseCase and GetUserByEmailUseCase looks good, but we should verify all consumers are updated.
Also applies to: 26-27
✅ Verification successful
All consumers have been properly updated to use the new use cases
Based on the search results, there are no remaining references to the old GetUserUseCase. The new GetUserByIdUseCase and GetUserByEmailUseCase are properly:
- Imported and provided in both
auth.module.tsandusers.module.ts - Injected and used correctly in
UsersService - Fully tested in their respective spec files
- Used appropriately in the resolver through the
UsersService
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to GetUserUseCase
rg "GetUserUseCase" --type ts
# Search for constructor injections of the new use cases
rg "constructor.*GetUserBy(Id|Email)UseCase" --type ts
Length of output: 86
Script:
#!/bin/bash
# Let's try a broader search for any references to these use cases
rg -i "getuserby(id|email)" --type ts -A 2
# Also search for any files that might contain "getuser" to catch potential missed updates
rg -i "getuser" --type ts -A 2
# Let's also check the specific file mentioned in the review
cat libs/auth/interface-adapters/src/lib/auth.module.ts
Length of output: 19203
libs/users/domain/src/lib/users.repository.spec.ts (1)
19-21: LGTM: findByEmail implementation is clean and efficient
The implementation correctly uses Array.find() and handles the null case appropriately.
libs/users/application/src/lib/users.service.spec.ts (1)
Line range hint 32-52: LGTM! Well-structured tests for findById
The tests cover both successful and unsuccessful cases appropriately.
libs/users/application/src/lib/use-cases/get-user-by-email.use-case.spec.ts
Outdated
Show resolved
Hide resolved
libs/users/application/src/lib/use-cases/get-user-by-email.use-case.spec.ts
Outdated
Show resolved
Hide resolved
libs/users/application/src/lib/use-cases/get-user-by-email.use-case.spec.ts
Outdated
Show resolved
Hide resolved
|
🎉 This PR is included in version 1.11.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary by CodeRabbit
Release Notes
New Features
GetUserByIdUseCaseandGetUserByEmailUseCase.UsersServiceto support both user lookups.Bug Fixes
Documentation