[feature] Get Posts/Post #59
Conversation
|
Warning Rate limit exceeded@gocanto has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 21 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThis change introduces a database-backed posts API endpoint, including repository, query filters, pagination utilities, handler logic, and response transformation. The router is updated to support the new posts handler, and supporting structs, methods, and utility functions are added for filtering, pagination, and response formatting. Minor Makefile and dependency updates are included. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Router
participant PostsHandler
participant PostsRepository
participant DB
Client->>Router: HTTP GET /posts
Router->>PostsHandler: Handle(request, response)
PostsHandler->>PostsRepository: GetPosts(filters, paginate)
PostsRepository->>DB: Query posts with filters, joins, pagination
DB-->>PostsRepository: Posts data
PostsRepository-->>PostsHandler: Paginated posts
PostsHandler->>PostsHandler: Transform posts to response format
PostsHandler-->>Router: Write JSON response
Router-->>Client: HTTP Response (paginated posts)
Estimated code review effort4 (~90 minutes) Possibly related PRs
Poem
✨ Finishing Touches
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 (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @gocanto, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request initiates the development of query functionality, specifically focusing on 'posts'. It establishes the necessary database integration within the application's routing layer, introduces a flexible and robust system for filtering and paginating post data, and sets up the foundational API endpoint for accessing this information. These changes are crucial for building out the application's data retrieval capabilities.
Highlights
- Database Integration: The
Routerstruct inboost/router.gohas been updated to include aDb *database.Connectionfield, and this database connection is now passed to the router during application initialization inboost/app.go. This change enables routing and handler logic to directly access the database. - Posts Querying Infrastructure: New structures
PostFiltersandPaginatedResult[T any]have been introduced indatabase/repository/posts.goanddatabase/repository/pagination.gorespectively. A comprehensiveGetPostsmethod has been added torepository.Posts, allowing for advanced filtering (by UUID, slug, title, author, category, tag) and robust pagination logic, including total record counts and next/previous page calculations. - New API Endpoint Setup: A new
/postsAPI endpoint is now registered within theRoutervia a newPosts()method. This endpoint is handled by a newly introducedPostsHandler(inhandler/posts.go), which leverages therepository.Poststo fetch data using the new filtering and pagination capabilities. - Development Tooling Update: The
watchcommand inconfig/makefile/app.mkhas been updated to run theairlive-reloading tool with the-d(debug) flag, which might provide more verbose output or enable specific debug features during development.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request starts the implementation of a query endpoint for posts. It introduces a new handler, repository methods for querying posts with filters and pagination, and related data structures. The review identifies a few critical issues, such as a compile error and use of log.Fatalf that would crash the server, along with several medium-severity suggestions to improve code clarity, fix bugs, and complete the implementation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
♻️ Duplicate comments (6)
boost/router.go (1)
34-43: Address handler signature compatibility and add HTTP method specification.There are two concerns with this implementation:
Handler signature compatibility: Based on past review comments, there may be a signature mismatch between the handler's
Handlemethod and the expectedhttp.ApiHandlerinterface.Missing HTTP method specification: Unlike other routes that specify "GET /path", this route only specifies "/posts", which will match all HTTP methods.
Consider these fixes:
func (r *Router) Posts() { repo := repository.Posts{DB: r.Db} abstract := handler.MakePostsHandler(&repo) resolver := r.PipelineFor( abstract.Handle, ) - r.Mux.HandleFunc("/posts", resolver) + r.Mux.HandleFunc("GET /posts", resolver) }Please verify that the handler's
Handlemethod signature is compatible withhttp.ApiHandler.database/repository/posts.go (1)
28-28: Implement missing IsPublished filter logic.The IsPublished filter is defined in PostFilters but not implemented in ApplyPostsFilters function, leading to incomplete filtering functionality.
This matches the previous review comment about the incomplete implementation. The ApplyPostsFilters function in
database/repository/queries/posts.goneeds to handle the IsPublished filter.database/repository/pagination.go (1)
10-19: Remove redundant Page field as noted in previous review.The
Pagefield duplicates the functionality ofCurrentPage, creating confusion and potential inconsistency in the API response.As mentioned in the previous review, remove the
Pagefield and use onlyCurrentPage. This will require updating the usage indatabase/repository/posts.goaccordingly.type Pagination[T any] struct { Data []T `json:"data"` - Page int `json:"page"` Total int64 `json:"total"` CurrentPage int `json:"current_page"` PageSize int `json:"page_size"` TotalPages int `json:"total_pages"` NextPage *int `json:"next_page,omitempty"` PreviousPage *int `json:"previous_page,omitempty"` }database/repository/queries/posts.go (1)
7-49: Implement missing IsPublished filter logic.The
IsPublishedfilter defined inPostFiltersis not implemented in this function, making the filtering incomplete. A post should be considered published ifPublishedAtis not NULL and is in the past.Add the missing implementation:
func ApplyPostsFilters(filters *PostFilters, query *gorm.DB) { if filters == nil { return } + if filters.IsPublished != nil { + if *filters.IsPublished { + query.Where("posts.published_at IS NOT NULL AND posts.published_at <= NOW()") + } else { + query.Where("posts.published_at IS NULL OR posts.published_at > NOW()") + } + } + // ... rest of the filters }handler/posts.go (1)
25-29: Address hardcoded parameters by parsing query parameters.The filter and pagination parameters are still hardcoded as noted in previous reviews. These values should be extracted from the HTTP request's query parameters to make the endpoint functional.
Consider implementing query parameter parsing:
func (h *PostsHandler) Handle(w baseHttp.ResponseWriter, r *baseHttp.Request) *http.ApiError { - filters := queries.PostFilters{Title: ""} - pagination := repository.Pagination[database.Post]{ - Page: 1, - PageSize: 10, - } + filters := queries.PostFilters{ + Title: r.URL.Query().Get("title"), + } + + page := 1 + if p := r.URL.Query().Get("page"); p != "" { + if parsed, err := strconv.Atoi(p); err == nil && parsed > 0 { + page = parsed + } + } + + pageSize := 10 + if ps := r.URL.Query().Get("page_size"); ps != "" { + if parsed, err := strconv.Atoi(ps); err == nil && parsed > 0 && parsed <= 100 { + pageSize = parsed + } + } + + pagination := repository.Pagination[database.Post]{ + Page: page, + PageSize: pageSize, + }handler/posts/transformer.go (1)
5-32: Fix field mapping bug in author transformation.There's a critical bug on line 29 where
p.UpdatedAtis used instead ofp.Author.UpdatedAtfor the author's UpdatedAt field.This issue was also identified in the handler file. The same fix applies here:
Author: UserData{ UUID: p.Author.UUID, FirstName: p.Author.FirstName, LastName: p.Author.LastName, Username: p.Author.Username, DisplayName: p.Author.DisplayName, Bio: p.Author.Bio, PictureFileName: p.Author.PictureFileName, ProfilePictureURL: p.Author.ProfilePictureURL, IsAdmin: p.Author.IsAdmin, CreatedAt: p.Author.CreatedAt, - UpdatedAt: p.UpdatedAt, + UpdatedAt: p.Author.UpdatedAt, },
🧹 Nitpick comments (5)
boost/router.go (1)
35-35: Consider using constructor pattern for repository.The repository is initialized directly instead of using a constructor. For consistency with the handler pattern, consider adding a repository constructor.
- repo := repository.Posts{DB: r.Db} + repo := repository.MakePosts(r.Db)database/repository/posts.go (1)
35-38: Consider performance implications of separate count query.The current approach executes two separate queries (count + data fetch). For better performance, consider implementing a single query approach or ensure proper indexing on filtered columns.
For high-traffic scenarios, consider:
- Caching count results when filters are static
- Adding database indexes on commonly filtered columns (title, published_at, author_id)
- Using window functions for count if the database supports it
handler/posts/response.go (2)
7-22: Consider adding omitempty tags for optional fields.Some fields like
Excerpt,Content, andCoverImageURLmight be empty and should useomitemptyin their JSON tags to avoid sending empty strings in responses.type PostResponse struct { UUID string `json:"uuid"` Author UserData `json:"author"` Slug string `json:"slug"` Title string `json:"title"` - Excerpt string `json:"excerpt"` - Content string `json:"content"` - CoverImageURL string `json:"cover_image_url"` + Excerpt string `json:"excerpt,omitempty"` + Content string `json:"content,omitempty"` + CoverImageURL string `json:"cover_image_url,omitempty"` PublishedAt *time.Time `json:"published_at"` CreatedAt time.Time `json:"created_at"` UpdatedAt time.Time `json:"updated_at"` // Associations Categories []CategoryData `json:"categories"` Tags []TagData `json:"tags"` }
24-36: Add omitempty tags for optional user fields.Fields like
Bio,PictureFileName, andProfilePictureURLare likely optional and should includeomitemptytags.type UserData struct { UUID string `json:"uuid"` FirstName string `json:"first_name"` LastName string `json:"last_name"` Username string `json:"username"` DisplayName string `json:"display_name"` - Bio string `json:"bio"` - PictureFileName string `json:"picture_file_name"` - ProfilePictureURL string `json:"profile_picture_url"` + Bio string `json:"bio,omitempty"` + PictureFileName string `json:"picture_file_name,omitempty"` + ProfilePictureURL string `json:"profile_picture_url,omitempty"` IsAdmin bool `json:"is_admin"` CreatedAt time.Time `json:"created_at"` UpdatedAt time.Time `json:"updated_at"` }handler/posts/transformer.go (1)
34-66: Consider optimizing slice allocation for better performance.Both
MapCategoriesandMapTagsfunctions could benefit from pre-allocating slices with known capacity to avoid multiple memory reallocations during append operations.func MapCategories(categories []database.Category) []CategoryData { - var data []CategoryData + data := make([]CategoryData, 0, len(categories)) func MapTags(tags []database.Tag) []TagData { - var data []TagData + data := make([]TagData, 0, len(tags))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
boost/app.go(2 hunks)boost/router.go(3 hunks)config/makefile/app.mk(1 hunks)database/connection.go(1 hunks)database/repository/pagination.go(1 hunks)database/repository/posts.go(2 hunks)database/repository/queries/filters.go(1 hunks)database/repository/queries/posts.go(1 hunks)go.mod(1 hunks)handler/posts.go(1 hunks)handler/posts/response.go(1 hunks)handler/posts/transformer.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
boost/app.go (1)
database/repository/posts.go (1)
Posts(11-15)
boost/router.go (3)
database/connection.go (1)
Connection(12-17)database/repository/posts.go (1)
Posts(11-15)handler/posts.go (1)
MakePostsHandler(18-22)
handler/posts/response.go (3)
database/repository/categories.go (1)
Categories(11-13)database/repository/tags.go (1)
Tags(13-15)database/repository/posts.go (1)
Posts(11-15)
handler/posts/transformer.go (2)
handler/posts/response.go (4)
PostResponse(7-22)UserData(24-36)CategoryData(38-45)TagData(47-57)database/model.go (2)
Category(86-98)Tag(107-119)
database/repository/queries/posts.go (1)
database/repository/queries/filters.go (1)
PostFilters(9-16)
database/repository/queries/filters.go (1)
database/model.go (2)
Category(86-98)Tag(107-119)
🔇 Additional comments (19)
config/makefile/app.mk (1)
28-28: Clarify the purpose of the-dflag addition.What specific functionality does the
-dflag enable for theaircommand? Please document this change in the comment above or in a commit message to help other developers understand the modification.go.mod (1)
13-13: Good practice moving indirect dependency to direct.This correctly reflects the direct usage of
golang.org/x/textin the new filtering functionality.boost/app.go (2)
45-45: Good integration of database connection.The database connection is properly assigned to the router, following the established dependency injection pattern.
73-73: Consistent route registration pattern.The posts route follows the same initialization pattern as other routes in the Boot method.
boost/router.go (2)
4-5: Good addition of required imports.The new imports for database and repository packages are appropriately added.
17-17: Database field properly integrated.The database connection field follows the existing struct pattern and enables repository access.
database/repository/posts.go (1)
7-7: LGTM! Import addition is appropriate.The new import for the queries package is necessary for the ApplyPostsFilters function usage.
database/repository/pagination.go (2)
70-87: LGTM! Excellent generic transformation utility.The
MapPaginatedResultfunction is well-designed with clear documentation and proper type safety. The implementation correctly preserves all pagination metadata while transforming the data.
78-86: MapPaginatedResult is safe to keep as-is
MapPaginatedResult only referencesCurrentPage(notPage), so removing the redundantPagefield fromPaginationwill not break this function. No changes required here.database/repository/queries/filters.go (2)
9-16: LGTM! Well-designed filter structure.The
PostFiltersstruct is well-designed with appropriate field types. The use of*boolforIsPublishedcorrectly allows for three states (true/false/nil), which is ideal for filtering logic.
18-42: Excellent sanitization approach with proper localization.The getter methods provide consistent sanitization using locale-aware case conversion and trimming. This ensures reliable case-insensitive filtering across the application.
database/repository/queries/posts.go (2)
7-11: LGTM! Proper null check for defensive programming.The null check for filters prevents potential nil pointer dereferences and handles the case where no filters are applied gracefully.
12-14: LGTM! Correct implementation of title filtering.The title filter correctly uses case-insensitive partial matching with proper parameterization to prevent SQL injection.
handler/posts.go (4)
1-12: LGTM: Clean package structure and imports.The package declaration and imports are well-organized with appropriate aliasing for the
baseHttppackage to avoid conflicts.
14-22: LGTM: Well-structured handler with dependency injection.The
PostsHandlerstruct and constructor follow good dependency injection patterns, making the code testable and maintainable.
33-37: LGTM: Improved error handling.Good improvement from the previous
log.Fatalfusage. The current implementation properly logs the error and returns an API error response without terminating the server.
39-42: LGTM: Clean data transformation.The use of
MapPaginatedResultwith the transformer function provides a clean separation of concerns for data transformation.handler/posts/transformer.go (2)
1-4: LGTM: Clean package structure.The package declaration and minimal imports are appropriate for this transformer module.
34-49: LGTM: Correct category mapping implementation.The
MapCategoriesfunction correctly transforms database categories to API response format with proper field mappings.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
handler/posts/transformer.go (1)
59-59: Fix misleading variable name in MapTags function.The loop variable is named
categorybut should betagsince it's iterating over tags.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
database/repository/pagination.go(1 hunks)database/repository/posts.go(2 hunks)handler/posts.go(1 hunks)handler/posts/transformer.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- database/repository/posts.go
- database/repository/pagination.go
- handler/posts.go
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
handler/posts/transformer.go (3)
34-34: Fix incorrect field mapping in Author data.The Author's
UpdatedAtfield is incorrectly using the post'sUpdatedAtvalue instead of the author's.
59-59: Fix misleading variable name in MapTags function.The loop variable is named
categorybut should betagsince it's iterating over tags. This could lead to confusion during maintenance.
77-91: Add validation for minimum pagination values.The function should validate that page and pageSize are positive values to prevent potential issues with zero or negative pagination parameters.
database/repository/pagination.go (2)
32-32: Remove redundant Page field from Pagination struct.The
Pagefield is redundant asCurrentPageserves the same purpose. Using a single field improves clarity and reduces potential confusion.Also applies to: 55-55, 57-57
65-73: Use CurrentPage consistently throughout pagination logic.After removing the redundant
Pagefield, all references should useCurrentPageconsistently.
🧹 Nitpick comments (1)
database/repository/pagination.go (1)
44-47: Improve default page size handling.Consider adding validation for the page number as well, similar to how page size is validated.
func Paginate[T any](data []T, attr PaginationAttr) *Pagination[T] { + page := attr.Page + if page <= 0 { + page = 1 + } + pSize := float64(attr.Limit) if pSize <= 0 { pSize = 10 }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
database/repository/pagination.go(1 hunks)database/repository/posts.go(2 hunks)handler/posts.go(1 hunks)handler/posts/transformer.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- database/repository/posts.go
- handler/posts.go
🔇 Additional comments (1)
database/repository/pagination.go (1)
102-110: Add missing Page field assignment in MapPaginatedResult.The
MapPaginatedResultfunction doesn't copy thePagefield from the source pagination. This creates inconsistency in the returned pagination structure.Apply this diff to fix the missing field:
return &Pagination[D]{ Data: mappedData, + Page: source.Page, Total: source.Total, CurrentPage: source.CurrentPage, PageSize: source.PageSize, TotalPages: source.TotalPages, NextPage: source.NextPage, PreviousPage: source.PreviousPage, }Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
database/repository/posts.go (3)
18-21: Add input validation for pagination parameters.The method should validate that pagination parameters are positive when provided to prevent potential issues with negative page numbers or zero/negative page sizes.
22-32: Fix potential query inconsistency between count and data queries.The use of
Distinct("posts.id, posts.published_at")on the main query but not on the count query could lead to incorrect pagination metadata. The count query should reflect the same distinct logic.
34-53: Remove redundant Distinct() call and optimize query execution.The
Distinct()call on line 42 is redundant since you already applied distinct selection earlier. This should be removed for cleaner code.handler/posts/transformer.go (3)
25-37: Fix incorrect field mapping in Author data.The Author's
UpdatedAtfield is incorrectly using the post'sUpdatedAtvalue instead of the author's.
61-61: Fix misleading variable name in MapTags function.The loop variable is named
categorybut should betagsince it's iterating over tags. This could lead to confusion during maintenance.
75-99: Add validation for minimum pagination values.The function should validate that page and pageSize are positive values to prevent potential issues with zero or negative pagination parameters.
🧹 Nitpick comments (1)
database/repository/pagination/pagination.go (1)
22-58: Add nil check and page bounds validation.The function should handle edge cases more robustly:
- Add a nil check for the
paginateparameter- Validate that the current page is within reasonable bounds
func MakePagination[T any](data []T, paginate *Paginate) *Pagination[T] { + if paginate == nil { + return nil + } + pSize := float64(paginate.Limit) if pSize <= 0 { pSize = 10 } totalPages := int( math.Ceil(paginate.GetNumItemsAsFloat() / pSize), ) + + // Ensure current page is at least 1 + currentPage := paginate.Page + if currentPage < 1 { + currentPage = 1 + } pagination := Pagination[T]{ Data: data, - Page: paginate.Page, + Page: currentPage, Total: paginate.GetNumItemsAsInt(), PageSize: paginate.Limit, TotalPages: totalPages, NextPage: nil, PreviousPage: nil, }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
database/repository/pagination/paginate.go(1 hunks)database/repository/pagination/pagination.go(1 hunks)database/repository/posts.go(2 hunks)handler/posts.go(1 hunks)handler/posts/transformer.go(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- database/repository/pagination/paginate.go
- handler/posts.go
🔇 Additional comments (3)
database/repository/posts.go (1)
7-8: LGTM! New imports are properly utilized.The pagination and queries imports are correctly added and used in the new GetPosts method.
database/repository/pagination/pagination.go (2)
5-20: LGTM! Well-designed generic pagination structure.The Pagination struct is well-designed with appropriate use of generics and pointer fields for optional next/previous page values. The JSON tags are correctly applied.
72-89: LGTM! Excellent generic transformation utility.The HydratePagination function is well-implemented with proper generic type handling, comprehensive documentation, and correct preservation of pagination metadata. The transformation logic is clean and efficient.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
database/connection.go (1)
76-78: Fix the GetSession method implementation - this critical issue was not properly addressed.The method still returns an unconnected
gorm.Sessionthat will cause runtime errors when used in repository operations. This is the same fundamental issue identified in the previous review.Apply this fix to create a proper GORM session connected to the database:
func (c *Connection) GetSession() *gorm.Session { - return &gorm.Session{QueryFields: true} + return c.driver.Session(&gorm.Session{QueryFields: true}) }Alternatively, if you need to return a
*gorm.DBconfigured with the session:-func (c *Connection) GetSession() *gorm.Session { - return &gorm.Session{QueryFields: true} +func (c *Connection) GetSession() *gorm.DB { + return c.driver.Session(&gorm.Session{QueryFields: true}) }
🧹 Nitpick comments (1)
database/repository/queries/posts_filters.go (1)
38-42: Consider optimizing caser creation for better performance.The sanitization logic is sound, but creating a new
cases.Caserinstance for each call could impact performance if called frequently.Consider creating the caser as a package-level variable:
+var englishLowerCaser = cases.Lower(language.English) + func (f PostFilters) sanitiseString(seed string) string { - caser := cases.Lower(language.English) - - return strings.TrimSpace(caser.String(seed)) + return strings.TrimSpace(englishLowerCaser.String(seed)) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
database/connection.go(1 hunks)database/repository/pagination/pagination.go(1 hunks)database/repository/posts.go(2 hunks)database/repository/queries/posts.go(1 hunks)database/repository/queries/posts_filters.go(1 hunks)handler/posts.go(1 hunks)handler/posts/response.go(1 hunks)handler/posts/transformer.go(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- handler/posts.go
🚧 Files skipped from review as they are similar to previous changes (4)
- handler/posts/response.go
- database/repository/posts.go
- database/repository/pagination/pagination.go
- handler/posts/transformer.go
🔇 Additional comments (5)
database/repository/queries/posts_filters.go (2)
9-16: Well-structured filter design with good tri-state logic.The
PostFiltersstruct is well-designed, particularly the use of*boolforIsPublishedto handle three states (true, false, unset). The filtering fields cover comprehensive search criteria.
18-36: Clean getter pattern with consistent sanitization.The getter methods provide a clean interface with consistent sanitization. Good separation of concerns.
database/repository/queries/posts.go (3)
8-11: Good defensive programming with nil check.Proper nil check prevents potential panics when filters are not provided.
17-24: Excellent fix for OR condition grouping.The text filtering now properly groups OR conditions in a single WHERE clause, addressing the previous review concern about logical grouping.
26-38: Great improvements to author filtering.The author filtering now includes proper grouping with parentheses and uses ILIKE for partial matching, addressing both issues from the previous review. Good addition of soft delete protection.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
go.mod (1)
3-3: Invalid Go version specifiedGo version 1.24 doesn't exist. The latest stable version is Go 1.22. This will cause build failures.
Apply this fix:
-go 1.24 +go 1.22
♻️ Duplicate comments (6)
database/connection.go (1)
76-78: Fix the GetSession method implementationSimilar to the existing
Session()method issue, this returns a disconnectedgorm.Sessionthat won't work with database operations. The session must be created from the database connection.Apply this fix:
func (c *Connection) GetSession() *gorm.Session { - return &gorm.Session{QueryFields: true} + return c.driver.Session(&gorm.Session{QueryFields: true}) }database/repository/posts.go (2)
18-21: Add input validation for pagination parameters.The method should validate that pagination parameters are positive to prevent potential issues with negative page numbers or invalid limits.
31-34: Ensure count query consistency with main query.The count query should use the same distinct logic as the main query to ensure accurate pagination metadata, especially when joins from filtering are involved.
#!/bin/bash # Verify if ApplyPostsFilters function uses joins that could affect count accuracy ast-grep --pattern 'func ApplyPostsFilters($_, $_) { $$$ }'database/repository/queries/posts.go (2)
45-51: Fix inconsistency between WHERE operators and parameter usage.The WHERE clause uses exact match operators (
=) but the parameters include wildcards (%), which won't work as expected. For partial matching, useILIKEoperators.Apply this fix to make category filtering consistent:
Where("("+ - "LOWER(categories.slug) = ? OR LOWER(categories.name) = ? OR LOWER(categories.description) = ?"+ + "LOWER(categories.slug) ILIKE ? OR LOWER(categories.name) ILIKE ? OR LOWER(categories.description) ILIKE ?"+ ")",
59-65: Fix inconsistency between WHERE operators and parameter usage.Same issue as category filtering - using exact match operators (
=) with wildcard parameters (%).Apply this fix to make tag filtering consistent:
Where("("+ - "LOWER(tags.slug) = ? OR LOWER(tags.name) = ? OR LOWER(tags.description) = ?"+ + "LOWER(tags.slug) ILIKE ? OR LOWER(tags.name) ILIKE ? OR LOWER(tags.description) ILIKE ?"+ ")",handler/posts/transformer.go (1)
105-107: Implement filter parsing from HTTP request.The function currently returns an empty
PostFiltersstruct instead of parsing actual filter parameters from the HTTP request. This needs to be implemented to make filtering functional.func GetFiltersFrom(r *baseHttp.Request) queries.PostFilters { - return queries.PostFilters{} + query := r.URL.Query() + return queries.PostFilters{ + Title: query.Get("title"), + Text: query.Get("text"), + Author: query.Get("author"), + Category: query.Get("category"), + Tag: query.Get("tag"), + } }Would you like me to help implement the complete filter parsing logic based on expected query parameters?
🧹 Nitpick comments (1)
database/repository/posts.go (1)
18-18: Consider using pointer for filters parameter consistency.The method uses a value receiver for
filtersbut a pointer forpaginate. For consistency and to avoid unnecessary copying, consider using a pointer for filters as well.-func (p Posts) GetPosts(filters queries.PostFilters, paginate pagination.Paginate) (*pagination.Pagination[database.Post], error) { +func (p Posts) GetPosts(filters *queries.PostFilters, paginate *pagination.Paginate) (*pagination.Pagination[database.Post], error) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
boost/app.go(2 hunks)boost/router.go(3 hunks)config/makefile/app.mk(1 hunks)database/connection.go(1 hunks)database/repository/pagination/paginate.go(1 hunks)database/repository/pagination/pagination.go(1 hunks)database/repository/posts.go(2 hunks)database/repository/queries/posts.go(1 hunks)database/repository/queries/posts_filters.go(1 hunks)go.mod(1 hunks)handler/posts.go(1 hunks)handler/posts/response.go(1 hunks)handler/posts/transformer.go(1 hunks)
🔇 Additional comments (21)
database/repository/pagination/paginate.go (1)
1-24: Well-designed pagination utility struct.The
Paginatestruct provides a clean foundation for pagination with appropriate type conversion methods and follows Go naming conventions effectively.handler/posts/response.go (1)
47-54: Circular reference issue has been resolved.The
TagDatastruct no longer contains thePostsfield that was mentioned in previous reviews, eliminating the circular reference concern. The current implementation is clean and appropriate for API responses.database/repository/posts.go (1)
38-44: LGTM on query structure and preloading.The preloading strategy and ordering are well-implemented for the posts retrieval functionality.
database/repository/queries/posts_filters.go (2)
9-16: Excellent filter struct design.The
PostFiltersstruct demonstrates good practices with:
- Appropriate field types for different filter scenarios
- Tri-state boolean using
*boolforIsPublished- Clear field documentation for partial matching behavior
18-42: Well-implemented sanitization methods.The getter methods with consistent sanitization using locale-aware case conversion (
cases.Lower(language.English)) and whitespace trimming provide robust input handling for filtering operations.database/repository/pagination/pagination.go (4)
5-6: Good constants for pagination bounds.The
MinPageandMaxLimitconstants provide clear boundaries for pagination parameters.
13-21: Excellent generic pagination struct design.The
Pagination[T]struct demonstrates excellent Go generics usage with:
- Comprehensive pagination metadata
- Proper JSON tags with omitempty for optional fields
- Pointer types for NextPage/PreviousPage to handle nil states cleanly
23-59: Robust MakePagination implementation.The function handles edge cases well with defensive programming:
- Fallback to default page size when invalid
- Correct total pages calculation using
math.Ceil- Proper boundary checking for next/previous page logic
73-90: Well-designed transformation utility.The
HydratePaginationfunction provides clean type transformation while preserving all pagination metadata. The comprehensive documentation and clear type parameter descriptions make it highly maintainable.handler/posts.go (2)
17-21: LGTM on the constructor implementation.The constructor function properly initializes the PostsHandler with the provided repository dependency.
23-47: Good improvement on error handling and parameter extraction.The handler correctly addresses the previously identified issues:
- Uses structured logging instead of
log.Fatalf- Extracts filter and pagination parameters from the HTTP request
- Returns appropriate HTTP error responses without exposing internal details
database/repository/queries/posts.go (3)
13-15: LGTM on title filtering implementation.The title filtering correctly uses ILIKE with wildcards for case-insensitive partial matching.
17-24: Good fix for text filtering logical grouping.The OR conditions for text filtering are now properly grouped within a single WHERE clause, addressing the previous logical grouping issue.
26-38: Excellent implementation of author filtering.The author filtering correctly:
- Uses proper JOIN syntax
- Includes soft delete checks
- Groups OR conditions properly
- Uses ILIKE with wildcards for flexible matching
handler/posts/transformer.go (4)
12-39: LGTM on post response transformation.The transformation function correctly maps all post fields including the fixed
UpdatedAtfield mapping for the author (now usesp.Author.UpdatedAtinstead ofp.UpdatedAt).
41-56: Clean implementation of category transformation.The category mapping function is well-structured and correctly transforms all category fields.
58-73: Good implementation of tag transformation.The tag mapping function correctly transforms tag data without the variable naming issues noted in previous reviews.
75-103: Excellent pagination parameter parsing with proper validation.The function now includes proper validation for pagination parameters, addressing the previous concern about minimum values. The implementation correctly:
- Uses constants from the pagination package
- Validates minimum page values
- Enforces maximum limit constraints
config/makefile/app.mk (1)
28-28: LGTM! Development tooling improvement.Adding the
-dflag to theaircommand enhances the development workflow by enabling daemon/debug mode for live reloading.boost/app.go (2)
73-73: Router.Posts method confirmed
ThePosts()method is defined on theRouterstruct inboost/router.go, so the call inboost/app.gois valid.• Location: boost/router.go, lines 34–43
45-45: Router struct’s Db field verified – changes approvedI’ve confirmed that in boost/router.go, the
Routerstruct includes the fieldDb *database.Connectionas expected. The added
router.Posts()call inBootaligns with the existing router setup pattern. No further changes needed—this is good to merge.
|
@gemini-code-assist review |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
pkg/http/request.go (1)
16-22: Consider simplifying the closer function structure.The defer inside the closer function that's returned but not immediately called can be confusing. Consider a simpler approach:
- closer := func() { - defer func(Body io.ReadCloser) { - if issue := Body.Close(); issue != nil { - slog.Error("ParseRequestBody: " + issue.Error()) - } - }(r.Body) - } + closer := func() { + if issue := r.Body.Close(); issue != nil { + slog.Error("ParseRequestBody: " + issue.Error()) + } + }database/repository/queries/posts_filters.go (1)
36-40: Minor optimization opportunity: redundant whitespace trimming.The
sanitiseStringmethod performs whitespace trimming twice - once inMakeStringableand once afterToLower(). SinceToLower()already trims whitespace, the additionalTrimSpacecall is redundant.func (f PostFilters) sanitiseString(seed string) string { str := pkg.MakeStringable(seed) - return strings.TrimSpace(str.ToLower()) + return str.ToLower() }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
boost/router.go(3 hunks)database/repository/posts.go(2 hunks)database/repository/queries/posts.go(1 hunks)database/repository/queries/posts_filters.go(1 hunks)handler/posts.go(1 hunks)handler/posts/response.go(1 hunks)handler/posts/transformer.go(1 hunks)pkg/http/request.go(1 hunks)pkg/http/response.go(1 hunks)pkg/stringable.go(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- boost/router.go
- database/repository/posts.go
- database/repository/queries/posts.go
🔇 Additional comments (9)
pkg/stringable.go (1)
22-26: Well-implemented internationalization-aware case conversion.The
ToLower()method correctly uses thegolang.org/x/text/casespackage for proper Unicode case conversion with English language rules, followed by whitespace trimming. This approach ensures consistent case-insensitive filtering.pkg/http/response.go (1)
75-87: Well-structured error constructor functions.The new
BadRequestErrorandNotFoundfunctions follow a consistent pattern with the existingInternalErrorfunction, using appropriate HTTP status codes and formatted error messages. The consistent lowercase styling in error messages maintains uniformity across the API.database/repository/queries/posts_filters.go (1)
8-34: Well-designed filter structure with consistent sanitization.The
PostFiltersstruct provides a clean interface for filtering posts with consistent sanitization across all filter fields. The getter methods ensure case-insensitive filtering by leveraging the newToLower()method.handler/posts/response.go (2)
55-62: Previous circular reference concern has been resolved.The
TagDatastruct no longer contains thePostsfield that was flagged in previous reviews, eliminating the potential circular reference issue withPostResponse.Tags.
7-30: Well-structured API response contracts.The request and response structs are well-designed with proper JSON tags, appropriate field types, and clear separation of concerns. The
IndexRequestBodyaligns with the filtering capabilities, andPostResponseprovides comprehensive post data with nested associations.handler/posts.go (1)
59-79: Well-implemented Show method.The method properly handles edge cases with appropriate HTTP status codes and clear error messages. Good error handling pattern.
handler/posts/transformer.go (3)
14-41: Correct implementation of post response mapping.The function properly maps all fields including the nested author data. The previously reported UpdatedAt field mapping issue has been correctly addressed.
60-75: Proper implementation with correct variable naming.The function correctly uses 'tag' as the loop variable, addressing the previously reported naming issue.
107-114: Complete implementation of filter parsing.The function properly maps request body fields to post filters, addressing the previously reported stub implementation issue.
There was a problem hiding this comment.
Code Review
The pull request introduces new API endpoints for listing and retrieving posts with filtering and pagination. The code includes new handler functions, repository methods, and data structures for handling post requests and responses. Several improvements can be made to error handling, query optimization, and code clarity.
|
@coderabbitai summary |
✅ Actions performedSummary regeneration triggered. |
This change introduces a database-backed posts API endpoint, including repository, query filters, pagination utilities, handler logic, and response transformation.
The router has been updated to support the new posts handler, and supporting structs, methods, and utility functions have been added for filtering, pagination, and response formatting. Minor Makefile and dependency updates are included.
Summary by CodeRabbit
New Features
Bug Fixes
Chores