Skip to content

Commit 9659efa

Browse files
committed
Fix #60 -- Adjust pre-AddField(db_default) for ForeignKey with callable defaults.
ForeignKey.get_default() cannot be safely called on instances that are not bound to a model yet; this will have to be adjusted if Django's schema editor is ever adjusted to operate from model states. Thanks @jzmiller1 for the report.
1 parent f097d52 commit 9659efa

File tree

5 files changed

+73
-4
lines changed

5 files changed

+73
-4
lines changed

CHANGELOG.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
- Avoid unnecessary prompting for a default value on `ManyToManyField`
77
removals. (#59)
8+
- Address a ``makemigration`` crash when adding a ``ForeignKey`` with a
9+
callable ``default``. (#60)
810

911
1.2.0
1012
=====

syzygy/operations.py

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from contextlib import contextmanager
22

3+
from django.db import models
34
from django.db.migrations import operations
45
from django.db.models.fields import NOT_PROVIDED
56
from django.utils.functional import cached_property
@@ -201,7 +202,22 @@ def get_pre_add_field_operation(model_name, name, field, preserve_default=True):
201202
"Fields with a db_default don't require a pre-deployment operation."
202203
)
203204
field = field.clone()
204-
field.db_default = field.get_default()
205+
if isinstance(field, models.ForeignKey):
206+
# XXX: Replicate ForeignKey.get_default() logic in a way that
207+
# doesn't require the field references to be pre-emptively
208+
# resolved. This will need to be fixed upstream if we ever
209+
# implement model state schema alterations.
210+
# See https://code.djangoproject.com/ticket/29898
211+
field_default = super(models.ForeignKey, field).get_default()
212+
if (
213+
isinstance(field_default, models.Model)
214+
and field_default._meta.label_lower == field.related_model.lower()
215+
):
216+
target_field = field.to_fields[0] or "pk"
217+
field_default = getattr(field_default, target_field)
218+
else:
219+
field_default = field.get_default()
220+
field.db_default = field_default
205221
operation = operations.AddField(model_name, name, field, preserve_default)
206222
return operation
207223

tests/models.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,10 @@
33

44
class Foo(models.Model):
55
pass
6+
7+
8+
class Bar(models.Model):
9+
name = models.CharField(max_length=100, unique=True)
10+
11+
class Meta:
12+
managed = False

tests/test_autodetector.py

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
)
2626
from syzygy.plan import get_migration_stage
2727

28+
from .models import Bar
29+
2830

2931
class AutodetectorTestCase(TestCase):
3032
style = color_style()
@@ -53,7 +55,7 @@ def get_changes(
5355

5456

5557
class AutodetectorTests(AutodetectorTestCase):
56-
def _test_field_addition(self, field):
58+
def _test_field_addition(self, field, expected_db_default=None):
5759
from_model = ModelState("tests", "Model", [])
5860
to_model = ModelState("tests", "Model", [("field", field)])
5961
changes = self.get_changes([from_model], [to_model])["tests"]
@@ -64,7 +66,10 @@ def _test_field_addition(self, field):
6466
pre_operation = changes[0].operations[0]
6567
if field_db_default_supported:
6668
self.assertIsInstance(pre_operation, migrations.AddField)
67-
self.assertEqual(pre_operation.field.db_default, field.default)
69+
self.assertEqual(
70+
pre_operation.field.db_default,
71+
expected_db_default or field.get_default(),
72+
)
6873
else:
6974
self.assertIsInstance(pre_operation, AddField)
7075
self.assertEqual(get_migration_stage(changes[1]), Stage.POST_DEPLOY)
@@ -81,10 +86,33 @@ def test_field_addition(self):
8186
fields = [
8287
models.IntegerField(default=42),
8388
models.IntegerField(null=True, default=42),
89+
models.IntegerField(default=lambda: 42),
90+
# Foreign keys with callable defaults should have their associated
91+
# db_default generated with care.
92+
(models.ForeignKey("tests.Model", models.CASCADE, default=42), 42),
93+
(
94+
models.ForeignKey(
95+
"tests.Bar", models.CASCADE, default=lambda: Bar(id=42)
96+
),
97+
42,
98+
),
99+
(
100+
models.ForeignKey(
101+
"tests.bar",
102+
models.CASCADE,
103+
to_field="name",
104+
default=lambda: Bar(id=123, name="bar"),
105+
),
106+
"bar",
107+
),
84108
]
85109
for field in fields:
110+
if isinstance(field, tuple):
111+
field, expected_db_default = field
112+
else:
113+
expected_db_default = None
86114
with self.subTest(field=field):
87-
self._test_field_addition(field)
115+
self._test_field_addition(field, expected_db_default)
88116

89117
def test_many_to_many_addition(self):
90118
from_model = ModelState("tests", "Model", [])

tests/test_migrations/null_field_removal/0001_initial.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,20 @@ class Migration(migrations.Migration):
1919
("bar", models.IntegerField()),
2020
],
2121
),
22+
migrations.CreateModel(
23+
"Bar",
24+
fields=[
25+
(
26+
"id",
27+
models.AutoField(
28+
auto_created=True,
29+
primary_key=True,
30+
serialize=False,
31+
verbose_name="ID",
32+
),
33+
),
34+
("name", models.CharField(unique=True)),
35+
],
36+
options={"managed": False},
37+
),
2238
]

0 commit comments

Comments
 (0)