Support for owner, admin and annotator role system
#3104
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
frascuchon
changed the title
owner, admin and annotator role system
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## develop #3104 +/- ##
===========================================
- Coverage 90.94% 90.93% -0.01%
===========================================
Files 215 216 +1
Lines 11347 11354 +7
===========================================
+ Hits 10319 10325 +6
- Misses 1028 1029 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
# Description This PR brings changed to support 3 kinds of roles: - `owner`: full access (old `admin` role) - `admin`: workspace-level operations (create/delete datasets, add records,...) - `annotator`: same as before. Just for annotation purposes. Most of the tests have been revisited regarding the `mocked_client` fixture. I've also added variations o tests to support most roles as possible for the tested functionality. Since a huge number of changes have been included here, I will add more details in future PRs. Refs #3094 **Type of change** (Please delete options that are not relevant. Remember to title the PR according to the type of change) - [x] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [x] Refactor (change restructuring the codebase without changing functionality) - [x] Improvement (change adding some improvement to an existing functionality) **How Has This Been Tested** (Please describe the tests that you ran to verify your changes. And ideally, reference `tests`) Tests have been updated **Checklist** - [x] I have merged the original branch into my forked branch - [x] I added relevant documentation - [ ] 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 - [x] I have added relevant notes to the CHANGELOG.md file (See https://keepachangelog.com/) --------- Co-authored-by: Gabriel Martín Blázquez <gmartinbdev@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
# Description This PRs mainly adds a new users task command to update an existing user (for now, only the user role can be used, but other fields like last name or even the password can be updated through this command) Also, some refactors have been applied: - Remove the role_callback function, since `typer` tries to cast the role into a valid `UserRole` value. - The default user is defined with the `admin` role, instead of `owner`. This still works for environments with one single user and workspace. **Type of change** (Please delete options that are not relevant. Remember to title the PR according to the type of change) - [x] New feature (non-breaking change which adds functionality) - [x] Refactor (change restructuring the codebase without changing functionality) - [x] Improvement (change adding some improvement to an existing functionality) **How Has This Been Tested** (Please describe the tests that you ran to verify your changes. And ideally, reference `tests`) New tests have been added **Checklist** - [x] I have merged the original branch into my forked branch - [ ] 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 - [x] I have added relevant notes to the CHANGELOG.md file (See https://keepachangelog.com/)
9 tasks
gabrielmbmb
added a commit
that referenced
this pull request
Jun 19, 2023
# Description As of the recent addition of the `owner` user at #3104, once we merged #3180, the unit tests were not passing in `develop` due to the addition of the new role. So on, we've refactored the `Workspace` unit tests for both the Python client and the SDK to use the `owner` user defined in `conftest.py` as the user to use to authenticate against the Argilla API and the one to add/delete from the workspace/s if applicable. Kudos to @gabrielmbmb for initially proposing this approach! **Type of change** - [X] Bug fix (non-breaking change which fixes an issue) - [X] Refactor (change restructuring the codebase without changing functionality) **How Has This Been Tested** - [X] Refactored unit tests to use `owner` - [X] Remove the API calls with the factories: `WorkspaceFactory` and `WorkspaceUserFactory` **Checklist** - [X] I have merged the original branch into my forked branch - [X] follows the style guidelines of this project - [X] I did a self-review of my code - [X] My changes generate no new warnings - [X] I have added tests that prove my fix is effective or that my feature works --------- Co-authored-by: gabrielmbmb <gmartinbdev@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR will be the main feature branch for a huge refactor that includes:
adminrole toownerkeeping all the current functionality but to this new role nameadminrole, with operation restrictions. Users with this role can work on scoped workspaces (must be specifically linked to those workspaces), and the supported operations will be:The rest of the operations will be denied for this role (users and workspace management,...)
Closes #3094
TODO
(The rest of the tasks will be tackled as separate PR over this branch)
admintoowner(current default credentials for quickstart images remain the same)adminrole (review and adapt policies)Update all doc sections with this new flowTackled in Update User management documentation #3212HOWTO guide to migrate the currentTackled in Update User management documentation #3212adminrole (the simplest way should be to provide a way to update a user role through a CLI task)Closes #3094
Type of change
(Please delete options that are not relevant. Remember to title the PR according to the type of change)
How Has This Been Tested
(Please describe the tests that you ran to verify your changes. And ideally, reference
tests)The tests have been updated
Checklist