change: delete on cascade responses when associated user is deleted (#…

…5126)

# Description

This PR adds the following changes:
* Change `Responses` SqlAlchemy ORM model to delete responses on cascade
when a user is deleted.
* Add a migration for `responses` doing the following:
* Removing all the responses rows with `user_id` equal to `NULL` so the
foreign key constraint can be modified.
* Change `user_id` constraint to delete on cascade responses when a user
is deleted.
  * Change `user_id` to be non nullable.


Closes #5109 

**Type of change**

- Improvement (change adding some improvement to an existing
functionality)

**How Has This Been Tested**

- [x] Manually tested on PostgreSQL and SQLite.
- [x] It should be tested on an environment with responses with
`user_id` equal to `NULL`.

**Checklist**

- [ ] I added relevant documentation
- [ ] follows the style guidelines of this project
- [ ] I did a self-review of my code
- [ ] I made corresponding changes to the documentation
- [ ] I confirm My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] I have added relevant notes to the CHANGELOG.md file (See
https://keepachangelog.com/)

---------

Co-authored-by: Paco Aranda <[email protected]>

.github/workflows/argilla-server.yml

@@ -115,7 +115,7 @@ jobs:
     if: |
       github.ref == 'refs/heads/main'
       || github.ref == 'refs/heads/develop'
-      || contains(github.ref, "releases/")
+      || contains(github.ref, 'releases/')
       || github.event_name == 'workflow_dispatch'
       || github.event_name == 'pull_request'
     needs:

argilla-server/CHANGELOG.md

@@ -18,6 +18,10 @@ These are the section headers that we use:
 
 ## [2.0.0rc1](https://github.com/argilla-io/argilla/compare/v1.29.0...v2.0.0rc1)
 
+### Changed
+
+- Change `responses` table to delete rows on cascade when a user is deleted. ([#5126](https://github.com/argilla-io/argilla/pull/5126))
+
 ### Removed
 
 - Removed all API v0 endpoints. ([#4852](https://github.com/argilla-io/argilla/pull/4852))

argilla-server/src/argilla_server/alembic/versions/d00f819ccc67_update_responses_user_id_foreign_key.py

@@ -0,0 +1,51 @@
+#  Copyright 2021-present, the Recognai S.L. team.
+#
+#  Licensed under the Apache License, Version 2.0 (the "License");
+#  you may not use this file except in compliance with the License.
+#  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+
+"""update responses user_id foreign key
+
+Revision ID: d00f819ccc67
+Revises: ca7293c38970
+Create Date: 2024-06-27 18:04:46.080762
+
+"""
+
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = "d00f819ccc67"
+down_revision = "ca7293c38970"
+branch_labels = None
+depends_on = None
+
+
+CONSTRAINT_NAME = "responses_user_id_fkey"
+NAMING_CONVENTION = {"fk": "%(table_name)s_%(column_0_name)s_fkey"}
+
+
+def upgrade() -> None:
+    op.execute("DELETE FROM responses WHERE user_id IS NULL")
+
+    with op.batch_alter_table("responses", naming_convention=NAMING_CONVENTION) as batch_op:
+        batch_op.alter_column("user_id", existing_type=sa.Uuid(), nullable=False)
+        batch_op.drop_constraint(CONSTRAINT_NAME, type_="foreignkey")
+        batch_op.create_foreign_key(CONSTRAINT_NAME, "users", ["user_id"], ["id"], ondelete="CASCADE")
+
+
+def downgrade() -> None:
+    with op.batch_alter_table("responses", naming_convention=NAMING_CONVENTION) as batch_op:
+        batch_op.alter_column("user_id", existing_type=sa.Uuid(), nullable=True)
+        batch_op.drop_constraint(CONSTRAINT_NAME, type_="foreignkey")
+        batch_op.create_foreign_key(CONSTRAINT_NAME, "users", ["user_id"], ["id"], ondelete="SET NULL")

argilla-server/src/argilla_server/api/schemas/v1/responses.py

@@ -90,7 +90,7 @@ class Response(BaseModel):
     values: Optional[ResponseValues]
     status: ResponseStatus
     record_id: UUID
-    user_id: Optional[UUID] = None  # Responses for delete users will have this field as None but still be present
+    user_id: UUID
     inserted_at: datetime
     updated_at: datetime
 

argilla-server/src/argilla_server/database.py

@@ -36,6 +36,8 @@
         "1.13": "1e629a913727",
         "1.17": "84f6b9ff6076",
         "1.18": "bda6fe24314e",
+        "1.28": "ca7293c38970",
+        "2.0": "d00f819ccc67",
     }
 )
 

argilla-server/src/argilla_server/models/database.py

@@ -87,7 +87,7 @@ class Response(DatabaseModel):
     values: Mapped[Optional[dict]] = mapped_column(MutableDict.as_mutable(JSON))
     status: Mapped[ResponseStatus] = mapped_column(ResponseStatusEnum, default=ResponseStatus.submitted, index=True)
     record_id: Mapped[UUID] = mapped_column(ForeignKey("records.id", ondelete="CASCADE"), index=True)
-    user_id: Mapped[Optional[UUID]] = mapped_column(ForeignKey("users.id", ondelete="SET NULL"), index=True)
+    user_id: Mapped[UUID] = mapped_column(ForeignKey("users.id", ondelete="CASCADE"), index=True)
 
     record: Mapped["Record"] = relationship(back_populates="responses")
     user: Mapped["User"] = relationship(back_populates="responses")
@@ -438,7 +438,12 @@ class User(DatabaseModel):
     workspaces: Mapped[List["Workspace"]] = relationship(
         secondary="workspaces_users", back_populates="users", order_by=WorkspaceUser.inserted_at.asc()
     )
-    responses: Mapped[List["Response"]] = relationship(back_populates="user")
+    responses: Mapped[List["Response"]] = relationship(
+        back_populates="user",
+        cascade="all, delete-orphan",
+        passive_deletes=True,
+        order_by=Response.inserted_at.asc(),
+    )
     datasets: Mapped[List["Dataset"]] = relationship(
         secondary="workspaces_users",
         primaryjoin="User.id == WorkspaceUser.user_id",

argilla-server/tests/unit/api/handlers/v1/test_datasets.py

@@ -5041,36 +5041,3 @@ async def create_dataset_with_user_responses(
         ]
 
         return dataset, questions, records, responses, suggestions
-
-    async def test_get_record_with_response_for_deleted_user(
-        self,
-        async_client: "AsyncClient",
-        db: "AsyncSession",
-        mock_search_engine: "SearchEngine",
-        owner: User,
-        owner_auth_header: dict,
-    ):
-        record = await RecordFactory.create()
-        user = await OwnerFactory.create()
-        response = await ResponseFactory.create(record=record, user=user)
-
-        mock_search_engine.search.return_value = SearchResponses(
-            items=[SearchResponseItem(record_id=record.id)], total=1
-        )
-
-        await db.delete(user)
-
-        http_response = await async_client.get(
-            f"/api/v1/datasets/{record.dataset.id}/records",
-            params={"include": ["responses"]},
-            headers=owner_auth_header,
-        )
-
-        response_json = http_response.json()
-        assert http_response.status_code == 200
-
-        response_items = response_json["items"]
-        assert len(response_items) == 1
-        assert response_items[0]["id"] == str(record.id)
-        assert response_items[0]["responses"][0]["id"] == str(response.id)
-        assert response_items[0]["responses"][0]["user_id"] is None