Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#15698 closed Bug (fixed)

Inconsistant handling of context_object_name in paginated MultipleObjectMixin

Reported by: Dave Hall Owned by: Dave Hall
Component: Generic views Version: 1.2
Severity: Normal Keywords:
Cc: Dave Hall Triage Stage: Ready for checkin
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Without pagination, the MultipleObjectMixin uses a context_object_name based on the name of the model being displayed, such as page_list.

However, when you use pagination by specifying paginate_by, the context_object_name becomes the less-than-helpful object_list.

The fix is simple:

https://github.com/etianen/django/commit/9db221e015185bf422795ef4783b31b154ba3bdd

Basically, the paginator overwrites the queryset with a simple Python list. This patch prevents that.

Attachments (1)

patch.diff (6.9 KB ) - added by Dave Hall 13 years ago.
Patch with tests

Download all attachments as: .zip

Change History (12)

comment:1 by Luke Plant, 13 years ago

Type: Bug

comment:2 by Luke Plant, 13 years ago

Severity: Normal

comment:3 by Julien Phalip, 13 years ago

Has patch: set
Needs tests: set
Triage Stage: UnreviewedAccepted

comment:4 by Dave Hall, 13 years ago

Owner: changed from nobody to Dave Hall
Status: newassigned

I'll add in a test to my patch today. Watch this space. :)

by Dave Hall, 13 years ago

Attachment: patch.diff added

Patch with tests

comment:5 by Dave Hall, 13 years ago

Needs tests: unset

comment:6 by Julien Phalip, 13 years ago

Needs documentation: set

Thanks, the patch looks good. Could you also update the doc, in particular in http://docs.djangoproject.com/en/dev/topics/pagination/ ?

comment:7 by Julien Phalip, 13 years ago

Triage Stage: AcceptedDesign decision needed

On a second thought, having two variables (for example "object_list" and "book_list" in your patch) pointing to the same thing doesn't feel very pythonic. Ideally I'd get rid of "object_list", but that would break backwards compatibility. I'd like to hear more viewpoints about this before proceeding further. Adding doc would still be useful though.

Marking as DDN for now as per my concern about the potential "unpythonicity". If someone -- other than etianen, the reporter =) -- thinks this is not a big deal, then please move back to "Accepted".

comment:8 by Dave Hall, 13 years ago

Cc: Dave Hall added

I'm not quite sure which docs need updating. The page you've linked to doesn't mention generic views at all.

This patch doesn't actually add any functionality. The only thing it does is make the dated generic views work as documented here: http://docs.djangoproject.com/en/1.3/ref/class-based-views/#multipleobjectmixin

I think there may be some confusion here. The 'object_list' and 'book_list' duality has existing since the 1.3 release. All I'm doing is making it work as it should! :P

comment:9 by Julien Phalip, 13 years ago

Triage Stage: Design decision neededReady for checkin

You're right, I totally got confused. Thanks for correcting me ;)

comment:10 by Jannis Leidel, 13 years ago

Resolution: fixed
Status: assignedclosed

In [16079]:

Fixed #15698 -- Fixed inconsistant handling of context_object_name in paginated MultipleObjectMixin views. Thanks, Dave Hall.

comment:11 by Jannis Leidel, 13 years ago

In [16080]:

[1.3.X] Fixed #15698 -- Fixed inconsistant handling of context_object_name in paginated MultipleObjectMixin views. Thanks, Dave Hall.

Backport from trunk (r16079).

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