From 7bc0878922d9d93ab8f4ef8a5c5ba7a1c671279f Mon Sep 17 00:00:00 2001 From: Jacob Kaplan-Moss Date: Tue, 17 Mar 2009 20:51:47 +0000 Subject: [PATCH] Fixed #8939: added a `list_editable` option to `ModelAdmin`; fields declared `list_editable` may be edited, in bulk, on the changelist page. Thanks, Alex Gaynor. git-svn-id: http://code.djangoproject.com/svn/django/trunk@10077 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/contrib/admin/options.py | 74 +++++++++++++++- .../admin/templates/admin/change_list.html | 88 +++++++++++++------ .../admin/templates/admin/pagination.html | 1 + .../contrib/admin/templatetags/admin_list.py | 22 ++++- django/contrib/admin/validation.py | 23 +++++ django/contrib/admin/views/main.py | 3 +- docs/ref/contrib/admin.txt | 26 ++++++ .../fixtures/admin-views-person.xml | 18 ++++ tests/regressiontests/admin_views/models.py | 24 +++++ tests/regressiontests/admin_views/tests.py | 72 ++++++++++++++- 10 files changed, 317 insertions(+), 34 deletions(-) create mode 100644 tests/regressiontests/admin_views/fixtures/admin-views-person.xml diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 859229e430..c79523c2ff 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -1,6 +1,6 @@ from django import forms, template from django.forms.formsets import all_valid -from django.forms.models import modelform_factory, inlineformset_factory +from django.forms.models import modelform_factory, modelformset_factory, inlineformset_factory from django.forms.models import BaseInlineFormSet from django.contrib.contenttypes.models import ContentType from django.contrib.admin import widgets @@ -16,6 +16,7 @@ from django.utils.safestring import mark_safe from django.utils.functional import curry from django.utils.text import capfirst, get_text_list from django.utils.translation import ugettext as _ +from django.utils.translation import ngettext from django.utils.encoding import force_unicode try: set @@ -108,7 +109,7 @@ class BaseModelAdmin(object): if db_field.__class__ in self.formfield_overrides: kwargs = dict(self.formfield_overrides[db_field.__class__], **kwargs) return db_field.formfield(**kwargs) - + # For any other type of field, just call its formfield() method. return db_field.formfield(**kwargs) @@ -177,6 +178,7 @@ class ModelAdmin(BaseModelAdmin): list_filter = () list_select_related = False list_per_page = 100 + list_editable = () search_fields = () date_hierarchy = None save_as = False @@ -313,6 +315,29 @@ class ModelAdmin(BaseModelAdmin): defaults.update(kwargs) return modelform_factory(self.model, **defaults) + def get_changelist_form(self, request, **kwargs): + """ + Returns a Form class for use in the Formset on the changelist page. + """ + defaults = { + "formfield_callback": curry(self.formfield_for_dbfield, request=request), + } + defaults.update(kwargs) + return modelform_factory(self.model, **defaults) + + def get_changelist_formset(self, request, **kwargs): + """ + Returns a FormSet class for use on the changelist page if list_editable + is used. + """ + defaults = { + "formfield_callback": curry(self.formfield_for_dbfield, request=request), + } + defaults.update(kwargs) + return modelformset_factory(self.model, + self.get_changelist_form(request), extra=0, + fields=self.list_editable, **defaults) + def get_formsets(self, request, obj=None): for inline in self.inline_instances: yield inline.get_formset(request, obj) @@ -685,7 +710,7 @@ class ModelAdmin(BaseModelAdmin): raise PermissionDenied try: cl = ChangeList(request, self.model, self.list_display, self.list_display_links, self.list_filter, - self.date_hierarchy, self.search_fields, self.list_select_related, self.list_per_page, self) + self.date_hierarchy, self.search_fields, self.list_select_related, self.list_per_page, 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' @@ -696,10 +721,53 @@ class ModelAdmin(BaseModelAdmin): return render_to_response('admin/invalid_setup.html', {'title': _('Database error')}) return HttpResponseRedirect(request.path + '?' + ERROR_FLAG + '=1') + # If we're allowing changelist editing, we need to construct a formset + # for the changelist given all the fields to be edited. Then we'll + # use the formset to validate/process POSTed data. + formset = cl.formset = None + + # Handle POSTed bulk-edit data. + if request.method == "POST" and self.list_editable: + FormSet = self.get_changelist_formset(request) + formset = cl.formset = FormSet(request.POST, request.FILES, queryset=cl.result_list) + if formset.is_valid(): + changecount = 0 + for form in formset.forms: + if form.has_changed(): + obj = self.save_form(request, form, change=True) + self.save_model(request, obj, form, change=True) + form.save_m2m() + change_msg = self.construct_change_message(request, form, None) + self.log_change(request, obj, change_msg) + changecount += 1 + + if changecount: + msg = ngettext("%(count)s %(singular)s was changed successfully.", + "%(count)s %(plural)s were changed successfully.", + changecount) % {'count': changecount, + 'singular': force_unicode(opts.verbose_name), + 'plural': force_unicode(opts.verbose_name_plural), + 'obj': force_unicode(obj)} + self.message_user(request, msg) + + return HttpResponseRedirect(request.get_full_path()) + + # Handle GET -- construct a formset for display. + elif self.list_editable: + FormSet = self.get_changelist_formset(request) + formset = cl.formset = FormSet(queryset=cl.result_list) + + # Build the list of media to be used by the formset. + if formset: + media = self.media + formset.media + else: + media = None + context = { 'title': cl.title, 'is_popup': cl.is_popup, 'cl': cl, + 'media': media, 'has_add_permission': self.has_add_permission(request), 'root_path': self.admin_site.root_path, 'app_label': app_label, diff --git a/django/contrib/admin/templates/admin/change_list.html b/django/contrib/admin/templates/admin/change_list.html index 5f8a43050c..dca5b80245 100644 --- a/django/contrib/admin/templates/admin/change_list.html +++ b/django/contrib/admin/templates/admin/change_list.html @@ -1,38 +1,76 @@ {% extends "admin/base_site.html" %} {% load adminmedia admin_list i18n %} -{% block extrastyle %}{{ block.super }}{% endblock %} +{% block extrastyle %} + {{ block.super }} + + {% if cl.formset %} + + + {{ media }} + {% endif %} +{% endblock %} {% block bodyclass %}change-list{% endblock %} -{% if not is_popup %}{% block breadcrumbs %}
{% trans "Home" %} › {{ app_label|capfirst }} › {{ cl.opts.verbose_name_plural|capfirst }}
{% endblock %}{% endif %} +{% if not is_popup %} + {% block breadcrumbs %} +
+ + {% trans "Home" %} + + › + + {{ app_label|capfirst }} + + › + {{ cl.opts.verbose_name_plural|capfirst }} +
+ {% endblock %} +{% endif %} {% block coltype %}flex{% endblock %} {% block content %} -
-{% block object-tools %} -{% if has_add_permission %} - -{% endif %} -{% endblock %} -
-{% block search %}{% search_form cl %}{% endblock %} -{% block date_hierarchy %}{% date_hierarchy cl %}{% endblock %} +
+ {% block object-tools %} + {% if has_add_permission %} + + {% endif %} + {% endblock %} + {% if cl.formset.errors %} +

+ {% blocktrans count cl.formset.errors|length as counter %}Please correct the error below.{% plural %}Please correct the errors below.{% endblocktrans %} +

+
    {% for error in cl.formset.non_field_errors %}
  • {{ error }}
    • {% endfor %}
+ {% endif %} +
+ {% block search %}{% search_form cl %}{% endblock %} + {% block date_hierarchy %}{% date_hierarchy cl %}{% endblock %} -{% block filters %} -{% if cl.has_filters %} -
-

{% trans 'Filter' %}

-{% for spec in cl.filter_specs %} - {% admin_list_filter cl spec %} -{% endfor %} -
-{% endif %} -{% endblock %} + {% block filters %} + {% if cl.has_filters %} +
+

{% trans 'Filter' %}

+ {% for spec in cl.filter_specs %}{% admin_list_filter cl spec %}{% endfor %} +
+ {% endif %} + {% endblock %} + + {% if cl.formset %} +
+ {{ cl.formset.management_form }} + {% endif %} -{% block result_list %}{% result_list cl %}{% endblock %} -{% block pagination %}{% pagination cl %}{% endblock %} -
-
+ {% block result_list %}{% result_list cl %}{% endblock %} + {% block pagination %}{% pagination cl %}{% endblock %} + {% if cl.formset %}{% endif %} +
+
{% endblock %} diff --git a/django/contrib/admin/templates/admin/pagination.html b/django/contrib/admin/templates/admin/pagination.html index 7694e4c5b0..58ade6ad0c 100644 --- a/django/contrib/admin/templates/admin/pagination.html +++ b/django/contrib/admin/templates/admin/pagination.html @@ -8,4 +8,5 @@ {% endif %} {{ cl.result_count }} {% ifequal cl.result_count 1 %}{{ cl.opts.verbose_name }}{% else %}{{ cl.opts.verbose_name_plural }}{% endifequal %} {% if show_all_url %}  {% trans 'Show all' %}{% endif %} +{% if cl.formset %}{% endif %}

diff --git a/django/contrib/admin/templatetags/admin_list.py b/django/contrib/admin/templatetags/admin_list.py index 37cdb91c3c..063ef0e4a1 100644 --- a/django/contrib/admin/templatetags/admin_list.py +++ b/django/contrib/admin/templatetags/admin_list.py @@ -133,7 +133,7 @@ def _boolean_icon(field_val): BOOLEAN_MAPPING = {True: 'yes', False: 'no', None: 'unknown'} return mark_safe(u'%s' % (settings.ADMIN_MEDIA_PREFIX, BOOLEAN_MAPPING[field_val], field_val)) -def items_for_result(cl, result): +def items_for_result(cl, result, form): first = True pk = cl.lookup_opts.pk.attname for field_name in cl.list_display: @@ -227,11 +227,25 @@ def items_for_result(cl, result): yield mark_safe(u'<%s%s>%s' % \ (table_tag, row_class, url, (cl.is_popup and ' onclick="opener.dismissRelatedLookupPopup(window, %s); return false;"' % result_id or ''), conditional_escape(result_repr), table_tag)) else: - yield mark_safe(u'%s' % (row_class, conditional_escape(result_repr))) + # By default the fields come from ModelAdmin.list_editable, but if we pull + # the fields out of the form instead of list_editable custom admins + # can provide fields on a per request basis + if form and field_name in form.fields: + bf = form[field_name] + result_repr = mark_safe(force_unicode(bf.errors) + force_unicode(bf)) + else: + result_repr = conditional_escape(result_repr) + yield mark_safe(u'%s' % (row_class, result_repr)) + if form: + yield mark_safe(force_unicode(form[cl.model._meta.pk.attname])) def results(cl): - for res in cl.result_list: - yield list(items_for_result(cl,res)) + if cl.formset: + for res, form in zip(cl.result_list, cl.formset.forms): + yield list(items_for_result(cl, res, form)) + else: + for res in cl.result_list: + yield list(items_for_result(cl, res, None)) def result_list(cl): return {'cl': cl, diff --git a/django/contrib/admin/validation.py b/django/contrib/admin/validation.py index ccade8a3ef..fa6d7e3e60 100644 --- a/django/contrib/admin/validation.py +++ b/django/contrib/admin/validation.py @@ -63,6 +63,29 @@ def validate(cls, model): if hasattr(cls, 'list_per_page') and not isinstance(cls.list_per_page, int): raise ImproperlyConfigured("'%s.list_per_page' should be a integer." % cls.__name__) + + # list_editable + if hasattr(cls, 'list_editable') and cls.list_editable: + check_isseq(cls, 'list_editable', cls.list_editable) + if not (opts.ordering or cls.ordering): + raise ImproperlyConfigured("'%s.list_editable' cannot be used " + "without a default ordering. Please define ordering on either %s or %s." + % (cls.__name__, cls.__name__, model.__name__)) + for idx, field in enumerate(cls.list_editable): + try: + opts.get_field_by_name(field) + except models.FieldDoesNotExist: + raise ImproperlyConfigured("'%s.list_editable[%d]' refers to a " + "field, '%s', not defiend on %s." % (cls.__name__, idx, field, model.__name__)) + if field not in cls.list_display: + raise ImproperlyConfigured("'%s.list_editable[%d]' refers to " + "'%s' which is not defined in 'list_display'." + % (cls.__name__, idx, field)) + if field in cls.list_display_links: + raise ImproperlyConfigured("'%s' cannot be in both '%s.list_editable'" + " and '%s.list_display_links'" + % (field, cls.__name__, cls.__name__)) + # search_fields = () if hasattr(cls, 'search_fields'): diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py index d70b6da1de..9580e912a4 100644 --- a/django/contrib/admin/views/main.py +++ b/django/contrib/admin/views/main.py @@ -32,7 +32,7 @@ ERROR_FLAG = 'e' EMPTY_CHANGELIST_VALUE = '(None)' 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, model_admin): + 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): self.model = model self.opts = model._meta self.lookup_opts = self.opts @@ -44,6 +44,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_editable = list_editable self.model_admin = model_admin # Get search parameters from the query string. diff --git a/docs/ref/contrib/admin.txt b/docs/ref/contrib/admin.txt index 35bfde217c..1813a191f6 100644 --- a/docs/ref/contrib/admin.txt +++ b/docs/ref/contrib/admin.txt @@ -403,6 +403,32 @@ the change list page:: Finally, note that in order to use ``list_display_links``, you must define ``list_display``, too. +``list_editable`` +~~~~~~~~~~~~~~~~~ + +.. versionadded:: 1.1 + +Set ``list_editable`` to a list of field names on the model which will allow +editing on the change list page. That is, fields listed in ``list_editable`` +will be displayed as form widgets on the change list page, allowing users to +edit and save multiple rows at once. + +.. note:: + + ``list_editable`` interacts with a couple of other options in particular + ways; you should note the following rules: + + * To use ``list_editable`` you must have defined ``ordering`` defined on + either your model or your ``ModelAdmin``. + + * Any field in ``list_editable`` must also be in ``list_display``. You + can't edit a field that's not displayed! + + * The same field can't be listed in both ``list_editable`` and + ``list_display_links`` -- a field can't be both a form and a link. + + You'll get a validation error if any of these rules are broken. + ``list_filter`` ~~~~~~~~~~~~~~~ diff --git a/tests/regressiontests/admin_views/fixtures/admin-views-person.xml b/tests/regressiontests/admin_views/fixtures/admin-views-person.xml new file mode 100644 index 0000000000..77928a834b --- /dev/null +++ b/tests/regressiontests/admin_views/fixtures/admin-views-person.xml @@ -0,0 +1,18 @@ + + + + John Mauchly + 1 + True + + + Grace Hooper + 1 + False + + + Guido van Rossum + 1 + True + + diff --git a/tests/regressiontests/admin_views/models.py b/tests/regressiontests/admin_views/models.py index d849a7b9c1..eeaf039444 100644 --- a/tests/regressiontests/admin_views/models.py +++ b/tests/regressiontests/admin_views/models.py @@ -134,6 +134,28 @@ class Thing(models.Model): class ThingAdmin(admin.ModelAdmin): list_filter = ('color',) +class Person(models.Model): + GENDER_CHOICES = ( + (1, "Male"), + (2, "Female"), + ) + name = models.CharField(max_length=100) + gender = models.IntegerField(choices=GENDER_CHOICES) + alive = models.BooleanField() + + def __unicode__(self): + return self.name + + class Meta: + ordering = ["id"] + +class PersonAdmin(admin.ModelAdmin): + list_display = ('name', 'gender', 'alive') + list_editable = ('gender', 'alive') + list_filter = ('gender',) + search_fields = ('name',) + ordering = ["id"] + class Persona(models.Model): """ A simple persona associated with accounts, to test inlining of related @@ -177,12 +199,14 @@ class PersonaAdmin(admin.ModelAdmin): BarAccountAdmin ) + admin.site.register(Article, ArticleAdmin) admin.site.register(CustomArticle, CustomArticleAdmin) admin.site.register(Section, inlines=[ArticleInline]) admin.site.register(ModelWithStringPrimaryKey) admin.site.register(Color) admin.site.register(Thing, ThingAdmin) +admin.site.register(Person, PersonAdmin) admin.site.register(Persona, PersonaAdmin) # We intentionally register Promo and ChapterXtra1 but not Chapter nor ChapterXtra2. diff --git a/tests/regressiontests/admin_views/tests.py b/tests/regressiontests/admin_views/tests.py index bf198bc9a2..33000d4f5a 100644 --- a/tests/regressiontests/admin_views/tests.py +++ b/tests/regressiontests/admin_views/tests.py @@ -11,7 +11,7 @@ from django.contrib.admin.util import quote from django.utils.html import escape # local test models -from models import Article, CustomArticle, Section, ModelWithStringPrimaryKey, Persona, FooAccount, BarAccount +from models import Article, CustomArticle, Section, ModelWithStringPrimaryKey, Person, Persona, FooAccount, BarAccount try: set @@ -729,6 +729,76 @@ class AdminViewUnicodeTest(TestCase): response = self.client.post('/test_admin/admin/admin_views/book/1/delete/', delete_dict) self.assertRedirects(response, '/test_admin/admin/admin_views/book/') + +class AdminViewListEditable(TestCase): + fixtures = ['admin-views-users.xml', 'admin-views-person.xml'] + + def setUp(self): + self.client.login(username='super', password='secret') + + def tearDown(self): + self.client.logout() + + def test_changelist_input_html(self): + response = self.client.get('/test_admin/admin/admin_views/person/') + # 2 inputs per object(the field and the hidden id field) = 6 + # 2 management hidden fields = 2 + # main form submit button = 1 + # search field and search submit button = 2 + # 6 + 2 + 1 + 2 = 11 inputs + self.failUnlessEqual(response.content.count("