fix(review): address PR feedback on account deletion

- database/users: add study, task_study, run_study, dataset_tag checks

- database/users: wrap user & group deletion in an explicit transaction

- routers/users: reuse direct query existence check without unused imports

- tests/users_test: add coverage for 409 conflict when resources exist

- tests/users_test: add db-level side-effect assertions for successful deletion

Author: Jayant-kernel

src/database/users.py

@@ -88,16 +88,47 @@ def get_user_resource_count(*, user_id: int, expdb: Connection) -> int:
         text("SELECT COUNT(*) FROM run WHERE uploader = :user_id"),
         parameters={"user_id": user_id},
     ).scalar() or 0
-    return int(dataset_count) + int(flow_count) + int(run_count)
 
-
-def delete_user(*, user_id: int, connection: Connection) -> None:
-    """Remove the user and their group memberships from the user database."""
-    connection.execute(
-        text("DELETE FROM users_groups WHERE user_id = :user_id"),
+    study_count = expdb.execute(
+        text("SELECT COUNT(*) FROM study WHERE creator = :user_id"),
         parameters={"user_id": user_id},
-    )
-    connection.execute(
-        text("DELETE FROM users WHERE id = :user_id"),
+    ).scalar() or 0
+    task_study_count = expdb.execute(
+        text("SELECT COUNT(*) FROM task_study WHERE uploader = :user_id"),
+        parameters={"user_id": user_id},
+    ).scalar() or 0
+    run_study_count = expdb.execute(
+        text("SELECT COUNT(*) FROM run_study WHERE uploader = :user_id"),
         parameters={"user_id": user_id},
+    ).scalar() or 0
+    dataset_tag_count = expdb.execute(
+        text("SELECT COUNT(*) FROM dataset_tag WHERE uploader = :user_id"),
+        parameters={"user_id": user_id},
+    ).scalar() or 0
+
+    return int(
+        dataset_count
+        + flow_count
+        + run_count
+        + study_count
+        + task_study_count
+        + run_study_count
+        + dataset_tag_count,
     )
+
+
+def delete_user(*, user_id: int, connection: Connection) -> None:
+    """Remove the user and their group memberships from the user database."""
+    with connection.begin_nested() as transaction:
+        try:
+            connection.execute(
+                text("DELETE FROM users_groups WHERE user_id = :user_id"),
+                parameters={"user_id": user_id},
+            )
+            connection.execute(
+                text("DELETE FROM users WHERE id = :user_id"),
+                parameters={"user_id": user_id},
+            )
+        except Exception:
+            transaction.rollback()
+            raise

src/routers/openml/users.py

@@ -42,16 +42,12 @@ def delete_account(
             detail={"code": str(int(UserError.NO_ACCESS)), "message": "No access granted"},
         )
 
-    # Verify the target user exists
-    from database.users import get_user_id_for  # noqa: PLC0415
-
-    # Check user exists by querying for them directly
-    from sqlalchemy import text
+    from sqlalchemy import text  # noqa: PLC0415
 
     existing = user_db.execute(
-        text("SELECT id FROM users WHERE id = :user_id"),
-        parameters={"user_id": user_id},
-    ).one_or_none()
+        text("SELECT 1 FROM users WHERE id = :id LIMIT 1"),
+        parameters={"id": user_id},
+    ).scalar()
 
     if existing is None:
         raise HTTPException(

tests/routers/openml/users_test.py

@@ -19,10 +19,28 @@ def test_delete_user_self(py_api: TestClient, user_test: Connection) -> None:
     )
     (new_id,) = user_test.execute(text("SELECT LAST_INSERT_ID()")).one()
 
+    # Add a users_groups entry to verify it gets deleted
+    user_test.execute(
+        text("INSERT INTO users_groups (user_id, group_id) VALUES (:id, 2)"),
+        parameters={"id": new_id},
+    )
+
     response = py_api.delete(f"/users/{new_id}?api_key=aaaabbbbccccddddaaaabbbbccccdddd")
     assert response.status_code == HTTPStatus.OK
     assert response.json() == {"user_id": new_id, "deleted": True}
 
+    # Verify DB side-effects: user and groups should be gone
+    user_count = user_test.execute(
+        text("SELECT COUNT(*) FROM users WHERE id = :id"),
+        parameters={"id": new_id},
+    ).scalar()
+    group_count = user_test.execute(
+        text("SELECT COUNT(*) FROM users_groups WHERE user_id = :id"),
+        parameters={"id": new_id},
+    ).scalar()
+    assert user_count == 0
+    assert group_count == 0
+
 
 @pytest.mark.mut
 def test_delete_user_as_admin(py_api: TestClient, user_test: Connection) -> None:
@@ -35,10 +53,23 @@ def test_delete_user_as_admin(py_api: TestClient, user_test: Connection) -> None
     )
     (new_id,) = user_test.execute(text("SELECT LAST_INSERT_ID()")).one()
 
+    # Add a users_groups entry
+    user_test.execute(
+        text("INSERT INTO users_groups (user_id, group_id) VALUES (:id, 2)"),
+        parameters={"id": new_id},
+    )
+
     response = py_api.delete(f"/users/{new_id}?api_key={ApiKey.ADMIN}")
     assert response.status_code == HTTPStatus.OK
     assert response.json() == {"user_id": new_id, "deleted": True}
 
+    # Verify DB side-effects
+    user_count = user_test.execute(
+        text("SELECT COUNT(*) FROM users WHERE id = :id"),
+        parameters={"id": new_id},
+    ).scalar()
+    assert user_count == 0
+
 
 def test_delete_user_no_auth(py_api: TestClient) -> None:
     """No API key → 401."""
@@ -58,3 +89,22 @@ def test_delete_user_not_found(py_api: TestClient) -> None:
     response = py_api.delete(f"/users/99999999?api_key={ApiKey.ADMIN}")
     assert response.status_code == HTTPStatus.NOT_FOUND
     assert response.json()["detail"]["code"] == "120"
+
+
+def test_delete_user_has_resources(py_api: TestClient, user_test: Connection) -> None:
+    """A user with resources (datasets, flows, runs) gets a 409 Conflict."""
+    # User 16 owns dataset 130 per tests/users.py definition
+    target_id = 16
+    response = py_api.delete(f"/users/{target_id}?api_key={ApiKey.DATASET_130_OWNER}")
+    
+    assert response.status_code == HTTPStatus.CONFLICT
+    assert response.json()["detail"]["code"] == "122"
+    assert "resource(s)" in response.json()["detail"]["message"]
+
+    # Verify user record was NOT deleted
+    user_count = user_test.execute(
+        text("SELECT COUNT(*) FROM users WHERE id = :id"),
+        parameters={"id": target_id},
+    ).scalar()
+    assert user_count == 1
+