error_messages for fields in Model don't carry over to ModelForm

[Aleksandar Micovic]

When having the optional error_messages parameter added to a field for some model, the errors don't get carried over to a ModelForm using that Model. For example:

MODEL:

class Contact(models.Model):

    ...

    first_name = models.CharField(max_length=30, verbose_name=u"first name", error_messages={'required':'A contact needs a first name.'})
    
    ...

FORM:

class ContactQuickAddForm(ModelForm):
    class Meta:
        model = Contact
        fields = ["first_name" ... ]

When validating, the error message doesn't change and it's default is shown: "this field is required."

A workaround (thanks to subsume on IRC) is to override your ModelForm's init.

    def __init__(self,*args,**kwargs):
         super(ContactQuickAddForm,self).__init__(*args,**kwargs)
         self.fields['first_name'].error_messages = {'required': 'FAS:DJFASKL:DJF'}

[furagu]

Patch added

[Honza Král]

This is not a straightforward issue as just this patch (which also breaks the test suite). The messages on model fields are meant to be used there and doesn't necessarily have to (and sometimes don't) coincide with the messages on form fields.

Marking this as wontfix since I don't believe this is an issue.

[Carl Meyer]

Marking this as wontfix since I don't believe this is an issue.

Honza, you certainly know a lot more than I do about model-validation, but I don't think I agree with the wontfix here. In general my mental model of ModelForms is that they pull as much information as possible from the model definition for the form default, to minimize the boilerplate needed to make a form for that model. Personally, I'd be surprised if I defined a custom validation error message on a model field and found that it didn't propagate to the ModelForm by default. It's possible that I'm missing something important, but I can't think of a case where I wouldn't want that as the default behavior. Maybe you can explain in a bit more detail why that's a bad idea? Or else reopen, and I'm willing to put in some time to work out the test-suite issues and any other problems with the patch.

[Honza Král]

Replying to carljm:

Marking this as wontfix since I don't believe this is an issue.

Honza, you certainly know a lot more than I do about model-validation, but I don't think I agree with the wontfix here. In general my mental model of ModelForms is that they pull as much information as possible from the model definition for the form default, to minimize the boilerplate needed to make a form for that model. Personally, I'd be surprised if I defined a custom validation error message on a model field and found that it didn't propagate to the ModelForm by default. It's possible that I'm missing something important, but I can't think of a case where I wouldn't want that as the default behavior. Maybe you can explain in a bit more detail why that's a bad idea? Or else reopen, and I'm willing to put in some time to work out the test-suite issues and any other problems with the patch.

My first issue is that it is mildly backwards incompatible (hence the broken tests) - nothing big but still. The other is that form validation and model validation serve different purposes and may work differently, requiring differently phrased error messages, for example the model DateTimeField accepts different date formats than the same field on the form level and the error message is worded differently because of that.

For me it's the question of locality - I define some validation on the model level and expect to use the model's error messages and similarly with forms. Otherwise I think it cold be confusing and we would need to keep the logic and message formats synchronized.

Another thing is the codes of the messages - half of them are called 'invalid' which is very generic and thus have a high probability of name clashes - I could define my validation on the model level, use 'invalid' for a code and then try to override the message only to see the message on completely unrelated validation result.

I completely agree that the idea is good and these are minor issues that can be fixed (although by introducing minor backwards incompatibilities) but until they are, this ticket doesn't bring enough positive to ignore them. So I am -1 on this until these issues are addressed and resolved which I don't think is the priority ATM.

Sorry for not providing more detailed explanation earlier, hope this helps.

[Carl Meyer]

Reopening this ticket based on Honza's comment "I completely agree that the idea is good and these are minor issues that can be fixed"; that means the ticket should be open, and marked "patch needs improvement" if the current patch does not adequately address these concerns.

[Karol Sikora]

Some discussion about these topic: ​https://groups.google.com/forum/?hl=pl&fromgroups#!topic/django-developers/5rSGXxAq25A
In my opinion, the simplest(and probably the best) solution is to use error_messages from field definition in model.
(should django version related in this ticket be changed to 1.4?)

[colegatron]

I am new to Django but I also think the DRY mantra should be aplicable to the error_messages inheritance from the model to the modelform.
Perhaps by a design point of view it is not a good idea, as points Honza in his first comment (22 months ago), but to redefine all the fields in the modelform or form classes to get the error_messages (and perhaps other attributes) is tedious and boilerplate.
Looking for a way to avoid this, I have find a post (​http://blog.brendel.com/2012/01/django-modelforms-setting-any-field.html) which gives a very nice, easy and elegant way to do it.

Probably that solution can be improved, but in my opinion is candidate to be imported as new feature in the base modelform class:

Example of use:

class AuthorForm(ExtendedMetaModelForm):
    class Meta:
        model = Author
        field_args = {
            "first_name" : {
                "error_messages" : {
                    "required" : "Please let us know what to call you!"
                }
            },
            "notes" : {
                "+error_messages" : {
                    "required" : "Please also enter some notes.",
                    "invalid"  : "This is not a valid note."
                },
                "widget" : forms.Textarea(attrs={'cols': 70, 'rows': 15}),
            }
        }

Extended modelform class:

class ExtendedMetaModelForm(forms.ModelForm):
    """
    Allow the setting of any field attributes via the Meta class.
    """
    def __init__(self, *args, **kwargs):
        """
        Iterate over fields, set attributes from Meta.field_args.
        """
        super(ExtendedMetaModelForm, self).__init__(*args, **kwargs)
        if hasattr(self.Meta, "field_args"):
            # Look at the field_args Meta class attribute to get
            # any (additional) attributes we should set for a field.
            field_args = self.Meta.field_args
            # Iterate over all fields...
            for fname, field in self.fields.items():
                # Check if we have something for that field in field_args
                fargs = field_args.get(fname)
                if fargs:
                    # Iterate over all attributes for a field that we
                    # have specified in field_args
                    for attr_name, attr_val in fargs.items():
                        if attr_name.startswith("+"):
                            merge_attempt = True
                            attr_name = attr_name[1:]
                        else:
                            merge_attempt = False
                        orig_attr_val = getattr(field, attr_name, None)
                        if orig_attr_val and merge_attempt and \
                                    type(orig_attr_val) == dict and \
                                    type(attr_val) == dict:
                            # Merge dictionaries together
                            orig_attr_val.update(attr_val)
                        else:
                            # Replace existing attribute
                            setattr(field, attr_name, attr_val)

Hope this two years ticket can be closed with that nice solution.

[loic84]

The general issue with error_messages and ModelForm was addressed in #20199.

The DRY syntax suggested in 7 was addressed in #20000.

I'm marking the ticket as invalid because error_messages defined on model fields shouldn't carry over to ModelForm in most cases:

1. The example given in the description defines a required error message at the Model level, however required is a Form concept, a rough equivalent at the Model level would be blank and null. Supporting this would work against the principle of loose coupling.

2. Form works at the presentation level; the ultimate decision regarding user-facing strings belongs there.

What may be a little confusing is that a lot of validation is duplicated at the Model and Form levels. For example both forms.IntegerField and models.IntergerField have their own error message for the invalid code, in that case the error message defined at the Form level is always used. Luckily for such cases #20000 helps to customize the error messages without requiring duplicating the whole form field.

error_messages defined on Model fields are only used when the ValidationError is raised during the model validation step and there are no error messages with matching codes defined at the form level. Hopefully the docs’ changes in [fba6c2ede7] make this behavior more obvious.

TL;DR: I think between #20199 and #20000 we made an acceptable compromise between DRY and loose coupling regarding this issue.