Skip to content

Commit 60d9340

Browse files
jfcalvofrascuchon
andauthored
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]>
1 parent e91bcb9 commit 60d9340

File tree

7 files changed

+66
-37
lines changed

7 files changed

+66
-37
lines changed

.github/workflows/argilla-server.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ jobs:
115115
if: |
116116
github.ref == 'refs/heads/main'
117117
|| github.ref == 'refs/heads/develop'
118-
|| contains(github.ref, "releases/")
118+
|| contains(github.ref, 'releases/')
119119
|| github.event_name == 'workflow_dispatch'
120120
|| github.event_name == 'pull_request'
121121
needs:

argilla-server/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@ These are the section headers that we use:
1818

1919
## [2.0.0rc1](https://github.com/argilla-io/argilla/compare/v1.29.0...v2.0.0rc1)
2020

21+
### Changed
22+
23+
- Change `responses` table to delete rows on cascade when a user is deleted. ([#5126](https://github.com/argilla-io/argilla/pull/5126))
24+
2125
### Removed
2226

2327
- Removed all API v0 endpoints. ([#4852](https://github.com/argilla-io/argilla/pull/4852))
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
# Copyright 2021-present, the Recognai S.L. team.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
"""update responses user_id foreign key
16+
17+
Revision ID: d00f819ccc67
18+
Revises: ca7293c38970
19+
Create Date: 2024-06-27 18:04:46.080762
20+
21+
"""
22+
23+
from alembic import op
24+
import sqlalchemy as sa
25+
26+
27+
# revision identifiers, used by Alembic.
28+
revision = "d00f819ccc67"
29+
down_revision = "ca7293c38970"
30+
branch_labels = None
31+
depends_on = None
32+
33+
34+
CONSTRAINT_NAME = "responses_user_id_fkey"
35+
NAMING_CONVENTION = {"fk": "%(table_name)s_%(column_0_name)s_fkey"}
36+
37+
38+
def upgrade() -> None:
39+
op.execute("DELETE FROM responses WHERE user_id IS NULL")
40+
41+
with op.batch_alter_table("responses", naming_convention=NAMING_CONVENTION) as batch_op:
42+
batch_op.alter_column("user_id", existing_type=sa.Uuid(), nullable=False)
43+
batch_op.drop_constraint(CONSTRAINT_NAME, type_="foreignkey")
44+
batch_op.create_foreign_key(CONSTRAINT_NAME, "users", ["user_id"], ["id"], ondelete="CASCADE")
45+
46+
47+
def downgrade() -> None:
48+
with op.batch_alter_table("responses", naming_convention=NAMING_CONVENTION) as batch_op:
49+
batch_op.alter_column("user_id", existing_type=sa.Uuid(), nullable=True)
50+
batch_op.drop_constraint(CONSTRAINT_NAME, type_="foreignkey")
51+
batch_op.create_foreign_key(CONSTRAINT_NAME, "users", ["user_id"], ["id"], ondelete="SET NULL")

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ class Response(BaseModel):
9090
values: Optional[ResponseValues]
9191
status: ResponseStatus
9292
record_id: UUID
93-
user_id: Optional[UUID] = None # Responses for delete users will have this field as None but still be present
93+
user_id: UUID
9494
inserted_at: datetime
9595
updated_at: datetime
9696

argilla-server/src/argilla_server/database.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@
3636
"1.13": "1e629a913727",
3737
"1.17": "84f6b9ff6076",
3838
"1.18": "bda6fe24314e",
39+
"1.28": "ca7293c38970",
40+
"2.0": "d00f819ccc67",
3941
}
4042
)
4143

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ class Response(DatabaseModel):
8787
values: Mapped[Optional[dict]] = mapped_column(MutableDict.as_mutable(JSON))
8888
status: Mapped[ResponseStatus] = mapped_column(ResponseStatusEnum, default=ResponseStatus.submitted, index=True)
8989
record_id: Mapped[UUID] = mapped_column(ForeignKey("records.id", ondelete="CASCADE"), index=True)
90-
user_id: Mapped[Optional[UUID]] = mapped_column(ForeignKey("users.id", ondelete="SET NULL"), index=True)
90+
user_id: Mapped[UUID] = mapped_column(ForeignKey("users.id", ondelete="CASCADE"), index=True)
9191

9292
record: Mapped["Record"] = relationship(back_populates="responses")
9393
user: Mapped["User"] = relationship(back_populates="responses")
@@ -438,7 +438,12 @@ class User(DatabaseModel):
438438
workspaces: Mapped[List["Workspace"]] = relationship(
439439
secondary="workspaces_users", back_populates="users", order_by=WorkspaceUser.inserted_at.asc()
440440
)
441-
responses: Mapped[List["Response"]] = relationship(back_populates="user")
441+
responses: Mapped[List["Response"]] = relationship(
442+
back_populates="user",
443+
cascade="all, delete-orphan",
444+
passive_deletes=True,
445+
order_by=Response.inserted_at.asc(),
446+
)
442447
datasets: Mapped[List["Dataset"]] = relationship(
443448
secondary="workspaces_users",
444449
primaryjoin="User.id == WorkspaceUser.user_id",

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

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -5041,36 +5041,3 @@ async def create_dataset_with_user_responses(
50415041
]
50425042

50435043
return dataset, questions, records, responses, suggestions
5044-
5045-
async def test_get_record_with_response_for_deleted_user(
5046-
self,
5047-
async_client: "AsyncClient",
5048-
db: "AsyncSession",
5049-
mock_search_engine: "SearchEngine",
5050-
owner: User,
5051-
owner_auth_header: dict,
5052-
):
5053-
record = await RecordFactory.create()
5054-
user = await OwnerFactory.create()
5055-
response = await ResponseFactory.create(record=record, user=user)
5056-
5057-
mock_search_engine.search.return_value = SearchResponses(
5058-
items=[SearchResponseItem(record_id=record.id)], total=1
5059-
)
5060-
5061-
await db.delete(user)
5062-
5063-
http_response = await async_client.get(
5064-
f"/api/v1/datasets/{record.dataset.id}/records",
5065-
params={"include": ["responses"]},
5066-
headers=owner_auth_header,
5067-
)
5068-
5069-
response_json = http_response.json()
5070-
assert http_response.status_code == 200
5071-
5072-
response_items = response_json["items"]
5073-
assert len(response_items) == 1
5074-
assert response_items[0]["id"] == str(record.id)
5075-
assert response_items[0]["responses"][0]["id"] == str(response.id)
5076-
assert response_items[0]["responses"][0]["user_id"] is None

0 commit comments

Comments
 (0)