Skip to content

Conversation

@codesungrape
Copy link
Collaborator

Description

Trello card: https://trello.com/c/dRDnYYYO
This PR introduces a configurable maximum offset value for pagination across reservation endpoints.

  • Adds a MAX_OFFSET environment variable with a fallback default of 2000.
  • Ensures offsets cannot exceed this maximum; otherwise, a validation error is raised.
  • Default offset remains 0 for normal pagination use.
  • Cleans up redundant offset validation logic and tests.

Fixes # (issue number if applicable)

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

  • Added unit tests for invalid offset values (negative, above max).
  • Updated reservation service and controller tests to validate new logic.
  • Verified default behavior when no MAX_OFFSET is provided.
  • Ran full test suite – all tests passing.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My individual commit messages are descriptive and follow our commit guidelines
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Copilot AI review requested due to automatic review settings September 11, 2025 08:54
Copy link

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 implements a configurable maximum offset value for pagination across book and reservation GET endpoints. It adds validation to ensure offset values cannot exceed a configurable maximum while maintaining the default behavior for normal pagination.

  • Adds MAX_OFFSET configuration with environment variable support and 5000 default fallback
  • Implements offset validation logic in both books and reservations endpoints
  • Updates test coverage to validate the new offset range restrictions

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
app/config.py Adds MAX_OFFSET configuration with safe integer parsing and fallback default
app/routes/legacy_routes.py Implements offset validation against MAX_OFFSET in books endpoint
app/routes/reservation_routes.py Implements offset validation against MAX_OFFSET in reservations endpoint
tests/test_pagination.py Adds test coverage for out-of-range offset validation in books endpoint
tests/test_reservations.py Adds test coverage for out-of-range offset validation in reservations endpoint
tests/conftest.py Removes debug print statement

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@codesungrape codesungrape merged commit 7c2cb6f into main Sep 11, 2025
2 checks passed
@codesungrape codesungrape deleted the Extend-pagination-endpoints-with-max-default-offset branch September 11, 2025 09:19
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