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

Improve pagination #285

Closed
wants to merge 1 commit into from
Closed

Improve pagination #285

wants to merge 1 commit into from

Conversation

mom1
Copy link
Contributor

@mom1 mom1 commented Nov 20, 2021

A proposal for allowing use view without **kwargs, but with pagination.

  • name_param for pagination data
  • no need **kwargs for use pagination
  • by default pagination argument not be passed to view function
  • ability to pass a parameter into the view

P.S. I also wanted to express my deep gratitude to you for this library.

@mom1
Copy link
Contributor Author

mom1 commented Nov 23, 2021

@vitalik maybe need something like this?

class PaginationBase(ABC):
    name_param: str = "pagination"
    pass_to_parameters = False
    ...
    def __init__(self, pass_to_parameters: bool = None, **kwargs: DictStrAny) -> None:
        if pass_to_parameters is not None:
            self.pass_to_parameters = pass_to_parameters
def view_with_pagination(*args: Tuple[Any], **kwargs: DictStrAny) -> Any:
    paginate_param = kwargs.get(paginator.name_param)
    if not paginator.pass_to_parameters:
        paginate_param = kwargs.pop(paginator.name_param)
    items = func(*args, **kwargs)
        return paginator.paginate_queryset(
            items, **{paginator.name_param: paginate_param}, **kwargs
    )

@vitalik
Copy link
Owner

vitalik commented Nov 23, 2021

Hi @mom1
Yeah, I'm still thinking for the best approach here...

I like what you did with **kwrags being not required

I think the final can be something like this

  • by default pagination argument maby not be passed to view function
  • and let user a way to pass it with something like:
@paginate(PageNumberPagination, per_page=50, pass_parameter='pagination')
def my_view(request, pagination):
       return queryset
     

@mom1
Copy link
Contributor Author

mom1 commented Nov 24, 2021

@vitalik I changed code according to your comment 😉

- name_param for pagination data
- no need `**kwargs` for use pagination
- by default pagination argument not be passed to view function
- ability to pass a parameter into the view
@SmileyChris
Copy link
Contributor

I wonder if it'd be better that the view requests the parameter, rather than the paginate decorator deciding on it. Something like:

@paginate(PageNumberPagination, per_page=50)
def my_view(request, pagination: PageNumberPagination.Input):
       return queryset

@vitalik
Copy link
Owner

vitalik commented Feb 1, 2022

Yeah, in my practise I never actually used to access kwargs for pagination, and forcing to add it is annoying
@mom1 - this is merged, I just adapted few things and a bit simplified, thank you


@SmileyChris

I wonder if it'd be better that the view requests the parameter, rather than the paginate decorator deciding on it. Something like:

def my_view(request, pagination: PageNumberPagination.Input):

yeah, that would be nice, but seems it will involve too much code reshaping

@vitalik vitalik closed this Feb 1, 2022
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.

None yet

3 participants