Record user deletions in a new DB table

Remove the CLI command for deleting a user: this interacts awkwardly
with recording deletions in a table because there's no authenticated
user to record as the requester of the deletion. There could be various
solutions to this but no one uses this CLI command anyway so let's just
delete it.

Author: seanh

h/cli/commands/user.py

@@ -101,30 +101,3 @@ def password(ctx, username, authority, password):
     request.tm.commit()
 
     click.echo(f"Password changed for {username}", err=True)
-
-
-@user.command()
-@click.argument("username")
-@click.option("--authority")
-@click.pass_context
-def delete(ctx, username, authority):
-    """
-    Delete a user with all their group memberships and annotations.
-
-    You must specify the username of a user to delete.
-    """
-    request = ctx.obj["bootstrap"]()
-
-    if not authority:
-        authority = request.default_authority
-
-    user = models.User.get_by_username(request.db, username, authority)
-    if user is None:
-        raise click.ClickException(
-            f'no user with username "{username}" and authority "{authority}"'
-        )
-
-    request.find_service(name="user_delete").delete_user(user)
-    request.tm.commit()
-
-    click.echo(f"User {username} deleted.", err=True)

h/models/__init__.py

@@ -38,6 +38,7 @@
 from h.models.subscriptions import Subscriptions
 from h.models.token import Token
 from h.models.user import User
+from h.models.user_deletion import UserDeletion
 from h.models.user_identity import UserIdentity
 
 __all__ = (
@@ -64,5 +65,6 @@
     "Subscriptions",
     "Token",
     "User",
+    "UserDeletion",
     "UserIdentity",
 )

h/models/helpers.py

@@ -0,0 +1,4 @@
+def repr_(obj, attrs):
+    class_name = type(obj).__name__
+    attrs = {attrname: getattr(obj, attrname) for attrname in attrs}
+    return f"{class_name}({', '.join(f'{name}={value!r}' for name, value in attrs.items())})"

h/models/job.py

@@ -41,6 +41,7 @@
 from sqlalchemy.dialects.postgresql import JSONB
 
 from h.db import Base
+from h.models import helpers
 
 
 class Job(Base):
@@ -67,10 +68,9 @@ class Job(Base):
     )
 
     def __repr__(self):
-        class_name = type(self).__name__
-        attrs = {
-            attrname: repr(getattr(self, attrname))
-            for attrname in [
+        return helpers.repr_(
+            self,
+            [
                 "id",
                 "name",
                 "enqueued_at",
@@ -79,6 +79,5 @@ def __repr__(self):
                 "priority",
                 "tag",
                 "kwargs",
-            ]
-        }
-        return f"{class_name}({', '.join(f'{name}={value}' for name, value in attrs.items())})"
+            ],
+        )

h/models/user_deletion.py

@@ -0,0 +1,53 @@
+from datetime import datetime
+
+from sqlalchemy import func
+from sqlalchemy.orm import Mapped, mapped_column
+
+from h.db import Base
+from h.models import helpers
+
+
+class UserDeletion(Base):
+    """A record of a user account that was deleted."""
+
+    __tablename__ = "user_deletion"
+
+    id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True)
+
+    userid: Mapped[str]
+    """The userid of the user who was deleted."""
+
+    requested_at: Mapped[datetime] = mapped_column(
+        server_default=func.now(),  # pylint:disable=not-callable
+    )
+    """The time at which the user deletion was requested."""
+
+    requested_by: Mapped[str]
+    """The userid of the user who requested the deletion."""
+
+    tag: Mapped[str]
+    """Just a string 'tag' field.
+
+    For example different views that delete users might put different tag
+    values here.
+    """
+
+    registered_date: Mapped[datetime]
+    """The time when the deleted user account was first registered."""
+
+    num_annotations: Mapped[int]
+    """The number of annotations that the deleted user account had."""
+
+    def __repr__(self) -> str:
+        return helpers.repr_(
+            self,
+            [
+                "id",
+                "userid",
+                "requested_at",
+                "requested_by",
+                "tag",
+                "registered_date",
+                "num_annotations",
+            ],
+        )

h/services/user_delete.py

@@ -1,6 +1,6 @@
 import sqlalchemy as sa
 
-from h.models import Annotation, Group, Token, User
+from h.models import Annotation, Group, Token, User, UserDeletion
 from h.services.annotation_delete import AnnotationDeleteService
 
 
@@ -13,7 +13,7 @@ def __init__(
         self._db = db_session
         self._annotation_delete_service = annotation_delete_service
 
-    def delete_user(self, user: User):
+    def delete_user(self, user: User, requested_by: User, tag: str):
         """
         Delete a user with all their group memberships and annotations.
 
@@ -46,6 +46,20 @@ def delete_user(self, user: User):
             else:
                 self._db.delete(group)
 
+        self._db.add(
+            UserDeletion(
+                userid=user.userid,
+                requested_by=requested_by.userid,
+                tag=tag,
+                registered_date=user.registered_date,
+                num_annotations=self._db.scalar(
+                    sa.select(
+                        sa.func.count(Annotation.id)  # pylint:disable=not-callable
+                    ).where(Annotation.userid == user.userid)
+                ),
+            )
+        )
+
         self._db.delete(user)
 
 

h/views/admin/users.py

@@ -134,7 +134,7 @@ def users_delete(request):
     user = _form_request_user(request)
     svc = request.find_service(name="user_delete")
 
-    svc.delete_user(user)
+    svc.delete_user(user, requested_by=request.user, tag=request.matched_route.name)
     request.session.flash(
         f"Successfully deleted user {user.username} with authority {user.authority}"
         "success",

tests/common/factories/__init__.py

@@ -20,4 +20,5 @@
 from tests.common.factories.subscriptions import Subscriptions
 from tests.common.factories.token import DeveloperToken, OAuth2Token
 from tests.common.factories.user import User
+from tests.common.factories.user_deletion import UserDeletion
 from tests.common.factories.user_identity import UserIdentity

tests/common/factories/user_deletion.py

@@ -0,0 +1,19 @@
+from factory import Faker, LazyAttribute
+
+from h import models
+
+from .base import ModelFactory
+
+
+class UserDeletion(ModelFactory):
+    class Meta:
+        model = models.UserDeletion
+        exclude = ("username", "requesting_username")
+
+    username = Faker("user_name")
+    userid = LazyAttribute(lambda o: f"acct:{o.username}@example.com")
+    requesting_username = Faker("user_name")
+    requested_by = LazyAttribute(lambda o: f"acct:{o.requesting_username}@example.com")
+    tag = "factory"
+    registered_date = Faker("date_time")
+    num_annotations = Faker("random_int")

tests/unit/h/cli/commands/user_test.py

@@ -195,46 +195,6 @@ def user(self, factories):
         return factories.User()
 
 
-class TestDeleteUserCommand:
-    def test_it_deletes_user(self, invoke_cli, user, user_delete_service):
-        result = invoke_cli(user_cli.delete, [user.username])
-
-        assert not result.exit_code
-        user_delete_service.delete_user.assert_called_once_with(user)
-
-    def test_it_deletes_user_with_specific_authority(
-        self, invoke_cli, user, user_delete_service
-    ):
-        user.authority = "partner.org"
-
-        result = invoke_cli(
-            user_cli.delete, ["--authority", "partner.org", user.username]
-        )
-
-        assert not result.exit_code
-        user_delete_service.delete_user.assert_called_once_with(user)
-
-    def test_it_errors_when_user_could_not_be_found(
-        self, invoke_cli, user_delete_service
-    ):
-        result = invoke_cli(user_cli.delete, ["bogus_username"])
-
-        assert result.exit_code == 1
-        user_delete_service.delete_user.assert_not_called()
-
-    def test_it_errors_when_user_with_specific_authority_could_not_be_found(
-        self, invoke_cli, user, user_delete_service
-    ):
-        result = invoke_cli(user_cli.delete, ["--authority", "foo.com", user.username])
-
-        assert result.exit_code == 1
-        user_delete_service.delete_user.assert_not_called()
-
-    @pytest.fixture
-    def user(self, factories):
-        return factories.User()
-
-
 @pytest.fixture
 def invoke_cli(cli, pyramid_request):
     pyramid_request.tm = mock.Mock()