Skip to content

Commit

Permalink
Fixed #28147 -- Fixed loss of parent pk in child model when saving ch…
Browse files Browse the repository at this point in the history
…ild after parent
  • Loading branch information
xmeowmeowx committed May 20, 2019
1 parent 5f9ad40 commit 893325e
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 10 deletions.
2 changes: 2 additions & 0 deletions AUTHORS
Expand Up @@ -271,6 +271,7 @@ answer newbie questions, and generally made Django that much better:
Erik Karulf <erik@karulf.com>
Erik Romijn <django@solidlinks.nl>
eriks@win.tue.nl
Erwin Junge <erwin@junge.nl>
Esdras Beleza <linux@esdrasbeleza.com>
Espen Grindhaug <http://grindhaug.org/>
Eugene Lazutkin <http://lazutkin.com/blog/>
Expand Down Expand Up @@ -744,6 +745,7 @@ answer newbie questions, and generally made Django that much better:
Robert Wittams
Rob Golding-Day <rob@golding-day.com>
Rob Hudson <https://rob.cogit8.org/>
Rob Nguyen <tienrobertnguyenn@gmail.com>
Robin Munn <http://www.geekforgod.com/>
Rodrigo Pinheiro Marques de Araújo <fenrrir@gmail.com>
Romain Garrigues <romain.garrigues.cs@gmail.com>
Expand Down
21 changes: 13 additions & 8 deletions django/db/models/base.py
Expand Up @@ -684,14 +684,19 @@ def save(self, force_insert=False, force_update=False, using=None,
# database to raise an IntegrityError if applicable. If
# constraints aren't supported by the database, there's the
# unavoidable risk of data corruption.
if obj and obj.pk is None:
# Remove the object from a related instance cache.
if not field.remote_field.multiple:
field.remote_field.delete_cached_value(obj)
raise ValueError(
"save() prohibited to prevent data loss due to "
"unsaved related object '%s'." % field.name
)
if obj:
if obj.pk is None:
# Remove the object from a related instance cache.
if not field.remote_field.multiple:
field.remote_field.delete_cached_value(obj)
raise ValueError(
"save() prohibited to prevent data loss due to "
"unsaved related object '%s'." % field.name
)
elif getattr(self, field.attname) is None:
# Use pk from related object if it has been saved after
# the original assignment
setattr(self, field.attname, obj.pk)
# If the relationship's pk/to_field was changed, clear the
# cached relationship.
if obj and getattr(obj, field.target_field.attname) != getattr(self, field.attname):
Expand Down
4 changes: 4 additions & 0 deletions django/db/models/fields/related.py
Expand Up @@ -1031,6 +1031,10 @@ def save_form_data(self, instance, data):
setattr(instance, self.name, data)
else:
setattr(instance, self.attname, data)
# remote field object must be cleared otherwise Model.save()
# will reassign attname using the related object pk
if data is None:
setattr(instance, self.name, data)

def _check_unique(self, **kwargs):
# Override ForeignKey since check isn't applicable here.
Expand Down
4 changes: 4 additions & 0 deletions tests/many_to_one/models.py
Expand Up @@ -74,6 +74,10 @@ class ToFieldChild(models.Model):
parent = models.ForeignKey(Parent, models.CASCADE, to_field='name', related_name='to_field_children')


class ChildNullableParent(models.Model):
parent = models.ForeignKey(Parent, models.CASCADE, null=True)


# Multiple paths to the same model (#7110, #7125)
class Category(models.Model):
name = models.CharField(max_length=20)
Expand Down
12 changes: 10 additions & 2 deletions tests/many_to_one/tests.py
Expand Up @@ -8,8 +8,8 @@
from django.utils.translation import gettext_lazy

from .models import (
Article, Category, Child, City, District, First, Parent, Record, Relation,
Reporter, School, Student, Third, ToFieldChild,
Article, Category, Child, ChildNullableParent, City, District, First,
Parent, Record, Relation, Reporter, School, Student, Third, ToFieldChild,
)


Expand Down Expand Up @@ -522,6 +522,14 @@ def test_fk_assignment_and_related_object_cache(self):
self.assertIsNot(c.parent, p)
self.assertEqual(c.parent, p)

def test_save_nullable_parent_then_child(self):
parent = Parent()
child = ChildNullableParent(parent=parent)
parent.save()
child.save()
child.refresh_from_db()
self.assertEqual(child.parent, parent)

def test_save_nullable_parent_then_child_to_field(self):
parent_id = "jeff"
parent = Parent(name=parent_id)
Expand Down

0 comments on commit 893325e

Please sign in to comment.