-
Notifications
You must be signed in to change notification settings - Fork 14k
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: Alpha are unable to perform a second modification to a Dataset when in Explore #20296
Conversation
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!
@@ -200,7 +200,8 @@ class DatasourceControl extends React.PureComponent { | |||
const { user } = this.props; | |||
const allowEdit = | |||
datasource.owners.map(o => o.id).includes(user.userId) || | |||
isUserAdmin(user); | |||
datasource.owners.map(o => o.value).includes(user.userId); |
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.
couldn't you write this in without the ||
o => o.id || o.value == user.userId
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.
don't think this is the right way of doing it though.
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.
also should this be an || or should you be amending allowEdit with the new values from o.value?
/testenv up |
@hughhhh Ephemeral environment spinning up at http://54.149.174.139:8080. Credentials are |
Codecov Report
@@ Coverage Diff @@
## master #20296 +/- ##
==========================================
- Coverage 66.65% 66.61% -0.05%
==========================================
Files 1729 1733 +4
Lines 64906 64953 +47
Branches 6842 6858 +16
==========================================
+ Hits 43266 43269 +3
- Misses 19891 19929 +38
- Partials 1749 1755 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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.
could we add some tests for this?
Ephemeral environment shutdown and build artifacts deleted. |
SUMMARY
Fix issue when Alpha users want to make a secondary change in
Edit Dataset
. The issue lies with the API fromdatasource/save
returning a different payload for datasource.owners. To handle we'll add an additional check for the key value to make sure the user will still be able to edit moving forward.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION