Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Django: ValueError: cannot assign none on ForeignField when trying to create in clean fails

I'm having a lot of trouble figuring out how to automatically create an instance of a model for a ForeignKey field when a form is submitted. Here's a simple toy website that illustrates the problem:

I have two models, Model1 and Model2. Model2 contains a ForeignKey to Model1. I want the user to be able to create an instance of Model2 by either specifically selecting an instance of Model1 to store in the ForeignKey, or by leaving that value blank and letting an instance of Model1 be automatically generated.

Here's what I feel like that code should look like. My models.py code is very straightforward:

# models.py
from django.db import models
from django.core.validators import MinValueValidator


class Model1(models.Model):

    # Note this field cannot be negative
    my_field1 = models.IntegerField(validators=[MinValueValidator(0)])


class Model2(models.Model):
    # blank = True will make key_to_model1 not required on the form,
    # but since null = False, I will still require the ForeignKey
    # to be set in the database.
    related_model1 = models.ForeignKey(Model1, blank=True)

    # Note this field cannot be negative
    my_field2 = models.IntegerField(validators=[MinValueValidator(0)])

forms.py is a bit involved, but what's going on is quite straightforward. If Model2Form does not receive an instance of Model1, it tries to automatically create one in the clean method, validates it, and if it's valid, it saves it. If it's not valid, it raises an exception.

#forms.py
from django import forms
from django.forms.models import model_to_dict

from .models import Model1, Model2


# A ModelForm used for validation purposes only.
class Model1Form(forms.ModelForm):
    class Meta:
        model = Model1


class Model2Form(forms.ModelForm):
    class Meta:
        model = Model2

    def clean(self):
        cleaned_data = super(Model2Form, self).clean()

        if not cleaned_data.get('related_model1', None):

            # Don't instantiate field2 if it doesn't exist.
            val = cleaned_data.get('my_field2', None)
            if not val:
                raise forms.ValidationError("My field must exist")

            # Generate a new instance of Model1 based on Model2's data
            new_model1 = Model1(my_field1=val)

            # validate the Model1 instance with a form form
            validation_form_data = model_to_dict(new_model1)
            validation_form = Model1Form(validation_form_data)

            if not validation_form.is_valid():
                raise forms.ValidationError("Could not create a proper instance of Model1.")

            # set the model1 instance to the related model and save it to the database.
            new_model1.save()
            cleaned_data['related_model1'] = new_model1

        return cleaned_data

However, this approach does not work. If I enter valid data into my form, it works fine. But, if I don't enter anything for the ForeignKey and put a negative value for the integer, I get a ValueError.

Traceback: File "/Library/Python/2.7/site-packages/django/core/handlers/base.py" in get_response 111. response = callback(request, *callback_args, **callback_kwargs) File "/Library/Python/2.7/site-packages/django/views/generic/base.py" in view 48. return self.dispatch(request, *args, **kwargs) File "/Library/Python/2.7/site-packages/django/views/generic/base.py" in dispatch 69. return handler(request, *args, **kwargs) File "/Library/Python/2.7/site-packages/django/views/generic/edit.py" in post 172. return super(BaseCreateView, self).post(request, *args, **kwargs) File "/Library/Python/2.7/site-packages/django/views/generic/edit.py" in post 137. if form.is_valid(): File "/Library/Python/2.7/site-packages/django/forms/forms.py" in is_valid 124. return self.is_bound and not bool(self.errors) File "/Library/Python/2.7/site-packages/django/forms/forms.py" in _get_errors 115. self.full_clean() File "/Library/Python/2.7/site-packages/django/forms/forms.py" in full_clean 272. self._post_clean() File "/Library/Python/2.7/site-packages/django/forms/models.py" in _post_clean 309. self.instance = construct_instance(self, self.instance, opts.fields, opts.exclude) File "/Library/Python/2.7/site-packages/django/forms/models.py" in construct_instance 51. f.save_form_data(instance, cleaned_data[f.name]) File "/Library/Python/2.7/site-packages/django/db/models/fields/init.py" in save_form_data 454. setattr(instance, self.name, data) File "/Library/Python/2.7/site-packages/django/db/models/fields/related.py" in set 362. (instance._meta.object_name, self.field.name))

Exception Type: ValueError at /add/ Exception Value: Cannot assign None: "Model2.related_model1" does not allow null values.

So, what's happening is that Django is catching my ValidationError and still creating an instance of Model2 even though validation fails.

I could fix this by overriding the _post_clean method to not create the instance of Model2 if there are errors. But, that solution is ugly. In particular, _post_clean's behavior is very helpful in general--In more complicated projects I need _post_clean to run for other reasons.

I could also allow the ForeignKey to be null but never set it to null in practice. But, again, that seems like a bad idea.

I could even set up a dummy Model1 that I use whenever validation on the attempted new Model1 fails, but that also seems hackish.

In general, I can think of lots of hacks to fix this, but I have no idea how to fix this in a reasonably clean, pythonic way.

like image 408
Noah Stephens-Davidowitz Avatar asked Apr 04 '13 17:04

Noah Stephens-Davidowitz


1 Answers

I've found a solution that I think might be acceptable, based somewhat on karthikr's discussion in the comments. I'm definitely still open to alternatives.

The idea is to use logic in the view to choose between two forms to do validation: One form is the standard model form and one is the model form without the ForeignKey field.

So, my models.py is identical.

My forms.py has two Model2 forms... one extremely simple one and one without the ForeignKey field and with new logic to dynamically generate a new instance of Model1 for the ForeignKey. The new form's clean logic is just the clean logic that I used to put in my Model2Form:

#forms.py
from django import forms
from django.forms.models import model_to_dict

from .models import Model1, Model2


# A ModelForm used for validation purposes only.
class Model1Form(forms.ModelForm):
    class Meta:
        model = Model1


class Model2Form(forms.ModelForm):
    class Meta:
        model = Model2

# This inherits from Model2Form so that any additional logic that I put in Model2Form
# will apply to it.
class Model2FormPrime(Model2Form):
    class Meta:
        model = Model2
        exclude = ('related_model1',)

    def clean(self):
        cleaned_data = super(Model2Form, self).clean()

        if cleaned_data.get('related_model1', None):
            raise Exception('Huh? This should not happen...')

        # Don't instantiate field2 if it doesn't exist.
        val = cleaned_data.get('my_field2', None)
        if not val:
            raise forms.ValidationError("My field must exist")

        # Generate a new instance of Model1 based on Model2's data
        new_model1 = Model1(my_field1=val)

        # validate the Model1 instance with a form form
        validation_form_data = model_to_dict(new_model1)
        validation_form = Model1Form(validation_form_data)

        if not validation_form.is_valid():
            raise forms.ValidationError("Could not create a proper instance of Model1.")

        # set the Model1 instance to the related model and save it to the database.
        cleaned_data['related_model1'] = new_model1

        return cleaned_data

    def save(self, commit=True):
        # Best to wait til save is called to save the instance of Model1
        # so that instances aren't created when the Model2Form is invalid
        self.cleaned_data['related_model1'].save()

        # Need to handle saving this way because otherwise related_model1 is excluded
        # from the save due to Meta.excludes
        instance = super(Model2FormPrime, self).save(False)
        instance.related_model1 = self.cleaned_data['related_model1']
        instance.save()

        return instance

And then my view logic uses one of the two forms to validate, depending on the post data. If it uses Model2FormPrime and validation fails, it will move the data and errors to a regular Model2Form to show the user:

# Create your views here.
from django.views.generic.edit import CreateView
from django.http import HttpResponseRedirect

from .forms import Model2Form, Model2FormPrime


class Model2CreateView(CreateView):
    form_class = Model2Form
    template_name = 'form_template.html'
    success_url = '/add/'

    def post(self, request, *args, **kwargs):
        if request.POST.get('related_model', None):
            # Complete data can just be sent to the standard CreateView form
            return super(Model2CreateView, self).post(request, *args, **kwargs)
        else:
            # super does this, and I won't be calling super.
            self.object = None

            # use Model2FormPrime to validate the post data without the related model.
            validation_form = Model2FormPrime(request.POST)
            if validation_form.is_valid():
                return self.form_valid(validation_form)
            else:
                # Create a normal instance of Model2Form to be displayed to the user
                # Insantiate it with post data and validation_form's errors
                form = Model2Form(request.POST)
                form._errors = validation_form._errors
                return self.form_invalid(form)

This solution works, and it's quite flexible. I can add logic to my models and to the base Model2Form without worrying too much about breaking it or violating DRY.

It's slightly ugly, though, since it requires me to use two forms to essentially do the job of one, pass errors between forms. So, I'm definitely open to alternative solutions if anyone can suggest anything.

like image 109
Noah Stephens-Davidowitz Avatar answered Nov 15 '22 08:11

Noah Stephens-Davidowitz