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

Fix vSphere provider url help text field #745

Merged
merged 1 commit into from
Oct 4, 2023

Conversation

sgratch
Copy link
Collaborator

@sgratch sgratch commented Oct 1, 2023

Reference: #646

As a followup for #736 , fix the vSphere provider's URL help text as follows:

  1. Rephrase the error, warning and successful/initial text messages to be aligned with the documnetation, other providers fields and Patternfly error msg recommendations.

  2. Set a url input ended with a "/" (i.e. "sdk/") as a valid url since it's a common use case.

  3. Fix a bug in which the warning text message string (helperTextMsgs.warning) is never displayed in the UI (the color is set to yellow to indicate the warning field validation state, but the text message was not changed accordingly).

    Before the fix, the warning text was the same as the successful text:

    Screenshot from 2023-10-01 20-13-47

    After the fix, the warning text is displayed:

    Screenshot from 2023-10-01 20-14-32

@sgratch
Copy link
Collaborator Author

sgratch commented Oct 1, 2023

cc:// @RichardHoch @anarnold97

@sgratch sgratch requested a review from yaacov October 1, 2023 17:28
@sgratch sgratch force-pushed the fix-vsphere-url-field-help-text branch from 847e94d to 02c5a4f Compare October 1, 2023 19:40
@sgratch sgratch requested review from yaacov and ahadas October 1, 2023 19:51
@ahadas
Copy link
Member

ahadas commented Oct 3, 2023

I've proposed a different phrasing on #744 , please check it out before making changes here

@sgratch
Copy link
Collaborator Author

sgratch commented Oct 3, 2023

@RichardHoch @ahadas your suggested changes were applied, please review

@@ -391,6 +390,7 @@
"URL must start with https:// or http:// and contain valid hostname and path": "URL must start with https:// or http:// and contain valid hostname and path",
"URL of the provider": "URL of the provider",
"URL of the provider, leave empty to use this providers URL": "URL of the provider, leave empty to use this providers URL",
"URL of the vCenter SDK endpoint. Ensure the URL includes the \"/sdk\" path. for example: https://vCenter-host-example.com/sdk": "URL of the vCenter SDK endpoint. Ensure the URL includes the \"/sdk\" path. for example: https://vCenter-host-example.com/sdk",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"URL of the vCenter SDK endpoint. Ensure the URL includes the \"/sdk\" path. for example: https://vCenter-host-example.com/sdk": "URL of the vCenter SDK endpoint. Ensure the URL includes the \"/sdk\" path. for example: https://vCenter-host-example.com/sdk",
"URL of the vCenter SDK endpoint. Ensure the URL includes the \"/sdk\" path. For example: https://vCenter-host-example.com/sdk": "URL of the vCenter SDK endpoint. Ensure the URL includes the \"/sdk\" path. For example: https://vCenter-host-example.com/sdk",

@@ -412,7 +412,7 @@
"vSphere REST API user name.": "vSphere REST API user name.",
"Warning concerns": "Warning concerns",
"Warning: The provided URL does not end with \"ovirt-engine/api\". Ensure it includes the correct path, like: https://rhv.com/ovirt-engine/api.": "Warning: The provided URL does not end with \"ovirt-engine/api\". Ensure it includes the correct path, like: https://rhv.com/ovirt-engine/api.",
"Warning: The provided URL does not end with \"sdk\". Ensure it includes the correct path, like: https://vcenter.com/sdk.": "Warning: The provided URL does not end with \"sdk\". Ensure it includes the correct path, like: https://vcenter.com/sdk.",
"Warning: The provided URL does not end with the SDK endpoint path: \"sdk\". Ensure the URL includes the correct path. For example: https://vCenter-host-example.com/sdk": "Warning: The provided URL does not end with the SDK endpoint path: \"sdk\". Ensure the URL includes the correct path. For example: https://vCenter-host-example.com/sdk",
Copy link
Member

Choose a reason for hiding this comment

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

nit-picking - we're not consistent here and also in #744 about the leading / in the path - here we don't have it (we say sdk) and below we have it (/sdk)

'Warning: The provided URL does not end with the SDK endpoint path: "sdk". Ensure the URL includes the correct path. For example: https://vCenter-host-example.com/sdk',
),
success: t(
'URL of the vCenter SDK endpoint. Ensure the URL includes the "/sdk" path. for example: https://vCenter-host-example.com/sdk',
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
'URL of the vCenter SDK endpoint. Ensure the URL includes the "/sdk" path. for example: https://vCenter-host-example.com/sdk',
'URL of the vCenter SDK endpoint. Ensure the URL includes the "/sdk" path. For example: https://vCenter-host-example.com/sdk',

@RichardHoch
Copy link

LGTM with the changes Arik suggested ("F" instead of "f" where marked).

@sgratch sgratch force-pushed the fix-vsphere-url-field-help-text branch from 3288af3 to 0910d72 Compare October 4, 2023 09:11
@yaacov
Copy link
Member

yaacov commented Oct 4, 2023

@sgratch can you take care of the git conflicts ?

As a followup for kubev2v#736, fix the vSphere provider's URL help text as follows:

1. Rephrase the error, warning and successful/initial text messages to be
   aligned with the documnetation, other providers fields and Patternfly error msg recommendations.
2. Set a url input ended with a "/" (i.e. "sdk/") as a valid url since it's a common used case.
3. Fix a bug in which the waring text message string (helperTextMsgs.warning) is never displayed
   in the UI (the color is set to yellow to indicate the warning field validation state, but the text message was not changed accordingly).

Signed-off-by: Sharon Gratch <sgratch@redhat.com>
@sgratch sgratch force-pushed the fix-vsphere-url-field-help-text branch from 0910d72 to 9d96bf5 Compare October 4, 2023 09:47
@sonarcloud
Copy link

sonarcloud bot commented Oct 4, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
3.9% 3.9% Duplication

@yaacov yaacov merged commit bf6ec95 into kubev2v:main Oct 4, 2023
5 checks passed
@sgratch sgratch deleted the fix-vsphere-url-field-help-text branch October 4, 2023 10:06
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.

4 participants