Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix oVirt provider url help text field #744

Merged
merged 1 commit into from
Oct 3, 2023

Conversation

sgratch
Copy link
Collaborator

@sgratch sgratch commented Oct 1, 2023

Reference: #646

As a followup for #732, fix the oVirt 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. "ovirt-engine/api/") as a valid url since it's mentioned on docs as valid and it's a common used 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 19-33-55

    After the fix, the warning text is displayed:

    Screenshot from 2023-10-01 19-35-03

@sgratch
Copy link
Collaborator Author

sgratch commented Oct 1, 2023

cc:// @RichardHoch @anarnold97

Copy link
Member

@yaacov yaacov left a comment

Choose a reason for hiding this comment

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

LGTM code wise, waiting for docs approval

@sgratch sgratch force-pushed the fix-ovirt-url-field-help-text branch from f772489 to 0e4d109 Compare October 1, 2023 19:50
@sgratch sgratch requested a review from ahadas October 2, 2023 14:26
@sgratch
Copy link
Collaborator Author

sgratch commented Oct 3, 2023

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

@sgratch sgratch force-pushed the fix-ovirt-url-field-help-text branch from c91d18f to 93426a2 Compare October 3, 2023 12:17
As a followup for kubev2v#732,
fix the oVirt 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. "ovirt-engine/api/")
   as a valid url since it's mentioned on docs as valid and 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-ovirt-url-field-help-text branch from 93426a2 to d8be031 Compare October 3, 2023 12:38
@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 3, 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
0.0% 0.0% Duplication

@yaacov yaacov merged commit 763a2f2 into kubev2v:main Oct 3, 2023
@@ -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 Red Hat Virtualization Manager (RHVM) API endpoint. Ensure the URL includes the \"/ovirt-engine/api\" path. for example: https://rhv-host-example.com/ovirt-engine/api": "URL of the Red Hat Virtualization Manager (RHVM) API endpoint. Ensure the URL includes the \"/ovirt-engine/api\" path. for example: https://rhv-host-example.com/ovirt-engine/api",
"User ID": "User ID",

Choose a reason for hiding this comment

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

@sgratch Should be "For example:..."

'Warning: The provided URL does not end with the RHVM API endpoint path: "ovirt-engine/api". Ensure the URL includes the correct path. For example: https://rhv-host-example.com/ovirt-engine/api',
),
success: t(
'URL of the Red Hat Virtualization Manager (RHVM) API endpoint. Ensure the URL includes the "/ovirt-engine/api" path. for example: https://rhv-host-example.com/ovirt-engine/api',

Choose a reason for hiding this comment

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

@sgratch Should be "For example:..."

@yaacov
Copy link
Member

yaacov commented Oct 4, 2023

@sgratch hi, can you make a PR for @RichardHoch suggestions above ^^ ?

@sgratch
Copy link
Collaborator Author

sgratch commented Oct 4, 2023

@sgratch hi, can you make a PR for @RichardHoch suggestions above ^^ ?

Done: #749

@sgratch sgratch deleted the fix-ovirt-url-field-help-text branch October 4, 2023 12:53
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants