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

Feat/tr/pagination #123

Merged
merged 36 commits into from
Jan 31, 2025
Merged

Feat/tr/pagination #123

merged 36 commits into from
Jan 31, 2025

Conversation

theodorehreuter
Copy link
Contributor

@theodorehreuter theodorehreuter commented Jan 3, 2025

Related Issue(s):

Proposed Changes:

Adding pagination for 4 endpoints:

  1. GET /orders
  2. GET /products
  3. GET /orders/{order_id}/statuses
  4. POST /products/{product_id}/opportunities

We are using token based approach to pagination.

BREAKING CHANGES:

  • Moving OrderCollection construction to root_router.py instead of root_backend.py
  • Pagination support on the backend methods has changed returned types in root_backend.py and product_backend.py to tuples, to include returning a new pagination token.

PR Checklist:

  • I have added my changes to the CHANGELOG or a CHANGELOG entry is not required.

…agination tests: creating incomplete test to start testing pagination
… feat: replacing token in url if new token is provided tests: continue to updae test to validate additional pieces of the limit/token expected functionality, still breaking on limit check feat: removing use or Orders type and going with tuple instead
… object if we are returned a token. tests: created simple test implementation to abstract out token handling and generation
…r bad token tests: reworking pagination tests tests: adding handle to max limit in test implemented backend
tests/application.py Outdated Show resolved Hide resolved
tests/application.py Outdated Show resolved Hide resolved
tests/application.py Outdated Show resolved Hide resolved
tests/test_order.py Outdated Show resolved Hide resolved
tests/test_order.py Outdated Show resolved Hide resolved
tests/test_order.py Outdated Show resolved Hide resolved
tests/test_order.py Outdated Show resolved Hide resolved
tests/test_order.py Outdated Show resolved Hide resolved
…getting records until no more are available ftests: added separate test to get 404 back with no orders and using a token
…y features after token lookup tests: separating empty orders check to separate test
… issue with in memory DB by adding missing __init__ to class. tests: added multiple orders to create_order fixture and passed this fixture to other tests using previously existing create_order_allowed_paylaods tests: added setup_pagination fixture that is now necessary again after fixing the in memory db init issue
tests/conftest.py Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
@philvarner
Copy link
Contributor

Done leaving comments.

…ethods to clean up endpoint business logic. tests: small tweaks to test based on PR feedback
while next_url:
url = next_url
if method == "POST":
next_url = next(
Copy link
Contributor

Choose a reason for hiding this comment

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

next is being extracted here to be used as a parameter -- it's supposed to be part of the POST body.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed so its extracting the body.

… next/limit must be passed as key/value pairs in the POST body. These are also now returned in the POST body in the 'next' link object returned by /opportunities tests: update tests to reflect changes in endpoint. POST body is now extracted wholesale from returned link object istead of just the params
tests/conftest.py Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
@philvarner
Copy link
Contributor

Couple of comments on some tests, other changes look good.

@theodorehreuter theodorehreuter merged commit 67ea960 into main Jan 31, 2025
2 checks passed
@theodorehreuter theodorehreuter deleted the feat/tr/pagination branch January 31, 2025 18:09
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.

3 participants