Is this approach correct to create forms using multiple models?

Question

I had to create a form from which some details go to default.auth.user model and some to my custom model so after searching from various sources I did this:

Django Version :1.7

model.py

class UserProfile(models.Model):
    user = models.OneToOneField(User)

    title_id = models.ForeignKey('Title')
    mobile_number = models.CharField(max_length=10)
    alternate_number = models.CharField(max_length=10)
    date_of_birth = models.DateField()
    profession_id = models.ForeignKey('Profession', null=True, blank=True)
    house_no = models.CharField(max_length=100, blank=True, default='NA')
    city_id = models.ForeignKey('City', null=True)
    country_id = models.ForeignKey('Country', null=True)
    state_id = models.ForeignKey('State', null=True)
    locality_id = models.ForeignKey('Locality', null=True)
    profile_picture_path = models.CharField(max_length=100, blank=True, default='NA')

forms.py:

class UserForm(forms.ModelForm):
    password = forms.CharField(widget=forms.PasswordInput(attrs={'id': 'password'}))
    email = forms.CharField(widget=forms.TextInput(attrs={'id': 'email_id'}))
    username = forms.CharField(widget=forms.TextInput(attrs={'id': 'username'}))
    first_name = forms.CharField(widget=forms.TextInput(attrs={'id': 'first_name'}))
    last_name = forms.CharField(widget=forms.TextInput(attrs={'id': 'last_name'}))

    class Meta:
        model = User
        fields = ('email', 'username', 'first_name', 'last_name', 'password')


class ExtraDetailsForm(UserForm):
    confirm_password = forms.CharField(widget=forms.PasswordInput(attrs=  
                                      {'id':'confirm_password'}),max_length=32,
                                       required=True,)
    class Meta:
        model = UserProfile
        fields = ('email', 'username', 'title_id', 'first_name', 'last_name',
                  'password', 'confirm_password',
                  'date_of_birth', 'mobile_number', )

My view.py is :

def register(request):
    # A boolean vakue for telling whether the registration was successful
    registered = False
    if request.method == 'POST':
        user_form = UserForm(data=request.POST)
        additional_details_form = ExtraDetailsForm(data=request.POST)

        if user_form.is_valid() and additional_details_form.is_valid():
            user = user_form.save()
            user.set_password(user.password)
            user.save()

            additional_details = additional_details_form.save(commit=False)
            additional_details.user = user
            additional_details.save()

            registered = True


        else:
            print(user_form.errors, additional_details_form.errors)
    else:
        user_form = UserForm
        additional_details_form = ExtraDetailsForm

    return render(request,
              'users/register.html',
        {'user_form' : user_form, 'additional_details_form': additional_details_form, 'registerered': registered})

regsiter.html:

{% if registerered %}
    <p>Thank you for register. check ur email , entered email was</p>
{% else %}
    <form action="/users/register/" method="post">{% csrf_token %}
        {{ additional_details_form.as_p }}

        <input type="submit" value="Register" />
    </form>
{% endif %}

Now the good thing is that everything is working fine and details are being stored as they should be. But the bad thing is I do not know whether it is a correct approach or not as I did not find any tutorial/resource where this approach is used?

Answer 1

Ok, firstly ExtraDetailsForm shouldn't inherit from UserForm because they are for different models. It should look something like this instead:

class UserForm(forms.ModelForm):
    confirm_password = forms.CharField(widget=forms.PasswordInput(attrs=  
                                      {'id':'confirm_password'}),max_length=32,
                                       required=True,)

    class Meta:
        model = User
        fields = ('email', 'username', 'first_name', 'last_name', 'password',
                  'confirm_password')


class ExtraDetailsForm(forms.ModelForm):
    class Meta:
        model = UserProfile
        fields = ('title_id', 'date_of_birth', 'mobile_number')

Then in your view:

from django.contrib.auth import login
from django.shortcuts import redirect, render


def register(request):
    user_form = UserForm(data=request.POST or None)
    profile_form = ExtraDetailsForm(data=request.POST or None)

    if all([user_form.is_valid(), profile_form.is_valid()]):
        user = user_form.save(commit=False)
        user.set_password(user.password)
        user.save()

        profile = profile_form.save()

        # probably at this point you want to login the new user:
        login(request, user)

        # it's good practice to do a redirect here, after a successful
        # form post, eg to display success page, as this will
        # prevent accidental re-posting data if user reloads the page
        return redirect('registration_success')
    else:
        print(user_form.errors, profile_form.errors)

    return render(
        request,
        'users/register.html',
        {
            'user_form' : user_form,
            'profile_form' : profile_form,
        }
    )


def registration_success(request):
    return render('registration_success.html')

Finally you need to output both forms in the template:

<form action="/users/register/" method="post">{% csrf_token %}
    {{ user_form.as_p }}
    {{ profile_form.as_p }}

    <input type="submit" value="Register" />
</form>

and a new template registration_success.html:

<p>Thank you for registering. Check your email, entered email was: {{ request.user.email }}</p>

Answer 2

I would recommend that you use just one form here that contains all fields.

There is no benefit to using two forms, especially since one inherits the other, this is odd behaviour when you are then passing the POST data into each of them.

Consolidate the fields into a single form and then override the 'clean' method of the form to be able to check that the two password fields match.

You can create a single form to save data into one or many different models and this is especially useful in your case since you need to validate the data for these different models together.

Answer 3

This is correct approach and you do it almost right. Just couple notes:

if user_form.is_valid() and additional_details_form.is_valid():

In this line if user_form is invalid then validation for additional_details_form will not run. To always validate both change it to:

if all([user_form.is_valid(), additional_details_form.is_valid()]):

In else statement you set form class to *_form variables. It should be form instances instead:

user_form = UserForm()
additional_details_form = ExtraDetailsForm()

And it may be a good idea to wrap your save code into one transaction :-)