Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: reviewing user tasks cli #3188

Conversation

frascuchon
Copy link
Member

@frascuchon frascuchon commented Jun 14, 2023

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)

  • New feature (non-breaking change which adds functionality)
  • Refactor (change restructuring the codebase without changing functionality)
  • 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

  • I have merged the original branch into my forked branch
  • 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
  • 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/)

@codecov
Copy link

codecov bot commented Jun 14, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.01 ⚠️

Comparison is base (091f849) 90.93% compared to head (c5652da) 90.93%.

Additional details and impacted files
@@                                 Coverage Diff                                  @@
##           3094-support-for-owner-and-workspace-admin-roles    #3188      +/-   ##
====================================================================================
- Coverage                                             90.93%   90.93%   -0.01%     
====================================================================================
  Files                                                   215      216       +1     
  Lines                                                 11333    11354      +21     
====================================================================================
+ Hits                                                  10306    10325      +19     
- Misses                                                 1027     1029       +2     
Flag Coverage Δ
pytest 90.93% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/argilla/tasks/users/create.py 91.11% <ø> (-4.45%) ⬇️
src/argilla/tasks/users/create_default.py 100.00% <ø> (ø)
src/argilla/tasks/users/__main__.py 100.00% <100.00%> (ø)
src/argilla/tasks/users/update.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@frascuchon frascuchon merged commit 64374b6 into 3094-support-for-owner-and-workspace-admin-roles Jun 14, 2023
@frascuchon frascuchon deleted the feat/reviewing-user-tasks-cli branch June 14, 2023 15:16
alvarobartt added a commit that referenced this pull request Jun 22, 2023
# Description

This PR creates the default user with `owner` role instead of `admin` in
`release.Dockerfile`, so that the default user has all the required
permissions matching the ones that the `admin` user had before, to avoid
disruption. Also this PR creates an `owner` user besides the current
`admin` and `annotator` users being created as part of the
`quickstart.Dockerfile`.

Reverts the PR at #3188

**Type of change**

- [X] Refactor (change restructuring the codebase without changing
functionality)

**How Has This Been Tested**

- [X] `docker build` both `quickstart.Dockerfile` and
`release.Dockerfile`

**Checklist**

- [X] I added relevant documentation
- [X] follows the style guidelines of this project
- [X] I did a self-review of my code
- [X] I made corresponding changes to the documentation
- [X] My changes generate no new warnings
- [ ] 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: Francisco Aranda <francis@argilla.io>
davidberenstein1957 pushed a commit that referenced this pull request Jun 26, 2023
# Description

This PR creates the default user with `owner` role instead of `admin` in
`release.Dockerfile`, so that the default user has all the required
permissions matching the ones that the `admin` user had before, to avoid
disruption. Also this PR creates an `owner` user besides the current
`admin` and `annotator` users being created as part of the
`quickstart.Dockerfile`.

Reverts the PR at #3188

**Type of change**

- [X] Refactor (change restructuring the codebase without changing
functionality)

**How Has This Been Tested**

- [X] `docker build` both `quickstart.Dockerfile` and
`release.Dockerfile`

**Checklist**

- [X] I added relevant documentation
- [X] follows the style guidelines of this project
- [X] I did a self-review of my code
- [X] I made corresponding changes to the documentation
- [X] My changes generate no new warnings
- [ ] 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: Francisco Aranda <francis@argilla.io>
davidberenstein1957 pushed a commit that referenced this pull request Jun 26, 2023
# Description

This PR creates the default user with `owner` role instead of `admin` in
`release.Dockerfile`, so that the default user has all the required
permissions matching the ones that the `admin` user had before, to avoid
disruption. Also this PR creates an `owner` user besides the current
`admin` and `annotator` users being created as part of the
`quickstart.Dockerfile`.

Reverts the PR at #3188

**Type of change**

- [X] Refactor (change restructuring the codebase without changing
functionality)

**How Has This Been Tested**

- [X] `docker build` both `quickstart.Dockerfile` and
`release.Dockerfile`

**Checklist**

- [X] I added relevant documentation
- [X] follows the style guidelines of this project
- [X] I did a self-review of my code
- [X] I made corresponding changes to the documentation
- [X] My changes generate no new warnings
- [ ] 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: Francisco Aranda <francis@argilla.io>
davidberenstein1957 pushed a commit that referenced this pull request Jun 26, 2023
This PR creates the default user with `owner` role instead of `admin` in
`release.Dockerfile`, so that the default user has all the required
permissions matching the ones that the `admin` user had before, to avoid
disruption. Also this PR creates an `owner` user besides the current
`admin` and `annotator` users being created as part of the
`quickstart.Dockerfile`.

Reverts the PR at #3188

**Type of change**

- [X] Refactor (change restructuring the codebase without changing
functionality)

**How Has This Been Tested**

- [X] `docker build` both `quickstart.Dockerfile` and
`release.Dockerfile`

**Checklist**

- [X] I added relevant documentation
- [X] follows the style guidelines of this project
- [X] I did a self-review of my code
- [X] I made corresponding changes to the documentation
- [X] My changes generate no new warnings
- [ ] 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: Francisco Aranda <francis@argilla.io>
damianpumar pushed a commit that referenced this pull request Jun 27, 2023
# Description

This PR creates the default user with `owner` role instead of `admin` in
`release.Dockerfile`, so that the default user has all the required
permissions matching the ones that the `admin` user had before, to avoid
disruption. Also this PR creates an `owner` user besides the current
`admin` and `annotator` users being created as part of the
`quickstart.Dockerfile`.

Reverts the PR at #3188

**Type of change**

- [X] Refactor (change restructuring the codebase without changing
functionality)

**How Has This Been Tested**

- [X] `docker build` both `quickstart.Dockerfile` and
`release.Dockerfile`

**Checklist**

- [X] I added relevant documentation
- [X] follows the style guidelines of this project
- [X] I did a self-review of my code
- [X] I made corresponding changes to the documentation
- [X] My changes generate no new warnings
- [ ] 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: Francisco Aranda <francis@argilla.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants