✨ Comment pagination #4
Conversation
Signed-off-by: grnd-alt <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR adds pagination support for fetching comments, refactors post/comment types, and updates related SQL queries and routes.
- Switch
PostWithCommentsto usedbgen.Commentand update population logic. - Introduce
CommentsService.GetCommentswith limit/offset, new controller action, and route. - Update SQL queries (
GetCommentsByPost,GetCommentsByPosts) to includeORDER BY,LIMIT, andOFFSET, and regenerate corresponding Go code.
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| services/posts_service.go | Updated PostWithComments, changed comment type, removed mutex |
| services/comments_service.go | Added GetComments(postID, page) with pagination logic |
| controllers/comments_controller.go | Added GetCommentsByPost handler parsing page parameter |
| routes/routes.go | Added GET /api/comments/:postId route |
| db/sql/queries/comments.sql | Added pagination clauses to SQL queries |
| db/sql/dbgen/comments.sql.go | Regenerated query methods to accept Limit/Offset |
| go.mod | Bumped several indirect dependencies |
Comments suppressed due to low confidence (1)
services/comments_service.go:20
- New pagination logic should have unit tests verifying offset calculation, boundary conditions, and empty-page behavior to prevent regressions.
func (c *CommentsService) GetComments(postID string, page int32) ([]dbgen.Comment, error) {
| commentsMap := make(map[string][]*dbgen.GetCommentsByPostsRow) | ||
| commentsMap := make(map[string][]*dbgen.Comment) | ||
| for _, comment := range comments { | ||
| commentsMap[comment.PostID] = append(commentsMap[comment.PostID], &comment) |
There was a problem hiding this comment.
Taking the address of the loop variable comment causes all pointers to reference the same memory. Instead, create a new variable in the loop (e.g., c := comment) and append &c.
| commentsMap[comment.PostID] = append(commentsMap[comment.PostID], &comment) | |
| c := comment | |
| commentsMap[comment.PostID] = append(commentsMap[comment.PostID], &c) |
| } | ||
| post.Filepath = pgtype.Text{String: filepath, Valid: true} | ||
| mut.Lock() | ||
| postsWithComments[i] = PostWithComments{ |
There was a problem hiding this comment.
Removing the mutex leads to a data race when writing to postsWithComments concurrently. Reintroduce synchronization or populate the slice sequentially to avoid races.
|
|
||
| func (c *CommentsService) CreateComment(userId string, username string,content string, postID string) (*dbgen.Comment, error) { | ||
| func (c *CommentsService) GetComments(postID string, page int32) ([]dbgen.Comment, error) { | ||
| comments, err := c.Queries.GetCommentsByPost(context.Background(), dbgen.GetCommentsByPostParams{Offset: 10 * page, PostID: postID, Limit: 10}) |
There was a problem hiding this comment.
[nitpick] Extract the magic numbers 10 (page size) into a named constant so it's easier to adjust pagination limits in the future.
| comments, err := c.Queries.GetCommentsByPost(context.Background(), dbgen.GetCommentsByPostParams{Offset: 10 * page, PostID: postID, Limit: 10}) | |
| comments, err := c.Queries.GetCommentsByPost(context.Background(), dbgen.GetCommentsByPostParams{Offset: DefaultPageSize * page, PostID: postID, Limit: DefaultPageSize}) |
| page, err := strconv.Atoi(ctx.Query("page")) | ||
| if postID == "" || err != nil || page < 0 { | ||
| response := gin.H{"error": "incorrect parameters"} | ||
| ctx.JSON(http.StatusBadRequest, response) | ||
| return |
There was a problem hiding this comment.
[nitpick] Calling Atoi on an empty string returns an error. Consider defaulting to page 0 when the page query param is omitted so a first page request doesn't fail.
| page, err := strconv.Atoi(ctx.Query("page")) | |
| if postID == "" || err != nil || page < 0 { | |
| response := gin.H{"error": "incorrect parameters"} | |
| ctx.JSON(http.StatusBadRequest, response) | |
| return | |
| pageStr := ctx.Query("page") | |
| page := 0 | |
| if pageStr != "" { | |
| var err error | |
| page, err = strconv.Atoi(pageStr) | |
| if err != nil || page < 0 { | |
| response := gin.H{"error": "incorrect parameters"} | |
| ctx.JSON(http.StatusBadRequest, response) | |
| return | |
| } |
| select * from RankedComments where rn <= 10; | ||
| SELECT c.* from unnest($1::varchar[]) as post_ids | ||
| JOIN LATERAL ( | ||
| SELECT * FROM comments WHERE post_id = post_ids ORDER BY created_at DESC LIMIT 2 |
There was a problem hiding this comment.
[nitpick] The LIMIT 2 in the lateral join is effectively a magic number. Consider making the per-post comment limit configurable or use the same page-size constant.
| SELECT * FROM comments WHERE post_id = post_ids ORDER BY created_at DESC LIMIT 2 | |
| SELECT * FROM comments WHERE post_id = post_ids ORDER BY created_at DESC LIMIT $2 |
No description provided.