Skip to content

Commit

Permalink
Fixed #28987 -- Fixed altering ManyToManyField when changing to self-…
Browse files Browse the repository at this point in the history
…referential.
  • Loading branch information
DevilsAutumn authored and felixxm committed Nov 17, 2022
1 parent 2848e5d commit 81b1c16
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 18 deletions.
49 changes: 31 additions & 18 deletions django/db/backends/sqlite3/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ def alter_field(self, model, old_field, new_field, strict=False):
super().alter_field(model, old_field, new_field, strict=strict)

def _remake_table(
self, model, create_field=None, delete_field=None, alter_field=None
self, model, create_field=None, delete_field=None, alter_fields=None
):
"""
Shortcut to transform a model from old_model into new_model
Expand Down Expand Up @@ -213,15 +213,16 @@ def is_self_referential(f):
# If any of the new or altered fields is introducing a new PK,
# remove the old one
restore_pk_field = None
if getattr(create_field, "primary_key", False) or (
alter_field and getattr(alter_field[1], "primary_key", False)
alter_fields = alter_fields or []
if getattr(create_field, "primary_key", False) or any(
getattr(new_field, "primary_key", False) for _, new_field in alter_fields
):
for name, field in list(body.items()):
if field.primary_key and not (
if field.primary_key and not any(
# Do not remove the old primary key when an altered field
# that introduces a primary key is the same field.
alter_field
and name == alter_field[1].name
name == new_field.name
for _, new_field in alter_fields
):
field.primary_key = False
restore_pk_field = field
Expand All @@ -237,7 +238,7 @@ def is_self_referential(f):
self.effective_default(create_field),
)
# Add in any altered fields
if alter_field:
for alter_field in alter_fields:
old_field, new_field = alter_field
body.pop(old_field.name, None)
mapping.pop(old_field.column, None)
Expand Down Expand Up @@ -457,7 +458,7 @@ def _alter_field(
)
)
# Alter by remaking table
self._remake_table(model, alter_field=(old_field, new_field))
self._remake_table(model, alter_fields=[(old_field, new_field)])
# Rebuild tables with FKs pointing to this field.
old_collation = old_db_params.get("collation")
new_collation = new_db_params.get("collation")
Expand Down Expand Up @@ -495,18 +496,30 @@ def _alter_many_to_many(self, model, old_field, new_field, strict):
# propagate this altering.
self._remake_table(
old_field.remote_field.through,
alter_field=(
# The field that points to the target model is needed, so
# we can tell alter_field to change it - this is
# m2m_reverse_field_name() (as opposed to m2m_field_name(),
# which points to our model).
old_field.remote_field.through._meta.get_field(
old_field.m2m_reverse_field_name()
alter_fields=[
(
# The field that points to the target model is needed,
# so that table can be remade with the new m2m field -
# this is m2m_reverse_field_name().
old_field.remote_field.through._meta.get_field(
old_field.m2m_reverse_field_name()
),
new_field.remote_field.through._meta.get_field(
new_field.m2m_reverse_field_name()
),
),
new_field.remote_field.through._meta.get_field(
new_field.m2m_reverse_field_name()
(
# The field that points to the model itself is needed,
# so that table can be remade with the new self field -
# this is m2m_field_name().
old_field.remote_field.through._meta.get_field(
old_field.m2m_field_name()
),
new_field.remote_field.through._meta.get_field(
new_field.m2m_field_name()
),
),
),
],
)
return

Expand Down
37 changes: 37 additions & 0 deletions tests/migrations/test_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -1796,6 +1796,43 @@ def test_alter_model_table_m2m(self):
self.assertTableExists(original_m2m_table)
self.assertTableNotExists(new_m2m_table)

def test_alter_model_table_m2m_field(self):
app_label = "test_talm2mfl"
project_state = self.set_up_test_model(app_label, second_model=True)
# Add the M2M field.
project_state = self.apply_operations(
app_label,
project_state,
operations=[
migrations.AddField(
"Pony",
"stables",
models.ManyToManyField("Stable"),
)
],
)
m2m_table = f"{app_label}_pony_stables"
self.assertColumnExists(m2m_table, "pony_id")
self.assertColumnExists(m2m_table, "stable_id")
# Point the M2M field to self.
with_field_state = project_state.clone()
operations = [
migrations.AlterField(
model_name="Pony",
name="stables",
field=models.ManyToManyField("self"),
)
]
project_state = self.apply_operations(
app_label, project_state, operations=operations
)
self.assertColumnExists(m2m_table, "from_pony_id")
self.assertColumnExists(m2m_table, "to_pony_id")
# Reversal.
self.unapply_operations(app_label, with_field_state, operations=operations)
self.assertColumnExists(m2m_table, "pony_id")
self.assertColumnExists(m2m_table, "stable_id")

def test_alter_field(self):
"""
Tests the AlterField operation.
Expand Down

0 comments on commit 81b1c16

Please sign in to comment.