Opened 18 years ago

Closed 14 years ago

Last modified 13 years ago

#2367 closed enhancement (fixed)

[patch] pagination for date based generic views

Reported by: andrewg@… Owned by: nobody
Component: Generic views Version: dev
Severity: normal Keywords:
Cc: DaNmarner@…, paolo@…, yatiohi@…, joesaccount@…, aidan@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Date based views do not have pagination. This patch adds pagination to the archive_day, archive_week, archive_month generic views. archive_index does not require pagination because of the num_latest argument.

Attachments (6)

date_based_paginate.diff (8.8 KB ) - added by apgwoz@… 18 years ago.
date_based_pagination.diff (11.8 KB ) - added by paolo@… 17 years ago.
pagination for each date_based
generic_views_pagination.diff (15.2 KB ) - added by paolo@… 17 years ago.
5992-pagination-for-datebased-generic-views.diff (16.4 KB ) - added by Øyvind Saltvik <oyvind@…> 17 years ago.
removed Http404 dependency in pagination.py, reraised InvalidPage, wrapped compute_pagination in a try except, use count instead of len on queryset.
django_pagination.diff (10.1 KB ) - added by Tyler <tyler@…> 16 years ago.
The previous patch uses the deprecated ObjectPaginator class and changes paginator.py. This patch changes only date_based.py and uses the newer interface as defined in list_detail.py's object_list method.
django_pagination.2.diff (10.1 KB ) - added by Tyler <tyler@…> 16 years ago.
small diff fix

Download all attachments as: .zip

Change History (25)

by apgwoz@…, 18 years ago

Attachment: date_based_paginate.diff added

comment:1 by Malcolm Tredinnick, 18 years ago

If we commit this, I think we should add pagination to archive_index as well, since num_latest does not help you view "the second page of the latest headlines".

comment:2 by Chris Beaven, 17 years ago

Triage Stage: UnreviewedDesign decision needed

in reply to:  1 comment:3 by anonymous, 17 years ago

Replying to mtredinnick:

If we commit this, I think we should add pagination to archive_index as well, since num_latest does not help you view "the second page of the latest headlines".

I agree. My original logic in that was flawed. Pagination should exist in all generic date based views.

comment:4 by paolo@…, 17 years ago

Cc: paolo@… added

My patch adds pagination capabilities to each date based generic view, except object_detail.

If it will be accepted for checkin I'll be glad to add a patch to update the documentation as well.

by paolo@…, 17 years ago

Attachment: date_based_pagination.diff added

pagination for each date_based

comment:5 by Gary Wilson <gary.wilson@…>, 17 years ago

Patch needs improvement: set

I like the factoring out of the common elements. Why not put everything in the Paginator object though, something along the idea of the patch attached to #1374? Also, whatever we do, it should be done for all the generic views, not just the date_based.

comment:6 by paolo@…, 17 years ago

Next week I'll work on that.

What I can do is to move inside Paginator the function for queryset computation and context updating. Then I'll change list_detail generic views to use this code.

You say that it should be done for *all the generic views*, *not just the date_based*. But after the new patch, both date_based and list_detail will use the same code for pagination. Simple generic views and CRUD shouldn't have the needing of pagination, isn't it?

comment:7 by Gary Wilson <gary.wilson@…>, 17 years ago

By all generic views, I meant all the ones where it makes sense, but I guess besides the date_based views, object_list is the only other one that makes sense.

comment:8 by paolo <paolo@…>, 17 years ago

Searching Django's related groups for more info about pagination I saw there have been a lot of discussions about "how Paginator should be" (to cite one, http://groups.google.com/group/django-users/browse_frm/thread/6963a1fdb6473352/d4622e25fc54bade). Even so, Paginator class hasn't changed in time (except for orphans handling), probably because features that actually provides are ok for the most of people. Thus I'd prefer not to change Paginator's code (that's working well btw).

In the light of what said, the latest patch (generic_views_pagination.diff) adds a new function at paginator module leaving Paginator class untouched. Then, it changes date_based and list_detail generic views to using this function to retrieve the paginated queryset and update the context with pagination variables.

Any chance anyone would comment and/or test the patch?

Thanks.

by paolo@…, 17 years ago

comment:9 by paolo@…, 17 years ago

Don't consider the last patch I uploaded, it has at least two issues, introduces a dependency on django.http and uses allow_empty which is not defined there. Et voilà!

by Øyvind Saltvik <oyvind@…>, 17 years ago

removed Http404 dependency in pagination.py, reraised InvalidPage, wrapped compute_pagination in a try except, use count instead of len on queryset.

comment:10 by Øyvind Saltvik <oyvind@…>, 17 years ago

Added a patch that fixes some of the issues mentioned.

Added a page argument to object_detail that is passed to context.

comment:11 by Jacob, 17 years ago

Triage Stage: Design decision neededAccepted

comment:12 by ctrochalakis, 17 years ago

Cc: yatiohi@… added

by Tyler <tyler@…>, 16 years ago

Attachment: django_pagination.diff added

The previous patch uses the deprecated ObjectPaginator class and changes paginator.py. This patch changes only date_based.py and uses the newer interface as defined in list_detail.py's object_list method.

by Tyler <tyler@…>, 16 years ago

Attachment: django_pagination.2.diff added

small diff fix

comment:13 by jos3ph, 16 years ago

Cc: joesaccount@… added

comment:14 by afitzpatrick, 15 years ago

Cc: aidan@… added

comment:15 by Jannis Leidel, 14 years ago

Needs tests: set

comment:16 by Eric Holscher, 14 years ago

Another hard part of paginating based on date is what happens when an item is added to the list when you're paginating? If i go to the "previous" page there is a possibility that I will miss and item because it has been bumped off of the page. I dunno if there's a good way to fix this (peg the pagination to a certain pk in the URL? bleeh). But it's at least worth noting in the docs if this gets merged.

comment:17 by Daniel Duan, 14 years ago

Cc: DaNmarner@… added
milestone: 1.3

comment:18 by Russell Keith-Magee, 14 years ago

Resolution: fixed
Status: newclosed

Function-based generic views were deprecated by the introduction of class-based views in [14254]. Class-based views should solve this problem.

comment:19 by Jacob, 13 years ago

milestone: 1.3

Milestone 1.3 deleted

Note: See TracTickets for help on using tickets.
Back to Top