From cf70c96ce08bec8834ada695451cc915f7558dbd Mon Sep 17 00:00:00 2001 From: Jannis Leidel Date: Thu, 8 Sep 2011 13:25:00 +0000 Subject: [PATCH] Fixed #15997 -- Added `list_max_show_all` option to `ModelAdmin` in replacement for a global module level variable. Thanks, jsdalton. git-svn-id: http://code.djangoproject.com/svn/django/trunk@16725 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/contrib/admin/options.py | 3 +- django/contrib/admin/validation.py | 5 ++ django/contrib/admin/views/main.py | 9 +-- docs/ref/contrib/admin/index.txt | 9 +++ .../regressiontests/admin_changelist/tests.py | 62 ++++++++++++++----- tests/regressiontests/admin_filters/tests.py | 2 +- tests/regressiontests/modeladmin/tests.py | 18 ++++++ 7 files changed, 85 insertions(+), 23 deletions(-) diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index cf7ea83c11..c9f2562142 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -278,6 +278,7 @@ class ModelAdmin(BaseModelAdmin): list_filter = () list_select_related = False list_per_page = 100 + list_max_show_all = 200 list_editable = () search_fields = () date_hierarchy = None @@ -1087,7 +1088,7 @@ class ModelAdmin(BaseModelAdmin): try: cl = ChangeList(request, self.model, list_display, self.list_display_links, self.list_filter, self.date_hierarchy, self.search_fields, - self.list_select_related, self.list_per_page, self.list_editable, self) + self.list_select_related, self.list_per_page, self.list_max_show_all, self.list_editable, self) except IncorrectLookupParameters: # Wacky lookup parameters were given, so redirect to the main # changelist page, without parameters, and pass an 'invalid=1' diff --git a/django/contrib/admin/validation.py b/django/contrib/admin/validation.py index 9a0c948120..733f89d06f 100644 --- a/django/contrib/admin/validation.py +++ b/django/contrib/admin/validation.py @@ -96,6 +96,11 @@ def validate(cls, model): raise ImproperlyConfigured("'%s.list_per_page' should be a integer." % cls.__name__) + # list_max_show_all + if hasattr(cls, 'list_max_show_all') and not isinstance(cls.list_max_show_all, int): + raise ImproperlyConfigured("'%s.list_max_show_all' should be an integer." + % cls.__name__) + # list_editable if hasattr(cls, 'list_editable') and cls.list_editable: check_isseq(cls, 'list_editable', cls.list_editable) diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py index f741fd0764..616b24957a 100644 --- a/django/contrib/admin/views/main.py +++ b/django/contrib/admin/views/main.py @@ -12,10 +12,6 @@ from django.contrib.admin import FieldListFilter from django.contrib.admin.options import IncorrectLookupParameters from django.contrib.admin.util import quote, get_fields_from_path -# The system will display a "Show all" link on the change list only if the -# total result count is less than or equal to this setting. -MAX_SHOW_ALL_ALLOWED = 200 - # Changelist settings ALL_VAR = 'all' ORDER_VAR = 'o' @@ -44,7 +40,7 @@ def field_needs_distinct(field): class ChangeList(object): def __init__(self, request, model, list_display, list_display_links, list_filter, date_hierarchy, search_fields, list_select_related, - list_per_page, list_editable, model_admin): + list_per_page, list_max_show_all, list_editable, model_admin): self.model = model self.opts = model._meta self.lookup_opts = self.opts @@ -56,6 +52,7 @@ class ChangeList(object): self.search_fields = search_fields self.list_select_related = list_select_related self.list_per_page = list_per_page + self.list_max_show_all = list_max_show_all self.model_admin = model_admin # Get search parameters from the query string. @@ -145,7 +142,7 @@ class ChangeList(object): else: full_result_count = self.root_query_set.count() - can_show_all = result_count <= MAX_SHOW_ALL_ALLOWED + can_show_all = result_count <= self.list_max_show_all multi_page = result_count > self.list_per_page # Get the list of objects to display on this page. diff --git a/docs/ref/contrib/admin/index.txt b/docs/ref/contrib/admin/index.txt index 40ef31ca99..d68197e57b 100644 --- a/docs/ref/contrib/admin/index.txt +++ b/docs/ref/contrib/admin/index.txt @@ -695,6 +695,15 @@ subclass:: The ``FieldListFilter`` API is currently considered internal and prone to refactoring. +.. attribute:: ModelAdmin.list_max_show_all + + .. versionadded:: 1.4 + + Set ``list_max_show_all`` to control how many items can appear on a "Show + all" admin change list page. The admin will display a "Show all" link on the + change list only if the total result count is less than or equal to this + setting. By default, this is set to ``200``. + .. attribute:: ModelAdmin.list_per_page Set ``list_per_page`` to control how many items appear on each paginated diff --git a/tests/regressiontests/admin_changelist/tests.py b/tests/regressiontests/admin_changelist/tests.py index 709fa766dd..09ec190785 100644 --- a/tests/regressiontests/admin_changelist/tests.py +++ b/tests/regressiontests/admin_changelist/tests.py @@ -1,6 +1,8 @@ +from __future__ import with_statement + from django.contrib import admin from django.contrib.admin.options import IncorrectLookupParameters -from django.contrib.admin.views.main import ChangeList, SEARCH_VAR +from django.contrib.admin.views.main import ChangeList, SEARCH_VAR, ALL_VAR from django.core.paginator import Paginator from django.template import Context, Template from django.test import TestCase @@ -24,7 +26,7 @@ class ChangeListTests(TestCase): request = self.factory.get('/child/') cl = ChangeList(request, Child, m.list_display, m.list_display_links, m.list_filter, m.date_hierarchy, m.search_fields, - m.list_select_related, m.list_per_page, m.list_editable, m) + m.list_select_related, m.list_per_page, m.list_max_show_all, m.list_editable, m) self.assertEqual(cl.query_set.query.select_related, {'parent': {'name': {}}}) def test_result_list_empty_changelist_value(self): @@ -37,7 +39,7 @@ class ChangeListTests(TestCase): m = ChildAdmin(Child, admin.site) cl = ChangeList(request, Child, m.list_display, m.list_display_links, m.list_filter, m.date_hierarchy, m.search_fields, - m.list_select_related, m.list_per_page, m.list_editable, m) + m.list_select_related, m.list_per_page, m.list_max_show_all, m.list_editable, m) cl.formset = None template = Template('{% load admin_list %}{% spaceless %}{% result_list cl %}{% endspaceless %}') context = Context({'cl': cl}) @@ -57,7 +59,7 @@ class ChangeListTests(TestCase): m = ChildAdmin(Child, admin.site) cl = ChangeList(request, Child, m.list_display, m.list_display_links, m.list_filter, m.date_hierarchy, m.search_fields, - m.list_select_related, m.list_per_page, m.list_editable, m) + m.list_select_related, m.list_per_page, m.list_max_show_all, m.list_editable, m) cl.formset = None template = Template('{% load admin_list %}{% spaceless %}{% result_list cl %}{% endspaceless %}') context = Context({'cl': cl}) @@ -86,7 +88,7 @@ class ChangeListTests(TestCase): m.list_editable = ['name'] cl = ChangeList(request, Child, m.list_display, m.list_display_links, m.list_filter, m.date_hierarchy, m.search_fields, - m.list_select_related, m.list_per_page, m.list_editable, m) + m.list_select_related, m.list_per_page, m.list_max_show_all, m.list_editable, m) FormSet = m.get_changelist_formset(request) cl.formset = FormSet(queryset=cl.result_list) template = Template('{% load admin_list %}{% spaceless %}{% result_list cl %}{% endspaceless %}') @@ -119,7 +121,7 @@ class ChangeListTests(TestCase): self.assertRaises(IncorrectLookupParameters, lambda: \ ChangeList(request, Child, m.list_display, m.list_display_links, m.list_filter, m.date_hierarchy, m.search_fields, - m.list_select_related, m.list_per_page, m.list_editable, m)) + m.list_select_related, m.list_per_page, m.list_max_show_all, m.list_editable, m)) def test_custom_paginator(self): new_parent = Parent.objects.create(name='parent') @@ -135,7 +137,7 @@ class ChangeListTests(TestCase): cl = ChangeList(request, Child, m.list_display, m.list_display_links, m.list_filter, m.date_hierarchy, m.search_fields, - m.list_select_related, m.list_per_page, m.list_editable, m) + m.list_select_related, m.list_per_page, m.list_max_show_all, m.list_editable, m) cl.get_results(request) self.assertIsInstance(cl.paginator, CustomPaginator) @@ -157,7 +159,7 @@ class ChangeListTests(TestCase): cl = ChangeList(request, Band, m.list_display, m.list_display_links, m.list_filter, m.date_hierarchy, m.search_fields, m.list_select_related, m.list_per_page, - m.list_editable, m) + m.list_max_show_all, m.list_editable, m) cl.get_results(request) @@ -180,7 +182,7 @@ class ChangeListTests(TestCase): cl = ChangeList(request, Group, m.list_display, m.list_display_links, m.list_filter, m.date_hierarchy, m.search_fields, m.list_select_related, m.list_per_page, - m.list_editable, m) + m.list_max_show_all, m.list_editable, m) cl.get_results(request) @@ -204,7 +206,7 @@ class ChangeListTests(TestCase): cl = ChangeList(request, Quartet, m.list_display, m.list_display_links, m.list_filter, m.date_hierarchy, m.search_fields, m.list_select_related, m.list_per_page, - m.list_editable, m) + m.list_max_show_all, m.list_editable, m) cl.get_results(request) @@ -228,7 +230,7 @@ class ChangeListTests(TestCase): cl = ChangeList(request, ChordsBand, m.list_display, m.list_display_links, m.list_filter, m.date_hierarchy, m.search_fields, m.list_select_related, m.list_per_page, - m.list_editable, m) + m.list_max_show_all, m.list_editable, m) cl.get_results(request) @@ -251,7 +253,7 @@ class ChangeListTests(TestCase): cl = ChangeList(request, Parent, m.list_display, m.list_display_links, m.list_filter, m.date_hierarchy, m.search_fields, m.list_select_related, m.list_per_page, - m.list_editable, m) + m.list_max_show_all, m.list_editable, m) # Make sure distinct() was called self.assertEqual(cl.query_set.count(), 1) @@ -271,7 +273,7 @@ class ChangeListTests(TestCase): cl = ChangeList(request, Parent, m.list_display, m.list_display_links, m.list_filter, m.date_hierarchy, m.search_fields, m.list_select_related, m.list_per_page, - m.list_editable, m) + m.list_max_show_all, m.list_editable, m) # Make sure distinct() was called self.assertEqual(cl.query_set.count(), 1) @@ -292,7 +294,8 @@ class ChangeListTests(TestCase): m = ChildAdmin(Child, admin.site) cl = ChangeList(request, Child, m.list_display, m.list_display_links, m.list_filter, m.date_hierarchy, m.search_fields, - m.list_select_related, m.list_per_page, m.list_editable, m) + m.list_select_related, m.list_per_page, m.list_max_show_all, + m.list_editable, m) self.assertEqual(cl.query_set.count(), 60) self.assertEqual(cl.paginator.count, 60) self.assertEqual(cl.paginator.page_range, [1, 2, 3, 4, 5, 6]) @@ -301,7 +304,8 @@ class ChangeListTests(TestCase): m = FilteredChildAdmin(Child, admin.site) cl = ChangeList(request, Child, m.list_display, m.list_display_links, m.list_filter, m.date_hierarchy, m.search_fields, - m.list_select_related, m.list_per_page, m.list_editable, m) + m.list_select_related, m.list_per_page, m.list_max_show_all, + m.list_editable, m) self.assertEqual(cl.query_set.count(), 30) self.assertEqual(cl.paginator.count, 30) self.assertEqual(cl.paginator.page_range, [1, 2, 3]) @@ -351,6 +355,34 @@ class ChangeListTests(TestCase): response.render() self.assertContains(response, 'Parent object') + def test_show_all(self): + parent = Parent.objects.create(name='anything') + for i in range(30): + Child.objects.create(name='name %s' % i, parent=parent) + Child.objects.create(name='filtered %s' % i, parent=parent) + + # Add "show all" parameter to request + request = self.factory.get('/child/', data={ALL_VAR: ''}) + + # Test valid "show all" request (number of total objects is under max) + m = ChildAdmin(Child, admin.site) + # 200 is the max we'll pass to ChangeList + cl = ChangeList(request, Child, m.list_display, m.list_display_links, + m.list_filter, m.date_hierarchy, m.search_fields, + m.list_select_related, m.list_per_page, 200, m.list_editable, m) + cl.get_results(request) + self.assertEqual(len(cl.result_list), 60) + + # Test invalid "show all" request (number of total objects over max) + # falls back to paginated pages + m = ChildAdmin(Child, admin.site) + # 30 is the max we'll pass to ChangeList for this test + cl = ChangeList(request, Child, m.list_display, m.list_display_links, + m.list_filter, m.date_hierarchy, m.search_fields, + m.list_select_related, m.list_per_page, 30, m.list_editable, m) + cl.get_results(request) + self.assertEqual(len(cl.result_list), 10) + class ParentAdmin(admin.ModelAdmin): list_filter = ['child__name'] diff --git a/tests/regressiontests/admin_filters/tests.py b/tests/regressiontests/admin_filters/tests.py index e089ce24fb..7f52f9c20d 100644 --- a/tests/regressiontests/admin_filters/tests.py +++ b/tests/regressiontests/admin_filters/tests.py @@ -119,7 +119,7 @@ class ListFiltersTests(TestCase): def get_changelist(self, request, model, modeladmin): return ChangeList(request, model, modeladmin.list_display, modeladmin.list_display_links, modeladmin.list_filter, modeladmin.date_hierarchy, modeladmin.search_fields, - modeladmin.list_select_related, modeladmin.list_per_page, modeladmin.list_editable, modeladmin) + modeladmin.list_select_related, modeladmin.list_per_page, modeladmin.list_max_show_all, modeladmin.list_editable, modeladmin) def test_datefieldlistfilter(self): modeladmin = BookAdmin(Book, site) diff --git a/tests/regressiontests/modeladmin/tests.py b/tests/regressiontests/modeladmin/tests.py index ad86f854a7..872fb0c3fe 100644 --- a/tests/regressiontests/modeladmin/tests.py +++ b/tests/regressiontests/modeladmin/tests.py @@ -1115,6 +1115,24 @@ class ValidationTests(unittest.TestCase): validate(ValidationTestModelAdmin, ValidationTestModel) + def test_max_show_all_allowed_validation(self): + + class ValidationTestModelAdmin(ModelAdmin): + list_max_show_all = 'hello' + + self.assertRaisesRegexp( + ImproperlyConfigured, + "'ValidationTestModelAdmin.list_max_show_all' should be an integer.", + validate, + ValidationTestModelAdmin, + ValidationTestModel, + ) + + class ValidationTestModelAdmin(ModelAdmin): + list_max_show_all = 200 + + validate(ValidationTestModelAdmin, ValidationTestModel) + def test_search_fields_validation(self): class ValidationTestModelAdmin(ModelAdmin):