Opened 13 months ago
Last modified 13 months ago
#34988 assigned Cleanup/optimization
Makemigrations shouldn't prompt for default values for non-nullable fields of other apps.
Reported by: | Sarah Boyce | Owned by: | Bhuvnesh |
---|---|---|---|
Component: | Migrations | Version: | dev |
Severity: | Normal | Keywords: | makemigrations |
Cc: | Bhuvnesh | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Have more than one app (app_1 and app_2). In app_1 have a model that has it's migrations applied then add a field that doesn't have a default and is not nullable (do not run makemigrations).
Then add a new model (or anything that would require a migration) to app_2.
Run manage.py makemigrations app_2
You get prompted for every app that has unapplied migrations missing defaults even though you're running migrations for a different app.
I would expect it not to care, as you can input temporary defaults to all these other apps and it doesn't do anything with them.
I think this might be a test case:
diff --git a/tests/migrations/test_commands.py b/tests/migrations/test_commands.py index a9c1cdf893..518ebef872 100644 --- a/tests/migrations/test_commands.py +++ b/tests/migrations/test_commands.py @@ -1979,6 +1979,39 @@ class MakeMigrationsTests(MigrationTestBase): self.assertIn("Remove field silly_field from sillymodel", out.getvalue()) self.assertIn("Add field silly_rename to sillymodel", out.getvalue()) + @override_settings( + INSTALLED_APPS=[ + "migrations", + "migrations.migrations_test_apps.migrated_app", + ] + ) + def test_makemigrations_interactive_not_null_addition_multiple_apps_single_call(self): + class Author(models.Model): + silly_author_field = models.BooleanField(null=False) + + class Meta: + app_label = "migrations" + + class NewModel1(models.Model): + class Meta: + app_label = "migrated_app" + + input_msg = ( + "It is impossible to add a non-nullable field 'silly_author_field' to " + "author without specifying a default. This is because the " + "database needs something to populate existing rows.\n" + "Please select a fix:\n" + " 1) Provide a one-off default now (will be set on all existing " + "rows with a null value for this column)\n" + " 2) Quit and manually define a default value in models.py." + ) + with self.temporary_migration_module(module="migrations.test_migrations"): + with mock.patch("builtins.input", return_value="1"): + with captured_stdout() as out: + call_command("makemigrations", "migrated_app", interactive=True) + output = out.getvalue() + self.assertNotIn(input_msg, output) + @mock.patch("builtins.input", return_value="Y") def test_makemigrations_model_rename_interactive(self, mock_input): class RenamedModel(models.Model):
Change History (6)
comment:1 by , 13 months ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 13 months ago
Just thinking about whether this is feasible to do: The autodetector is documented as being designed to run on entire projects but I wonder if it's possible to delay prompting the questioner for various things it requests until after _trim_changes()
is called.
comment:4 by , 13 months ago
Cc: | added |
---|
comment:5 by , 13 months ago
Type: | Uncategorized → Cleanup/optimization |
---|
comment:6 by , 13 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
The autodetector is documented as being designed to run on entire projects but I wonder if it's possible to delay prompting the questioner for various things it requests until after _trim_changes() is called.
Hi, David ! I cannot fully understand what you are trying the propose, it will be helpful if you could please elaborate this a bit. 😅
I'm working on one more approach - passing app_label from auto-detector and restricting questioner for apps other than specified apps. This is working for independent apps but not for dependent apps. Need to look more into this.
Thanks again for another report 🏆🏆
Confirmed this is indeed happening for apps with mutually independent migrations ✓
What's more is that if you run
makemigrations
a second time it asks for a default but then reports "No changes detected in app <app-specified>"