Code

Opened 6 years ago

Closed 6 years ago

#6369 closed (duplicate)

ModelForm fields ordering when Meta.fields is set

Reported by: Luke Garner <dj-t@…> Owned by: nobody
Component: Forms Version: master
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: yes
Easy pickings: UI/UX:

Description (last modified by gwilson)

Field ordering is currently determined by the order the fields were originally defined in the model. However, if the user specifically defines the fields to use in Meta.fields, it's safe to assume that's the order the user wants the fields to be outputted in.

Right now, this is accomplished with:

class myForm(forms.ModelForm):
    def __init__(self, *args, **kwargs):
        super(myForm, self).__init__(*args, **kwargs)
        self.fields.keyOrder = self.Meta.fields #This line should be unnecessary
    class Meta:
        model = myModel
        fields = ['field_3', 'field_2', 'field_9']

The new version would just be:

class myForm(forms.ModelForm):
    class Meta:
        model = myModel
        fields = ['field_3', 'field_2', 'field_9']

Attachments (4)

modelfield_order.diff (584 bytes) - added by Luke Garner <dj-t@…> 6 years ago.
modelfield_order_2.diff (555 bytes) - added by Luke Garner <dj-t@…> 6 years ago.
modelfield_order_3.diff (875 bytes) - added by Luke Garner <dj-t@…> 6 years ago.
ModelField fields ordering (now with error handling)
modelfield_order_3.2.diff (876 bytes) - added by Luke Garner <dj-t@…> 6 years ago.
ModelField fields ordering (now with error handling)

Download all attachments as: .zip

Change History (9)

Changed 6 years ago by Luke Garner <dj-t@…>

comment:1 Changed 6 years ago by gwilson

  • Description modified (diff)
  • Needs documentation set
  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

Sounds like the right thing to do. It's even a TODO in the code comments :)

As far as the patch goes, assigning directly to keyOrder is not a good idea. IMO, keyOrder looks like it should be a private attribute. Anyway, by blindly assigning to keyOrder without checking to see if those keys exist in the SortedDict would cause bad things to happen.

comment:2 Changed 6 years ago by gwilson

We should put the fields in the correct order in the newforms/models.py code.

Changed 6 years ago by Luke Garner <dj-t@…>

comment:3 Changed 6 years ago by Luke Garner <dj-t@…>

As far as I'm aware, there's no way to change the order of a SortedDict other than directly editing keyOrder. I've added a new patch which checks to ensure the fields are in keyOrder before reassigning them. I'm not sure this is 100% necessary, though. If Meta.fields contains a field that isn't in the attrsbase_fields? SortedDict, then the program is going to fail anyway.

Do you think the best approach is to put in a descriptive error message to raise, if Model.fields is incorrect?

Changed 6 years ago by Luke Garner <dj-t@…>

ModelField fields ordering (now with error handling)

Changed 6 years ago by Luke Garner <dj-t@…>

ModelField fields ordering (now with error handling)

comment:4 Changed 6 years ago by anonymous

How about performing a set.difference between opts.fields and keyOrder? This way, the error message could return all of the erroneous fields instead of just the first one.

comment:5 Changed 6 years ago by ubernostrum

  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #5986, which has several proposals and recent activity.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.