Conversation
I have zero idea whether this works or not but iv'e tried to make it as accurate to the db template as possible. Will add filtering later
small edits
Updated the games route to include query parameters for pagination and filtering. Added zod validation. Tried to make it match the format of pre-existed profile and auth routes
|
@Thisissandy07 do try to follow as for the code @Rex-8 will review soon |
Removed unnecessary queries
There was a problem hiding this comment.
Pull request overview
This pull request adds a new GET /games endpoint that allows authenticated users to retrieve a paginated list of active games with filtering capabilities.
Changes:
- Added a new games route file with GET /games endpoint supporting pagination and filtering by search term, likes count, and creation date
- Implemented Zod schema validation for query parameters (page, limit, search, maxLikes, createdAfter, createdBefore)
- Added filtering logic for active games with support for title search and date range filtering
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| const whereClause = and(...conditions); | ||
|
|
||
| /*Data retrival*/ |
There was a problem hiding this comment.
Missing space after comment delimiter. Comment should be formatted as "/* Data retrieval */" to match standard formatting conventions. Also, there's a spelling error: "retrival" should be "retrieval".
| /*Data retrival*/ | |
| /* Data retrieval */ |
| /** | ||
| * GET /games | ||
| * | ||
| * gives all active games |
There was a problem hiding this comment.
The documentation comment has inconsistent capitalization. It should start with a capital letter like other route documentation in the codebase. Change "gives all active games" to "Retrieves all active games" or "Returns all active games" to match the documentation style seen in auth.ts and profile.ts (e.g., "Initiates Google OAuth flow", "Handles Google OAuth callback", "Complete initial profile setup").
| * gives all active games | |
| * Retrieves all active games |
| * - createdAfter (YYYY-MM-DD) | ||
| * - createdBefore (YYYY-MM-DD) | ||
| * | ||
|
|
There was a problem hiding this comment.
The documentation has an extra blank line at line 45 before the closing comment delimiter. This is inconsistent with the documentation style in other routes (see auth.ts and profile.ts) which don't have blank lines before the closing delimiter. Remove this blank line for consistency.
| * - page | ||
| * - limit | ||
| * - search | ||
| * - minLikes |
There was a problem hiding this comment.
The documentation comment mentions 'minLikes' as a query parameter, but this parameter is not defined in the Zod schema (line 19-26) and is not implemented in the filtering logic. Either add the minLikes parameter to the schema and implement the filtering logic, or remove it from the documentation.
| * - minLikes |
| const afterTimestamp = Math.floor( | ||
| new Date(createdAfter).getTime() / 1000, | ||
| ); | ||
| conditions.push(gte(games.createdAt, afterTimestamp)); |
There was a problem hiding this comment.
When an invalid date string is provided (e.g., "invalid-date"), new Date() will create an Invalid Date object, and getTime() will return NaN. Math.floor(NaN) will also return NaN, which will then be used in the database query and could cause unexpected behavior. The date validation should be improved in the schema (as noted in another comment), or you should add a check here to handle invalid dates and return an appropriate error message.
| const beforeTimestamp = Math.floor( | ||
| new Date(createdBefore).getTime() / 1000, | ||
| ); | ||
| conditions.push(lte(games.createdAt, beforeTimestamp)); |
There was a problem hiding this comment.
When an invalid date string is provided (e.g., "invalid-date"), new Date() will create an Invalid Date object, and getTime() will return NaN. Math.floor(NaN) will also return NaN, which will then be used in the database query and could cause unexpected behavior. The date validation should be improved in the schema (as noted in another comment), or you should add a check here to handle invalid dates and return an appropriate error message.
| const totalResult = await db | ||
| .select({ value: count() }) | ||
| .from(games) | ||
| .where(whereClause); | ||
|
|
||
| const total = totalResult[0]?.value ?? 0; |
There was a problem hiding this comment.
The total count query runs separately from the main data query, which means two database queries are executed for every request. For large datasets with complex filters, the count query could become a performance bottleneck. Consider adding database indexes on frequently filtered columns (isActive, countLikes, createdAt) to improve query performance. Additionally, for very large datasets, you might want to consider caching the total count or using cursor-based pagination instead of offset-based pagination.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
zod date validation Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@Thisissandy07 Is there a reason for choosing offset pagination over cursor pagination. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (createdAfter) { | ||
| const afterTimestamp = Math.floor( | ||
| new Date(createdAfter).getTime() / 1000, | ||
| ); | ||
| conditions.push(gte(games.createdAt, afterTimestamp)); | ||
| } | ||
|
|
||
| if (createdBefore) { | ||
| const beforeTimestamp = Math.floor( | ||
| new Date(createdBefore).getTime() / 1000, | ||
| ); | ||
| conditions.push(lte(games.createdAt, beforeTimestamp)); | ||
| } |
There was a problem hiding this comment.
createdAt is defined as an integer timestamp column with mode: "timestamp" in the schema (used elsewhere with new Date() values). For consistency and to avoid type/serialization issues, compare games.createdAt against Date objects instead of manually converting to epoch seconds. Also, with createdBefore as YYYY-MM-DD, using midnight with lte will exclude most of that day; consider filtering by < startOfNextDay(createdBefore) (or otherwise making the end date inclusive).
|
|
||
| export default gamesRoute; |
There was a problem hiding this comment.
This route is exported but is not currently mounted anywhere in src/index.ts (only /auth and /profile are routed), so GET /games will never be reachable. Add it to the main app router (e.g., app.route("/games", gamesRoute)) to actually expose the endpoint.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Thisissandy07
left a comment
There was a problem hiding this comment.
added cursor based pagination instead of page based. Restructured comments to be more organized
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const hasNextPage = gamesList.length > limit; | ||
| const results = hasNextPage ? gamesList.slice(0, limit) : gamesList; | ||
| const nextCursor = hasNextPage ? results[results.length - 1].createdAt : null; | ||
|
|
||
| return c.json({ | ||
| success: true, | ||
| limit, | ||
| nextCursor, | ||
| games: results, |
There was a problem hiding this comment.
nextCursor is returned as results[...].createdAt. If createdAt is a Date, the JSON response will contain an ISO string cursor, but the request schema currently expects a numeric cursor, so the next request will fail validation. Make the response and request cursor formats consistent.
There was a problem hiding this comment.
You can try returning the timestamps as numbers instead of ISO strings.
GET/Games endpoint route
Summary
API endpoint route what allows active users to access games
Type
feat: new feature
refactor: code restructuring
Changes
-pagination added
-filtering of games by likes and date added
Checklist
Conventional Commits