Fixing overwrite and save slice permissions for a give role #298
Conversation
|
It fixes #143 |
|
|
| @@ -468,7 +468,7 @@ def explore(self, datasource_type, datasource_id): | |||
|
|
|||
| action = request.args.get('action') | |||
| if action in ('save', 'overwrite'): | |||
| return self.save(request.args, slc) | |||
| return self.saveoroverwriteslice(request.args, slc) | |||
There was a problem hiding this comment.
underscore_between_words_please
For admin self.appbuilder.sm.has_access('can_edit', 'Slices') and self.appbuilder.sm.has_access('can_add', 'Slices') permissions are False. The only permission available to admin on Slices is [has menu access]. I have changed the view name to SliceModelView instead of Slices and it works. Tested. |
|
Nice, ideally we'd disable the buttons in the UI as well. All you have to do is pass the same |
|
Thats a good catch. I thought about the same but didn't find any implementation in the current code to disable UI features based on access controls. I have updated and tested the code for disabling the button (basically calling appbuilder functions in the template). I don't see a need for {{ "disabled" if can_add }}. We can simply add it in the if condition to show the button or not. And now the backend handles the same as well. Let me know if you need further modifications in the way its implemented in this PR. I will then create another PR to do the same for some other important features like - save dashboard, download csv. |
|
Fyi - I tried replacing appbuilder.sm.has_access('can_edit', 'SliceModelView') with can_edit and it returned false. After reading template documentation I noticed one correct way to implement this is by calling objects passed in the render_template function. I don't see any current definition of can_edit and can_add in the current template code. |
|
|
|
I meant you'd pass |
1 similar comment
|
in the latest changes, I did the same. Im passing these parameters in the template to disable the buttons. So I think we are on the same page. Having said that I can remove these conditions from views.py (which seem redundant). What do you say? |
|
or may be you meant passing the values of can_edit and can_add in the template from explore function in the views.py. I think thats the same as replacing can_edit and can_add by self.appbuilder.sm.has_access('can_add', 'SliceModelView) in the template itself. Whatever you prefer :) |
|
I think you overlooked my first comment ;) Thats a good catch. I thought about the same but didn't find any implementation in the current code to disable UI features based on access controls. I have updated and tested the code for disabling the button (basically calling appbuilder functions in the template). I don't see a need for {{ "disabled" if can_add }}. We can simply add it in the if condition to show the button or not. And now the backend handles the same as well. Let me know if you need further modifications in the way its implemented in this PR. I will then create another PR to do the same for some other important features like - save dashboard, download csv. |
|
|
|
From my previous comment: " I prefer disabling the buttons to hiding them" |
|
My bad. I didn't know the difference :/ I am testing the disabling feature on the UI - http://www.w3schools.com/bootstrap/tryit.asp?filename=trybs_button_active&stacked=h |
|
|
|
Awesome! Mergin' |
This commit will only work if pull request apache#298 at cloudera/impyla is accepted, but it may not do no harm if not. It has also touched Hive impersonation, though it should not affect it.
* build: fix demo build * fix: version * build: add yarn.lock * fix: clean up webpack config
* build: fix demo build * fix: version * build: add yarn.lock * fix: clean up webpack config
* build: fix demo build * fix: version * build: add yarn.lock * fix: clean up webpack config
* build: fix demo build * fix: version * build: add yarn.lock * fix: clean up webpack config
* build: fix demo build * fix: version * build: add yarn.lock * fix: clean up webpack config
mistercrunch
added
🏷️ bot
@mistercrunch - I have fixed and tested slice save and slice overwrite permissions. Now a role can have flexibility to overwrite or save a slice. It will help in use cases where admin doesn't want users to overwrite a slice to mess up the dashboard.