refactor(user): deactivate more accurately describes this action

Author: thekaveman

compiler_admin/commands/user/__init__.py

@@ -1,8 +1,8 @@
 import click
 
-from compiler_admin.commands.user.alumni import alumni
 from compiler_admin.commands.user.convert import convert
 from compiler_admin.commands.user.create import create
+from compiler_admin.commands.user.deactivate import deactivate
 from compiler_admin.commands.user.delete import delete
 from compiler_admin.commands.user.offboard import offboard
 from compiler_admin.commands.user.reset import reset
@@ -18,9 +18,9 @@ def user():
     pass
 
 
-user.add_command(alumni)
 user.add_command(convert)
 user.add_command(create)
+user.add_command(deactivate)
 user.add_command(delete)
 user.add_command(offboard)
 user.add_command(reset)

compiler_admin/commands/user/convert.py

@@ -1,11 +1,9 @@
 import click
 
 from compiler_admin import RESULT_FAILURE
-from compiler_admin.commands.user.alumni import alumni
 from compiler_admin.services.google import (
     GROUP_PARTNERS,
     GROUP_STAFF,
-    OU_ALUMNI,
     OU_CONTRACTORS,
     OU_PARTNERS,
     OU_STAFF,
@@ -19,24 +17,11 @@
 )
 
 
-ACCOUNT_TYPE_OU = {"alumni": OU_ALUMNI, "contractor": OU_CONTRACTORS, "partner": OU_PARTNERS, "staff": OU_STAFF}
+ACCOUNT_TYPE_OU = {"contractor": OU_CONTRACTORS, "partner": OU_PARTNERS, "staff": OU_STAFF}
 
 
 @click.command()
 @click.option("-f", "--force", is_flag=True, help="Don't ask for confirmation.")
-@click.option(
-    "-n", "--notify", help="An email address to send the new password notification. Only valid for alumni conversion."
-)
-@click.option(
-    "-e",
-    "--recovery-email",
-    help="An email address to use as the new recovery email. Only valid for alumni conversion.",
-)
-@click.option(
-    "-p",
-    "--recovery-phone",
-    help="A phone number to use as the new recovery phone number. Only valid for alumni conversion.",
-)
 @click.argument("username")
 @click.argument("account_type", type=click.Choice(ACCOUNT_TYPE_OU.keys(), case_sensitive=False))
 @click.pass_context
@@ -52,11 +37,7 @@ def convert(ctx: click.Context, username: str, account_type: str, **kwargs):
 
     click.echo(f"User exists, converting to: {account_type} for {account}")
 
-    if account_type == "alumni":
-        # call the alumni command
-        ctx.forward(alumni)
-
-    elif account_type == "contractor":
+    if account_type == "contractor":
         if user_is_partner(account):
             remove_user_from_group(account, GROUP_PARTNERS)
             remove_user_from_group(account, GROUP_STAFF)

compiler_admin/commands/user/alumni.py renamed to compiler_admin/commands/user/deactivate.py

@@ -8,6 +8,7 @@
     move_user_ou,
     user_account_name,
     user_exists,
+    user_is_deactivated,
 )
 
 
@@ -17,34 +18,40 @@
 @click.option(
     "-e",
     "--recovery-email",
+    default="",
     help="An email address to use as the new recovery email. Without a value, clears the recovery email.",
 )
 @click.option(
     "-p",
     "--recovery-phone",
+    default="",
     help="A phone number to use as the new recovery phone number. Without a value, clears the recovery phone number.",
 )
 @click.argument("username")
 @click.pass_context
-def alumni(
+def deactivate(
     ctx: click.Context, username: str, force: bool = False, recovery_email: str = "", recovery_phone: str = "", **kwargs
 ):
     """
-    Convert a user to a Compiler alumni.
+    Deactivate (but do not delete) a user.
     """
     account = user_account_name(username)
 
     if not user_exists(account):
         click.echo(f"User does not exist: {account}")
         raise SystemExit(RESULT_FAILURE)
 
+    if user_is_deactivated(account):
+        click.echo("User is already deactivated")
+        raise SystemExit(RESULT_FAILURE)
+
     if not force:
-        cont = input(f"Convert account to alumni for {account}? (Y/n): ")
+        cont = input(f"Deactivate account {account}? (Y/n): ")
         if not cont.lower().startswith("y"):
-            click.echo("Aborting conversion.")
+            click.echo("Aborting deactivation")
             raise SystemExit(RESULT_SUCCESS)
 
-    click.echo(f"User exists, converting to alumni: {account}")
+    click.echo(f"User exists, deactivating: {account}")
 
     click.echo("Removing from groups")
     CallGAMCommand(("user", account, "delete", "groups"))
@@ -71,3 +78,5 @@ def alumni(
     click.echo("Turning off 2FA")
     command = ("user", account, "turnoff2sv")
     CallGAMCommand(command)
+
+    click.echo(f"User is deactivated: {account}")

compiler_admin/commands/user/offboard.py

@@ -3,7 +3,7 @@
 import click
 
 from compiler_admin import RESULT_FAILURE, RESULT_SUCCESS
-from compiler_admin.commands.user.alumni import alumni
+from compiler_admin.commands.user.deactivate import deactivate
 from compiler_admin.commands.user.delete import delete
 from compiler_admin.services.google import (
     USER_ARCHIVE,
@@ -49,8 +49,8 @@ def offboard(ctx: click.Context, username: str, alias: str = "", force: bool = F
 
     click.echo(f"User exists, offboarding: {account}")
 
-    # call the alumni command
-    ctx.forward(alumni)
+    # call the deactivate command
+    ctx.forward(deactivate)
 
     click.echo("Backing up email")
     CallGYBCommand(("--service-account", "--email", account, "--action", "backup"))

tests/commands/user/test_convert.py

@@ -4,11 +4,6 @@
 from compiler_admin.commands.user.convert import convert, __name__ as MODULE
 
 
-@pytest.fixture
-def mock_commands_alumni(mock_commands_alumni):
-    return mock_commands_alumni(MODULE)
-
-
 @pytest.fixture
 def mock_google_user_exists(mock_google_user_exists):
     return mock_google_user_exists(MODULE)
@@ -86,15 +81,6 @@ def test_convert_user_exists_bad_account_type(cli_runner, mock_google_move_user_
     assert mock_google_move_user_ou.call_count == 0
 
 
-@pytest.mark.usefixtures("mock_google_user_exists_true")
-def test_convert_alumni(cli_runner, mock_commands_alumni, mock_google_move_user_ou):
-    result = cli_runner.invoke(convert, ["username", "alumni"])
-
-    assert result.exit_code == RESULT_SUCCESS
-    mock_commands_alumni.callback.assert_called_once()
-    mock_google_move_user_ou.assert_called_once()
-
-
 @pytest.mark.usefixtures(
     "mock_google_user_exists_true", "mock_google_user_is_partner_false", "mock_google_user_is_staff_false"
 )

tests/commands/user/test_alumni.py renamed to tests/commands/user/test_deactivate.py

@@ -1,7 +1,7 @@
 import pytest
 
 from compiler_admin import RESULT_FAILURE, RESULT_SUCCESS
-from compiler_admin.commands.user.alumni import alumni, __name__ as MODULE
+from compiler_admin.commands.user.deactivate import deactivate, __name__ as MODULE
 from compiler_admin.services.google import OU_ALUMNI
 
 
@@ -40,10 +40,10 @@ def mock_google_user_exists(mock_google_user_exists):
     return mock_google_user_exists(MODULE)
 
 
-def test_alumni_user_does_not_exists(cli_runner, mock_google_user_exists, mock_google_CallGAMCommand):
+def test_deactivate_user_does_not_exists(cli_runner, mock_google_user_exists, mock_google_CallGAMCommand):
     mock_google_user_exists.return_value = False
 
-    result = cli_runner.invoke(alumni, ["username"])
+    result = cli_runner.invoke(deactivate, ["username"])
 
     assert result.exit_code == RESULT_FAILURE
     assert result.exception
@@ -52,37 +52,37 @@ def test_alumni_user_does_not_exists(cli_runner, mock_google_user_exists, mock_g
 
 
 @pytest.mark.usefixtures("mock_input_yes")
-def test_alumni_confirm_yes(
+def test_deactivate_confirm_yes(
     cli_runner, mock_google_user_exists, mock_google_CallGAMCommand, mock_google_move_user_ou, mock_commands_reset
 ):
     mock_google_user_exists.return_value = True
 
-    result = cli_runner.invoke(alumni, ["username"])
+    result = cli_runner.invoke(deactivate, ["username"])
 
     assert result.exit_code == RESULT_SUCCESS
     mock_google_CallGAMCommand.assert_called()
     mock_google_move_user_ou.assert_called_once_with("username@compiler.la", OU_ALUMNI)
 
 
 @pytest.mark.usefixtures("mock_input_no")
-def test_alumni_confirm_no(
+def test_deactivate_confirm_no(
     cli_runner, mock_google_user_exists, mock_google_CallGAMCommand, mock_google_move_user_ou, mock_commands_reset
 ):
     mock_google_user_exists.return_value = True
 
-    result = cli_runner.invoke(alumni, ["username"])
+    result = cli_runner.invoke(deactivate, ["username"])
 
     assert result.exit_code == RESULT_SUCCESS
     mock_google_CallGAMCommand.assert_not_called()
     mock_google_move_user_ou.assert_not_called()
 
 
-def test_alumni_force(
+def test_deactivate_force(
     cli_runner, mock_google_user_exists, mock_google_CallGAMCommand, mock_google_move_user_ou, mock_commands_reset
 ):
     mock_google_user_exists.return_value = True
 
-    result = cli_runner.invoke(alumni, ["--force", "username"])
+    result = cli_runner.invoke(deactivate, ["--force", "username"])
 
     assert result.exit_code == RESULT_SUCCESS
     mock_google_CallGAMCommand.assert_called()

tests/commands/user/test_init.py

@@ -3,6 +3,6 @@
 from compiler_admin.commands.user import user
 
 
-@pytest.mark.parametrize("command", ["alumni", "convert", "create", "delete", "offboard", "reset", "restore", "signout"])
+@pytest.mark.parametrize("command", ["convert", "create", "deactivate", "delete", "offboard", "reset", "restore", "signout"])
 def test_user_commands(command):
     assert command in user.commands

tests/commands/user/test_offboard.py

@@ -20,8 +20,8 @@ def mock_NamedTemporaryFile(mock_NamedTemporaryFile_with_readlines):
 
 
 @pytest.fixture
-def mock_commands_alumni(mock_commands_alumni):
-    return mock_commands_alumni(MODULE)
+def mock_commands_deactivate(mock_commands_deactivate):
+    return mock_commands_deactivate(MODULE)
 
 
 @pytest.fixture
@@ -51,7 +51,7 @@ def test_offboard_confirm_yes(
     mock_google_CallGAMCommand,
     mock_google_CallGYBCommand,
     mock_NamedTemporaryFile,
-    mock_commands_alumni,
+    mock_commands_deactivate,
     mock_commands_delete,
 ):
     mock_google_user_exists.return_value = True
@@ -63,7 +63,7 @@ def test_offboard_confirm_yes(
     mock_google_CallGYBCommand.assert_called_once()
     mock_NamedTemporaryFile.assert_called_once()
 
-    mock_commands_alumni.callback.assert_called_once()
+    mock_commands_deactivate.callback.assert_called_once()
     mock_commands_delete.callback.assert_called_once()
 
 
@@ -73,7 +73,7 @@ def test_offboard_confirm_no(
     mock_google_user_exists,
     mock_google_CallGAMCommand,
     mock_google_CallGYBCommand,
-    mock_commands_alumni,
+    mock_commands_deactivate,
     mock_commands_delete,
 ):
     mock_google_user_exists.return_value = True
@@ -84,7 +84,7 @@ def test_offboard_confirm_no(
     mock_google_CallGAMCommand.assert_not_called()
     mock_google_CallGYBCommand.assert_not_called()
 
-    mock_commands_alumni.callback.assert_not_called()
+    mock_commands_deactivate.callback.assert_not_called()
     mock_commands_delete.callback.assert_not_called()
 
 

tests/conftest.py

@@ -77,9 +77,9 @@ def __mock_command(module):
 
 
 @pytest.fixture
-def mock_commands_alumni(mock_command):
-    """Fixture returns a function that patches the alumni function in a given module."""
-    return mock_command("alumni")
+def mock_commands_deactivate(mock_command):
+    """Fixture returns a function that patches the deactivate function in a given module."""
+    return mock_command("deactivate")
 
 
 @pytest.fixture