Skip to content
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

Fix offset validation and parsing in Angor routes #40

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

miladsoft
Copy link
Member

Pagination Offset Handling Improvements

Changes Made:

  1. Modified offset handling in API endpoints to be more consistent with Blockcore Indexer:

    • Empty offset (?offset=) now defaults to 0
    • Explicit offset=0 (?offset=0) now defaults to 0
    • No offset parameter (?limit=10) defaults to 0
  2. Fixed validation logic for offset parameter:

    • Now correctly validates empty string and "0" values
    • Error messages now show under correct "offset" key instead of "limit"
    • Improved type handling for undefined/null cases
  3. Updated pagination header handling:

    • Modified setPaginationAndLinkHeaders to handle offset=0 cases consistently
    • First page link now includes explicit offset=0
    • Pagination-Offset header now always returns a numeric value

Testing Scenarios:

The following API calls now work consistently:

  • ?limit=10 → offset=0
  • ?limit=10&offset= → offset=0
  • ?limit=10&offset=0 → offset=0
  • ?limit=10&offset=10 → offset=10

Backwards Compatibility:

These changes maintain backwards compatibility while aligning behavior with Blockcore Indexer expectations.

Technical Details:

  • Default offset value changed from undefined to 0
  • Validation logic simplified to handle empty strings and "0" values uniformly
  • Pagination links updated to always include explicit offset values

@dangershony
Copy link
Member

Empty offset (?offset=) now defaults to 0

How do we deal with offset 0 then?
Normally offset 0 should return the first elements, and offset null should return he latest elements.
Instead of defaulting null to zero we need to allow null and treat null and latest items.

@dangershony dangershony requested a review from Copilot March 14, 2025 17:33
Copy link

@Copilot Copilot AI left a 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 improves offset handling and validation in Angor API endpoints to ensure consistent behavior with Blockcore Indexer expectations. Changes include:

  • Updating offset parameter validation to treat empty and "0" values consistently
  • Correcting error message keys from "limit" to "offset" where appropriate
  • Adjusting pagination header links to always include an explicit offset value

@@ -360,14 +361,14 @@ class AngorRoutes {
}
}

if (typeof queryOffset === 'string') {
if (typeof queryOffset === 'string' && queryOffset !== '' && queryOffset !== '0') {
Copy link
Preview

Copilot AI Mar 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The added condition "queryOffset !== '0'" may cause an explicit offset value of '0' to bypass validation, which seems inconsistent with the intended default handling. Consider revisiting this condition to ensure that an explicit '0' is processed uniformly.

Suggested change
if (typeof queryOffset === 'string' && queryOffset !== '' && queryOffset !== '0') {
if (typeof queryOffset === 'string' && queryOffset !== '') {

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants