Merge pull request #791 from seapagan/bugfix/cli-database-initialization-crash

Fix CLI crash when database not initialized

Author: seapagan

TODO.md

@@ -39,9 +39,6 @@
 - Admin users CAN delete themselves, but this should not be allowed. They should
   be able to delete other users, but not themselves. Or should they?? Need to
   think about this.
-- the `api-admin user` 'list', 'search' and prob others crash if the database is
-  not initialized. This should be handled gracefully, and the CLI should
-  prompt the user to initialize the database if it is not already done.
 
 ## Auth
 

app/commands/db.py

@@ -17,6 +17,7 @@
 from sqlalchemy.exc import SQLAlchemyError
 
 from app.database.db import async_session
+from app.database.helpers import is_database_initialized
 from app.managers.user import ErrorMessages, UserManager
 from app.models.enums import RoleType
 
@@ -266,6 +267,14 @@ async def _populate_db(num_regular_users: int, num_admins: int) -> None:
 
     try:
         async with async_session() as session:
+            # Check if database is initialized
+            if not await is_database_initialized(session):
+                rprint(
+                    "\n[red]-> ERROR: Database has not been initialized.\n"
+                    "[yellow]Please run [bold]'api-admin db init'[/bold] "
+                    "to initialize the database first.\n"
+                )
+                raise typer.Exit(1)
             # Create admin users
             admin_count = 0
             for _ in range(num_admins):
@@ -385,6 +394,16 @@ def _validate_csv_file(csv_file: Path) -> list[dict[str, str]]:
 async def _seed_users_from_csv(csv_file: Path) -> None:
     """Import users from a CSV file into the database."""
     try:
+        # Check if database is initialized first
+        async with async_session() as check_session:
+            if not await is_database_initialized(check_session):
+                rprint(
+                    "\n[red]-> ERROR: Database has not been initialized.\n"
+                    "[yellow]Please run [bold]'api-admin db init'[/bold] "
+                    "to initialize the database first.\n"
+                )
+                raise typer.Exit(1)  # noqa: TRY301
+
         # Validate the CSV file and get rows
         rows = _validate_csv_file(csv_file)
 

app/commands/user.py

@@ -13,6 +13,7 @@
 from sqlalchemy.exc import SQLAlchemyError
 
 from app.database.db import async_session
+from app.database.helpers import is_database_initialized
 from app.managers.user import UserManager
 from app.models.enums import RoleType
 from app.models.user import User
@@ -21,10 +22,30 @@
 if TYPE_CHECKING:  # pragma: no cover
     from collections.abc import Sequence
 
+    from sqlalchemy.ext.asyncio import AsyncSession
+
 
 app = typer.Typer(no_args_is_help=True)
 
 
+async def check_db_initialized(session: AsyncSession) -> None:
+    """Check if database is initialized and exit if not.
+
+    Args:
+        session: The database session to check
+
+    Raises:
+        typer.Exit: If database is not initialized
+    """
+    if not await is_database_initialized(session):
+        rprint(
+            "\n[red]-> ERROR: Database has not been initialized.\n"
+            "[yellow]Please run [bold]'api-admin db init'[/bold] "
+            "to initialize the database first.\n"
+        )
+        raise typer.Exit(1)
+
+
 def show_table(title: str, user_list: Sequence[User]) -> None:
     """Show User data in a tabulated format."""
     console = Console()
@@ -111,6 +132,7 @@ async def _create_user(user_data: dict[str, str | RoleType]) -> None:
         """Async function to create a new user."""
         try:
             async with async_session() as session:
+                await check_db_initialized(session)
                 await UserManager.register(user_data, session)
                 await session.commit()
 
@@ -151,6 +173,7 @@ async def _list_users() -> Sequence[User]:
         """Async function to list all users in the database."""
         try:
             async with async_session() as session:
+                await check_db_initialized(session)
                 user_list = await UserManager.get_all_users(session)
 
         except SQLAlchemyError as exc:
@@ -180,6 +203,7 @@ async def _show_user() -> User:
         """Async function to show details for a single user."""
         try:
             async with async_session() as session:
+                await check_db_initialized(session)
                 user = await UserManager.get_user_by_id(user_id, session)
         except HTTPException as exc:
             rprint(
@@ -208,6 +232,7 @@ async def _verify_user(user_id: int) -> User | None:
         """Async function to verify a user by id."""
         try:
             async with async_session() as session:
+                await check_db_initialized(session)
                 user = await session.get(User, user_id)
                 if user:
                     user.verified = True
@@ -249,6 +274,7 @@ async def _ban_user(user_id: int, *, unban: bool | None) -> User | None:
         """Async function to ban or unban a user."""
         try:
             async with async_session() as session:
+                await check_db_initialized(session)
                 user = await session.get(User, user_id)
                 if user:
                     user.banned = not unban
@@ -296,6 +322,7 @@ async def _toggle_admin(
         """Async function to toggle admin status for a user."""
         try:
             async with async_session() as session:
+                await check_db_initialized(session)
                 user = await session.get(User, user_id)
                 if user:
                     user.role = RoleType.user if remove else RoleType.admin
@@ -333,6 +360,7 @@ async def _delete_user(user_id: int) -> User | None:
         """Async function to delete a user."""
         try:
             async with async_session() as session:
+                await check_db_initialized(session)
                 user = await session.get(User, user_id)
                 if user:
                     await session.delete(user)
@@ -389,6 +417,7 @@ async def _search_users() -> list[User]:
         """Async function to search for users."""
         try:
             async with async_session() as session:
+                await check_db_initialized(session)
                 query = await UserManager.search_users(
                     search_term, field_enum, exact_match=exact
                 )

app/database/helpers.py

@@ -5,7 +5,8 @@
 from uuid import UUID
 
 from passlib.context import CryptContext
-from sqlalchemy import insert, select, update
+from sqlalchemy import insert, select, text, update
+from sqlalchemy.exc import SQLAlchemyError
 from sqlalchemy.ext.asyncio import AsyncSession
 
 from app.models.api_key import ApiKey
@@ -58,6 +59,28 @@ def verify_password(password: str, hashed_password: str) -> bool:
         raise ValueError(error_invalid) from exc
 
 
+async def is_database_initialized(session: AsyncSession) -> bool:
+    """Check if the database has been initialized with Alembic migrations.
+
+    This checks for the existence of the alembic_version table, which is
+    created when Alembic runs migrations.
+
+    Args:
+        session: The database session
+
+    Returns:
+        bool: True if database is initialized, False otherwise
+    """
+    try:
+        # Try to query the alembic_version table
+        await session.execute(text("SELECT version_num FROM alembic_version"))
+    except SQLAlchemyError:
+        # Table doesn't exist or other database error
+        return False
+    else:
+        return True
+
+
 async def get_user_by_id_(user_id: int, session: AsyncSession) -> User | None:
     """Return a user by ID."""
     return await session.get(User, user_id)

tests/cli/test_cli_create_user.py

@@ -26,6 +26,12 @@ class TestCLI:
 
     patch_register_user = "app.commands.user.UserManager.register"
     patch_async_session = "app.commands.user.async_session"
+    patch_is_db_initialized = "app.commands.user.is_database_initialized"
+
+    @pytest.fixture(autouse=True)
+    def _mock_db_initialized(self, mocker) -> None:
+        """Mock is_database_initialized to return True for all tests."""
+        mocker.patch(self.patch_is_db_initialized, return_value=True)
 
     def test_create_user_success(
         self, runner: CliRunner, mocker, fake_user_data

tests/cli/test_cli_db_command.py

@@ -298,6 +298,24 @@ def test_populate_command_custom_count(self, mocker) -> None:
         # Verify _populate_db was called with correct arguments
         populate_mock.assert_called_once_with(7, 2)
 
+    def test_populate_database_not_initialized(self, mocker) -> None:
+        """Test populate command shows error when database not initialized."""
+        # Mock is_database_initialized to return False
+        mocker.patch(
+            "app.commands.db.is_database_initialized", return_value=False
+        )
+
+        # Mock the async_session
+        session_mock = mocker.AsyncMock()
+        session_mock.__aenter__.return_value = session_mock
+        mocker.patch("app.commands.db.async_session", return_value=session_mock)
+
+        result = CliRunner().invoke(app, ["db", "populate", "--count", "5"])
+
+        assert result.exit_code == 1
+        assert "Database has not been initialized" in result.output
+        assert "api-admin db init" in result.output
+
 
 @pytest.mark.asyncio
 class TestPopulateDB:

tests/cli/test_cli_db_seed.py

@@ -172,6 +172,30 @@ def test_seed_missing_default_file(self) -> None:
             assert "Error: File" in result.output
             assert "does not exist" in result.output
 
+    def test_seed_database_not_initialized(self, mocker, tmp_path) -> None:
+        """Test seed command shows error when database not initialized."""
+        # Create a dummy CSV file
+        dummy_file = tmp_path / "users.seed"
+        dummy_file.touch()
+
+        # Mock is_database_initialized to return False
+        mocker.patch(
+            "app.commands.db.is_database_initialized", return_value=False
+        )
+
+        # Mock the async_session
+        session_mock = mocker.AsyncMock()
+        session_mock.__aenter__.return_value = session_mock
+        mocker.patch("app.commands.db.async_session", return_value=session_mock)
+
+        result = CliRunner().invoke(
+            app, ["db", "seed", str(dummy_file), "--force"]
+        )
+
+        assert result.exit_code == 1
+        assert "Database has not been initialized" in result.output
+        assert "api-admin db init" in result.output
+
 
 class TestValidateCsvFile:
     """Test the _validate_csv_file function."""

tests/cli/test_cli_user_command.py

@@ -82,6 +82,12 @@ class TestCLI:
     patch_get_all_users = "app.commands.user.UserManager.get_all_users"
     patch_get_user_by_id = "app.commands.user.UserManager.get_user_by_id"
     patch_async_session = "app.commands.user.async_session"
+    patch_is_db_initialized = "app.commands.user.is_database_initialized"
+
+    @pytest.fixture(autouse=True)
+    def _mock_db_initialized(self, mocker) -> None:
+        """Mock is_database_initialized to return True for all tests."""
+        mocker.patch(self.patch_is_db_initialized, return_value=True)
 
     def test_no_command_should_give_help(self, runner: CliRunner) -> None:
         """Test that running with no command should give help."""
@@ -812,3 +818,65 @@ def test_search_users_db_error(self, runner: CliRunner, mocker) -> None:
         # Check error message
         assert "ERROR searching Users" in result.output
         assert "Database connection failed" in result.output
+
+    # ------------------------------------------------------------------------ #
+    #                  test database initialization check                      #
+    # ------------------------------------------------------------------------ #
+    @pytest.mark.parametrize(
+        "command_args",
+        [
+            ["list"],
+            ["show", "1"],
+            ["search", "test"],
+            ["verify", "1"],
+            ["ban", "1"],
+            ["admin", "1"],
+            ["delete", "1"],
+        ],
+        ids=[
+            "list",
+            "show",
+            "search",
+            "verify",
+            "ban",
+            "admin",
+            "delete",
+        ],
+    )
+    def test_user_commands_database_not_initialized(
+        self, runner: CliRunner, mocker, command_args
+    ) -> None:
+        """Test user commands show error when database not initialized."""
+        # Override the autouse fixture by patching with False
+        mocker.patch(self.patch_is_db_initialized, return_value=False)
+
+        result = runner.invoke(app, ["user", *command_args])
+        assert result.exit_code == 1
+        assert "Database has not been initialized" in result.output
+        assert "api-admin db init" in result.output
+
+    def test_create_user_database_not_initialized(
+        self, runner: CliRunner, mocker, faker
+    ) -> None:
+        """Test create command shows error when database not initialized."""
+        # Override the autouse fixture by patching with False
+        mocker.patch(self.patch_is_db_initialized, return_value=False)
+
+        result = runner.invoke(
+            app,
+            [
+                "user",
+                "create",
+                "--email",
+                faker.email(),
+                "--first_name",
+                faker.first_name(),
+                "--last_name",
+                faker.last_name(),
+                "--password",
+                faker.password(),
+            ],
+        )
+        assert result.exit_code == 1
+        assert "Database has not been initialized" in result.output
+        assert "api-admin db init" in result.output