-
Notifications
You must be signed in to change notification settings - Fork 14
Fix size=0 query parameter to return all floors instead of default 30 #201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: KYLN24 <[email protected]>
- Added check for raw query parameters before validation applies defaults - When both size=0 and offset=0, return all floors instead of limiting to 30 - Added test case to verify the fix - Added treehole_next binary to .gitignore Co-authored-by: KYLN24 <[email protected]>
Co-authored-by: KYLN24 <[email protected]>
Co-authored-by: KYLN24 <[email protected]>
Co-authored-by: KYLN24 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a bug where the API endpoint /api/holes/{hole_id}/floors?size=0&offset=0 was incorrectly returning only 30 floors instead of all floors. The issue occurs because the ListModel schema has a default:"30" tag that causes the validation framework to replace size=0 with the default value of 30, preventing the special case logic from detecting the request for all floors.
Key Changes:
- Added raw query parameter checking before validation to detect explicit
size=0&offset=0requests - Updated
ListFloorsInAHoleto passnilfor both offset and size toMakeQuerySetwhen the special case is detected, returning all floors without limit - Added test case to verify all 50 test floors are returned with
size=0&offset=0parameters
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
.gitignore |
Adds treehole_next binary to gitignore |
tests/floor_test.go |
Adds test case verifying size=0&offset=0 returns all 50 floors in the test hole |
apis/floor/apis.go |
Implements raw query parameter check before validation to detect size=0&offset=0 and bypass pagination, matching ListFloorsOld behavior for backward compatibility |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The API endpoint
/api/holes/{hole_id}/floors?size=0&offset=0was returning only 30 floors instead of all floors. The validation framework applies a default value of 30 whensize=0is passed, preventing the special case from being detected.Changes
apis/floor/apis.go: Check raw query parameters before validation to detect explicitsize=0&offset=0and passniltoMakeQuerySetfor unlimited resultstests/floor_test.go: Add test case verifying all 50 test floors are returned.gitignore: Addtreehole_nextbinaryExample
This matches the existing behavior in
ListFloorsOldfor backward compatibility.Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.