Use ORM for user delete

Author: azvoleff

gefapi/routes/api/v1/users.py

@@ -639,14 +639,14 @@ def delete_profile():
     logger.info("[ROUTER]: Delete me")
     identity = current_user
     try:
-        user = UserService.delete_user(str(identity.id))
+        user_data = UserService.delete_user(str(identity.id))
     except UserNotFound as e:
         logger.error("[ROUTER]: " + e.message)
         return error(status=404, detail=e.message)
     except Exception as e:
         logger.error("[ROUTER]: " + str(e))
         return error(status=500, detail="Generic Error")
-    return jsonify(data=user.serialize()), 200
+    return jsonify(data=user_data), 200
 
 
 @endpoints.route(
@@ -979,14 +979,14 @@ def delete_user(user):
     if not can_delete_user(identity):
         return error(status=403, detail="Forbidden")
     try:
-        user = UserService.delete_user(user)
+        user_data = UserService.delete_user(user)
     except UserNotFound as e:
         logger.error("[ROUTER]: " + e.message)
         return error(status=404, detail=e.message)
     except Exception as e:
         logger.error("[ROUTER]: " + str(e))
         return error(status=500, detail="Generic Error")
-    return jsonify(data=user.serialize()), 200
+    return jsonify(data=user_data), 200
 
 
 @endpoints.route(

gefapi/services/user_service.py

@@ -760,90 +760,86 @@ def delete_user(user_id):
             raise UserNotFound(
                 message="User with ID " + str(user_id) + " does not exist"
             )
+
+        # Serialize user data before deletion
+        user_data = user.serialize()
+
         try:
-            from sqlalchemy import text
+            from sqlalchemy import String, cast
 
-            user_uuid = str(user.id)
+            from gefapi.models import Execution, ExecutionLog, Script, StatusLog
+            from gefapi.models.password_reset_token import PasswordResetToken
+            from gefapi.models.refresh_token import RefreshToken
+            from gefapi.models.script_log import ScriptLog
 
-            # Use PostgreSQL DELETE...USING for efficient JOIN-based deletes
-            # This is faster than subqueries as PostgreSQL can use indexes
+            user_uuid = user.id
 
-            # Delete status logs via JOIN (StatusLog uses String FK)
-            logger.info("[DB]: Deleting status logs for user's executions")
-            db.session.execute(
-                text("""
-                    DELETE FROM status_log s
-                    USING execution e
-                    WHERE s.execution_id = CAST(e.id AS VARCHAR)
-                    AND e.user_id = :user_id
-                """),
-                {"user_id": user_uuid},
+            # Get execution IDs for this user (needed for related table deletions)
+            execution_ids_uuid = db.session.query(Execution.id).filter(
+                Execution.user_id == user_uuid
+            )
+            # Cast to string for StatusLog which uses String(36) for execution_id
+            execution_ids_str = db.session.query(cast(Execution.id, String)).filter(
+                Execution.user_id == user_uuid
             )
 
-            # Delete execution logs via JOIN (ExecutionLog uses UUID FK)
-            logger.info("[DB]: Deleting execution logs for user's executions")
-            db.session.execute(
-                text("""
-                    DELETE FROM execution_log el
-                    USING execution e
-                    WHERE el.execution_id = e.id
-                    AND e.user_id = :user_id
-                """),
-                {"user_id": user_uuid},
+            # Get script IDs for this user (needed for script log deletions)
+            script_ids_uuid = db.session.query(Script.id).filter(
+                Script.user_id == user_uuid
             )
 
+            # Delete status logs for user's executions
+            # StatusLog.execution_id is String(36), so use string-cast query
+            logger.info("[DB]: Deleting status logs for user's executions")
+            StatusLog.query.filter(
+                StatusLog.execution_id.in_(execution_ids_str)
+            ).delete(synchronize_session=False)
+
+            # Delete execution logs (uses UUID foreign key)
+            logger.info("[DB]: Deleting execution logs for user's executions")
+            ExecutionLog.query.filter(
+                ExecutionLog.execution_id.in_(execution_ids_uuid)
+            ).delete(synchronize_session=False)
+
             # Delete executions
             logger.info("[DB]: Deleting executions")
-            db.session.execute(
-                text("DELETE FROM execution WHERE user_id = :user_id"),
-                {"user_id": user_uuid},
+            Execution.query.filter(Execution.user_id == user_uuid).delete(
+                synchronize_session=False
             )
 
-            # Delete script logs via JOIN
+            # Delete script logs for user's scripts
             logger.info("[DB]: Deleting script logs for user's scripts")
-            db.session.execute(
-                text("""
-                    DELETE FROM script_log sl
-                    USING script s
-                    WHERE sl.script_id = s.id
-                    AND s.user_id = :user_id
-                """),
-                {"user_id": user_uuid},
+            ScriptLog.query.filter(ScriptLog.script_id.in_(script_ids_uuid)).delete(
+                synchronize_session=False
             )
 
             # Delete scripts
             logger.info("[DB]: Deleting scripts")
-            db.session.execute(
-                text("DELETE FROM script WHERE user_id = :user_id"),
-                {"user_id": user_uuid},
+            Script.query.filter(Script.user_id == user_uuid).delete(
+                synchronize_session=False
             )
 
             # Delete password reset tokens
             logger.info("[DB]: Deleting password reset tokens")
-            db.session.execute(
-                text("DELETE FROM password_reset_token WHERE user_id = :user_id"),
-                {"user_id": user_uuid},
-            )
+            PasswordResetToken.query.filter(
+                PasswordResetToken.user_id == user_uuid
+            ).delete(synchronize_session=False)
 
             # Delete refresh tokens
             logger.info("[DB]: Deleting refresh tokens")
-            db.session.execute(
-                text("DELETE FROM refresh_tokens WHERE user_id = :user_id"),
-                {"user_id": user_uuid},
+            RefreshToken.query.filter(RefreshToken.user_id == user_uuid).delete(
+                synchronize_session=False
             )
 
             # Now delete the user
             logger.info("[DB]: DELETE user")
-            db.session.execute(
-                text("DELETE FROM public.user WHERE id = :user_id"),
-                {"user_id": user_uuid},
-            )
+            db.session.delete(user)
             db.session.commit()
         except Exception as error:
             db.session.rollback()
             rollbar.report_exc_info()
             raise error
-        return user
+        return user_data
 
     @staticmethod
     def authenticate_user(email, password):

pytest.ini

@@ -16,6 +16,7 @@ markers =
     auth: marks tests related to authentication
     admin: marks tests requiring admin privileges
     celery: marks tests related to Celery tasks
+    standalone: marks tests as standalone (no database or Flask app needed)
 filterwarnings =
     ignore::DeprecationWarning
     ignore::PendingDeprecationWarning