integr8ly / tutorial-web-app

Solution Explorer provides the front door into the Integreatly initiative. It hosts the various Solution Patterns, as well as providing a dashboard of installed applications/products/services.
Apache License 2.0
36 stars 54 forks source link

APPDUX-80: Make Settings icon inactive for non-admin users #580

Closed mfrances17 closed 4 years ago

mfrances17 commented 4 years ago

Motivation

https://issues.redhat.com/browse/APPDUX-80

What

Make the Settings icon inactive for non-admin users and display a tooltip explanation.

Why

Recent requirement to not allow non-admins access to certain areas of the Solution Explorer UI.

Verification Steps

To test:

  1. Log into Solution Explorer on an OpenShift 4 cluster as an Administrator e.g. kube:admin.
  2. Verify that the Settings cog in the masthead is clickable and opens the Settings page.

  3. Close the Solution Explorer browser tab.
  4. In OpenShift, log out as and click github to log in with your dev credentials.

  5. Launch Solution Explorer from the application launcher.
  6. Click from the masthead and select Log out.
  7. Log back into Solution Explorer with your GitHub credentials.
  8. Verify that the Settings cog is now grayed out and when you hover over it, a tooltip appears and explains why.

Checklist:

Progress

Additional Notes

For testing, you can point your OpenShift cluster to my Solution Explorer docker image at:
docker.io/mfrances17/dev-tutorial-web-app:latest

For a limited time, you can also view the changes on the UXD OpenShift 4 cluster:
 https://tutorial-web-app-redhat-rhmi-solution-explorer.apps.uxd.cloudservices.rhmw.io/

Screencaps

Masthead logged in as Dev: dev_masthead

Masthead logged in as Admin: admin_masthead

Settings page after click as Admin: admin_after_click

tiffanynolan commented 4 years ago

@mfrances17 Looking good! For the tooltip, is there a way to either (1) change the background color to a gray or (2) move it further away from the hover point? The detail and shape of it are getting lost in the masthead since it looks like the same background color.

tiffanynolan commented 4 years ago

@mfrances17 Will the update to change the settings page to add the empty state be part of this PR or a different one? I can still get to and change the settings page as a "developer". image

mfrances17 commented 4 years ago

@mfrances17 Looking good! For the tooltip, is there a way to either (1) change the background color to a gray or (2) move it further away from the hover point? The detail and shape of it are getting lost in the masthead since it looks like the same background color.

I'll take a look and see if there's something we can do, and run it past you.

Update: @tiffanynolan I've increased the tooltip distance, how is this:

tooltip_distance

mfrances17 commented 4 years ago

@mfrances17 Will the update to change the settings page to add the empty state be part of this PR or a different one? I can still get to and change the settings page as a "developer".

I assume you got there by typing /Settings in the browser address bar... yes this will be handled in a separate JIRA: https://issues.redhat.com/browse/APPDUX-113

pb82 commented 4 years ago

@mfrances17 we should also test this with one of the customer-admin users (e.g. customer-admin01 when using the testing-idp). They are not cluster admins, but should still be able to access those settings.

mfrances17 commented 4 years ago

Will do, thanks.

Update: @pb82 I tested on the uxd server using testing-idp and a customer admin account, which failed, so I included a fix to openshift services to fix it. It's testing well on the uxd cluster if you want to verify. Good catch, thanks.

tiffanynolan commented 4 years ago

Update: @tiffanynolan I've increased the tooltip distance, how is this:

Yes, much better. Thanks.