Re: [Django] #36259: Unsaved related object with primary_key=True field does not raise usaved object error

[Django]

#36259: Unsaved related object with primary_key=True field does not raise usaved
object error
-------------------------------------+-------------------------------------
Reporter: Clifford Gama | Owner: Clifford
| Gama
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Clifford Gama):

Thank you, Sarah!

I should have mentioned in the report that this incorrect use of
`_is_pk_set()` affects also three other cases:
1. Access of a generated field on an unsaved instance with a specified
primary key.
{{{#!diff
diff --git a/tests/model_fields/test_generatedfield.py
b/tests/model_fields/test_generatedfield.py
index e00a665ec8..ab80536e38 100644
--- a/tests/model_fields/test_generatedfield.py
+++ b/tests/model_fields/test_generatedfield.py
@@ -180,9 +180,12 @@ class GeneratedFieldTestMixin:

def test_unsaved_error(self):
m = self.base_model(a=1, b=2)
+ manual_pk_obj = GeneratedModelNonAutoPk(id=101, a="a")
msg = "Cannot read a generated field from an unsaved model."
with self.assertRaisesMessage(AttributeError, msg):
m.field
+ with self.assertRaisesMessage(AttributeError, msg): # Raises
integrity error instead
+ manual_pk_obj.b

def test_full_clean(self):
m = self.base_model(a=1, b=2)
}}}
2. `QuerySet.contains(unsaved_with_manual_pk)` raises integrity error.
{{{#!diff
diff --git a/tests/queries/test_contains.py
b/tests/queries/test_contains.py
index 2aa4badc72..aef3b475f9 100644
--- a/tests/queries/test_contains.py
+++ b/tests/queries/test_contains.py
@@ -1,6 +1,6 @@
from django.test import TestCase

-from .models import DumbCategory, NamedCategory, ProxyCategory
+from .models import DateTimePK, DumbCategory, NamedCategory,
ProxyCategory


class ContainsTests(TestCase):
@@ -13,6 +13,8 @@ class ContainsTests(TestCase):
msg = "QuerySet.contains() cannot be used on unsaved objects."
with self.assertRaisesMessage(ValueError, msg):
DumbCategory.objects.contains(DumbCategory())
+ with self.assertRaisesMessage(ValueError, msg):
+ DateTimePK.objects.contains(DateTimePK())

def test_obj_type(self):
msg = "'obj' must be a model instance."
}}}
3. `filter|exclude(obj=unsaved_obj_with_manual_pk)`
{{{#!diff
diff --git a/tests/queries/tests.py b/tests/queries/tests.py
index c429a93af3..f30dbb66ff 100644
--- a/tests/queries/tests.py
+++ b/tests/queries/tests.py
@@ -1982,6 +1982,8 @@ class Queries5Tests(TestCase):
Employment.objects.filter(employer__in=[company,
Company(name="unsaved")])
with self.assertRaisesMessage(ValueError, msg):
StaffUser.objects.filter(staff=Staff(name="unsaved"))
+ with self.assertRaisesMessage(ValueError, msg):
+ ExtraInfo.objects.filter(date=DateTimePK())


class SelectRelatedTests(TestCase):
@@ -3360,6 +3362,8 @@ class ExcludeTests(TestCase):
Employment.objects.exclude(employer__in=[company,
Company(name="unsaved")])
with self.assertRaisesMessage(ValueError, msg):
StaffUser.objects.exclude(staff=Staff(name="unsaved"))
+ with self.assertRaisesMessage(ValueError, msg):
+ ExtraInfo.objects.exclude(date=DateTimePK())


class ExcludeTest17600(TestCase):
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36259#comment:6>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36259: Unsaved related object with primary_key=True field does not raise usaved
object error
-------------------------------------+-------------------------------------
Reporter: Clifford Gama | Owner: Clifford
| Gama
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Sarah Boyce):

Thank you for the extra details. I've checked and all of these are still
failing on 5.1 so are not regressions introduced by 5.2
I also think each case is roughly covered by the title, with the
GeneratedField being a special case
--
Ticket URL: <https://code.djangoproject.com/ticket/36259#comment:7>