-
Notifications
You must be signed in to change notification settings - Fork 5
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
Feat/tr/pagination #123
Conversation
…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
…t: use include_query_params instead of parsing query param string ourselves
…r bad token tests: reworking pagination tests tests: adding handle to max limit in test implemented backend
…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
…re not being torn down for every 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
Done leaving comments. |
…ethods to clean up endpoint business logic. tests: small tweaks to test based on PR feedback
tests/conftest.py
Outdated
while next_url: | ||
url = next_url | ||
if method == "POST": | ||
next_url = next( |
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.
next is being extracted here to be used as a parameter -- it's supposed to be part of the POST body.
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.
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
Couple of comments on some tests, other changes look good. |
Related Issue(s):
Proposed Changes:
Adding pagination for 4 endpoints:
GET /orders
GET /products
GET /orders/{order_id}/statuses
POST /products/{product_id}/opportunities
We are using token based approach to pagination.
BREAKING CHANGES:
root_router.py
instead ofroot_backend.py
root_backend.py
andproduct_backend.py
to tuples, to include returning a new pagination token.PR Checklist: