Opened 6 years ago

Closed 6 years ago

Last modified 6 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: UI/UX:

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 6 years ago.
Patch with tests

Download all attachments as: .zip

Change History (12)

comment:1 Changed 6 years ago by Luke Plant

Type: Bug

comment:2 Changed 6 years ago by Luke Plant

Severity: Normal

comment:3 Changed 6 years ago by Julien Phalip

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

comment:4 Changed 6 years ago by Dave Hall

Owner: changed from nobody to Dave Hall
Status: newassigned

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

Changed 6 years ago by Dave Hall

Attachment: patch.diff added

Patch with tests

comment:5 Changed 6 years ago by Dave Hall

Needs tests: unset

comment:6 Changed 6 years ago by Julien Phalip

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 Changed 6 years ago by Julien Phalip

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 Changed 6 years ago by Dave Hall

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 Changed 6 years ago by Julien Phalip

Triage Stage: Design decision neededReady for checkin

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

comment:10 Changed 6 years ago by Jannis Leidel

Resolution: fixed
Status: assignedclosed

In [16079]:

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

comment:11 Changed 6 years ago by Jannis Leidel

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