Code

Opened 2 years ago

Closed 2 years ago

Last modified 12 months ago

#17924 closed New feature (wontfix)

Proposal: DRY/elegant/flexible ModelForm field attribute overrides

Reported by: DrMeers Owned by: DrMeers
Component: Forms Version: 1.3
Severity: Normal Keywords: modelform, attributes, override, DRY
Cc: charette.s@…, real.human@… Triage Stage: Accepted
Has patch: no Needs documentation: yes
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Currently overriding certain attributes of fields in ModelForms is somewhat limited/inflexible unless you resort to using the undocumented and inelegant formfield_callback functionality.

For example, you can change a field's widget class quite elegantly by using the ModelForm.Meta.widgets dictionary. However if you want to change a different aspect such as label, required, form_class, etc you have to either:

  • replace the field with a manually defined one and lose all attributes inherited from the corresponding Models.Field (as described in the caveat note)
  • define a formfield_callback function (undocumented / inelegant)

Two solutions spring to mind:

  • addition of labels, required, form_classes analogues of ModelForm.Meta.widgets (I don't like where this leads)
  • addition of an overrides attribute to ModelForm.Meta which can encompass the existing widgets functionality (potentially deprecatable) but also provides the ability to override other field attributes. For example
class MyForm(forms.ModelForm):
    class Meta:
        overrides = {
            'description': {
                'widget': forms.Textarea,
                'label': 'Message',
            },
            'start_date': {'label': 'Date Range'},
            'end_date': {'label': 'to'},
            'type': {
                'form_class': ModelChoiceFieldWithDescriptions,
                'empty_label': None,
            },
        }

This keeps inherited attributes DRY, but allows easy/elegant overriding of arbitrary field attributes. The patch for this is actually quite simple, which I'm very happy to polish and provide, I just thought I'd first seek out opinions regarding the approach before investing time in it.

Attachments (1)

17924.diff (13.2 KB) - added by DrMeers 2 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 2 years ago by charettes

  • Cc charette.s@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

I agree that this could be useful but can't you just override __init__ and access self.fields directly?

class MyModel(models.Model):
  is_cool = models.BooleanField()
  content = models.CharField(max_length=255)

class MyForm(forms.ModelForm):
  class Meta:
    model = MyModel
  
  def __init__(self, *args, **kwargs):
    super(MyForm, self).__init__(*args, **kwargs)
    self.fields['is_cool'].widget.attrs.update(attr='value')
    self.fields['content'].widget = forms.TextArea()

comment:2 Changed 2 years ago by DrMeers

Yes, though:

  • that is more code, and less elegant
  • the form field has already been created at that point, so you can't change it's class, etc (e.g. the form_class = ModelChoiceFieldWithDescriptions above)

comment:3 Changed 2 years ago by charettes

I agree that it's easier to define it using your overrides but you can achieve form field replacement this way.

class MyForm(forms.ModelForm):
  class Meta:
    model = MyModel
  
  def __init__(self, *args, **kwargs):
    super(MyForm, self).__init__(*args, **kwargs)
    self.fields['fk_field'] = ModelChoiceFieldWithDescriptions()

Or directly call formfield which is a bit ugly

self._meta.model._meta.get_field('fk_field').formfield(form_class=ModelChoiceFieldWithDescriptions)
Last edited 2 years ago by charettes (previous) (diff)

comment:4 Changed 2 years ago by DrMeers

True. Though the former loses all attribute inheritance from the models.Field and the latter just reinforces my point. IMHO the simplicity and flexibility of the changes required would not be burdensome to the core codebase, and the result would make the form customization interface much nicer.

comment:5 Changed 2 years ago by charettes

  • Triage Stage changed from Unreviewed to Accepted

Marking as Accepted since you convinced me such a feature would be useful.

Another reason why this could be a great addition to the form API is that there's no documented way of doing this yet while it's quite a common use case.

comment:6 Changed 2 years ago by charettes

  • Needs documentation set
  • Needs tests set

comment:7 Changed 2 years ago by DrMeers

In the interest of flexibilty/DRYness, it might be nice to also allow field classes or callables as overrides dictionary keys to target multiple fields. For example:

    overrides = {
        models.DateField: {
            'widget': MyCustomDateWidget,
        },
        lambda f: f.name.startswith('question_'): {
            'required': True,
        },
    }

So a string dictionary key is a shortcut for lambda f: f.name == key and a class dictionary key is a shortcut for lambda f: isinstance(f, key). This is still a fairly simple patch.

Thoughts?

comment:8 Changed 2 years ago by jezdez

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

I'm violently -1 on the whole topic "meta programming form fields after they've been instantiated", it's a mess. Yes it would be more DRY, but also much harder to know how the hell the form fields are composed as. Just override the form field, it's not a huge deal code wise and much easier to grok.

Changed 2 years ago by DrMeers

comment:10 Changed 23 months ago by mrmachine

  • Cc real.human@… added

comment:11 Changed 12 months ago by loic84

Duplicate of #20000 to some extent.

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.