Skip to content
This repository has been archived by the owner on Jun 14, 2024. It is now read-only.

Commit

Permalink
fix: allow fetch responses from deleted users (#57)
Browse files Browse the repository at this point in the history
<!-- Thanks for your contribution! As part of our Community Growers
initiative 🌱, we're donating Justdiggit bunds in your name to reforest
sub-Saharan Africa. To claim your Community Growers certificate, please
contact David Berenstein in our Slack community or fill in this form
https://tally.so/r/n9XrxK once your PR has been merged. -->

# Description

This PR allows returning records with deleted users' responses
(user_id=None). This change will allow SDK admins to work with those
responses.


**Type of change**

(Please delete options that are not relevant. Remember to title the PR
according to the type of change)

- [X] Bug fix (non-breaking change which fixes an issue)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)

**How Has This Been Tested**

(Please describe the tests that you ran to verify your changes. And
ideally, reference `tests`)

Tested locally

- [ ] Test A
- [ ] Test B

**Checklist**

- [X] I followed the style guidelines of this project
- [ ] I did a self-review of my code
- [ ] My changes generate no new warnings
- [X] I have added tests that prove my fix is effective or that my
feature works
- [ ] I filled out [the contributor form](https://tally.so/r/n9XrxK)
(see text above)
- [X] I have added relevant notes to the `CHANGELOG.md` file (See
https://keepachangelog.com/)

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
frascuchon and pre-commit-ci[bot] authored Mar 8, 2024
1 parent efe3bce commit 0b7a132
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 4 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ These are the section headers that we use:

## [Unreleased]()

### Fixed

- Fixed error when returning responses from deleted users (which contains user_id=None). ([#57](https://github.com/argilla-io/argilla-server/pull/57))

## [1.25.0](https://github.com/argilla-io/argilla-server/compare/v1.24.0...v1.25.0)

>[!IMPORTANT]
Expand Down
2 changes: 1 addition & 1 deletion src/argilla_server/schemas/v1/responses.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class Response(BaseModel):
values: Optional[Dict[str, ResponseValue]]
status: ResponseStatus
record_id: UUID
user_id: UUID
user_id: Optional[UUID] = None # Responses for delete users will have this field as None but still be present
inserted_at: datetime
updated_at: datetime

Expand Down
34 changes: 34 additions & 0 deletions tests/unit/api/v1/test_datasets.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
LabelSelectionQuestionFactory,
MetadataPropertyFactory,
MultiLabelSelectionQuestionFactory,
OwnerFactory,
QuestionFactory,
RatingQuestionFactory,
RecordFactory,
Expand Down Expand Up @@ -5459,3 +5460,36 @@ 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
6 changes: 3 additions & 3 deletions tests/unit/api/v1/test_records.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@
# limitations under the License.

from datetime import datetime
from typing import TYPE_CHECKING, Any, Callable, Type
from typing import TYPE_CHECKING, Any, Awaitable, Callable, Type
from unittest.mock import call
from uuid import UUID, uuid4

import pytest
from argilla_server.constants import API_KEY_HEADER_NAME
from argilla_server.enums import ResponseStatus
from argilla_server.models import Dataset, Record, Response, Suggestion, User, UserRole
from argilla_server.search_engine import SearchEngine
from argilla_server.search_engine import SearchEngine, SearchResponseItem, SearchResponses
from sqlalchemy import func, select
from sqlalchemy.orm import Session

Expand Down Expand Up @@ -922,7 +922,7 @@ async def test_create_record_response_with_wrong_response_value(
self,
async_client: "AsyncClient",
owner_auth_header: dict,
create_questions_func: Callable[["Dataset"], None],
create_questions_func: Callable[["Dataset"], Awaitable[None]],
responses: dict,
expected_error_msg: str,
):
Expand Down

0 comments on commit 0b7a132

Please sign in to comment.