Fixed #35813 -- Add AlterField functionality to makemigrations for unmanaged models

django/db/migrations/autodetector.py

@@ -3,7 +3,6 @@
 from collections import defaultdict, namedtuple
 from enum import Enum
 from graphlib import TopologicalSorter
-from itertools import chain
 
 from django.conf import settings
 from django.db import models
@@ -152,23 +151,17 @@ def _detect_changes(self, convert_apps=None, graph=None):
         # proxy models and ignoring unmigrated apps.
         self.old_model_keys = set()
         self.old_proxy_keys = set()
-        self.old_unmanaged_keys = set()
         self.new_model_keys = set()
         self.new_proxy_keys = set()
-        self.new_unmanaged_keys = set()
         for (app_label, model_name), model_state in self.from_state.models.items():
-            if not model_state.options.get("managed", True):
-                self.old_unmanaged_keys.add((app_label, model_name))
-            elif app_label not in self.from_state.real_apps:
+            if app_label not in self.from_state.real_apps:
                 if model_state.options.get("proxy"):
                     self.old_proxy_keys.add((app_label, model_name))
                 else:
                     self.old_model_keys.add((app_label, model_name))
 
         for (app_label, model_name), model_state in self.to_state.models.items():
-            if not model_state.options.get("managed", True):
-                self.new_unmanaged_keys.add((app_label, model_name))
-            elif app_label not in self.from_state.real_apps or (
+            if app_label not in self.from_state.real_apps or (
                 convert_apps and app_label in convert_apps
             ):
                 if model_state.options.get("proxy"):
@@ -238,7 +231,6 @@ def _prepare_field_lists(self):
         """
         self.kept_model_keys = self.old_model_keys & self.new_model_keys
         self.kept_proxy_keys = self.old_proxy_keys & self.new_proxy_keys
-        self.kept_unmanaged_keys = self.old_unmanaged_keys & self.new_unmanaged_keys
         self.through_users = {}
         self.old_field_keys = {
             (app_label, model_name, field_name)
@@ -656,12 +648,10 @@ def generate_created_models(self):
         Defer any model options that refer to collections of fields that might
         be deferred (e.g. unique_together).
         """
-        old_keys = self.old_model_keys | self.old_unmanaged_keys
+        old_keys = self.old_model_keys
         added_models = self.new_model_keys - old_keys
-        added_unmanaged_models = self.new_unmanaged_keys - old_keys
-        all_added_models = chain(
-            sorted(added_models, key=self.swappable_first_key, reverse=True),
-            sorted(added_unmanaged_models, key=self.swappable_first_key, reverse=True),
+        all_added_models = sorted(
+            added_models, key=self.swappable_first_key, reverse=True
         )
         for app_label, model_name in all_added_models:
             model_state = self.to_state.models[app_label, model_name]
@@ -755,11 +745,6 @@ def generate_created_models(self):
                 beginning=True,
             )
 
-            # Don't add operations which modify the database for unmanaged
-            # models
-            if not model_state.options.get("managed", True):
-                continue
-
             # Generate operations for each related field
             for name, field in sorted(related_fields.items()):
                 dependencies = self._get_dependencies_for_foreign_key(
@@ -913,20 +898,16 @@ def generate_created_proxies(self):
 
     def generate_deleted_models(self):
         """
-        Find all deleted models (managed and unmanaged) and make delete
-        operations for them as well as separate operations to delete any
-        foreign key or M2M relationships (these are optimized later, if
-        possible).
+        Find all deleted models and make delete operations for them as well
+        as separate operations to delete any foreign key or M2M relationships
+        (these are optimized later, if possible).
 
         Also bring forward removal of any model options that refer to
         collections of fields - the inverse of generate_created_models().
         """
-        new_keys = self.new_model_keys | self.new_unmanaged_keys
+        new_keys = self.new_model_keys
         deleted_models = self.old_model_keys - new_keys
-        deleted_unmanaged_models = self.old_unmanaged_keys - new_keys
-        all_deleted_models = chain(
-            sorted(deleted_models), sorted(deleted_unmanaged_models)
-        )
+        all_deleted_models = sorted(deleted_models)
         for app_label, model_name in all_deleted_models:
             model_state = self.from_state.models[app_label, model_name]
             # Gather related fields
@@ -1840,9 +1821,7 @@ def generate_altered_unique_together(self):
         self._generate_altered_foo_together(operations.AlterUniqueTogether)
 
     def generate_altered_db_table(self):
-        models_to_check = self.kept_model_keys.union(
-            self.kept_proxy_keys, self.kept_unmanaged_keys
-        )
+        models_to_check = self.kept_model_keys.union(self.kept_proxy_keys)
         for app_label, model_name in sorted(models_to_check):
             old_model_name = self.renamed_models.get(
                 (app_label, model_name), model_name
@@ -1861,9 +1840,7 @@ def generate_altered_db_table(self):
                 )
 
     def generate_altered_db_table_comment(self):
-        models_to_check = self.kept_model_keys.union(
-            self.kept_proxy_keys, self.kept_unmanaged_keys
-        )
+        models_to_check = self.kept_model_keys.union(self.kept_proxy_keys)
         for app_label, model_name in sorted(models_to_check):
             old_model_name = self.renamed_models.get(
                 (app_label, model_name), model_name
@@ -1890,11 +1867,6 @@ def generate_altered_options(self):
         """
         models_to_check = self.kept_model_keys.union(
             self.kept_proxy_keys,
-            self.kept_unmanaged_keys,
-            # unmanaged converted to managed
-            self.old_unmanaged_keys & self.new_model_keys,
-            # managed converted to unmanaged
-            self.old_model_keys & self.new_unmanaged_keys,
         )
 
         for app_label, model_name in sorted(models_to_check):

docs/releases/6.1.txt

@@ -274,6 +274,11 @@ Management Commands
   :data:`~django.db.models.signals.m2m_changed` signals with ``raw=True`` when
   loading fixtures.
 
+* The :djadmin:`makemigrations` command now tracks all changes to unmanaged
+  models, including field additions, removals, alterations, constraints and
+  model renames. After upgrading, you will see new migrations detected for
+  unmanaged models that have changed.
+
 Models
 ~~~~~~
 

tests/migrations/test_autodetector.py

@@ -712,6 +712,72 @@ class AutodetectorTests(BaseAutodetectorTests):
     author_unmanaged = ModelState(
         "testapp", "AuthorUnmanaged", [], {"managed": False}, ("testapp.author",)
     )
+    author_unmanaged_empty = ModelState(
+        "testapp",
+        "Author",
+        [("id", models.AutoField(primary_key=True))],
+        {"managed": False},
+        ("testapp.author",),
+    )
+    author_unmanaged_name_check_constraint = ModelState(
+        "testapp",
+        "Author",
+        [
+            ("id", models.AutoField(primary_key=True)),
+            ("name", models.CharField(max_length=200, default="Ada Lovelace")),
+        ],
+        {
+            "managed": False,
+            "constraints": [
+                models.CheckConstraint(
+                    condition=models.Q(name__contains="Bob"), name="name_contains_bob"
+                )
+            ],
+        },
+        ("testapp.author",),
+    )
+    author_unmanaged_with_book = ModelState(
+        "testapp",
+        "Author",
+        [
+            ("id", models.AutoField(primary_key=True)),
+            ("name", models.CharField(max_length=200)),
+            ("book", models.ForeignKey("otherapp.Book", models.CASCADE)),
+        ],
+        {"managed": False},
+        ("testapp.author",),
+    )
+    author_unmanaged_name_default = ModelState(
+        "testapp",
+        "Author",
+        [
+            ("id", models.AutoField(primary_key=True)),
+            ("name", models.CharField(max_length=200, default="Ada Lovelace")),
+        ],
+        {"managed": False},
+        ("testapp.author",),
+    )
+    author_unmanaged_name_longer = ModelState(
+        "testapp",
+        "Author",
+        [
+            ("id", models.AutoField(primary_key=True)),
+            ("name", models.CharField(max_length=400)),
+        ],
+        {"managed": False},
+        ("testapp.author",),
+    )
+    author_unmanaged_renamed_with_book = ModelState(
+        "testapp",
+        "Writer",
+        [
+            ("id", models.AutoField(primary_key=True)),
+            ("name", models.CharField(max_length=200)),
+            ("book", models.ForeignKey("otherapp.Book", models.CASCADE)),
+        ],
+        {"managed": False},
+        ("testapp.author",),
+    )
     author_unmanaged_managed = ModelState(
         "testapp", "AuthorUnmanaged", [], {}, ("testapp.author",)
     )
@@ -829,6 +895,17 @@ class AutodetectorTests(BaseAutodetectorTests):
         ],
         {"db_table": "author_three"},
     )
+    book_unmanaged = ModelState(
+        "otherapp",
+        "Book",
+        [
+            ("id", models.AutoField(primary_key=True)),
+            ("author", models.ForeignKey("testapp.Author", models.CASCADE)),
+            ("title", models.CharField(max_length=200)),
+        ],
+        {"managed": False},
+        ("otherapp.book",),
+    )
     contract = ModelState(
         "testapp",
         "Contract",
@@ -968,6 +1045,17 @@ class AutodetectorTests(BaseAutodetectorTests):
             ("title", models.CharField(max_length=200)),
         ],
     )
+    book_with_author_unmanaged_renamed = ModelState(
+        "otherapp",
+        "Book",
+        [
+            ("id", models.AutoField(primary_key=True)),
+            ("author", models.ForeignKey("testapp.Writer", models.CASCADE)),
+            ("title", models.CharField(max_length=200)),
+        ],
+        {"managed": False},
+        ("otherapp.book",),
+    )
     book_with_field_and_author_renamed = ModelState(
         "otherapp",
         "Book",
@@ -4019,8 +4107,45 @@ def test_proxy_to_mti_with_fk_to_proxy_proxy(self):
         )
         self.assertEqual(fk_field.remote_field.model, "testapp.AAuthorProxyProxy")
 
+    def test_unmanaged_add_constraints(self):
+        """Test change detection of new constraints."""
+        changes = self.get_changes(
+            [self.author_unmanaged_name_default],
+            [self.author_unmanaged_name_check_constraint],
+        )
+        self.assertNumberMigrations(changes, "testapp", 1)
+        self.assertOperationTypes(changes, "testapp", 0, ["AddConstraint"])
+        added_constraint = models.CheckConstraint(
+            condition=models.Q(name__contains="Bob"), name="name_contains_bob"
+        )
+        self.assertOperationAttributes(
+            changes, "testapp", 0, 0, model_name="author", constraint=added_constraint
+        )
+
+    def test_unmanaged_add_field(self):
+        """Tests autodetection of new fields."""
+        changes = self.get_changes(
+            [self.author_unmanaged_empty], [self.author_unmanaged_name_default]
+        )
+        # Right number/type of migrations?
+        self.assertNumberMigrations(changes, "testapp", 1)
+        self.assertOperationTypes(changes, "testapp", 0, ["AddField"])
+        self.assertOperationAttributes(changes, "testapp", 0, 0, name="name")
+
+    def test_unmanaged_alter_field(self):
+        """Tests autodetection of new fields on an unmanaged model."""
+        changes = self.get_changes(
+            [self.author_unmanaged_name_default], [self.author_unmanaged_name_longer]
+        )
+        # Right number/type of migrations?
+        self.assertNumberMigrations(changes, "testapp", 1)
+        self.assertOperationTypes(changes, "testapp", 0, ["AlterField"])
+        self.assertOperationAttributes(
+            changes, "testapp", 0, 0, name="name", preserve_default=True
+        )
+
     def test_unmanaged_create(self):
-        """The autodetector correctly deals with managed models."""
+        """The autodetector correctly deals with unmanaged models."""
         # First, we test adding an unmanaged model
         changes = self.get_changes(
             [self.author_empty], [self.author_empty, self.author_unmanaged]
@@ -4032,6 +4157,50 @@ def test_unmanaged_create(self):
             changes, "testapp", 0, 0, name="AuthorUnmanaged", options={"managed": False}
         )
 
+    def test_unmanaged_remove_field(self):
+        """Tests autodetection of removed fields."""
+        changes = self.get_changes(
+            [self.author_unmanaged_name_default], [self.author_unmanaged_empty]
+        )
+        # Right number/type of migrations?
+        self.assertNumberMigrations(changes, "testapp", 1)
+        self.assertOperationTypes(changes, "testapp", 0, ["RemoveField"])
+        self.assertOperationAttributes(
+            changes, "testapp", 0, 0, name="name", model_name="author"
+        )
+
+    def test_unmanaged_remove_constraints(self):
+        """Test change detection of new constraints."""
+        changes = self.get_changes(
+            [self.author_unmanaged_name_check_constraint],
+            [self.author_unmanaged_name_default],
+        )
+        self.assertNumberMigrations(changes, "testapp", 1)
+        self.assertOperationTypes(changes, "testapp", 0, ["RemoveConstraint"])
+        self.assertOperationAttributes(
+            changes, "testapp", 0, 0, model_name="author", name="name_contains_bob"
+        )
+
+    def test_unmanaged_rename_model(self):
+        """Tests autodetection of renamed models."""
+        changes = self.get_changes(
+            [self.author_unmanaged_with_book, self.book_unmanaged],
+            [
+                self.author_unmanaged_renamed_with_book,
+                self.book_with_author_unmanaged_renamed,
+            ],
+            MigrationQuestioner({"ask_rename_model": True}),
+        )
+        # Right number/type of migrations?
+        self.assertNumberMigrations(changes, "testapp", 1)
+        self.assertOperationTypes(changes, "testapp", 0, ["RenameModel"])
+        self.assertOperationAttributes(
+            changes, "testapp", 0, 0, old_name="Author", new_name="Writer"
+        )
+        # Now that RenameModel handles related fields too, there should be
+        # no AlterField for the related field.
+        self.assertNumberMigrations(changes, "otherapp", 0)
+
     def test_unmanaged_delete(self):
         changes = self.get_changes(
             [self.author_empty, self.author_unmanaged], [self.author_empty]