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

Table.page handle incorrect page number #68

Closed
andir opened this issue Mar 29, 2012 · 5 comments
Closed

Table.page handle incorrect page number #68

andir opened this issue Mar 29, 2012 · 5 comments

Comments

@andir
Copy link

andir commented Mar 29, 2012

Hi,
Are there any thoughts yet on changing the handling of invalid page numbers to provide a default behaviour (empty table/first page/HttpRedirect/..)?

In the django documentation (https://docs.djangoproject.com/en/dev/topics/pagination/?from=olddocs#using-paginator-in-a-view) is a reference implementatoin which handels the EmptyPage and PageNotAnInteger exceptions accordingly.

In my current straight forward use case (example code below) I'd have to catch the Http404 Exception raised by the Paginator during the render() call, call table.page() and handle the 2 exceptions accordingly which doesn't feel very good. I could also implement an own Paginator (derived from django.core.Paginator) which does the handling but still feels like a bit too much "work".

It would also be alot more uglier with multiple tables on the same page..

My proposal would be some sort of Meta options flag on how to handle invalid page numbers.


 table = Table(queryset)
 table.RequestConfig(request).configure(table)
 return render(request,'xyz.html',{'table':table}, context_instance=request_context)
@bradleyayers
Copy link
Collaborator

I hadn't thought about this before, but I totally agree. I think Table.page should be rewritten to not handle the exception, that code should be moved to SingleTableMixin. However as the code should be customisable.

I feel like perhaps pagination.page(number) should actually be executed in Table.paginate, so that EmptyPage and PageNotAnInteger are raised those that call, rather than being delayed until Table.page.

One possible pattern would be to just require get_table to be implemented in subclasses:

class Example(tables.Table):
    # ...

    def get_table(self):
        try:
            return super(Example, self).get_table()
        except PageNotAnInteger:
            # ...
        except EmptyPage:
            # ...

I'm not sure how the Meta flag you option would allow arbitrary handling of pagination exceptions (and really the general case here is table configuration errors)

@andir
Copy link
Author

andir commented Mar 29, 2012

Yes, I agree moving pagination.page(n) to Table.paginate would actually work quiet well. The pattern you mentioned would be even better (one place to implement the handling vs. dozens of (nearly) equal code parts).

With the Meta flag I just thought about a way to switch between the current and possible new behavior for compatibility reasons. So the "new" behavior would be opt-in for a while. There might be people checking for the Http404-Exception atm.

@bradleyayers
Copy link
Collaborator

Any thoughts on the Meta variable name? Alternatively it could be passed to Table.paginate(), and set as a default in SimpleTableMixin.

@andir
Copy link
Author

andir commented Mar 30, 2012

Something along the lines of "page_out_of_bounds_action"(=404/page1/...) ? But thats terribly long....
I'm also thinking about a way to do things like an HttpRedirect in that case.. so maybe re-raising should be also possible?

@bradleyayers
Copy link
Collaborator

RequestConfig now handles pagination errors silently. This happens when silent=True is included in its paginate argument, e.g. RequestConfig(..., paginate={"silent": True}) (or when it's not specified). It handles exceptions as follows:

  • EmptyPage -> render last page
  • PageNotAnInteger -> render first page

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

No branches or pull requests

2 participants