-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
fix(datasets): add support for removing owners #16461
fix(datasets): add support for removing owners #16461
Conversation
Codecov Report
@@ Coverage Diff @@
## master #16461 +/- ##
==========================================
- Coverage 76.63% 76.58% -0.06%
==========================================
Files 1002 1002
Lines 53680 53684 +4
Branches 6852 6852
==========================================
- Hits 41138 41112 -26
- Misses 12305 12335 +30
Partials 237 237
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Code LGTM, a little concern if we have to avoid users to removing all owners ? |
f1299f7
to
f61d977
Compare
f61d977
to
1c6b699
Compare
🏷 2021.34 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* fix(datasets): add support for removing owners * default to current user (cherry picked from commit c5a5cf7)
* fix(datasets): add support for removing owners * default to current user
* fix(datasets): add support for removing owners * default to current user
SUMMARY
Currently removing all owners from the datasets modal raises the following error:
This makes the following changes to the endpoint:
OLD_API_CHECK_DATASET_OWNERSHIP
config is set to the default value (True
), we both check ownership and make sure the user doesn't lock themselves out of the object. See feat(dao): admin can remove self from object owners #15149 for more details on how this works for Admin vs regular usersOLD_API_CHECK_DATASET_OWNERSHIP
config flag is set toFalse
, the behavior is unchanged (other than fixing the error whendatasource_dict["owners"] == None
). Note that in this scenario, a user is able to both edit datasets that they don't own and also save without adding themselves to the list of owners. See fix(datasets): add support for removing owners #16461 for context on this FF.AFTER
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION