Opened 16 years ago

Closed 16 years ago

Last modified 16 years ago

#10512 closed (fixed)

Ordering of non-model fields in a ModelForm, after #8164

Reported by: Marco Bonetti Owned by: Alex Gaynor
Component: Forms Version: dev
Severity: Keywords: modelform, field order
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

After #8164 was closed in [10062], it is not longer possible to define non-model fields in the fields list, as the ordering process raises a KeyError on forms/models.py:166

It would be great if the fields list order could be applied to non-model fields defined in the Form class as well.

Attachments (1)

modelforms-fields-order.diff (1.6 KB ) - added by Alex Gaynor 16 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 by Alex Gaynor, 16 years ago

Triage Stage: UnreviewedAccepted
Version: 1.0SVN

As I mentioned in IRC, the issue here is not that you can't have extra fields on the Form, it's that you can't use them in the fields ordering option.

by Alex Gaynor, 16 years ago

comment:2 by anonymous, 16 years ago

Has patch: set
Needs tests: set

comment:3 by korpios, 16 years ago

fields_for_model is actually the wrong place to be sorting; you want to do that in ModelFormMetaclass.__new__, otherwise you'll miss sorting entirely on fields from declared_fields. Just reassign fields.keyOrder, e.g.:

        if opts.fields:
            fields.keyOrder = [f for f in opts.fields if f in fields.keyOrder]

But, yeah — the changes in r10062 made things go breaky over here. ^_^

comment:4 by Alex Gaynor, 16 years ago

Nope this works fine(as the test show) since None will get put in the fields_dict and then get reassigned to the new value which doesn't change the ordering.

comment:5 by Alex Gaynor, 16 years ago

Just to be clear I'm not necessarily saying this is the right place for it, just that it does work. I'm not sure why it would be causing a break for anyone, why do you have fields that aren't on the Model in the fields option?

comment:6 by Marco Bonetti, 16 years ago

Thank you Alex, your patch worked as expected here.

comment:7 by Russell Keith-Magee, 16 years ago

Resolution: fixed
Status: newclosed

(In [10070]) Fixed #10512 -- Corrected the handling of extra fields on a ModelForm?. Thanks to Alex Gaynor for the patch.

comment:8 by korpios, 16 years ago

Alex: for the sake of ordering them. The fix in r10070 doesn't address that.

comment:9 by Alex Gaynor, 16 years ago

Yes it does, the behavior of the SortedDict is that on update it doesn't reset the position of the key. So basically what happens is it tries to get a non Model field, it doesn't find it, it puts a None in the fields_dict, then later that gets updated which keeps that key position. And the tests demonstrate this.

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