-
Notifications
You must be signed in to change notification settings - Fork 6
Layered Phase 2 - Comments #169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…typo in feed summary
…s and design principles
…ethods - Updated article response DTO to encapsulate article details within an 'article' object. - Refactored articles service methods to improve type handling and ensure consistent article data mapping. - Removed unused query parameters in article fetching logic for cleaner service calls. - Enhanced error handling in article creation and update methods to ensure articles are found post-operation. - Cleaned up comments and organized imports for better code readability.
- Introduced a new mapping function `toFeedResponse` for transforming article data into the feed response format. - Updated the articles controller to utilize `toFeedResponse` for article data mapping. - Modified DTO exports to include `article-feed-response.dto` and reorganized existing exports for clarity. - Changed the author type in the article feed interface to `ProfileFeed` for better alignment with the new response structure.
…handling - Replaced `articlesPlugin` with `articlesController` in the app module for improved structure. - Updated article schema imports across various files to maintain consistency. - Refactored `CommentsRepository` to use a new `CreateCommentDto` type for better clarity and type safety. - Cleaned up unused imports and comments for enhanced code readability.
- Modified summary descriptions in the articles controller for clarity, changing 'Article Feed' to 'Feed Articles', 'Add Comment to Article' to 'Add Comments to an Article', and 'Get Comments from Article' to 'Get Comments from an Article'. - Updated the tags plugin summary from 'List Tags' to 'Get Tags' for consistency. - Adjusted user plugin summaries, changing 'Register' to 'Registeration', 'Login' to 'Authentication', and 'Current User' to 'Get Current User' for improved clarity.
- Moved the article creation endpoint to a new position within the articles controller for improved readability and structure. - Ensured the endpoint retains its functionality, including user authentication and response formatting. - Updated the endpoint's summary details to maintain consistency with other routes.
- Replaced `ListArticlesQueryDto` with `ArticleFeedQueryDto` in the articles controller to enhance query handling for the article feed. - Added a detailed description for the feed articles endpoint, specifying additional query parameters. - Removed the now obsolete `article-feed-response.dto.ts` file to streamline the DTO structure. - Updated DTO exports in the index file to reflect the changes.
- Added a detailed description to the 'List Articles' endpoint, outlining the default behavior and available query parameters for filtering results. - Clarified that authentication is optional and specified the ordering of returned articles.
…entation - Added commentsController to the app module for managing comments functionality. - Updated commentsController to include detailed descriptions for API endpoints, clarifying authentication requirements for adding, retrieving, and deleting comments.
- Updated CommentsRepository and CommentsService to use NewCommentRow interface instead of deprecated CommentToCreate type. - Introduced a new mapper function, toNewCommentRow, to streamline the transformation of comment data. - Enhanced the interfaces index to export the new comment row interface for better organization.
- Removed the comments.mapper file and consolidated mapping functions into a new index file for better organization. - Updated commentsController to utilize the new toCommentsResponse function for improved response handling. - Adjusted imports in comments.service.ts to reflect the new structure, enhancing maintainability.
- Streamlined the import statements in articles.controller.ts and articles.service.ts to utilize a centralized mappers index file, enhancing code organization and maintainability.
- Introduced a new toDomain mapper function to streamline the transformation of comment data in CommentsService. - Updated the comments.service.ts to utilize the new toDomain function for improved code clarity and maintainability. - Adjusted the mappers index file to include the new toDomain export, ensuring better organization of mapping functions.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis change refactors the comments feature by extracting all comment-related logic, routes, services, and mappers from the articles module into a dedicated comments module and controller. The articles controller and module no longer handle comments. New DTOs, mappers, and interfaces for comments are introduced and restructured. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API (commentsController)
participant AuthService
participant CommentsService
participant ProfilesService
participant Database
Client->>API (commentsController): POST /articles/:slug/comments (with auth)
API->>AuthService: Validate user from header
API->>CommentsService: createComment(slug, comment, userId)
CommentsService->>Database: Insert new comment row
CommentsService->>ProfilesService: Get author profile
CommentsService->>API: Return created comment (mapped)
API->>Client: Respond with CommentResponseDto
Client->>API (commentsController): GET /articles/:slug/comments (optional auth)
API->>CommentsService: getComments(slug, [userId])
CommentsService->>Database: Query comments for article
CommentsService->>ProfilesService: Get authors' profiles
CommentsService->>API: Return comments (mapped)
API->>Client: Respond with CommentsResponseDto
Client->>API (commentsController): DELETE /articles/:slug/comments/:id (with auth)
API->>AuthService: Validate user from header
API->>CommentsService: deleteComment(slug, commentId, userId)
CommentsService->>Database: Delete comment if authorized
API->>Client: Respond with 204 No Content
Possibly related PRs
Suggested labels
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
src/comments/comments.repository.ts (1)
60-75: 🛠️ Refactor suggestionArchitectural concern: Article query in comments repository.
The
findBySlugmethod queries thearticlestable, which violates the separation of concerns principle. A comments repository should not be responsible for article-related operations.Consider one of these approaches:
- Move this method to the articles repository where it belongs
- Remove it if it's no longer needed after the refactoring
- If comments need article data, inject the articles repository as a dependency
- async findBySlug(slug: string): Promise<ArticleRow | null> { - const result = await this.db.query.articles.findFirst({ - where: eq(articles.slug, slug), - with: { - author: { - with: { - followers: true, - }, - }, - favoritedBy: true, - tags: true, - }, - }); - - return result ?? null; - }src/comments/comments.schema.ts (1)
9-14:⚠️ Potential issueUse integer() instead of serial() for foreign key columns.
Foreign key columns should use
integer()rather thanserial(). Serial creates auto-incrementing columns, which is inappropriate for foreign keys that should reference existing primary keys.- articleId: serial('article_id') + articleId: integer('article_id') .notNull() .references(() => articles.id, { onDelete: 'cascade' }), - authorId: serial('author_id') + authorId: integer('author_id') .notNull() .references(() => users.id, { onDelete: 'cascade' }),
🧹 Nitpick comments (8)
src/articles/mappers/to-new-article-row.mapper.ts (1)
1-13: Mapper implementation solid, but cover slug edge cases with tests
ThetoNewArticleRowfunction correctly mapsCreateArticleInputtoNewArticleRowand generates slugs viaslugify. Consider adding unit tests for edge cases (e.g., special characters, duplicate titles) to validate slug behavior.src/comments/interfaces/comment-row.interface.ts (1)
1-8: EnsureCommentRowaligns with persisted record shape
Verify thatCommentRowaccurately represents the stored comment schema (types and field names). If the comment structure grows, consider extracting shared fields into a base interface to reduce duplication.src/articles/mappers/to-feed-domain.mapper.ts (1)
3-15: Consider simplifying the mapper implementation.The current implementation explicitly maps each property, but since
ArticleFeedRowandIArticleFeedhave identical structures, this could be simplified.-export function toFeedDomain(article: ArticleFeedRow): IArticleFeed { - return { - slug: article.slug, - title: article.title, - description: article.description, - tagList: article.tagList, - createdAt: article.createdAt, - updatedAt: article.updatedAt, - favorited: article.favorited, - favoritesCount: article.favoritesCount, - author: article.author, - }; -} +export function toFeedDomain(article: ArticleFeedRow): IArticleFeed { + return { ...article }; +}This approach is more concise and automatically adapts if properties are added to both types. However, the explicit mapping provides better clarity about the transformation, so both approaches are valid depending on team preferences.
src/comments/mappers/to-comments-response.mapper.ts (1)
4-6: Consider defining a dedicated interface for better type clarity.The current return type using
ReturnType<typeof toCommentResponse>['comment'][]is verbose and reduces readability. Consider defining a dedicated interface for the response.+import type { CommentsResponseDto } from '../dto/comments-response.dto'; + -export function toCommentsResponse(comments: IComment[]): { - comments: ReturnType<typeof toCommentResponse>['comment'][]; -} { +export function toCommentsResponse(comments: IComment[]): CommentsResponseDto {src/comments/mappers/to-new-comment-row.mapper.ts (1)
3-7: Consider simplifying the body parameter structure.The current parameter
body: { body: string }followed by accessingbody.bodycreates unnecessary nesting. If this structure isn't required by the calling context, consider simplifying.-export function toNewCommentRow( - body: { body: string }, - articleId: number, - authorId: number, -): NewCommentRow { +export function toNewCommentRow( + body: string, + articleId: number, + authorId: number, +): NewCommentRow { return { - body: body.body, + body, articleId, authorId, }; }However, if the nested structure aligns with your DTO design, the current implementation is correct.
src/comments/comments.module.ts (1)
9-22: Consider adding explicit return type for better type safety.The
setupCommentsfunction would benefit from an explicit return type annotation to improve type safety and developer experience.-export const setupComments = () => { +export const setupComments = (): Elysia<'', { commentsService: CommentsService; authService: AuthService }> => {src/comments/mappers/to-domain.mapper.ts (1)
4-8: Consider extracting the author type if used elsewhere.The
ToCommentsDomainAuthortype is well-defined but consider moving it to a shared interfaces file if this pattern is used in other mappers.src/comments/comments.controller.ts (1)
47-53: Consider simplifying the userId null handling.The null-to-undefined conversion may be unnecessary depending on how
commentsService.getCommentshandles null values. Consider checking if the service can acceptnulldirectly to simplify the code.- const userId = await store.authService.getOptionalUserIdFromHeader( - request.headers, - ); - const comments = await store.commentsService.getComments( - params.slug, - userId === null ? undefined : userId, - ); + const userId = await store.authService.getOptionalUserIdFromHeader( + request.headers, + ); + const comments = await store.commentsService.getComments( + params.slug, + userId, + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
src/app.module.ts(2 hunks)src/articles/articles.controller.ts(1 hunks)src/articles/articles.module.ts(0 hunks)src/articles/articles.schema.ts(1 hunks)src/articles/articles.service.ts(1 hunks)src/articles/mappers/articles.mapper.ts(0 hunks)src/articles/mappers/index.ts(1 hunks)src/articles/mappers/to-create-article-input.mapper.ts(1 hunks)src/articles/mappers/to-domain.mapper.ts(1 hunks)src/articles/mappers/to-feed-domain.mapper.ts(1 hunks)src/articles/mappers/to-feed-response.mapper.ts(1 hunks)src/articles/mappers/to-new-article-row.mapper.ts(1 hunks)src/articles/mappers/to-response.mapper.ts(1 hunks)src/comments/comments.controller.ts(1 hunks)src/comments/comments.module.ts(1 hunks)src/comments/comments.repository.ts(1 hunks)src/comments/comments.schema.ts(2 hunks)src/comments/comments.service.ts(3 hunks)src/comments/dto/comment-author.dto.ts(0 hunks)src/comments/dto/comment-response.dto.ts(1 hunks)src/comments/dto/comments-response.dto.ts(1 hunks)src/comments/dto/index.ts(1 hunks)src/comments/interfaces/comment-row.interface.ts(1 hunks)src/comments/interfaces/index.ts(1 hunks)src/comments/interfaces/new-comment-row.interface.ts(1 hunks)src/comments/mappers/index.ts(1 hunks)src/comments/mappers/to-comment-response.mapper.ts(1 hunks)src/comments/mappers/to-comments-response.mapper.ts(1 hunks)src/comments/mappers/to-domain.mapper.ts(1 hunks)src/comments/mappers/to-new-comment-row.mapper.ts(1 hunks)src/comments/schema/comment-relations.schema.ts(0 hunks)src/comments/schema/index.ts(0 hunks)src/database.providers.ts(1 hunks)
💤 Files with no reviewable changes (5)
- src/comments/schema/index.ts
- src/articles/articles.module.ts
- src/comments/schema/comment-relations.schema.ts
- src/comments/dto/comment-author.dto.ts
- src/articles/mappers/articles.mapper.ts
🧰 Additional context used
🧬 Code Graph Analysis (15)
src/app.module.ts (1)
src/comments/comments.controller.ts (1)
commentsController(10-90)
src/articles/mappers/to-new-article-row.mapper.ts (3)
src/articles/interfaces/create-article-input.interface.ts (1)
CreateArticleInput(1-6)src/articles/interfaces/new-article-row.interface.ts (1)
NewArticleRow(4-4)src/utils/slugify.ts (1)
slugify(26-39)
src/articles/mappers/to-feed-response.mapper.ts (1)
src/articles/interfaces/article-feed.interface.ts (1)
IArticleFeed(3-13)
src/articles/mappers/to-feed-domain.mapper.ts (2)
src/articles/interfaces/article-feed-row.interface.ts (1)
ArticleFeedRow(3-13)src/articles/interfaces/article-feed.interface.ts (1)
IArticleFeed(3-13)
src/comments/mappers/to-comments-response.mapper.ts (2)
src/comments/interfaces/comment.interface.ts (1)
IComment(1-12)src/comments/mappers/to-comment-response.mapper.ts (1)
toCommentResponse(4-17)
src/articles/mappers/to-response.mapper.ts (1)
src/articles/interfaces/article.interface.ts (1)
IArticle(3-21)
src/comments/comments.repository.ts (2)
src/database.providers.ts (2)
db(13-21)Database(22-22)src/comments/interfaces/new-comment-row.interface.ts (1)
NewCommentRow(1-5)
src/comments/comments.schema.ts (2)
src/articles/articles.schema.ts (1)
articles(14-25)src/users/users.model.ts (1)
users(12-23)
src/comments/mappers/to-new-comment-row.mapper.ts (1)
src/comments/interfaces/new-comment-row.interface.ts (1)
NewCommentRow(1-5)
src/comments/mappers/to-comment-response.mapper.ts (1)
src/comments/interfaces/comment.interface.ts (1)
IComment(1-12)
src/comments/comments.module.ts (6)
src/comments/comments.repository.ts (1)
CommentsRepository(8-82)src/database.providers.ts (1)
db(13-21)src/profiles/profiles.repository.ts (1)
ProfilesRepository(5-76)src/profiles/profiles.service.ts (1)
ProfilesService(6-90)src/comments/comments.service.ts (1)
CommentsService(8-110)src/auth/auth.service.ts (1)
AuthService(8-104)
src/articles/mappers/to-create-article-input.mapper.ts (1)
src/articles/interfaces/create-article-input.interface.ts (1)
CreateArticleInput(1-6)
src/comments/mappers/to-domain.mapper.ts (2)
src/comments/interfaces/comment-row.interface.ts (1)
CommentRow(1-8)src/comments/interfaces/comment.interface.ts (1)
IComment(1-12)
src/articles/mappers/to-domain.mapper.ts (2)
src/articles/interfaces/article-row.interface.ts (1)
ArticleRow(8-13)src/articles/interfaces/article.interface.ts (1)
IArticle(3-21)
src/comments/comments.controller.ts (6)
src/comments/comments.module.ts (1)
setupComments(9-22)src/comments/mappers/to-comment-response.mapper.ts (1)
toCommentResponse(4-17)src/comments/dto/comment-response.dto.ts (2)
CommentResponseDto(3-16)CommentResponseDto(18-18)src/comments/comments.schema.ts (1)
comments(6-17)src/comments/mappers/to-comments-response.mapper.ts (1)
toCommentsResponse(4-8)src/comments/dto/comments-response.dto.ts (2)
CommentsResponseDto(4-6)CommentsResponseDto(7-7)
🔇 Additional comments (30)
src/articles/articles.schema.ts (1)
2-2: Correct import path for comments schemaThis change updates the import to match the new comments module structure and ensures the
commentstable is referenced correctly in article relations.src/database.providers.ts (1)
2-2: Align comments schema import in database providerThe import has been updated to reflect the relocated comments schema (
@comments/comments.schema), keeping the database schema registration consistent.src/app.module.ts (2)
14-14: Register comments controller in app moduleAdding the
commentsControllerimport wires in the new comments routes alongside other feature controllers. Ensure the path and named export match the controller file.
64-64: Mount comments controller in API groupInserting
.use(commentsController)under/apiintegrates comment endpoints with the existing middleware chain. Ordering with related controllers is consistent.src/articles/articles.service.ts (1)
13-13: Consolidate mapper imports via indexSwitching to the barrel import (
./mappers) improves modularity and matches the new file structure. Verify thatindex.tsin the mappers folder exports all referenced functions.src/comments/interfaces/index.ts (1)
2-3: Export new comment row interfacesRe-exporting
comment-row.interfaceandnew-comment-row.interfacealongsidecomment.interfacecentralizes type definitions for the comments module.src/comments/dto/index.ts (1)
3-3: Expose consolidatedCommentsResponseDtoin DTO barrel
Addingcomments-response.dtoaligns with the new response schema consolidation. Ensure that all imports ofCommentsResponseDtouse this barrel export in controllers and services.src/comments/interfaces/new-comment-row.interface.ts (1)
1-5: ValidateNewCommentRowagainst database schema
Confirm thatNewCommentRowmatches the database insert model (e.g., usingInferInsertModel<typeof comments>) to avoid mismatches during persistence.src/comments/dto/comments-response.dto.ts (1)
1-7:CommentsResponseDtoschema looks correct
Definingcomments: Type.Array(CommentResponseDto.properties.comment)ensures consistency with individual comment DTOs.src/comments/mappers/to-comment-response.mapper.ts (1)
4-17: Well-structured mapper implementation.The mapper correctly transforms domain objects to API response format, including proper date serialization to ISO strings. The wrapping of the result in a
commentproperty follows common REST API conventions.src/comments/comments.repository.ts (2)
5-6: Import and type changes look good.The migration from absolute to relative imports and the updated type from
CreateCommentDtotoNewCommentRowaligns well with the refactoring objectives.
11-11: Parameter type update is consistent with refactoring.The change from
CreateCommentDtotoNewCommentRowmaintains consistency with the new interface structure defined in the comments module.src/articles/articles.controller.ts (2)
11-11: Clean separation of concerns achieved.The updated imports reflect the new modular mapper structure and the removal of comment-related functionality. This aligns well with the refactoring objectives to separate article and comment concerns.
13-218: Articles controller now properly focused.The removal of comment-related routes successfully separates article and comment concerns. The controller now handles only article operations, which improves maintainability and follows single responsibility principle.
src/comments/mappers/to-comments-response.mapper.ts (1)
7-7: LGTM! Clean and efficient implementation.The mapping logic correctly transforms the array and extracts the nested comment property from each mapped result.
src/comments/comments.schema.ts (1)
19-30: LGTM! Proper relational mappings defined.The comment relations are correctly defined with appropriate one-to-one relationships to articles and users, including descriptive relation names.
src/articles/mappers/to-create-article-input.mapper.ts (1)
4-13: LGTM! Clean mapper with proper null handling.The implementation correctly maps the DTO structure to the input interface and provides a sensible default for the optional
tagListfield.src/comments/mappers/index.ts (1)
1-4: LGTM! Clean modular architecture.The index file properly consolidates mapper exports, creating a single entry point for comment mappers. This modular approach aligns well with the layered architecture goals and improves maintainability.
src/articles/mappers/to-response.mapper.ts (1)
4-25: Well-structured mapper function.The
toResponsefunction correctly transforms the domain model to DTO format. The author profile extraction is clean, and the date-to-ISO string conversion is appropriate for API responses. Good separation of concerns and type safety.src/articles/mappers/index.ts (1)
1-6: Excellent modularization approach.The index file creates a clean entry point for all article mappers, replacing the monolithic structure. This approach is consistent with the comments mappers architecture and improves maintainability through better separation of concerns.
src/comments/dto/comment-response.dto.ts (1)
9-14: Good simplification of DTO structure.The inline author definition eliminates the external
CommentAuthorDtodependency while maintaining the same API contract. All necessary fields are properly typed, and this approach simplifies the overall DTO architecture.src/comments/comments.module.ts (1)
10-17: Well-structured dependency injection setup.The manual dependency injection pattern is clean and follows good practices. The service instantiation order correctly handles dependencies.
src/comments/mappers/to-domain.mapper.ts (1)
17-34: Excellent mapper implementation with clear documentation.The function signature is type-safe, the JSDoc is comprehensive, and the mapping logic correctly transforms all required fields from the database row to the domain model.
src/articles/mappers/to-feed-response.mapper.ts (1)
4-23: Clean and well-implemented mapper function.The function correctly handles date conversion to ISO strings for API responses and properly uses the nullish coalescing operator for the articlesCount fallback. The mapping logic is straightforward and type-safe.
src/comments/comments.service.ts (3)
5-6: Good import organization for the new mappers and interfaces.The import statements are clean and well-organized, bringing in the necessary types and mapper functions for the refactored service.
25-41: Excellent use of mapper functions in createComment.The refactoring successfully replaces manual object construction with the
toNewCommentRowandtoDomainmappers, making the code more maintainable and consistent. The comment about the author always being the current user is helpful context.
64-76: Clean refactoring with toDomain mapper in getComments.The use of the
toDomainmapper in the map operation is well-implemented. The author object construction inline is appropriate here since it's extracting specific fields from the joined comment data.src/articles/mappers/to-domain.mapper.ts (1)
1-34: LGTM! Well-implemented domain mapper.The mapper function correctly transforms
ArticleRowtoIArticlewith appropriate handling of optional fields and computed properties. The use offind()for checking relationships and nullish coalescing fortagListdemonstrates good defensive programming practices.src/comments/comments.controller.ts (2)
69-69: Good practice using parseInt with radix.The explicit radix parameter in
Number.parseInt(params.id, 10)is excellent practice for preventing potential issues with number parsing.
30-30: Proper authentication implementation.The controller correctly applies authentication middleware to routes that modify data (POST/DELETE) while leaving the read-only GET route with optional authentication. This follows REST security best practices.
Also applies to: 74-74
- Updated import statements in articles.schema.ts, articles.service.ts, article-row.interface.ts, article.interface.ts, and to-new-article-row.mapper.ts to use absolute paths for better consistency and maintainability. - Enhanced code organization by ensuring uniformity in import paths throughout the articles module.
…ration - Integrated ArticlesService and TagsService into CommentsService for improved comment handling related to articles and tags. - Removed the deprecated findBySlug method from CommentsRepository, streamlining the repository's functionality. - Updated setupComments function to instantiate and utilize the new services, enhancing modularity and maintainability.
- Modified the deleteComment method to accept the comment ID as a string instead of parsing it to a number, simplifying the parameter handling. - Updated the parameter validation in commentsController to enforce numeric type for the comment ID, enhancing data integrity and validation consistency.
- Introduced a separate count query to improve performance when filtering articles by tags and favorited status. - Consolidated filtering conditions for tags and authors into reusable variables, enhancing code readability and maintainability. - Updated the base query to utilize the new count query for accurate article counts based on applied filters.
- Replaced the use of find with some in the toDomain mapper for articles and profiles, improving performance and readability. - Updated favorited and following properties to utilize more efficient array methods, enhancing code clarity.
- Changed articleId and authorId fields in comments schema from serial to integer type for improved data integrity and consistency. - Updated import statements to include the integer type from drizzle-orm/pg-core, enhancing schema definition clarity.
… layered-phase2-comments
aruaycodes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
honestly, the code was so well-written, that I should use it as a bible when i do my own project.
Description
PR Checklist
bun docsSummary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores