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

Make request consistently available in pagination classes #8764

Merged
merged 2 commits into from
Nov 17, 2022
Merged

Make request consistently available in pagination classes #8764

merged 2 commits into from
Nov 17, 2022

Conversation

c-w
Copy link
Contributor

@c-w c-w commented Nov 16, 2022

Description

As discussed in #8761, this pull request modifies the CursorPagination such that it stores the current request in an instance field. This ensures that all three pagination classes consistently have access to a request reference.

As a related change, this pull request also updates the PageNumberPagination and LimitOffsetPagination to be consistent and both store the request at the start of the paginate_queryset method: previously LimitOffsetPagination stored the request early in the flow whereas PageNumberPagination stored it late.

Copy link
Member

@tomchristie tomchristie left a comment

Choose a reason for hiding this comment

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

Yup. Thanks.

@@ -195,6 +195,7 @@ def paginate_queryset(self, queryset, request, view=None):
Paginate a queryset if required, either returning a
page object, or `None` if pagination is not configured for this view.
"""
self.request = request
Copy link
Member

Choose a reason for hiding this comment

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

I like that you've moved these, so that the assignment is clearly consistent across all three cases.

@tomchristie tomchristie merged commit 759fc6f into encode:master Nov 17, 2022
@c-w c-w deleted the cursor-pagination-request branch December 1, 2022 18:56
@adamantike
Copy link

This is a really useful change for when trying to override methods like get_paginated_response. Looking forward for a new release to be tagged!

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.

4 participants