From 519016e5f25d7c0a040015724f9920581551cab0 Mon Sep 17 00:00:00 2001 From: Rob Date: Mon, 20 May 2019 22:06:30 +1000 Subject: [PATCH] Fixed #28147 -- Fixed loss of assigned parent when saving child after parent. Thanks Erwin Junge for the initial patch. --- AUTHORS | 2 ++ django/db/models/base.py | 21 +++++++++++++-------- django/db/models/fields/related.py | 4 ++++ tests/many_to_one/models.py | 4 ++++ tests/many_to_one/tests.py | 12 ++++++++++-- 5 files changed, 33 insertions(+), 10 deletions(-) diff --git a/AUTHORS b/AUTHORS index a9d4bd4359..4487126aa3 100644 --- a/AUTHORS +++ b/AUTHORS @@ -271,6 +271,7 @@ answer newbie questions, and generally made Django that much better: Erik Karulf Erik Romijn eriks@win.tue.nl + Erwin Junge Esdras Beleza Espen Grindhaug Eugene Lazutkin @@ -744,6 +745,7 @@ answer newbie questions, and generally made Django that much better: Robert Wittams Rob Golding-Day Rob Hudson + Rob Nguyen Robin Munn Rodrigo Pinheiro Marques de Araújo Romain Garrigues diff --git a/django/db/models/base.py b/django/db/models/base.py index eeb5163b96..0d704684f1 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -684,14 +684,19 @@ class Model(metaclass=ModelBase): # 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 + # an 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): diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index 276bcb11ff..1e54bd6c6e 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -1031,6 +1031,10 @@ class OneToOneField(ForeignKey): 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. diff --git a/tests/many_to_one/models.py b/tests/many_to_one/models.py index 96b84ccddb..6230129bbd 100644 --- a/tests/many_to_one/models.py +++ b/tests/many_to_one/models.py @@ -70,6 +70,10 @@ class Child(models.Model): parent = models.ForeignKey(Parent, models.CASCADE) +class ChildNullableParent(models.Model): + parent = models.ForeignKey(Parent, models.CASCADE, null=True) + + class ToFieldChild(models.Model): parent = models.ForeignKey(Parent, models.CASCADE, to_field='name', related_name='to_field_children') diff --git a/tests/many_to_one/tests.py b/tests/many_to_one/tests.py index 8ad467b6b6..7538f238f2 100644 --- a/tests/many_to_one/tests.py +++ b/tests/many_to_one/tests.py @@ -8,8 +8,8 @@ from django.test import TestCase 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, ) @@ -522,6 +522,14 @@ class ManyToOneTests(TestCase): self.assertIsNot(c.parent, p) self.assertEqual(c.parent, p) + def test_save_nullable_fk_after_parent(self): + parent = Parent() + child = ChildNullableParent(parent=parent) + parent.save() + child.save() + child.refresh_from_db() + self.assertEqual(child.parent, parent) + def test_save_nullable_fk_after_parent_with_to_field(self): parent = Parent(name='jeff') child = ToFieldChild(parent=parent)