-
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
Disable user access request #4405
Disable user access request #4405
Conversation
123335b
to
117db38
Compare
117db38
to
5f3862b
Compare
You may want to disable the ModelView as well |
@mistercrunch Done. I left the access requests on by default. Is it better off by default? |
d94539c
to
63a7cdf
Compare
Let's flip it off by default. |
63a7cdf
to
410e764
Compare
Done |
4ff61ea
to
fb24089
Compare
Wait you're deleting all those tests :( Why not just have them look at the same config/feature flag? |
Ah, true. I'll do it that way instead. |
ccc1b05
to
5550e58
Compare
5550e58
to
c4c21a2
Compare
Do you have idea how does it join with #4352 ? |
@maver1ck This PR only gives different deployments the the option to remove the ability to request access from users. It doesn't change whether gamma has access to dashboards it created. |
* add feature flag to config * wrap check around a feature flag * add flag to the model view * remove access request from seurity tests
* add feature flag to config * wrap check around a feature flag * add flag to the model view * remove access request from seurity tests
The request access feature isn't fully implemented and it can potentially be misleading to users who click the access request button and expect to gain access eventually. This PR makes the access request button optional via a feature flag that allows it by default.
If you set the flag to false, you'll no longer see the image below whenever you don't have access to any of the slices in a dashboard.
Instead, you'll see all the slices and an error message for any one you don't have access to. Like in the image below.
@john-bodley @michellethomas @mistercrunch