#25349: ModelForm regression when setting None into a ForeignKey
-------------------------------+--------------------
     Reporter:  jpaulett       |      Owner:  nobody
         Type:  Uncategorized  |     Status:  new
    Component:  Forms          |    Version:  1.8
     Severity:  Normal         |   Keywords:
 Triage Stage:  Unreviewed     |  Has patch:  0
Easy pickings:  0              |      UI/UX:  0
-------------------------------+--------------------
 It appears that when using a ModelForm to "unset" a non-required foreign
 key field, Django 1.8 no longer will set the foreign key to None when
 saving the ModelForm, and the prior value for the ForeignKey remains.

 In practice, imagine a form where users can pick a category to assign to
 the instance, but can un-assign its category by picking the empty_label
 value in the <select>.

 The behaviour worked previously in Django, including 1.6 and 1.7.9, but
 appears when testing against Django 1.8.5.  I have also tested the
 stable/1.8.x and master branches on github, which still show the issue.  I
 have reviewed the Release Notes and ModelForms documentation and can not
 see anything that would indicate this behaviour has changed.

 models.py:
 {{{
 from django.db import models

 class Child(models.Model):
     name = models.TextField()

     def __unicode__(self):
         return self.name

 class Parent(models.Model):
     name = models.TextField()
     child = models.ForeignKey(Child, null=True)

     def __unicode__(self):
         return self.name
 }}}

 forms.py:
 {{{
 from django import forms

 from .models import Parent, Child

 class ParentForm(forms.ModelForm):
     # field definition only set so `required = False`.  Have also tried
     # a custom ParentForm.__init__ with `self.fields['child'].required =
 False`
     child = forms.ModelChoiceField(
         queryset=Child.objects.all(),
         required=False
     )

     class Meta:
         model = Parent
         fields = ('name', 'child')

     def __init__(self, *args, **kwargs):
         super(ParentForm, self).__init__(*args, **kwargs)
         # self.fields['child'].required = False
 }}}

 tests.py:
 {{{
 from django.test import TestCase

 from .forms import ParentForm
 from .models import Parent, Child

 class ParentFormTest(TestCase):
     def test_unset(self):
         p = Parent.objects.create(
             name='p',
             child=Child.objects.create(name='c')
         )

         form = ParentForm(
             data={'name': 'new name', 'child': None},
             instance=p
         )
         self.assertTrue(form.is_valid())

         form.save()

         obj = Parent.objects.get()
         self.assertEqual(obj.name, 'new name')
         self.assertIsNone(obj.child)  # ASSERTION FAILS

     def test_set_alternative(self):
         p = Parent.objects.create(
             name='p',
             child=Child.objects.create(name='c')
         )

         form = ParentForm(
             data={'name': 'new name',
                   'child': Child.objects.create(name='alt').id},
             instance=p
         )
         self.assertTrue(form.is_valid())

         form.save()

         obj = Parent.objects.get()
         self.assertEqual(obj.name, 'new name')
         self.assertEqual(obj.child.name, 'alt')  # ASSERTION WORKS
 }}}

 Result when running against Django 1.8.5:
 {{{
 Creating test database for alias 'default'...
 .F
 ======================================================================
 FAIL: test_unset (demo.tests.ParentFormTest)
 ----------------------------------------------------------------------
 Traceback (most recent call last):
   File "/home/user/modelform-issue/demo/tests.py", line 24, in test_unset
     self.assertIsNone(obj.child)  # ASSERTION FAILS
 AssertionError: <Child: c> is not None

 ----------------------------------------------------------------------
 Ran 2 tests in 0.007s

 FAILED (failures=1)
 Destroying test database for alias 'default'...
 }}}

 I have tried tracing through the ModelForm.save code with pdb.  The
 form.cleaned_data appears correct (`{'name': u'new name', 'child': None}`.
 At the point that `save_instance` is called, it appears that the
 `instance.child`.

--
Ticket URL: <https://code.djangoproject.com/ticket/25349>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/051.c4115984633c6a9d5348efe33710983c%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to