Record user deletions in a new DB table

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()

tests/unit/h/models/helpers_test.py

@@ -0,0 +1,9 @@
+from unittest.mock import Mock
+
+from h.models import helpers
+
+
+def test_repr_():
+    obj = Mock(foo="FOO", bar="BAR")
+
+    assert helpers.repr_(obj, ["foo", "bar"]) == "Mock(foo='FOO', bar='BAR')"

tests/unit/h/models/job_test.py

@@ -1,43 +1,29 @@
-from datetime import datetime
+import pytest
 
-from h.models.job import Job
 
+def test___repr__(factories, helpers):
+    job = factories.Job()
+    repr_ = repr(job)
 
-class TestJob:
-    def test___repr__(self):
-        job = Job(
-            id=42,
-            name="job_name",
-            enqueued_at=datetime(
-                year=2024,
-                month=5,
-                day=8,
-                hour=11,
-                minute=51,
-                second=23,
-            ),
-            scheduled_at=datetime(
-                year=2024,
-                month=6,
-                day=1,
-                hour=0,
-                minute=0,
-                second=0,
-            ),
-            expires_at=datetime(
-                year=2025,
-                month=1,
-                day=1,
-                hour=0,
-                minute=0,
-                second=0,
-            ),
-            priority=3,
-            tag="job_tag",
-            kwargs={"foo": "FOO", "bar": "BAR"},
-        )
+    helpers.repr_.assert_called_once_with(
+        job,
+        [
+            "id",
+            "name",
+            "enqueued_at",
+            "scheduled_at",
+            "expires_at",
+            "priority",
+            "tag",
+            "kwargs",
+        ],
+    )
+    assert repr_ == helpers.repr_.return_value
 
-        assert (
-            repr(job)
-            == "Job(id=42, name='job_name', enqueued_at=datetime.datetime(2024, 5, 8, 11, 51, 23), scheduled_at=datetime.datetime(2024, 6, 1, 0, 0), expires_at=datetime.datetime(2025, 1, 1, 0, 0), priority=3, tag='job_tag', kwargs={'foo': 'FOO', 'bar': 'BAR'})"
-        )
+
+@pytest.fixture(autouse=True)
+def helpers(mocker):
+    helpers = mocker.patch("h.models.job.helpers")
+    # __repr__() needs to return a string or repr() raises.
+    helpers.repr_.return_value = "test_string_representation"
+    return helpers

tests/unit/h/models/user_deletion_test.py

@@ -0,0 +1,28 @@
+import pytest
+
+
+def test___repr__(factories, helpers):
+    user_deletion = factories.UserDeletion()
+    repr_ = repr(user_deletion)
+
+    helpers.repr_.assert_called_once_with(
+        user_deletion,
+        [
+            "id",
+            "userid",
+            "requested_at",
+            "requested_by",
+            "tag",
+            "registered_date",
+            "num_annotations",
+        ],
+    )
+    assert repr_ == helpers.repr_.return_value
+
+
+@pytest.fixture(autouse=True)
+def helpers(mocker):
+    helpers = mocker.patch("h.models.user_deletion.helpers")
+    # __repr__() needs to return a string or repr() raises.
+    helpers.repr_.return_value = "test_string_representation"
+    return helpers