Django forms.ValidationError not displayed in template and submit not working

Question

In my Django application, I have a contact us template with fields: company name, contact number, email address.

I have defined a def clean() to validate the email address and phone number as follows:

class ContactForm(forms.Form):
    def clean(self):
        super(ContactForm, self).clean()
        email_address = self.cleaned_data.get('email_address')
        contact_no = self.cleaned_data.get('contact_no')

        if validate_email(email_address):
            pattern = re.compile('^[2-9]\\d{9}$')
            if bool(pattern.match(contact_no)):
                return self.cleaned_data
            else: # error not displayed
                raise forms.ValidationError(
                    "Please enter a valid phone number")
        else: # error not displayed
            raise forms.ValidationError(
                "Please enter a valid email address")
        return self.cleaned_data

In the 'contact_us' template, I am correctly rendering the errors as well:

<form method="post" action="{% url 'contact' %}">
    {% csrf_token %}
    {{contact_form.as_p}
    <input type="submit" value="Submit">
</form>

{% if contact_form.errors %}
    {% for field in contact_form %}
        {% for error in field.errors %}
        <div class="alert alert-danger">
            <strong>{{ error|escape }}</strong>
        </div>
        {% endfor %}
    {% endfor %}
{% endif %}

In views.py, even if the form is valid and an email is sent successfully, the form is not rendering again(as a fresh new form) on the template.

 def contact(request):
     contact_form = ContactForm()

     if request.method == 'POST':
        contact_form = ContactForm(request.POST)
        if contact_form.is_valid():
            company_name = contact_form.cleaned_data.get('company_name')
            contact_no = contact_form.cleaned_data.get('contact_no')
            email_address = contact_form.cleaned_data.get('email_address')

            # send mail code

            contact_form.save()
            contact_form = ContactForm()
            return render(request, './mysite/contact_us.html', {
                'contact_form': contact_form, 'successful_submit': True})
     return render(request, './mysite/contact_us.html', {
         'contact_form': contact_form,
     })

Answer 1

You're raising ValidationError from clean(), so the error is stored in form.non_field_errors - which you are not displaying so obviously you can't see them indeed.

A documented solution is to use self.add_error(<fieldname>, <msg>) instead,

BUT:

but i am performing validation on two fields, hence i used the common clean() method instead of two separate clean_field() methods.

form.clean() is for cross validation - validating two fields that depend on each other. In your example, there's absolutely no dependency between both fields, you don't need the value of the email to validate the phone number and you don't need the phone number to validate the email. IOW, you should REALLY be using distinct clean_field methods.

Note that even if you had some cross validation, you can (and should) still use clean_field methods for what can be validated alone. In your case you should have a validate_email_address method to validate the email itself (or just use a form.EmailField FWIW), and in clean() just check if the email has been validated (if not you'll find some error in self._errors['email_address']) before.

And while we're at it, there are are quite a few other issues with your code. Here:

    if validate_email(email_address):
        pattern = re.compile('^[2-9]\\d{9}$')
        if bool(pattern.match(contact_no)):
            return self.cleaned_data
        else: # error not displayed
            raise forms.ValidationError(
                "Please enter a valid phone number")
    else: # error not displayed
        raise forms.ValidationError(
            "Please enter a valid email address")
    return self.cleaned_data

1/ precompiling a regex this way is totally useless. Either you precompile it at the module's top-level so it's only compiled once (per process at least), or you don't compile it at all and use re functions instead, ie if re.match(r"your expression here", your_variable) - the re module will not only compile the expression, but also cache it for future reuse.

2/ if bool(pattern.match(contact_no)) -> the call to bool is redundant, Python WILL do it anyway.

3/ but Django DOES have a regexp validator already so that's what you should be using.

4/ the control flow is uselessly complicated, and the last return statement will actually never be executed. This:

    if validate_email(email_address):
        pattern = re.compile('^[2-9]\\d{9}$')
        if bool(pattern.match(contact_no)):
            return self.cleaned_data
        else: # error not displayed
            raise forms.ValidationError(
                "Please enter a valid phone number")
    else: # error not displayed
        raise forms.ValidationError(
            "Please enter a valid email address")

would be much more readable using early exit:

    if not validate_email(email_address):
        raise forms.ValidationError(
            "Please enter a valid email address"
            )

    pattern = re.compile('^[2-9]\\d{9}$')
    if not pattern.match(contact_no)):
        raise forms.ValidationError(
            "Please enter a valid phone number"
            )

    return self.cleaned_data

5/ but this is still wrong, as validation will stop on the first detected error - you want to catch as many validation errors as you can in one go so the user doesn't have to read the error message, correct one field, re-submit, find another error that could have been detected the first time, fix it, re-resubmit etc. Well, unless your goal is to make it so hard for the user that he gives up trying to use your site, of course.

6/ your view code is doubly wrong too. First, you should save the form (I assume a ModelForm) BEFORE sending the mail, so you don't loose the data if anything gos wrong, and then as Daniel already mentionned, you should always redirect after a successful post (read this for the why)