Hide restricted ui elements, remove <br> from error message

[jasnovak]

Only show explore and edit buttons for a slice if user has explore or edit access. Don't show save buttons on dashboard if user doesn't have save access.

Also, remove a "<br>" in the error message of the SaveModal. It displays on screen since the message is not html.

[codecov-io]

Codecov Report

Merging #4900 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #4900      +/-   ##
==========================================
+ Coverage   77.08%   77.09%   +<.01%     
==========================================
  Files          44       44              
  Lines        8549     8551       +2     
==========================================
+ Hits         6590     6592       +2     
  Misses       1959     1959

Impacted Files	Coverage Δ
superset/views/core.py	74.71% <100%> (+0.03%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d6e59a...7260e8b. Read the comment docs.

[mistercrunch]

Let's use camel case for props. The guideline is it's ok to reference objects keys coming out of python in underscore notation, but for everything declared on the js side (const, let, props) we use camel case.

[jasnovak]

Yes, I generally follow that convention as well. But the code is not consistent in that. In this code, props for permissions are usually written in snake_case on the js side. So I used snake_case as well to match the current style:

Here's a few examples:
in ExploreChartHeader.jsx:
const propTypes = {
actions: PropTypes.object.isRequired,
addHistory: PropTypes.func,
can_overwrite: PropTypes.bool.isRequired,
can_download: PropTypes.bool.isRequired,

or in ExploreViewContainer:
can_overwrite: !!explore.can_overwrite,
can_download: !!explore.can_download,
column_formats: explore.datasource ? explore.datasource.column_formats : null,

I can do either way of course, if you'd rather I copy the current style, or use camel case.

[mistercrunch]

Let's do camel case here and we should fix up ExploreChartHeader and ExploreViewContainer eventually. I'm surprised the linter doesn't fire for this.

[jasnovak]

Sounds good! Will do. Yes, there are other places where snake case is used in the js besides the examples I gave -- could use a pass to fix them all up for sure.

[mistercrunch]

here too

[jasnovak]

Same as above. I matched the prevalent style in the code for permission variables, but can do camel case instead if you'd like.

[mistercrunch]

@jasnovak also note that there's ongoing work on rebuilding the dashboard app pretty much from scratch. Hopefully the work here is taken into account on that side

@williaster @graceguo-supercat

[jasnovak]

@mistercrunch Yes -- and there may be other places where this type of thing could be applied, i.e. not showing controls for which a user doesn't have access. I took care of these cases as they were important for us with our read-only, dashboard level access controls.

[williaster]

Yep I think we're on top of these in the new dashboard.

[graceguo-supercat]

@jasnovak @mistercrunch If user is not the owner of a dashboard, we used to allow user save-as a new copy of this dashboard ( and allow copy all slices under current dashboard).
With this PR, user can't save-as other owner's dashboard anymore. Is this intended behavior?

before:

after:

[jasnovak]

Yes, this is intended behavior. We have a read-only role that we don't want to give the ability to create dashboards. So we don't give them save access, which for this case should also mean no save-as.

[graceguo-supercat]

@jasnovak Thanks for clarify. Do you have some roles that can view and save-as other owner's dashboards, but cannot edit/overwrite changes on the original dashboard?

[jasnovak]

@graceguo-supercat No, we don't have that case. But I think to support both cases, you may need another permission, say can_save_as_dash

[graceguo-supercat]

@jasnovak In Superset build-in roles: both Alpha user and Gamma user can save dashboard, and can save other owner's dashboard as their copy. There are tests to assure this feature. We have a lot of users want to copy other's dashboard, but not change the original one. But with this PR, all Alpha and Gamma users can't save dashboard as copy anymore.

I want to make some changes that still allow Alpha+Gamma users save dashboard as copy. For the users in your system that don't have save access, are they any of build-in roles, or any role that removes can_save_dash? thanks!

[jasnovak]

Yes, we remove can_save_dash from our read-only users. So, if you want to make can_save_dash allow people to copy other's dashboards, that would work fine. Thanks for asking!