feat: add pagination support to GET /api/orders#56
feat: add pagination support to GET /api/orders#56srija-pixel wants to merge 1 commit intofuzziecoder:mainfrom
Conversation
|
@srija-pixel is attempting to deploy a commit to the Revon Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe GET /api/orders endpoint has been refactored to implement pagination. The single-order retrieval logic was replaced with multi-parameter filtering (spotId, userId) combined with limit and offset query parameters. Results are validated, sliced, and returned with pagination metadata. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/server.js`:
- Around line 131-138: The code currently calls database.getOrders without
pagination and then slices the full result in server.js, which forces a full
fetch and mapping; update database.getOrders (in backend/db.js) to accept limit
and offset, perform a COUNT(*) query to compute total, run the paged SELECT with
LIMIT ? OFFSET ? to fetch only the requested orders, then fetch/map items only
for the paged order IDs; change the server call (database.getOrders) to pass
limit and offset and consume the new return shape (e.g., { total, orders }) so
server.js uses the DB-provided total and paged orders directly.
- Around line 123-126: The offset parsing currently coerces empty or
whitespace-only values to 0 because it calls Number(offsetParam); update the
validation in the block that reads offsetParam (the code that sets offset =
Number(offsetParam) and calls sendJson) to first reject empty or whitespace-only
strings by checking something like if (typeof offsetParam === 'string' &&
offsetParam.trim() === '') then call sendJson(res, 400, { error: 'Invalid offset
parameter' }) before converting to Number; keep the existing integer and
non-negative checks for offset after conversion.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
backend/server.js
| if (offsetParam !== null) { | ||
| offset = Number(offsetParam); | ||
| if (!Number.isInteger(offset) || offset < 0) { | ||
| sendJson(res, 400, { error: 'Invalid offset parameter' }); |
There was a problem hiding this comment.
Reject empty/whitespace offset as malformed.
Line 124 uses Number(offsetParam), so ?offset= and ?offset= are coerced to 0 and currently pass. That conflicts with the “malformed values return 400” behavior.
Suggested fix
if (offsetParam !== null) {
- offset = Number(offsetParam);
- if (!Number.isInteger(offset) || offset < 0) {
+ if (!/^\d+$/.test(offsetParam)) {
+ sendJson(res, 400, { error: 'Invalid offset parameter' });
+ return;
+ }
+ offset = Number(offsetParam);
+ if (!Number.isInteger(offset) || offset < 0) {
sendJson(res, 400, { error: 'Invalid offset parameter' });
return;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (offsetParam !== null) { | |
| offset = Number(offsetParam); | |
| if (!Number.isInteger(offset) || offset < 0) { | |
| sendJson(res, 400, { error: 'Invalid offset parameter' }); | |
| if (offsetParam !== null) { | |
| if (!/^\d+$/.test(offsetParam)) { | |
| sendJson(res, 400, { error: 'Invalid offset parameter' }); | |
| return; | |
| } | |
| offset = Number(offsetParam); | |
| if (!Number.isInteger(offset) || offset < 0) { | |
| sendJson(res, 400, { error: 'Invalid offset parameter' }); | |
| return; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/server.js` around lines 123 - 126, The offset parsing currently
coerces empty or whitespace-only values to 0 because it calls
Number(offsetParam); update the validation in the block that reads offsetParam
(the code that sets offset = Number(offsetParam) and calls sendJson) to first
reject empty or whitespace-only strings by checking something like if (typeof
offsetParam === 'string' && offsetParam.trim() === '') then call sendJson(res,
400, { error: 'Invalid offset parameter' }) before converting to Number; keep
the existing integer and non-negative checks for offset after conversion.
| // Get full filtered result first | ||
| const allOrders = database.getOrders({ spotId, userId }); | ||
|
|
||
| if (!order) { | ||
| sendJson(res, 404, { error: `Order not found: ${orderId}` }); | ||
| return; | ||
| } | ||
| const total = allOrders.length; | ||
|
|
||
| // Apply pagination | ||
| const paginatedOrders = allOrders.slice(offset, offset + limit); | ||
|
|
There was a problem hiding this comment.
Move pagination to the database layer to avoid full fetches.
Line 132 fetches all filtered orders, then Line 137 slices in memory. Per backend/db.js (getOrders, lines 163-330), this means loading and mapping every matched order (and items) before pagination, which defeats scalability and can block the request path under larger datasets.
Suggested direction
- const allOrders = database.getOrders({ spotId, userId });
- const total = allOrders.length;
- const paginatedOrders = allOrders.slice(offset, offset + limit);
+ const { data: paginatedOrders, total } = database.getOrders({
+ spotId,
+ userId,
+ limit,
+ offset
+ });Then update backend/db.js:getOrders to:
- run
COUNT(*)fortotal - run paged select with
LIMIT ? OFFSET ? - fetch/map items only for paged order IDs
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/server.js` around lines 131 - 138, The code currently calls
database.getOrders without pagination and then slices the full result in
server.js, which forces a full fetch and mapping; update database.getOrders (in
backend/db.js) to accept limit and offset, perform a COUNT(*) query to compute
total, run the paged SELECT with LIMIT ? OFFSET ? to fetch only the requested
orders, then fetch/map items only for the paged order IDs; change the server
call (database.getOrders) to pass limit and offset and consume the new return
shape (e.g., { total, orders }) so server.js uses the DB-provided total and
paged orders directly.
1 participant
Adds pagination support to GET /api/orders.
Previously, the endpoint returned the full dataset without paging.
This update introduces limit/offset pagination with metadata.
Changes Made
Added limit and offset query parameters
Default pagination: limit = 10, offset = 0
Response now includes meta object:
total
limit
offset
hasMore
Added validation for invalid pagination parameters
Returns 400 Bad Request for malformed limit/offset values
Summary by CodeRabbit