Opened 17 years ago

Closed 10 years ago

#5986 closed New feature (fixed)

Easy way to customize ordering of fields on forms that use inheritance

Reported by: Michał Sałaban Owned by: nobody
Component: Forms Version: dev
Severity: Normal Keywords: field order weight form newforms
Cc: marc.garcia@…, matti.haavikko@…, sime, Simon Charette, Loic Bistuer, tanner Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Consider this example:

from django import newforms as forms

class UserForm(forms.Form):
    # Used for registering new accounts.
    username = forms.CharField(max_length=20)
    email = forms.EmailField()
    password = forms.CharField(max_length=20)
    password2 = forms.CharField(max_length=20)

    # Lots of validators which check if username is unique,
    # password1 == password2 and so on.

class UserProfileForm(UserForm):
    # Inherits all UserForm fields and validators and adds
    # optional profile entries.
    first_name = forms.CharField(max_length=30)
    last_name = forms.CharField(max_length=30)
    jabber_id = forms.EmailField(required=False)

The UserProfileForm can inherit all the goods of UserForm: fields and validators. But the field order may look a bit messy then:

>>> UserProfileForm.base_fields.keys()
['username',
 'email',
 'password',
 'password2',
 'first_name',
 'last_name',
 'jabber_id']

It would be nice to have email grouped with jabber_id, first_name and last_name with username, etc. It's of course possible to do it using custom template, but violates DRY principle and renders as_*() methods useless.

The attached patch allows to specify a custom Field order within a Form, even with inherited fields.

Every Field may have an additional weight parameter with default value of 0. All fields are sorted in ascending weight order.

The example forms customized with weight parameters:

from django import newforms as forms

class UserForm(forms.Form):
    username = forms.CharField(max_length=20, weight=-2)
    email = forms.EmailField()
    password = forms.CharField(max_length=20, weight=1)
    password2 = forms.CharField(max_length=20, weight=1)

class UserProfileForm(UserForm):
    first_name = forms.CharField(max_length=30, weight=-1)
    last_name = forms.CharField(max_length=30, weight=-1)
    jabber_id = forms.EmailField()

And the effect:

>>> UserProfileForm.base_fields.keys()
['username',
 'first_name',
 'last_name',
 'email',
 'jabber_id',
 'password',
 'password2']

Attachments (4)

django-fields-weight.patch (1.9 KB ) - added by Michał Sałaban 17 years ago.
The patch adding weight parameter to newforms.Field
django-fields-order.patch (1.4 KB ) - added by Michał Sałaban 17 years ago.
Second approach, with Form.Meta.fields_order
django-fields-order.2.patch (4.9 KB ) - added by Patryk Zawadzki <patrys@…> 17 years ago.
Correct patch including regression tests
django-fields-order.3.patch (5.6 KB ) - added by Patryk Zawadzki <patrys@…> 17 years ago.
Added support for form_for_model

Download all attachments as: .zip

Change History (45)

by Michał Sałaban, 17 years ago

Attachment: django-fields-weight.patch added

The patch adding weight parameter to newforms.Field

comment:1 by patrys@…, 17 years ago

Might be more useful to reimplement it as a meta-property containing the desired list of field names (the way you define columns to display for admin interface). This way each form can have its own independent list there and you can actually try to extend two forms at once while maintaining the correct and predictable output.

comment:2 by Michał Sałaban, 17 years ago

So, here it goes, with Form.Meta.fields_order list.

Example:

from django import newforms as forms

class UserForm(forms.Form):
    # Used for registering new accounts.
    username = forms.CharField(max_length=20)
    email = forms.EmailField()
    password = forms.CharField(max_length=20)
    password2 = forms.CharField(max_length=20)

    # Lots of validators which check if username is unique,
    # password1 == password2 and so on.


class UserProfileForm(UserForm):
    # Inherits all UserForm fields and validators and adds
    # optional profile entries.
    first_name = forms.CharField(max_length=30)
    last_name = forms.CharField(max_length=30)
    jabber_id = forms.EmailField()

    class Meta:
        fields_order = ['username', 'first_name', 'last_name',
                'email', 'jabber_id', 'password']

by Michał Sałaban, 17 years ago

Attachment: django-fields-order.patch added

Second approach, with Form.Meta.fields_order

by Patryk Zawadzki <patrys@…>, 17 years ago

Attachment: django-fields-order.2.patch added

Correct patch including regression tests

comment:3 by Patryk Zawadzki <patrys@…>, 17 years ago

Summary: Custom field order in newforms[PATCH] Custom field order in newforms

by Patryk Zawadzki <patrys@…>, 17 years ago

Attachment: django-fields-order.3.patch added

Added support for form_for_model

comment:4 by Patryk Zawadzki <patrys@…>, 17 years ago

Has patch: set

comment:5 by jkocherhans, 17 years ago

I'm sorry to be blunt, but I couldn't be more against this change, or rather the weight=X syntax. I'm working on a new class called ModelForm right now (search django-dev for the relevant thread) that should allow something similar to the fields_order attribute above. It's just called fields and it will actually restrict the fields that occur in the form as well.

in reply to:  5 comment:6 by Patryk Zawadzki <patrys@…>, 17 years ago

Replying to jkocherhans:

I'm sorry to be blunt, but I couldn't be more against this change, or rather the weight=X syntax. I'm working on a new class called ModelForm right now (search django-dev for the relevant thread) that should allow something similar to the fields_order attribute above. It's just called fields and it will actually restrict the fields that occur in the form as well.

This has little or nothing to do with form_for_* syntax. This adds the ordering ability for all Form subclasses as it only operates inside the metaclass. If your ModelForm class or any other class extends Form then it gains this feature for free.

The only relevant bit would be removing the small block of code including the comment about form_for_* once these functions die.

I think the patch is still appropriate to commit and it adds a nice regression test so you can be sure things work like they did and will continue to do so.

in reply to:  5 comment:7 by Michał Sałaban <michal@…>, 17 years ago

Replying to jkocherhans:

I'm sorry to be blunt, but I couldn't be more against this change, or rather the weight=X syntax. I'm working on a new class called ModelForm right now (search django-dev for the relevant thread) that should allow something similar to the fields_order attribute above. It's just called fields and it will actually restrict the fields that occur in the form as well.

The weight syntax is obsolete. Please, see the latest patch and example.

I found the discussion about ModelForms but don't see if they deal with the order of fields. And how can they be used to create Forms which are not Model-based?

Anyway, I see no problem in coexistence of ModelForms and Meta.fields_order above.

comment:8 by bear330, 17 years ago

This might be useless, but my way to order the fields is:

def sortDictByList(dic, seq):
    """
    Sort dictionary by giving a sequence.

    @param dic The dictionary instance you want to sort.
    @param seq The sequence you want to specify how to sort.
    @return Sorted dictionary.
    """
    n = SortedDict()
    for k in seq:
        n[k] = dic[k]

    for k, v in dic.iteritems():
        if n.has_key(k):
            continue
        n[k] = v
    return n

class SortFormFieldsMixin(object):
    """
    Sort the form fields by 'fieldsOrder' attribute in mixined form class.
    The fieldsOrder attribute is a list to figure out the order of form fields.
    """
    def __new__(cls):
        if not hasattr(cls, 'fieldsOrder'):
            raise RuntimeError("Bitch! You use SortFormFieldsMixin but no "
                "fieldsOrder attribute in it, stupid man~~~!!")
                                        
        cls.base_fields = sortDictByList(cls.base_fields, cls.fieldsOrder)
        return super(SortFormFieldsMixin, cls).__new__(cls)
    
BaseAForm = forms.form_for_model(models.A, BaseForm)

class FinalForm(BaseAForm, SortFormFieldsMixin):
    fieldsOrder = ['name', 'desc', 'link']

Anyway, the class Meta with fields_order should be a better way because the coordination with django's convention.

I posted here just for a reference. :)

comment:9 by Pete Crosier, 17 years ago

Needs documentation: set
Summary: [PATCH] Custom field order in newformsCustom field order in newforms

comment:10 by MichaelBishop, 17 years ago

Triage Stage: UnreviewedDesign decision needed

Decision needs to be made whether adding a custom field order is a "good thing". IMHO this doesn't appear to be a critical feature. Either way a design decision is needed.

comment:11 by James Bennett, 17 years ago

#6369 and #6878 were closed as duplicates.

comment:12 by David Cramer, 17 years ago

Can we call it ordering to stick w/ a variable name thats already used instead of creating more?

Michael, features don't need to be critical to be wanted :)

comment:14 by Marc Garcia, 16 years ago

Cc: marc.garcia@… added
milestone: 1.0 beta

+1 on this.

Specially because it's also needed for ModelForm, where fields Meta attribute couldn't be used for sorting. Without a order parameter, it's not easy to position in the correct place an additional parameter of a ModelForm.

comment:15 by Michael Radziej, 16 years ago

milestone: 1.0 betapost-1.0

not a bug => not milestone 1.0 beta.

comment:16 by Matti Haavikko, 16 years ago

Cc: matti.haavikko@… added

comment:17 by GertSteyn, 16 years ago

After reeding CookBookNewFormsFieldOrdering this came to mind...

class FooForm(forms.ModelForm):

class Meta:

fields = ('first_field', 'second_field', 'third_field',)

def init(self, *args, kwargs):

super(FooForm, self).init(*args, kwargs)
self.fields.keyOrder = self.Meta.fields

The patch has now been reduced to a one liner:
"self.fields.keyOrder = self.Meta.fields"

comment:18 by sime, 16 years ago

Cc: sime added

comment:19 by Grigoriy Petukhov, 16 years ago

The patch has now been reduced to a one liner: "self.fields.keyOrder = self.Meta.fields"

I think that is not correct. In this case only those fields that described in Meta.fields will be displayed.
Custom fields that have been added in the init or in the class will be truncated.

comment:20 by miracle2k, 16 years ago

comment:21 by Alex Gaynor, 16 years ago

Resolution: duplicate
Status: newclosed

Closing as a dpue of #8164 since it has the better API.

comment:22 by (none), 16 years ago

milestone: post-1.0

Milestone post-1.0 deleted

comment:23 by Markus Bertheau, 10 years ago

Easy pickings: unset
Resolution: duplicate
Severity: Normal
Status: closednew
Type: Uncategorized
UI/UX: unset

This is not a dupe of #8164 since that only talks about and implements field ordering for ModelForms.

comment:24 by Collin Anderson, 10 years ago

Summary: Custom field order in newformsCustom form field order
Triage Stage: Design decision neededUnreviewed
Type: UncategorizedNew feature

I haven't tried this myself, but would a syntax like this work?

class UserProfileForm(UserForm):
    first_name = forms.CharField(max_length=30, weight=-1)
    last_name = forms.CharField(max_length=30, weight=-1)
    username = UserForm.base_fields['username']
    password = UserForm.base_fields['password']
    password2 = UserForm.base_fields['password2']
    jabber_id = forms.EmailField()
Version 0, edited 10 years ago by Collin Anderson (next)

comment:25 by Tim Graham, 10 years ago

Has patch: unset
Needs documentation: unset
Summary: Custom form field orderEasy way to customize ordering of fields on forms that use inheritance
Triage Stage: UnreviewedAccepted

Pre-Django 1.7, the following worked, but relied on internal implementation details (that self.fields was SortedDict; now it's OrderedDict):

class EditForm(forms.Form):
    summary = forms.CharField()
    description = forms.CharField(widget=forms.TextArea)


class CreateForm(EditForm):
    name = forms.CharField()

    def __init__(self, *args, **kwargs):
        super(CreateForm, self).__init__(*args, **kwargs)
        self.fields.keyOrder = ['name', 'summary', 'description']

Adding an official API for making ordering easier on forms that use inheritance seems reasonable. I'm not sure if recommending base_fields is the best approach as that's not public API as of now.

comment:26 by Tim Graham, 10 years ago

Has patch: set

#23936 was a duplicate and included a pull request.

comment:27 by Loic Bistuer, 10 years ago

Can't say I'm convinced with this ticket, IMO ordering fields belongs in the templates.

in reply to:  27 comment:28 by Alasdair Nicol, 10 years ago

Replying to loic:

Can't say I'm convinced with this ticket, IMO ordering fields belongs in the templates.

I used self.field.keyOrder in previous versions of Django, and would find an official API useful. If you render the form in the template with {{ form }} or {{ form.ul }}, then it's much easier to change the field order in the form than the template.

The PasswordChangeForm in the contrib.auth app changes the field order by changing base_fields. I think it's much better to change it there than to tell users to put 'old password' before 'new password' in their template. It would be even better if we used a public API to change the field order.

in reply to:  27 comment:29 by Simon Charette, 10 years ago

Cc: Simon Charette added

Replying to loic:

Can't say I'm convinced with this ticket, IMO ordering fields belongs in the templates.

The thing is field ordering also affects the order of clean_<field_name> calls.

comment:30 by Carl Meyer, 10 years ago

Hmm, I always thought that relying on ordering of clean_<field_name> calls was unsupported; a clean_<field_name> method should only deal with its field and nothing else, if you need to validate multiple fields it should be done in clean.

I think that in most cases forms are better rendered explicitly in the template, and that's where field-ordering belongs. But there are use cases (more generic applications such as the admin) where that's not practical. The fact that we reorder form fields ourselves in Django is a pretty strong argument that there should be a public API for it.

in reply to:  30 comment:31 by Loic Bistuer, 10 years ago

Cc: Loic Bistuer added

Replying to carljm:

Hmm, I always thought that relying on ordering of clean_<field_name> calls was unsupported; a clean_<field_name> method should only deal with its field and nothing else, if you need to validate multiple fields it should be done in clean.

+1, especially now that add_error() makes it very easy to do.


I think that in most cases forms are better rendered explicitly in the template, and that's where field-ordering belongs. But there are use cases (more generic applications such as the admin) where that's not practical. The fact that we reorder form fields ourselves in Django is a pretty strong argument that there should be a public API for it.

Quoting from the PR regarding Form.field_order: "Fields missing in the list are removed". I'm worried this adds yet another way of excluding fields, ModelForm is already pretty confusing with both fields and exclude (and the possible overlap of the two). There is also the interaction with ModelAdmin that already supports fields ordering through fields and fieldsets.

Finally I'd like to point out that the proposed API doesn't play well with inheritance. We wouldn't be able to use it in PasswordChangeForm for instance as it would break any subclasses with extra fields.

Doesn't sorting self.fields in __init__() achieves the same result anyway?

comment:32 by tanner, 10 years ago

I have revised my PR to make it fully backwards-compatible.

The field_order attribute/argument is used to rearrange existing fields.

If a key of an existing field is missing in the list, the field is appended to the fields in the original order.
This way new fields in subclasses are just added (as before) if the subclass does not override field_order.

If a key in field_order refers to a non-existing field, it is simply ignored.
This make is possible to remove a field in a subclass by overriding it with None, without overriding field_order.
Again, this won't break existing subclasses.

If you do not define Form.field_order or set it None, the default order is kept.

There are now three ways to set the field order:

  1. set the class variable as default for all instances
  1. pass field_order as argument when instantiating a Form, e.g., when calling the parent constructor
  1. use the order_fields() method to rearrange the field order of an instance, e.g., in a subclass constructor after some extra fields have been added.

comment:33 by tanner, 10 years ago

Cc: tanner added

comment:34 by tanner, 10 years ago

anyone willing to review the PR? https://github.com/django/django/pull/3652

comment:35 by Berker Peksag, 10 years ago

Patch needs improvement: set

Thanks. I've reviewed the patch.

comment:36 by tanner, 10 years ago

Patch needs improvement: unset

thank you. patch has been revised

comment:37 by Berker Peksag, 10 years ago

use the order_fields() method to rearrange the field order of an instance, e.g., in a subclass constructor after some extra fields have been added.

It would be good if users can avoid to call self.order_fields() manually (see https://github.com/django/django/pull/3652#issuecomment-65085465 for a use case). Perhaps we can call this method in self.full_clean(), __call__ or somewhere else?

comment:38 by tanner, 10 years ago

why? that use case is very rare and I think it's acceptable to call order_fields explicitly in this case. The field order has an effect on many other methods and must be set after initialization, long before validation.

comment:39 by Tim Graham, 10 years ago

Patch needs improvement: set

Cosmetic suggests to go. Please bump patch the ticket to "Ready for checkin" after you make those updates.

comment:40 by tanner, 10 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:41 by Tim Graham <timograham@…>, 10 years ago

Resolution: fixed
Status: newclosed

In 28986da:

Fixed #5986 -- Added ability to customize order of Form fields

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