fix: correct utsav pagination offset and include utsavs without packages#285
fix: correct utsav pagination offset and include utsavs without packages#285
Conversation
vendz
commented
Mar 22, 2026
- Fix offset calculation: (page-1)*(pageSize-1) → (page-1)*pageSize Previous formula caused page 2 to overlap with page 1 by 1 item, creating infinite pagination loop that crashed React Query for users with 9+ upcoming utsavs (REACT-NATIVE-KK)
- Change JOIN → LEFT JOIN on utsav_packages_db so utsavs with no packages are still returned, with packages as empty JSON array
- Fix offset calculation: (page-1)*(pageSize-1) → (page-1)*pageSize Previous formula caused page 2 to overlap with page 1 by 1 item, creating infinite pagination loop that crashed React Query for users with 9+ upcoming utsavs (REACT-NATIVE-KK) - Change JOIN → LEFT JOIN on utsav_packages_db so utsavs with no packages are still returned, with packages as empty JSON array Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Summary of ChangesHello, 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 addresses a pagination issue in the upcoming utsavs endpoint and ensures that all utsavs are returned, regardless of whether they have associated packages. The offset calculation was incorrect, causing an infinite loop, and the query was updated to include utsavs without packages. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe 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 by creating a comment using either
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 Limitations & Feedback Gemini Code Assist 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Pull request overview
This PR fixes client-facing issues in the “upcoming utsavs” endpoint by correcting pagination math and ensuring utsavs without any packages are still returned.
Changes:
- Fix pagination offset calculation to avoid overlapping pages.
- Switch packages join to a
LEFT JOINand returnpackagesas an empty JSON array when no packages exist.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const page = parseInt(req.query.page) || 1; | ||
| const pageSize = parseInt(req.query.page_size) || 10; |
There was a problem hiding this comment.
Pagination offset math changed here, and FetchUpcoming now has new behavior for utsavs with no packages. There are integration tests for other booking controllers under tests/controllers/client/, but none for this controller; adding coverage for non-overlapping page results and packages: [] when no packages exist would help prevent regressions.
| const page = parseInt(req.query.page) || 1; | |
| const pageSize = parseInt(req.query.page_size) || 10; | |
| const rawPage = parseInt(req.query.page, 10); | |
| const rawPageSize = parseInt(req.query.page_size, 10); | |
| const page = Number.isNaN(rawPage) || rawPage < 1 ? 1 : rawPage; | |
| const DEFAULT_PAGE_SIZE = 10; | |
| const MAX_PAGE_SIZE = 100; | |
| const pageSizeUnclamped = Number.isNaN(rawPageSize) || rawPageSize < 1 ? DEFAULT_PAGE_SIZE : rawPageSize; | |
| const pageSize = Math.min(pageSizeUnclamped, MAX_PAGE_SIZE); |
| IF(COUNT(t2.id) = 0, JSON_ARRAY(), JSON_ARRAYAGG( | ||
| JSON_OBJECT( | ||
| 'package_id', t2.id, | ||
| 'package_name', t2.name, | ||
| 'package_start', t2.start_date, | ||
| 'package_end', t2.end_date, | ||
| 'package_amount', t2.amount | ||
| ) | ||
| ) AS packages | ||
| )) AS packages | ||
| FROM utsav_db t1 | ||
| JOIN utsav_packages_db t2 ON t1.id = t2.utsavid | ||
| LEFT JOIN utsav_packages_db t2 ON t1.id = t2.utsavid |
There was a problem hiding this comment.
FetchUpcoming now includes utsavs with no packages via LEFT JOIN and returns packages as []. However FetchUtsavById in the same controller still uses an inner JOIN utsav_packages_db, so selecting an utsav from the upcoming list that has no packages will likely 404 on the details endpoint. Consider updating FetchUtsavById to also use LEFT JOIN and the same empty-array handling for packages to keep the API consistent.
There was a problem hiding this comment.
Code Review
This pull request introduces two key fixes. First, it corrects the pagination offset calculation, resolving an issue that caused overlapping data between pages. Second, it modifies a database query by changing an INNER JOIN to a LEFT JOIN. This ensures that utsavs without associated packages are included in the results, with their packages represented as an empty JSON array. The changes are well-implemented and effectively address the described issues. The code looks solid.