Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#15698 closed Bug (fixed)

Inconsistant handling of context_object_name in paginated MultipleObjectMixin

Reported by: etianen Owned by: etianen
Component: Generic views Version: 1.2
Severity: Normal Keywords:
Cc: etianen 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 etianen 4 years ago.
Patch with tests

Download all attachments as: .zip

Change History (12)

comment:1 Changed 4 years ago by lukeplant

  • Type set to Bug

comment:2 Changed 4 years ago by lukeplant

  • Severity set to Normal

comment:3 Changed 4 years ago by julien

  • Has patch set
  • Needs documentation unset
  • Needs tests set
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:4 Changed 4 years ago by etianen

  • Owner changed from nobody to etianen
  • Status changed from new to assigned

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

Changed 4 years ago by etianen

Patch with tests

comment:5 Changed 4 years ago by etianen

  • Needs tests unset

comment:6 Changed 4 years ago by julien

  • 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 4 years ago by julien

  • Triage Stage changed from Accepted to Design 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 4 years ago by etianen

  • Cc etianen 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 4 years ago by julien

  • Triage Stage changed from Design decision needed to Ready for checkin

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

comment:10 Changed 4 years ago by jezdez

  • Resolution set to fixed
  • Status changed from assigned to closed

In [16079]:

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

comment:11 Changed 4 years ago by jezdez

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