Skip to content

Commit 89fd9a9

Browse files
seapaganclaude
andcommitted
Fix CLI delete to use admin protection logic
Updates the CLI delete command to use UserManager.delete_user() instead of direct session manipulation, ensuring consistent admin protection between CLI and API endpoints. Changes: - Refactored delete command to call UserManager.delete_user() - Added HTTPException handling for validation errors - Updated all delete tests to mock UserManager instead of session - Added test_delete_last_admin_blocked for admin protection - Removed unnecessary try/except that immediately re-raised The CLI now correctly prevents deletion of the last admin user, matching the behavior of the API endpoint. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent dfcf950 commit 89fd9a9

File tree

2 files changed

+46
-47
lines changed

2 files changed

+46
-47
lines changed

app/commands/user.py

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -356,31 +356,25 @@ def delete(
356356
) -> None:
357357
"""Delete the user with the given id."""
358358

359-
async def _delete_user(user_id: int) -> User | None:
359+
async def _delete_user(user_id: int) -> None:
360360
"""Async function to delete a user."""
361361
try:
362362
async with async_session() as session:
363363
await check_db_initialized(session)
364-
user = await session.get(User, user_id)
365-
if user:
366-
await session.delete(user)
367-
await session.commit()
364+
await UserManager.delete_user(user_id, user_id, session)
365+
await session.commit()
366+
except HTTPException as exc:
367+
rprint(f"\n[RED]-> ERROR deleting that User : [bold]{exc.detail}\n")
368+
raise typer.Exit(1) from exc
368369
except SQLAlchemyError as exc:
369370
rprint(f"\n[RED]-> ERROR deleting that User : [bold]{exc}\n")
370371
raise typer.Exit(1) from exc
371-
else:
372-
return user
373372

374-
user = aiorun(_delete_user(user_id))
375-
376-
if user:
377-
rprint(
378-
f"\n[green]-> User [bold]{user_id}[/bold] "
379-
f"[red]DELETED[/red] succesfully."
380-
)
381-
else:
382-
rprint("\n[red]-> ERROR deleting that User : [bold]User not found\n")
383-
raise typer.Exit(1)
373+
aiorun(_delete_user(user_id))
374+
rprint(
375+
f"\n[green]-> User [bold]{user_id}[/bold] "
376+
f"[red]DELETED[/red] succesfully."
377+
)
384378

385379

386380
@app.command()

tests/cli/test_cli_user_command.py

Lines changed: 35 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -561,40 +561,29 @@ def test_admin_missing_user(
561561
# ------------------------------------------------------------------------ #
562562
def test_delete_user(self, runner: CliRunner, mocker, test_user) -> None:
563563
"""Test that the 'delete' command works."""
564-
mock_session = mocker.patch(
565-
self.patch_async_session,
566-
)
567-
568-
mock_session.return_value.__aenter__.return_value.get.return_value = (
569-
test_user
564+
mock_manager = mocker.patch(
565+
"app.commands.user.UserManager.delete_user",
566+
return_value=None,
570567
)
568+
mocker.patch(self.patch_async_session)
571569

572570
result = runner.invoke(app, ["user", "delete", str(test_user.id)])
573571
assert result.exit_code == 0
574572

575-
assert mock_session.called
576-
assert mock_session.return_value.__aenter__.return_value.commit.called
577-
573+
assert mock_manager.called
578574
assert f"User {test_user.id} DELETED" in result.output
579575

580576
def test_delete_sqlalchemy_error(
581577
self, runner: CliRunner, mocker, test_user
582578
) -> None:
583579
"""Test that the 'delete' command exits when there is an error."""
584-
mock_session = mocker.patch(
585-
self.patch_async_session,
586-
)
587-
mock_session.return_value.__aenter__.return_value.get.side_effect = (
588-
SQLAlchemyError("Ooooops!!")
589-
)
590-
result = runner.invoke(app, ["user", "delete", str(test_user.id)])
591-
assert result.exit_code == 1
592-
593-
assert mock_session.called
594-
assert (
595-
not mock_session.return_value.__aenter__.return_value.commit.called
580+
mocker.patch(
581+
"app.commands.user.UserManager.delete_user",
582+
side_effect=SQLAlchemyError("Ooooops!!"),
596583
)
584+
mocker.patch(self.patch_async_session)
597585

586+
result = runner.invoke(app, ["user", "delete", str(test_user.id)])
598587
assert result.exit_code == 1
599588

600589
assert "ERROR deleting that User" in result.output
@@ -604,23 +593,39 @@ def test_delete_missing_user(
604593
self, runner: CliRunner, mocker, test_user
605594
) -> None:
606595
"""Test that the 'delete' command exits when the user is missing."""
607-
mock_session = mocker.patch(
608-
self.patch_async_session,
609-
)
610-
mock_session.return_value.__aenter__.return_value.get.return_value = (
611-
None
596+
mocker.patch(
597+
"app.commands.user.UserManager.delete_user",
598+
side_effect=HTTPException(
599+
status_code=status.HTTP_404_NOT_FOUND,
600+
detail=ErrorMessages.USER_INVALID,
601+
),
612602
)
603+
mocker.patch(self.patch_async_session)
613604

614605
result = runner.invoke(app, ["user", "delete", str(test_user.id)])
615606
assert result.exit_code == 1
616607

617-
assert mock_session.called
618-
assert (
619-
not mock_session.return_value.__aenter__.return_value.commit.called
608+
assert "ERROR deleting that User" in result.output
609+
assert ErrorMessages.USER_INVALID in result.output
610+
611+
def test_delete_last_admin_blocked(
612+
self, runner: CliRunner, mocker, test_admin
613+
) -> None:
614+
"""Test that deleting the last admin is blocked."""
615+
mocker.patch(
616+
"app.commands.user.UserManager.delete_user",
617+
side_effect=HTTPException(
618+
status_code=status.HTTP_400_BAD_REQUEST,
619+
detail=ErrorMessages.CANT_DELETE_LAST_ADMIN,
620+
),
620621
)
622+
mocker.patch(self.patch_async_session)
623+
624+
result = runner.invoke(app, ["user", "delete", str(test_admin.id)])
625+
assert result.exit_code == 1
621626

622627
assert "ERROR deleting that User" in result.output
623-
assert "User not found" in result.output
628+
assert ErrorMessages.CANT_DELETE_LAST_ADMIN in result.output
624629

625630
# ------------------------------------------------------------------------ #
626631
# test 'search' subcommand #

0 commit comments

Comments
 (0)