Bug 1978222: make Users list empty state message more clear

[rhamilto]

Can edit identity providers and none exist:

Can edit identity providers and one or more exist, but no users from the identity provider have logged in:

Cannot edit identity providers:

[openshift-ci]

@rhamilto: This pull request references Bugzilla bug 1978222, which is invalid:

• expected the bug to target the "4.9.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1978222: make Users list empty state message more clear

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[rhamilto]

/bugzilla refresh

[openshift-ci]

@rhamilto: This pull request references Bugzilla bug 1978222, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target release (4.9.0) matches configured target release for branch (4.9.0)
• bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @yapei

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[itsptk]

I personally would lean to leave the empty state as-is for the 'no idp configured' case, and add a new "No Users found" empty state when an idp is added that doesn't include the "Add IDP" button and mentions in the sub-text again that "Users are automatically added the first time they log in" or similar.

[spadgett]

I'd get the OAuth resource to check if an IDP exists and use that to decide whether to show the button. We do this for the "Getting started" box at the top of the Home -> Overview page IIRC. If the user can't get the OAuth resource, they wouldn't be able to add an IDP anyway, so we can hide the button. Ideally we'd also perform an access review to see if the user can patch the OAuth resource.

[TheRealJon]

Should we just say "No users have logged in." for the title, then in the detailed message we can clarify that the list will be empty until at least one user has logged in for the first time. Also, we might consider excluding the IDP link if at least one IDP is already configured.

[TheRealJon]

@spadgett @rhamilto @itsptk Does this cover all cases?

1. The user can't see or patch the OAuth resource:
   Use the current empty state without mentioning identity providers because we don't know if one exists or not.

2. The user can see but not patch the OAuth resource:
   a. An IDP exists:

   "No users have logged in yet."
   "Users will be added to this list after their first login"
   

   b. An IDP doesn't exist

   "No Users Found."
   "A cluster administrator needs to configure an identity provider for this cluster"
   

3. The user can see and patch the OAuth resource:
   a. An IDP exists: Same as 2a
   b. An IDP doesn't exist: Keep current empty state.

[openshift-ci]

@rhamilto: This pull request references Bugzilla bug 1978222, which is valid.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target release (4.9.0) matches configured target release for branch (4.9.0)
• bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @yapei

In response to this:

Bug 1978222: make Users list empty state message more clear

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[rhamilto]

Thanks for the input! Take 2.

Note this does not cover all the cases @TheRealJon outlined in #9437 (comment), but gets us most of the way there.

[openshift-ci]

@rhamilto: This pull request references Bugzilla bug 1978222, which is valid.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target release (4.9.0) matches configured target release for branch (4.9.0)
• bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @yapei

In response to this:

Bug 1978222: make Users list empty state message more clear

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[spadgett]

Thanks @rhamilto

[openshift-ci]

@rhamilto: This pull request references Bugzilla bug 1978222, which is valid.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target release (4.9.0) matches configured target release for branch (4.9.0)
• bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @yapei

In response to this:

Bug 1978222: make Users list empty state message more clear

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[rhamilto]

Thanks, @spadgett. Updated.

[TheRealJon]

/lgtm

[itsptk]

@spadgett @rhamilto @itsptk Does this cover all cases?

1. The user can't see or patch the OAuth resource:
   Use the current empty state without mentioning identity providers because we don't know if one exists or not.

2. The user can see but not patch the OAuth resource:
   a. An IDP exists:

   "No users have logged in yet."
   "Users will be added to this list after their first login"
   

   b. An IDP doesn't exist

   "No Users Found."
   "A cluster administrator needs to configure an identity provider for this cluster"
   

3. The user can see and patch the OAuth resource:
   a. An IDP exists: Same as 2a
   b. An IDP doesn't exist: Keep current empty state.

This generally sounds good to me. We discussed this bug some in a UX team meeting this morning and I think our only change would be for 2a to just stick with the "No Users found" main line, since that follows our Empty state convention and avoids any possible confusion whether this is a list of currently logged in users. We think the sub-text of "Users will be added to this list after their first login" would cover conveying that to someone looking at this list. Looks good. 👍

[rhamilto]

@spadgett @rhamilto @itsptk Does this cover all cases?

It does not cover them as @TheRealJon outlined them as we don't cover the case where a user can view oauth but not patch it (case 2). We assume that if you cannot patch, you also cannot view (see the third screenshot above).

[spadgett]

We assume that if you cannot patch, you also cannot view (see the third screenshot above).

This seems fine because you can't add an IDP through the web console if you can't view, even if you have patch. I'm having trouble thinking of a scenario where it makes sense to give a user patch without view for this resource.

[itsptk]

LGTM

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: itsptk, rhamilto, TheRealJon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• frontend/OWNERS [rhamilto]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[openshift-ci]

@rhamilto: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-gcp-console	c7cf3e7	link	/test e2e-gcp-console

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[openshift-bot]

/retest

Please review the full test history for this PR and help us cut down flakes.

[openshift-ci]

@rhamilto: All pull requests linked via external trackers have merged:

• openshift/console#9437

Bugzilla bug 1978222 has been moved to the MODIFIED state.

In response to this:

Bug 1978222: make Users list empty state message more clear

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.