Skip to content
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

Bug 1955737: Fix error modal text for cert/CORS errors #1202

Merged
merged 2 commits into from May 11, 2021

Conversation

ibolton336
Copy link
Contributor

@ibolton336 ibolton336 commented May 10, 2021

image

@ibolton336 ibolton336 requested a review from a team May 10, 2021 16:44
@github-actions
Copy link

valid bug 1955737

rayfordj
rayfordj previously approved these changes May 10, 2021
in one of the clusters. Navigate to the following URL and accept the certificate:
A network error has occurred and we are unable to fetch one or more resources. We
are unable to determine the precise cause due to browser obfuscation, but based on
the content of the error it is likely one of two root issues:
Copy link

@apinnick apinnick May 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure the use will understand "browser obfuscation" or whether that information would be helpful. You might want to describe possible courses of action without explaining why the cause of the error is unknown.

Suggestion:

"A network error has occurred. The likely cause of the error is one of the following issues:"

store.
<ul>
<li>
* Your cluster API and/or its services are using self-signed certificates and
Copy link

@apinnick apinnick May 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion:

"The cluster is using self-signed certificates and you have not installed the certificate authority so that it is trusted. To bypass the certificate check, navigate to the following URL to accept the certificate:"

<br />
NOTE: It is recommended if using a self-signed certificate to install the
trusted CA in your browser’s supported store prior to using the Migration
Toolkit.
Copy link

@apinnick apinnick May 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested text:

"The best practice is to install the certificate authority in the browser's trust store."

a Cross Origin Resource Sharing rejection. This is something that is configured
in the cluster’s API server settings by the MTC operator and can take up to 10
minutes to propagate across all related services. Please wait and check back in
at a later point in time.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested text:

"Cross-origin resource sharing has not yet been configured. This process can take up to 10 minutes. Wait for the process to complete and then refresh your browser." (or "log in" or whatever is required)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eriknelson @ibolton336 , rather than "up to 10 minutes", consider:

"This process can take at least 10 minutes."

Because we know it is designed to eventually complete and we have no control over how long that will take, does it make more sense to highlight a [typical] lower-limit (at least 10 min) while leaving the upper-limit unbound?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 it should be phrased the way @rayfordj described

@rayfordj rayfordj dismissed their stale review May 11, 2021 13:20

PR discussion continues.

@ibolton336
Copy link
Contributor Author

image

Copy link
Contributor

@eriknelson eriknelson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thank you everyone for making this happen quickly!

@ibolton336 ibolton336 merged commit ceb1c16 into migtools:master May 11, 2021
@ibolton336 ibolton336 deleted the bz-1955737 branch May 11, 2021 14:24
ibolton336 added a commit to ibolton336/mig-ui that referenced this pull request May 11, 2021
ibolton336 added a commit that referenced this pull request May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants