Opened 11 years ago

Closed 11 years ago

#20000 closed New feature (fixed)

Allow overriding `label`, `help_text` and `error_messages` for the default fields in `ModelForm`

Reported by: loic84 Owned by: loic84
Component: Forms Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Currently ModelForm requires to completely redefine fields in order to customize user-facing strings such as labels, help_texts and error_messages.

This leads to a lot of boilerplate code to anyone who wants to customize these, but most importantly it's an error prone process. The bigger the ModelForm becomes, the more difficult it is to ensure functional parity between the Model fields and the Form fields for the critical parts such as blank/required, default, validators, etc.

One shouldn't be at risk of completely breaking a form functionality in order to make small cosmetic changes.

It's understood that we have to draw a line somewhere, else we would have field definitions in the Meta, but I believe that line should be drawn at "anything cosmetic" available through Meta overrides, and "anything functional" through fields overrides. I believe the currently provided widgets lie in between the two, so it's almost surprising that it was added as a convenience in a first place instead of the purely cosmetic options.

Change History (15)

comment:2 by Jannis Leidel, 11 years ago

Resolution: wontfix
Status: newclosed

This has been previously discussed and wontfixed before. Feel free to raise the issue on the developer list.

comment:3 by Carl Meyer, 11 years ago

FWIW, I think this patch would be a positive improvement. It makes common customization cases simpler, and avoids the error-prone alternative of overriding the field entirely and the extra-boilerplate alternative of overriding __init__ and patching fields there. I also think the supplied new customization points are very similar in nature to the existing Meta.widgets, and it's an odd inconsistency to have one but not the others. What argument is there for Meta.widgets that does not apply equally to labels, help texts, and error messages?

In other words, if you decide to bring it up on django-developers, I am +1 for the change.

comment:4 by Carl Meyer, 11 years ago

Resolution: wontfix
Status: closednew
Triage Stage: UnreviewedDesign decision needed

comment:5 by Jacob, 11 years ago

Triage Stage: Design decision neededAccepted

I agree with Carl.

comment:7 by Łukasz Rekucki, 11 years ago

Just for the record, this is a duplicate of #17924.

comment:8 by Tim Graham, 11 years ago

I've updated the pull request to merge cleanly and also added the labels, help_texts, and error_messages arguments to the model(form|formset)_factory functions: https://github.com/django/django/pull/1251

comment:9 by Jannis Leidel, 11 years ago

I've previously warned that introducing the widgets parameter to the Meta class would open up the possibility "to ask for more". I can't believe Jacob or Carl would become soft in the strict separation of concern between form class and field.

Since I can't provide an alternative as requirement to veto a feature request other than "DON'T DO IT!!1!" I'm forced to change to -0. But I reserve the right to say "I told you so" when a ticket is opened to allow overriding arbitrary field options in the Meta class.

comment:10 by Tim Graham, 11 years ago

An alternative which may mitigate the underlying issue of making simple customizations on a ModelForm without having to redeclare the options that are generated by the default form:

FooFormField = MyModel._meta.get_field('foo').formfield() # would probably need a better API

class MyModelForm(ModelForm):
  foo = FooFormField(label='My Foo')

Alternatively, we could recommend doing customization's in __init__, but it doesn't see like the cleanest solution.

comment:11 by loic84, 11 years ago

I dislike the FooFormField suggestion, it's very verbose and the intent (override the label) is not immediately obvious.

Also I can't think of any other places in Django where we use such pattern so it feels particularly out of place.

comment:12 by Tim Graham, 11 years ago

But it could be a way to override selected options on the the default ModelForm field without having to redeclare all of that field's default options (i.e. for non-cosmetic changes).

comment:13 by Tai Lee, 11 years ago

As jezdez points out, this opens up the possibility to "ask for more"... I liked #17924 and I am asking for more :) If we're going to hard code support for a few special cases, why not just support anything in a clean and DRY/elegant/flexible way as was suggested before...

Last edited 11 years ago by Tai Lee (previous) (diff)

comment:14 by Carl Meyer, 11 years ago

I think we should move ahead with this ticket as outlined in the description.

I might favor the version in #17924 if we were designing the API from scratch, but the problem with #17924 is that for consistent API it would imply deprecating Meta.widgets, and I just don't think the benefit of a "universal" approach is worth a deprecation cycle for already-existing functionality that's in wide use. (#17924 also carries the minor disadvantage of requiring a lot more commas, quotes, and curly braces for simple customizations.) If we have the common cases covered via direct Meta options, overriding __init__ is still always an option for the uncommon cases.

comment:15 by Tim Graham <timograham@…>, 11 years ago

Resolution: fixed
Status: newclosed

In 9e50833e2293aa2734f14f9873abe4f83d03b8c6:

Fixed #20000 -- Allowed ModelForm meta overrides for label, help_text and error_messages

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