-
Notifications
You must be signed in to change notification settings - Fork 875
Solidify permissions and licensing checks in the UI for Org feature set #13915
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
Comments
Related: #14057 |
Something that needs to be addressed is that you might see organizations you cannot edit in the sidebar. Right now I am showing a "you cannot edit this org" message, but maybe we will want to filter out organizations the user cannot edit (no easy way to do this right now I think aside from making an auth check for each org)? But there is also currently a halfway state where the user cannot edit the org but they can see the audit log link, so in that case I am not sure what to do, because we still need the org to show up on the sidebar so they can get to the audit link. If we decide to get rid of the per-org audit links then it will resolve itself though. We could have that page be a sort of summary page for the org instead of a settings page if you do not have permissions. Just displays the org name and description or something like that. Edit: actually just going to still show the form but make it ineditable for now. |
@code-asher I think we should do an auth check for each menu item, and show it conditionally. Some pages like
I think this is a great idea. Then someone with no sidebar item perms, but can still see the org with |
we do that in edit: err, that PR is about to be merged but hasn't been yet #14174 |
Oh dope! I do actually run individual authcheck queries for the links under each org (so you should only see the sub-links you have permissions for), but I only do it at the point when the menu item is clicked and expanded because I was worried about firing off N queries all at once, but it sounds like that is probably the better way to do this, since that lets me hide the main org item entirely if the user has no permissions at all. Thanks for the guidance all! And I will add a summary page for users who can see at least one link but cannot edit. |
@code-asher authchecks can be batched into a single api call. So if you know all the auth checks up front, you could send all N in 1 call |
Good grief how did I not realize that, I should have read Kayla's PR more closely, thank you that makes things incredibly easy. Edit: oh wait I thought maybe there was some fancy nesting I could do but I think I just prefix IDs or something like that to do a multi-org query Edit 2: yeah going to do |
@code-asher the |
Ah yup that makes sense to me, could bring the same license UI that is on the new org page I think. And/or show the summary instead of the settings? Not sure we want to hide in case the license expires and then they can no longer see their orgs. |
@bpmct I don't think we should hide all org stuff without the license. There is still an existence of organizations, and licenses can expire. Maybe we should have a conversation how to best handle this. |
For now I made it show the summary page, but I def think we should rethink especially since that summary page is pretty lame, and it does not explain why you are seeing the summary page instead of the form (it should probably show that the feature is not entitled or something, maybe using the entitled badge) |
Orgs is an enterprise feature. We should ensure it is effectively and coherently gated prior to releasing as an experiment.
Requirements
organization.<org_id>.update
Known issues
The text was updated successfully, but these errors were encountered: