862: allow piccolo migrations new all --auto (#864)

* allow `piccolo migrations new all --auto`

* remove unused import

* fix old docstring

* make sure the correct app config is being used

* rearrange print statements

* improve UI when making migrations

* add test for app sorting

piccolo/apps/migrations/commands/backwards.py

@@ -11,6 +11,7 @@
 )
 from piccolo.apps.migrations.tables import Migration
 from piccolo.conf.apps import AppConfig, MigrationModule
+from piccolo.utils.printing import print_heading
 
 
 class BackwardsMigrationManager(BaseMigrationManager):
@@ -136,8 +137,7 @@ async def run_backwards(
         if _continue not in "yY":
             return MigrationResult(success=False, message="user cancelled")
         for _app_name in sorted_app_names:
-            print(f"\n{_app_name.upper():^64}")
-            print("-" * 64)
+            print_heading(_app_name)
             manager = BackwardsMigrationManager(
                 app_name=_app_name,
                 migration_id="all",

piccolo/apps/migrations/commands/forwards.py

@@ -10,6 +10,7 @@
 )
 from piccolo.apps.migrations.tables import Migration
 from piccolo.conf.apps import AppConfig, MigrationModule
+from piccolo.utils.printing import print_heading
 
 
 class ForwardsMigrationManager(BaseMigrationManager):
@@ -109,8 +110,7 @@ async def run_forwards(
     if app_name == "all":
         sorted_app_names = BaseMigrationManager().get_sorted_app_names()
         for _app_name in sorted_app_names:
-            print(f"\n{_app_name.upper():^64}")
-            print("-" * 64)
+            print_heading(_app_name)
             manager = ForwardsMigrationManager(
                 app_name=_app_name,
                 migration_id="all",

piccolo/apps/migrations/commands/new.py

@@ -21,6 +21,7 @@
 )
 from piccolo.conf.apps import AppConfig, Finder
 from piccolo.engine import SQLiteEngine
+from piccolo.utils.printing import print_heading
 
 from .base import BaseMigrationManager
 
@@ -217,7 +218,8 @@ async def new(
     Creates a new migration file in the migrations folder.
 
     :param app_name:
-        The app to create a migration for.
+        The app to create a migration for. Specify a value of 'all' to create
+        migrations for all apps (use in conjunction with --auto).
     :param auto:
         Auto create the migration contents.
     :param desc:
@@ -228,22 +230,41 @@ async def new(
         entered. For example, --auto_input='y'.
 
     """
-    print("Creating new migration ...")
-
     engine = Finder().get_engine()
     if auto and isinstance(engine, SQLiteEngine):
         sys.exit("Auto migrations aren't currently supported by SQLite.")
 
-    app_config = Finder().get_app_config(app_name=app_name)
+    if app_name == "all" and not auto:
+        raise ValueError(
+            "Only use `--app_name=all` in conjunction with `--auto`."
+        )
 
-    _create_migrations_folder(app_config.migrations_folder_path)
-    try:
-        await _create_new_migration(
-            app_config=app_config,
-            auto=auto,
-            description=desc,
-            auto_input=auto_input,
+    app_names = (
+        sorted(
+            BaseMigrationManager().get_app_names(
+                sort_by_migration_dependencies=False
+            )
         )
-    except NoChanges:
-        print("No changes detected - exiting.")
-        sys.exit(0)
+        if app_name == "all"
+        else [app_name]
+    )
+
+    for app_name in app_names:
+        print_heading(app_name)
+        print("🚀 Creating new migration ...")
+
+        app_config = Finder().get_app_config(app_name=app_name)
+
+        _create_migrations_folder(app_config.migrations_folder_path)
+
+        try:
+            await _create_new_migration(
+                app_config=app_config,
+                auto=auto,
+                description=desc,
+                auto_input=auto_input,
+            )
+        except NoChanges:
+            print("🏁 No changes detected.")
+
+    print("\n✅ Finished\n")

piccolo/conf/apps.py

@@ -1,6 +1,5 @@
 from __future__ import annotations
 
-import functools
 import inspect
 import itertools
 import os
@@ -13,6 +12,7 @@
 
 from piccolo.engine.base import Engine
 from piccolo.table import Table
+from piccolo.utils.graphlib import TopologicalSorter
 from piccolo.utils.warnings import Level, colored_warning
 
 
@@ -159,26 +159,38 @@ def __post_init__(self):
         if isinstance(self.migrations_folder_path, pathlib.Path):
             self.migrations_folder_path = str(self.migrations_folder_path)
 
+        self._migration_dependency_app_configs: t.Optional[
+            t.List[AppConfig]
+        ] = None
+
     def register_table(self, table_class: t.Type[Table]):
         self.table_classes.append(table_class)
         return table_class
 
     @property
     def migration_dependency_app_configs(self) -> t.List[AppConfig]:
         """
-        Get all of the AppConfig instances from this app's migration
+        Get all of the ``AppConfig`` instances from this app's migration
         dependencies.
         """
-        modules: t.List[PiccoloAppModule] = [
-            t.cast(PiccoloAppModule, import_module(module_path))
-            for module_path in self.migration_dependencies
-        ]
-        return [i.APP_CONFIG for i in modules]
+        # We cache the value so it's more efficient, and also so we can set the
+        # underlying value in unit tests for easier mocking.
+        if self._migration_dependency_app_configs is None:
+
+            modules: t.List[PiccoloAppModule] = [
+                t.cast(PiccoloAppModule, import_module(module_path))
+                for module_path in self.migration_dependencies
+            ]
+            self._migration_dependency_app_configs = [
+                i.APP_CONFIG for i in modules
+            ]
+
+        return self._migration_dependency_app_configs
 
     def get_table_with_name(self, table_class_name: str) -> t.Type[Table]:
         """
-        Returns a Table subclass with the given name from this app, if it
-        exists. Otherwise raises a ValueError.
+        Returns a ``Table`` subclass with the given name from this app, if it
+        exists. Otherwise raises a ``ValueError``.
         """
         filtered = [
             table_class
@@ -426,44 +438,69 @@ def get_app_modules(self) -> t.List[PiccoloAppModule]:
 
         return app_modules
 
-    def get_app_names(self, sort: bool = True) -> t.List[str]:
+    def get_app_names(
+        self, sort_by_migration_dependencies: bool = True
+    ) -> t.List[str]:
         """
         Return all of the app names.
 
-        :param sort:
+        :param sort_by_migration_dependencies:
             If True, sorts the app names using the migration dependencies, so
             dependencies are before dependents in the list.
 
         """
-        modules = self.get_app_modules()
-        configs: t.List[AppConfig] = [module.APP_CONFIG for module in modules]
+        return [
+            i.app_name
+            for i in self.get_app_configs(
+                sort_by_migration_dependencies=sort_by_migration_dependencies
+            )
+        ]
+
+    def get_sorted_app_names(self) -> t.List[str]:
+        """
+        Just here for backwards compatibility - use ``get_app_names`` directly.
+        """
+        return self.get_app_names(sort_by_migration_dependencies=True)
 
-        if not sort:
-            return [i.app_name for i in configs]
+    def sort_app_configs(
+        self, app_configs: t.List[AppConfig]
+    ) -> t.List[AppConfig]:
+        app_config_map = {
+            app_config.app_name: app_config for app_config in app_configs
+        }
 
-        def sort_app_configs(app_config_1: AppConfig, app_config_2: AppConfig):
-            return (
-                app_config_1 in app_config_2.migration_dependency_app_configs
-            )
+        sorted_app_names = TopologicalSorter(
+            {
+                app_config.app_name: [
+                    i.app_name
+                    for i in app_config.migration_dependency_app_configs
+                ]
+                for app_config in app_config_map.values()
+            }
+        ).static_order()
 
-        sorted_configs = sorted(
-            configs, key=functools.cmp_to_key(sort_app_configs)
-        )
-        return [i.app_name for i in sorted_configs]
+        return [app_config_map[i] for i in sorted_app_names]
 
-    def get_sorted_app_names(self) -> t.List[str]:
+    def get_app_configs(
+        self, sort_by_migration_dependencies: bool = True
+    ) -> t.List[AppConfig]:
         """
-        Just here for backwards compatibility.
+        Returns a list of ``AppConfig``, optionally sorted by migration
+        dependencies.
         """
-        return self.get_app_names(sort=True)
+        app_configs = [i.APP_CONFIG for i in self.get_app_modules()]
+
+        return (
+            self.sort_app_configs(app_configs=app_configs)
+            if sort_by_migration_dependencies
+            else app_configs
+        )
 
     def get_app_config(self, app_name: str) -> AppConfig:
         """
         Returns an ``AppConfig`` for the given app name.
         """
-        modules = self.get_app_modules()
-        for module in modules:
-            app_config = module.APP_CONFIG
+        for app_config in self.get_app_configs():
             if app_config.app_name == app_name:
                 return app_config
         raise ValueError(f"No app found with name {app_name}")
@@ -487,7 +524,7 @@ def get_table_classes(
     ) -> t.List[t.Type[Table]]:
         """
         Returns all ``Table`` classes registered with the given apps. If
-        ``app_names`` is ``None``, then ``Table`` classes will be returned
+        ``include_apps`` is ``None``, then ``Table`` classes will be returned
         for all apps.
         """
         if include_apps and exclude_apps:

piccolo/utils/printing.py

@@ -1,8 +1,18 @@
 def get_fixed_length_string(string: str, length=20) -> str:
     """
-    Add spacing to the end of the string so it's a fixed length.
+    Add spacing to the end of the string so it's a fixed length, or truncate
+    if it's too long.
     """
     if len(string) > length:
         return f"{string[: length - 3]}..."
     spacing = "".join(" " for _ in range(length - len(string)))
     return f"{string}{spacing}"
+
+
+def print_heading(string: str, width: int = 64) -> None:
+    """
+    Prints out a nicely formatted heading to the console. Useful for breaking
+    up the output in large CLI commands.
+    """
+    print(f"\n{string.upper():^{width}}")
+    print("-" * width)

tests/apps/migrations/commands/test_new.py

@@ -18,7 +18,7 @@
 
 
 class TestNewMigrationCommand(TestCase):
-    def test_create_new_migration(self):
+    def test_manual(self):
         """
         Create a manual migration (i.e. non-auto).
         """
@@ -46,17 +46,50 @@ def test_create_new_migration(self):
 
     @engines_only("postgres")
     @patch("piccolo.apps.migrations.commands.new.print")
-    def test_new_command(self, print_: MagicMock):
+    def test_auto(self, print_: MagicMock):
         """
         Call the command, when no migration changes are needed.
         """
-        with self.assertRaises(SystemExit) as manager:
-            run_sync(new(app_name="music", auto=True))
+        run_sync(new(app_name="music", auto=True))
 
-        self.assertEqual(manager.exception.code, 0)
+        self.assertListEqual(
+            print_.call_args_list,
+            [
+                call("🚀 Creating new migration ..."),
+                call("🏁 No changes detected."),
+                call("\n✅ Finished\n"),
+            ],
+        )
+
+    @engines_only("postgres")
+    @patch("piccolo.apps.migrations.commands.new.print")
+    def test_auto_all(self, print_: MagicMock):
+        """
+        Try auto migrating all apps.
+        """
+        run_sync(new(app_name="all", auto=True))
+        self.assertListEqual(
+            print_.call_args_list,
+            [
+                call("🚀 Creating new migration ..."),
+                call("🏁 No changes detected."),
+                call("🚀 Creating new migration ..."),
+                call("🏁 No changes detected."),
+                call("\n✅ Finished\n"),
+            ],
+        )
 
-        self.assertTrue(
-            print_.mock_calls[-1] == call("No changes detected - exiting.")
+    @engines_only("postgres")
+    def test_auto_all_error(self):
+        """
+        Call the command, when no migration changes are needed.
+        """
+        with self.assertRaises(ValueError) as manager:
+            run_sync(new(app_name="all", auto=False))
+
+        self.assertEqual(
+            manager.exception.__str__(),
+            "Only use `--app_name=all` in conjunction with `--auto`.",
         )
 
 

tests/conf/test_apps.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 import pathlib
 from unittest import TestCase
 
@@ -270,3 +272,35 @@ def test_get_table_classes(self):
             finder.get_table_classes(
                 exclude_apps=["music"], include_apps=["mega"]
             )
+
+    def test_sort_app_configs(self):
+        """
+        Make sure we can sort ``AppConfig`` based on their migration
+        dependencies.
+        """
+        app_config_1 = AppConfig(
+            app_name="app_1",
+            migrations_folder_path="",
+        )
+
+        app_config_1._migration_dependency_app_configs = [
+            AppConfig(
+                app_name="app_2",
+                migrations_folder_path="",
+            )
+        ]
+
+        app_config_2 = AppConfig(
+            app_name="app_2",
+            migrations_folder_path="",
+        )
+
+        app_config_2._migration_dependency_app_configs = []
+
+        sorted_app_configs = Finder().sort_app_configs(
+            [app_config_2, app_config_1]
+        )
+
+        self.assertListEqual(
+            [i.app_name for i in sorted_app_configs], ["app_2", "app_1"]
+        )