Django Email Change Form Setup

March 29, 2023 Post a Comment

My email change form for users works, but I feel like my code is not written correctly. If I did it the way I have done below, I'd need a thousand else statements so that the page

Solution 1:

I would suggest moving the validation to the form clean method:

#form
class EmailChangeForm():
..
..
 def clean(self):
     if self.cleaned_data.get('email1', None) != self.cleaned_data.get('email1', None):
             raise forms.ValidationError('Validation Failed')


@login_required('/login/') //You can check the user is logged in using the decorator
def email_change(request):
    form = Email_Change_Form()
    if request.method=='POST':
        form = Email_Change_Form(request.POST)
        if form.is_valid():
                    user = request.user //Don't know why you want to get the object from database when you already have it
                    user.email = form.cleaned_data['email1'] 
                    user.save()
                    return HttpResponseRedirect("/accounts/profile/")
    else:
        return render_to_response("email_change.html", {'form':form}, context_instance=RequestContext(request))

Update: Doing this is redundant:

user = request.user
u = User.objects.get(username=user.username)

Because user is going to be the same as u i.e. user = u

Solution 2:

I would suggest a complete change on how you looked at this. In my opinion, you should have all the implementation on the form side.

`forms.py`

I've implemented a class based on the SetPasswordForm that is more complete:

class EmailChangeForm(forms.Form):
    """
    A form that lets a user change set their email while checking for a change in the 
    e-mail.
    """
    error_messages = {
        'email_mismatch': _("The two email addresses fields didn't match."),
        'not_changed': _("The email address is the same as the one already defined."),
    }

    new_email1 = forms.EmailField(
        label=_("New email address"),
        widget=forms.EmailInput,
    )

    new_email2 = forms.EmailField(
        label=_("New email address confirmation"),
        widget=forms.EmailInput,
    )

    def __init__(self, user, *args, **kwargs):
        self.user = user
        super(EmailChangeForm, self).__init__(*args, **kwargs)

    def clean_new_email1(self):
        old_email = self.user.email
        new_email1 = self.cleaned_data.get('new_email1')
        if new_email1 and old_email:
            if new_email1 == old_email:
                raise forms.ValidationError(
                    self.error_messages['not_changed'],
                    code='not_changed',
                )
        return new_email1

    def clean_new_email2(self):
        new_email1 = self.cleaned_data.get('new_email1')
        new_email2 = self.cleaned_data.get('new_email2')
        if new_email1 and new_email2:
            if new_email1 != new_email2:
                raise forms.ValidationError(
                    self.error_messages['email_mismatch'],
                    code='email_mismatch',
                )
        return new_email2

    def save(self, commit=True):
        email = self.cleaned_data["new_email1"]
        self.user.email = email
        if commit:
            self.user.save()
        return self.user

This class checks both if the e-mail have in fact changed (very useful if you need to validate the e-mail or update mail chimp for example) and produce the appropriate errors, so they are helpful for the user in the form view.

`views.py`

Your code adapted to my class:

@login_required() 
def email_change(request):
    form = EmailChangeForm()
    if request.method=='POST':
        form = EmailChangeForm(user, request.POST)
        if form.is_valid():
            form.save()
            return HttpResponseRedirect("/accounts/profile/")
    else:
        return render_to_response("email_change.html", {'form':form},
                                  context_instance=RequestContext(request))

As you can see the view is simplified, assuring everything on the form level. To ensure the login I set a decorator (See the docs).

Ps: I changed email1 and email2 to new_email1 and new_email2 to be consistent with the Django approach on passwords. I also changed the form Email_Change_Form to EmailChangeForm according to Python guidelines for classes.

Solution 3:

You will create more complicated code with nested if, if you write every bit of logic in your views. You need to break them in appropriate sections. Like, for every form related validations, do it in forms like -

if `email1` is same as `email2`, 
 and if email1 is valid

check it in your form. You should check that in clean or clean_FieldName methods. Refer here: https://docs.djangoproject.com/en/dev/ref/forms/validation/#cleaning-and-validating-fields-that-depend-on-each-other

Another check you applied for authentication - if the user is authenticated or not. In this case, can a Unauthorised user change his email - well no. So why should I let my code run for it. It would be better to check this condition as soon as possible and then send the user to login page. @login_required is used to check this condition as a decorator of your view. See here : https://docs.djangoproject.com/en/dev/topics/auth/#the-login-required-decorator

If you really want to check your user authentication in your view, I think the good approach would be -

def email_change(request):
    if not request.user.is_authenticated:
        // Do what you need to say to user or send them to login
        // return HttpResponse object / HttpResponseRedirect

    form = Email_Change_Form(request.POST) 
    if request.method=='POST': 
        if form.is_valid():
            ...
    else:
        ...  // Display form.

Python College