refactor: ♻️ Move dto and resolver to interface-adapters #57
Conversation
WalkthroughThe changes in this pull request involve significant restructuring of the application architecture, particularly in the categorization of layers. The Changes
Possibly related PRs
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 (
|
There was a problem hiding this comment.
Copilot reviewed 13 out of 26 changed files in this pull request and generated no suggestions.
Files not reviewed (13)
- libs/users/interface-adapters/project.json: Language not supported
- libs/users/interface-adapters/tsconfig.json: Language not supported
- libs/users/interface-adapters/tsconfig.lib.json: Language not supported
- libs/users/interface-adapters/tsconfig.spec.json: Language not supported
- libs/users/presentation/dto/project.json: Language not supported
- libs/users/presentation/dto/tsconfig.lib.json: Language not supported
- libs/users/presentation/dto/tsconfig.spec.json: Language not supported
- libs/users/presentation/dto/README.md: Evaluated as low risk
- libs/users/presentation/dto/eslint.config.js: Evaluated as low risk
- libs/users/presentation/dto/src/index.ts: Evaluated as low risk
- libs/users/presentation/resolver/README.md: Evaluated as low risk
- libs/users/interface-adapters/src/lib/resolver/users.resolver.ts: Evaluated as low risk
- README.md: Evaluated as low risk
Comments skipped due to low confidence (1)
libs/users/interface-adapters/src/lib/resolver/users.resolver.spec.ts:5
- The import path for UsersResolver is incorrect. It should be updated to reflect the new location in interface-adapters.
import { UsersResolver } from '../../../../presentation/resolver/src/lib/users.resolver';
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (5)
libs/users/interface-adapters/src/index.ts (2)
1-4: Clean architecture restructuring looks good! ✨Moving DTOs and resolvers to the interface-adapters layer is a solid architectural decision. This better separates the concerns between the domain logic and the presentation layer, following clean architecture principles where interface adapters handle the conversion of data between the inner and outer layers.
1-4: Consider adding JSDoc module documentationTo improve maintainability, consider adding a JSDoc comment at the top of the file describing the purpose of these exports and their role in the interface adapters layer.
+/** + * @module users/interface-adapters + * @description Exports DTOs and resolvers that handle the conversion between + * domain entities and external representations in the users domain. + */ // dto export * from './lib/dto/user.dto'; // resolver export * from './lib/resolver/users.resolver';tsconfig.base.json (1)
28-30: LGTM! Good architectural improvement.Moving DTOs and Resolvers under interface-adapters aligns well with clean architecture principles, as these components serve as adapters between the domain and external layers.
README.md (2)
57-62: Well-structured architectural reorganization! 👍The move of DTOs and resolvers to the interface-adapters layer better aligns with clean architecture principles, providing clearer separation between the presentation and domain layers. The addition of the service layer also helps isolate core application logic from framework dependencies.
65-65: Minor: Remove redundancy in repository descriptionThe phrase "specific details" is redundant. Consider simplifying to just "details".
-By defining this interface, we can decouple database access: the specific details of data access will be done by implementation classes, such as specific implementations using tools like Mongoose, TypeORM, Prisma, and so on. +By defining this interface, we can decouple database access: the details of data access will be done by implementation classes, such as specific implementations using tools like Mongoose, TypeORM, Prisma, and so on.🧰 Tools
🪛 LanguageTool
[style] ~65-~65: This phrase is redundant. Consider writing “details”.
Context: ...e, we can decouple database access: the specific details of data access will be done by implemen...(SPECIFIC_DETAILS)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (24)
README.md(1 hunks)libs/users/interface-adapters/README.md(1 hunks)libs/users/interface-adapters/eslint.config.js(1 hunks)libs/users/interface-adapters/jest.config.ts(1 hunks)libs/users/interface-adapters/project.json(1 hunks)libs/users/interface-adapters/src/index.ts(1 hunks)libs/users/interface-adapters/src/lib/resolver/users.resolver.spec.ts(1 hunks)libs/users/interface-adapters/src/lib/resolver/users.resolver.ts(1 hunks)libs/users/interface-adapters/tsconfig.json(1 hunks)libs/users/interface-adapters/tsconfig.lib.json(1 hunks)libs/users/interface-adapters/tsconfig.spec.json(1 hunks)libs/users/presentation/dto/README.md(0 hunks)libs/users/presentation/dto/eslint.config.js(0 hunks)libs/users/presentation/dto/project.json(0 hunks)libs/users/presentation/dto/src/index.ts(0 hunks)libs/users/presentation/dto/tsconfig.lib.json(0 hunks)libs/users/presentation/dto/tsconfig.spec.json(0 hunks)libs/users/presentation/resolver/README.md(0 hunks)libs/users/presentation/resolver/eslint.config.js(0 hunks)libs/users/presentation/resolver/jest.config.ts(0 hunks)libs/users/presentation/resolver/project.json(0 hunks)libs/users/presentation/resolver/src/index.ts(0 hunks)libs/users/presentation/resolver/tsconfig.json(0 hunks)tsconfig.base.json(1 hunks)
💤 Files with no reviewable changes (12)
- libs/users/presentation/dto/README.md
- libs/users/presentation/dto/eslint.config.js
- libs/users/presentation/dto/project.json
- libs/users/presentation/dto/src/index.ts
- libs/users/presentation/dto/tsconfig.lib.json
- libs/users/presentation/dto/tsconfig.spec.json
- libs/users/presentation/resolver/README.md
- libs/users/presentation/resolver/eslint.config.js
- libs/users/presentation/resolver/jest.config.ts
- libs/users/presentation/resolver/project.json
- libs/users/presentation/resolver/src/index.ts
- libs/users/presentation/resolver/tsconfig.json
✅ Files skipped from review due to trivial changes (7)
- libs/users/interface-adapters/README.md
- libs/users/interface-adapters/eslint.config.js
- libs/users/interface-adapters/project.json
- libs/users/interface-adapters/src/lib/resolver/users.resolver.spec.ts
- libs/users/interface-adapters/src/lib/resolver/users.resolver.ts
- libs/users/interface-adapters/tsconfig.json
- libs/users/interface-adapters/tsconfig.lib.json
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~65-~65: This phrase is redundant. Consider writing “details”.
Context: ...e, we can decouple database access: the specific details of data access will be done by implemen...
(SPECIFIC_DETAILS)
🔇 Additional comments (2)
libs/users/interface-adapters/tsconfig.spec.json (1)
4-4: LGTM! Path adjustment aligns with architectural changes.
The updated outDir path correctly reflects the new directory structure after moving components to the interface-adapters layer.
Let's verify the consistency of this path with other configuration files:
✅ Verification successful
Path adjustment is correct and consistent with the codebase structure
The verification shows that:
- Both
tsconfig.spec.jsonandtsconfig.lib.jsonin the same directory use the consistent path"../../../dist/out-tsc" - The path aligns with other similar modules at the same directory level:
libs/users/domain/: "../../../dist/out-tsc"libs/users/application/: "../../../dist/out-tsc"libs/prompt/domain/: "../../../dist/out-tsc"
- Deeper nested modules use four levels:
"../../../../dist/out-tsc" - Apps use two levels:
"../../dist/out-tsc"
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the outDir path consistency across related TypeScript configs
# Expected: All TypeScript configs in the interface-adapters directory should use the same relative path
# Check all TypeScript configs in the interface-adapters directory
fd tsconfig.*.json -p libs/users/interface-adapters --exec grep -l "outDir"
# Verify the path in each found config
fd tsconfig.*.json -p libs/users/interface-adapters --exec grep "outDir"
Length of output: 154
Script:
#!/bin/bash
# Let's try a broader search to find all tsconfig files and their outDir settings
# First, find the specific file we're reviewing
echo "=== Current file under review ==="
cat libs/users/interface-adapters/tsconfig.spec.json
echo -e "\n=== Related tsconfig files in the same directory ==="
ls -la libs/users/interface-adapters/tsconfig*.json
echo -e "\n=== Content of all tsconfig files in the directory ==="
for f in libs/users/interface-adapters/tsconfig*.json; do
echo "=== $f ==="
cat "$f" 2>/dev/null
done
echo -e "\n=== Similar tsconfig.spec.json files in other directories ==="
fd tsconfig.spec.json --exec grep -H "outDir" {} \;
Length of output: 3864
libs/users/interface-adapters/jest.config.ts (1)
2-3: LGTM! Path adjustments align with the architectural changes.
The configuration changes correctly reflect the move to interface-adapters, with appropriate updates to the displayName and directory paths.
Let's verify the path adjustments are correct:
Also applies to: 9-9
✅ Verification successful
Path adjustments in jest.config.ts are correct and consistent
The verification confirms:
- Jest preset file exists at repository root
- Directory structure matches the new paths (
libs/users/interface-adapters/) - Path depth (
../../../) is consistent with other modules at the same level (application, domain) - Only the mongoose infrastructure module uses deeper paths (
../../../../) due to its nested location
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the Jest preset path and ensure the coverage directory exists
# Check if the Jest preset file exists at the specified path
if [ -f "jest.preset.js" ]; then
echo "Jest preset file found at repository root"
fi
# Verify the directory structure matches the new paths
fd -t d "users" libs/
Length of output: 154
Script:
#!/bin/bash
# Let's verify the complete directory structure under libs/users to ensure paths are correct
fd . libs/users/ -t d
# Also check if there are any other jest.config.ts files to compare structure
fd jest.config.ts libs/users/
Length of output: 758
Script:
#!/bin/bash
# Let's check the content of other jest.config.ts files to verify path consistency
cat libs/users/application/jest.config.ts libs/users/domain/jest.config.ts libs/users/infrastructure/mongoose/jest.config.ts
Length of output: 1130
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
apps/users/src/users/users.module.ts (1)
Line range hint
18-24: Consider documenting the Federation version constraint.The comment about the Federation version constraint references a GitHub issue. While this is helpful, consider adding more context about:
- What specific issues this version constraint solves
- When this constraint might be removable
- What version is actually being used
Consider updating the comment to:
/** * MEMO: * Federation v2 is explicitly specified due to schema composition issues * with automatic version detection. * Issue: https://github.com/nestjs/graphql/issues/2646#issuecomment-1567381944 * TODO: Monitor issue for resolution to remove version constraint */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
apps/users-e2e/src/users/users.spec.ts(1 hunks)apps/users/src/app/app.controller.spec.ts(1 hunks)apps/users/src/app/app.module.ts(1 hunks)apps/users/src/app/app.service.spec.ts(1 hunks)apps/users/src/app/app.service.ts(1 hunks)apps/users/src/users/users.module.ts(1 hunks)libs/users/interface-adapters/src/lib/resolver/users.resolver.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/users/src/app/app.module.ts
- apps/users/src/app/app.service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/users/interface-adapters/src/lib/resolver/users.resolver.ts
🔇 Additional comments (5)
apps/users-e2e/src/users/users.spec.ts (2)
Line range hint 1-10: Consider updating related documentation.
Since this change is part of a larger refactoring effort moving components to interface-adapters, ensure that any API documentation or README files mentioning this endpoint are updated to reflect the new message.
#!/bin/bash
# Description: Check for documentation files that might need updates
# Search for documentation files that might reference the API response
echo "Checking documentation files..."
fd -e md -e txt | xargs rg "Hello.*Service"8-8: LGTM! Verify consistency across the codebase.
The updated assertion aligns with the changes made in AppService. Let's verify the consistency of this message across all related files.
✅ Verification successful
Message update is consistent across the users module
The message "Hello Users Module" is consistently used across all relevant files in the users module:
- apps/users/src/app/app.service.ts
- apps/users/src/app/app.service.spec.ts
- apps/users/src/app/app.controller.spec.ts
- apps/users-e2e/src/users/users.spec.ts
The gateway module uses its own distinct message ("Hello Gateway"), which is correct as it's a separate module.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining instances of the old message and verify the new message is used consistently
# Search for any remaining instances of the old message
echo "Checking for old message..."
rg -l "Hello User Service"
# Search for new message to ensure consistency
echo "Checking new message usage..."
rg -l "Hello Users Module"
Length of output: 213
Script:
#!/bin/bash
# Let's try a more targeted search with proper quoting and separate commands
# Search for old message variations
echo "Searching for old message variations..."
rg -l "Hello User Service"
rg -l "Hello Users Service"
# Search for new message
echo "Searching for new message..."
rg -l "Hello Users Module"
# Search in app service files specifically
echo "Checking app service files..."
fd "app\.(service|controller)\.(ts|js)" --exec rg -l "Hello"
Length of output: 601
Script:
#!/bin/bash
# Let's check the actual content of these files to verify the message consistency
echo "Content of app.service.ts:"
cat ./apps/users/src/app/app.service.ts
echo -e "\nContent of app.service.spec.ts:"
cat ./apps/users/src/app/app.service.spec.ts
echo -e "\nContent of app.controller.spec.ts:"
cat ./apps/users/src/app/app.controller.spec.ts
echo -e "\nContent of gateway app.service.ts:"
cat ./apps/gateway/src/app/app.service.ts
Length of output: 1974
apps/users/src/app/app.service.spec.ts (1)
17-18: LGTM! Test updates align with service changes.
The test description and assertion have been properly updated to reflect the new service response.
Let's verify the consistency of these changes across related files:
✅ Verification successful
✅ Test changes are consistent across the codebase
The verification confirms that:
- The new message "Hello Users Module" is consistently used across all related files:
- app.service.ts (implementation)
- app.service.spec.ts (unit test)
- app.controller.spec.ts (controller test)
- users.spec.ts (e2e test)
- No instances of the old message "Hello User Service" remain
- The service implementation correctly returns the updated message
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all related test files have been updated consistently
# Expected: All test assertions should expect "Hello Users Module"
echo "Checking for consistent test expectations across files..."
rg -l 'Hello Users Module' 'apps/**/*spec.ts'
echo "\nChecking for any remaining old expectations..."
rg -l 'Hello User Service' 'apps/**/*spec.ts'
echo "\nVerifying the actual service implementation..."
rg -A 1 'getData.*{' 'apps/users/src/app/app.service.ts'
Length of output: 689
Script:
#!/bin/bash
# Let's fix the syntax and search patterns
echo "Checking for consistent test expectations..."
rg "Hello Users Module" apps/
echo -e "\nChecking for any remaining old expectations..."
rg "Hello User Service" apps/
echo -e "\nVerifying the actual service implementation..."
rg "getData" apps/users/src/app/app.service.ts
Length of output: 1029
apps/users/src/app/app.controller.spec.ts (1)
17-17: LGTM! Test updates align with the module renaming.
The changes to the test description and expected message are consistent with the broader refactoring effort mentioned in the PR objectives. The test structure follows NestJS best practices.
Let's verify the consistency of this change across related files:
Also applies to: 20-20
✅ Verification successful
The verification results show that the old message "Hello User Service" is not found anywhere in the codebase, while the new message "Hello Users Module" is consistently used across all relevant files:
- Controller test file (the one under review)
- Service implementation file
- Service test file
- E2E test file
Let me verify the actual content of these files to ensure the message is used correctly in each context:
✅ Message update is consistent across all related files
The message "Hello Users Module" is correctly and consistently used across all relevant files:
- Service implementation returns the message
- Service test verifies the message
- Controller test (under review) verifies the message
- E2E test validates the message in API response
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the message has been updated consistently across all related files
# Expected: All occurrences should use "Hello Users Module"
echo "Checking for any inconsistent messages across the codebase:"
rg -l "Hello User Service"
echo "Verifying the new message is used consistently:"
rg -l "Hello Users Module"
Length of output: 445
Script:
#!/bin/bash
# Check the context of "Hello Users Module" usage in each file
echo "=== app.service.ts ==="
rg -C 2 "Hello Users Module" apps/users/src/app/app.service.ts
echo -e "\n=== app.service.spec.ts ==="
rg -C 2 "Hello Users Module" apps/users/src/app/app.service.spec.ts
echo -e "\n=== app.controller.spec.ts ==="
rg -C 2 "Hello Users Module" apps/users/src/app/app.controller.spec.ts
echo -e "\n=== users.spec.ts ==="
rg -C 2 "Hello Users Module" apps/users-e2e/src/users/users.spec.ts
Length of output: 1192
apps/users/src/users/users.module.ts (1)
9-9: Verify the new import path and architectural alignment.
The import path change from @users/presentation-resolver to @users/interface-adapters aligns with clean architecture principles, as resolvers act as interface adapters between the GraphQL layer and the application core.
Let's verify the new import path is correctly configured:
✅ Verification successful
Import path change is correctly configured and architecturally sound
The verification confirms:
- The resolver exists at the new location:
libs/users/interface-adapters/src/lib/resolver/users.resolver.ts - It's properly exported in the interface-adapters package's index.ts
- The
@users/interface-adapterspath alias is correctly configured in tsconfig - The resolver implementation follows clean architecture principles by:
- Using domain entities and DTOs for data transformation
- Depending on application layer services (
@users/application)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the new import path exists and is correctly configured
# Check if the resolver exists in the new location
fd -t f "users.resolver.ts" -x echo "Found resolver at: {}"
# Verify the exports in the interface-adapters package
fd -t f "index.ts" -p "**/users/interface-adapters/src" -x cat {}
# Check for any remaining references to the old import path
rg -l "@users/presentation-resolver"
Length of output: 369
Script:
#!/bin/bash
# Let's verify the exports and structure of the interface-adapters package
# Check the content of the resolver file
cat ./libs/users/interface-adapters/src/lib/resolver/users.resolver.ts
# Check for any index files that might be exporting the resolver
fd -t f "index.ts" -p "libs/users/interface-adapters" -x cat {}
# Check the tsconfig paths configuration for @users alias
fd -t f "tsconfig.*json" -x cat {} | grep -A 5 -B 5 "@users/"
Length of output: 1688
|
🎉 This PR is included in version 1.2.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Related: #47
Summary by CodeRabbit
Release Notes
New Features
servicelayer in the application architecture.users-interface-adapterslibrary with testing instructions.Bug Fixes
Documentation
users-presentation-dtoandusers-presentation-resolverlibraries.Chores
users-presentation-dtoandusers-presentation-resolverlibraries.