Reputation: 282865
I have a method on my user registration form that looks like this:
def save(self):
user = User(
username = self.cleaned_data['username'],
email = self.cleaned_data['email1'],
first_name = self.cleaned_data['first_name'],
last_name = self.cleaned_data['last_name'],
)
user.set_password(self.cleaned_data['password1'])
user.profile = Profile(
primary_phone = self.cleaned_data['phone'],
)
user.profile.address = Address(
country = self.cleaned_data['country'],
province = self.cleaned_data['province'],
city = self.cleaned_data['city'],
postal_code = self.cleaned_data['postal_code'],
street1 = self.cleaned_data['street1'],
street2 = self.cleaned_data['street2'],
street3 = self.cleaned_data['street3'],
)
user.save()
return user
Problem is when I call form.save()
it creates the user
object as expected, but doesn't save his profile or address. Why doesn't it cascade and save all the sub-models? I suspect I could call user.profile.save()
and user.profile.address.save()
manually, but I want the whole thing to succeed or fail together. What's the best way to do this?
Current solution:
def save(self):
address = Address(
country = self.cleaned_data['country'],
province = self.cleaned_data['province'],
city = self.cleaned_data['city'],
postal_code = self.cleaned_data['postal_code'],
street1 = self.cleaned_data['street1'],
street2 = self.cleaned_data['street2'],
street3 = self.cleaned_data['street3'],
)
address.save()
user = User(
username = self.cleaned_data['username'],
email = self.cleaned_data['email1'],
first_name = self.cleaned_data['first_name'],
last_name = self.cleaned_data['last_name'],
)
user.set_password(self.cleaned_data['password1'])
user.save()
profile = Profile(
primary_phone = self.cleaned_data['phone'],
)
profile.address = address
profile.user = user
profile.save()
I had to make profile
the "central" object. Needed to set profile.user = user
rather than user.profile = profile
to make it work (I guess because the key is on the profile model, not on the user model).
Newer solution:
I took a hint from this article suggested in this answer.
Now I have separated my model forms and moved the logic into the view:
def register(request):
if request.POST:
account_type_form = forms.AccountTypeForm(request.POST)
user_form = forms.UserForm(request.POST)
profile_form = forms.ProfileForm(request.POST)
address_form = forms.AddressForm(request.POST)
if user_form.is_valid() and profile_form.is_valid() and address_form.is_valid():
user = user_form.save()
address = address_form.save()
profile = profile_form.save(commit=False)
profile.user = user
profile.address = address
profile.save()
return HttpResponseRedirect('/thanks/')
else:
account_type_form = forms.AccountTypeForm()
user_form = forms.UserForm()
profile_form = forms.ProfileForm()
address_form = forms.AddressForm()
return render_to_response(
'register.html',
{'account_type_form': account_type_form, 'user_form': user_form, 'address_form': address_form, 'profile_form': profile_form},
context_instance=RequestContext(request)
)
I'm not too fond of shifting the burden to the view, but I guess I get a bit more flexibility this way?
Upvotes: 7
Views: 5071
Reputation: 33716
The problem is that you're trying to create or update fields in a User object that doesn't even exist yet. So the other fields aren't really updated because they aren't associated to any primary keys of the child fields.
Every time you're instantiating a new model field, you have to make sure you're saving so that a child model field has an id (primary key) to associate with.
You need something more like this:
def save(self):
user = User(
username = self.cleaned_data['username'],
email = self.cleaned_data['email1'],
first_name = self.cleaned_data['first_name'],
last_name = self.cleaned_data['last_name'],
)
## save user so we get an id
user.save()
## make sure we have a user.id
if user.id:
## this doesn't save the password, just updates the working instance
user.set_password(self.cleaned_data['password1'])
user.profile = Profile(
primary_phone = self.cleaned_data['phone'],
)
## save the profile so we get an id
user.profile.save()
## make sure we have a profile.id
if user.profile.id:
user.profile.address = Address(
country = self.cleaned_data['country'],
province = self.cleaned_data['province'],
city = self.cleaned_data['city'],
postal_code = self.cleaned_data['postal_code'],
street1 = self.cleaned_data['street1'],
street2 = self.cleaned_data['street2'],
street3 = self.cleaned_data['street3'],
)
## save the profile address
user.profile.address.save()
## final save to commit password and profile changes
user.save()
return user
This cascading save()
thing you have going on here just doesn't feel right. You're prone to way too many errors there, if any of the fields doesn't save you'll end up with a partially complete user instance and posisbly end up with duplicates if the user has to go back and try again. Not fun!
Edit: Removed the 2nd half of this because it was not accurate.
Upvotes: 2
Reputation: 798606
It doesn't cascade-save because it doesn't actually know whether or not the other objects need to be saved.
To do it in one go, first start a transaction:
@transaction.commit_on_success
def save(self):
....
Then save the subobjects in order:
user.profile.address.save()
user.profile.save()
user.save()
Upvotes: 8