Skip to content

Commit

Permalink
fix: return all workspaces in system for owner users (#3343)
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 changes the `GET /api/v1/users/:user_id/workspaces` endpoint
introduced in #3308 to return all workspaces for owner users

Also, returns 404 if provided user id does not exist


**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)

**How Has This Been Tested**

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

- [ ] Test A
- [ ] Test B

**Checklist**

- [ ] I added relevant documentation
- [x] follows the style guidelines of this project
- [x] I did a self-review of my code
- [ ] I made corresponding changes to the documentation
- [x] 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/)
  • Loading branch information
frascuchon committed Jul 5, 2023
1 parent 3589703 commit 6a6996b
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 7 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ These are the section headers that we use:

### Added

- Added `GET /api/v1/users/{user_id}/workspaces` endpoint to list the workspaces to which a user belongs ([#3308](https://github.com/argilla-io/argilla/pull/3308)).
- Added `GET /api/v1/users/{user_id}/workspaces` endpoint to list the workspaces to which a user belongs ([#3308](https://github.com/argilla-io/argilla/pull/3308) and [#3343](https://github.com/argilla-io/argilla/pull/3343)).
- Added `HuggingFaceDatasetMixIn` for internal usage, to detach the `FeedbackDataset` integrations from the class itself, and use MixIns instead ([#3326](https://github.com/argilla-io/argilla/pull/3326)).
- Added `GET /api/v1/records/{record_id}/suggestions` API endpoint to get the list of suggestions for the responses associated to a record ([#3304](https://github.com/argilla-io/argilla/pull/3304)).
- Added `POST /api/v1/records/{record_id}/suggestions` API endpoint to create a suggestion for a response associated to a record ([#3304](https://github.com/argilla-io/argilla/pull/3304)).
Expand Down
18 changes: 15 additions & 3 deletions src/argilla/server/apis/v1/handlers/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@

from uuid import UUID

from fastapi import APIRouter, Depends, Security
from fastapi import APIRouter, Depends, HTTPException, Security, status
from sqlalchemy.ext.asyncio import AsyncSession

from argilla.server.contexts import accounts
from argilla.server.database import get_async_db
from argilla.server.models import User
from argilla.server.models import User, UserRole
from argilla.server.policies import UserPolicyV1, authorize
from argilla.server.schemas.v1.workspaces import Workspaces
from argilla.server.security import auth
Expand All @@ -35,5 +35,17 @@ async def list_user_workspaces(
current_user: User = Security(auth.get_current_user),
):
await authorize(current_user, UserPolicyV1.list_workspaces)
workspaces = await accounts.list_workspaces_by_user_id(db, user_id)

user = await accounts.get_user_by_id(db, user_id)
if not user:
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail=f"User with id `{user_id}` not found",
)

if user.is_owner:
workspaces = await accounts.list_workspaces(db)
else:
workspaces = await accounts.list_workspaces_by_user_id(db, user_id)

return Workspaces(items=workspaces)
2 changes: 1 addition & 1 deletion src/argilla/server/contexts/accounts.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ async def delete_workspace(db: "AsyncSession", workspace: Workspace):
return workspace


async def get_user_by_id(db: Session, user_id: UUID) -> Union[User, None]:
async def get_user_by_id(db: "AsyncSession", user_id: UUID) -> Union[User, None]:
return await db.get(User, user_id)


Expand Down
26 changes: 24 additions & 2 deletions tests/server/api/v1/test_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@
# 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.

import uuid
from typing import TYPE_CHECKING

import pytest
from argilla._constants import API_KEY_HEADER_NAME
from argilla.server.models import UserRole
from argilla.server.models import User, UserRole

from tests.factories import UserFactory, WorkspaceFactory

Expand Down Expand Up @@ -45,6 +45,24 @@ async def test_list_user_workspaces(self, client: "TestClient", owner_auth_heade
]
}

async def test_list_user_workspaces_for_owner(self, client: "TestClient"):
workspaces = await WorkspaceFactory.create_batch(5)
owner = await UserFactory.create(role=UserRole.owner)

response = client.get(f"/api/v1/users/{owner.id}/workspaces", headers={API_KEY_HEADER_NAME: owner.api_key})
assert response.status_code == 200
assert response.json() == {
"items": [
{
"id": str(workspace.id),
"name": workspace.name,
"inserted_at": workspace.inserted_at.isoformat(),
"updated_at": workspace.updated_at.isoformat(),
}
for workspace in workspaces
]
}

@pytest.mark.parametrize("role", [UserRole.annotator, UserRole.admin])
async def test_list_user_workspaces_as_restricted_user(self, client: "TestClient", role: UserRole):
workspaces = await WorkspaceFactory.create_batch(3)
Expand All @@ -56,3 +74,7 @@ async def test_list_user_workspaces_as_restricted_user(self, client: "TestClient
)

assert response.status_code == 403

async def test_list_user_for_non_existing_user(self, client: "TestClient", owner_auth_header: dict):
response = client.get(f"/api/v1/users/{uuid.uuid4()}/workspaces", headers=owner_auth_header)
assert response.status_code == 404

0 comments on commit 6a6996b

Please sign in to comment.