Enhance OpenStack provider help text fields #861
Conversation
|
cc:// @RichardHoch @anarnold97 |
| <Trans t={t} ns="plugin__forklift-console-plugin"> | ||
| { | ||
| 'Error: The format of the provided URL is invalid. Ensure the URL includes a scheme, a domain name, and a path. For example: <strong>https://identity_service.com:5000/v3</strong>.' | ||
| } | ||
| </Trans> |
There was a problem hiding this comment.
| <Trans t={t} ns="plugin__forklift-console-plugin"> | |
| { | |
| 'Error: The format of the provided URL is invalid. Ensure the URL includes a scheme, a domain name, and a path. For example: <strong>https://identity_service.com:5000/v3</strong>.' | |
| } | |
| </Trans> | |
| <Trans t={t} ns="plugin__forklift-console-plugin"> | |
| Error: The format of the provided URL is invalid. Ensure the URL includes | |
| a scheme, a domain name, and a path. For example: | |
| <strong>https://identity_service.com:5000/v3</strong>. | |
| </Trans> |
sgratch
force-pushed
the
enhance-openstack-provider-help-text-fields
branch
2 times, most recently
from
0b0c102 to
10fd83c
Compare
Regarding code duplication - this was the situation with the old code as well. We dup the code per authentication type. Do you want me to change that now? |
| ), | ||
| validated: 'error', | ||
| }; | ||
| const urlValidationHook: ValidationHookType = (value) => { |
There was a problem hiding this comment.
urlValidationHook is a validator, it should be implemented in the validations part of our project, please move it to currect directory.
There was a problem hiding this comment.
This was not added as part of this PR, I just added more logic to that method.
In addition, this validation method exists for all providers as part of the modal code (ocp, oVirt,vsphere), so better do that in a separate follow up PR
There was a problem hiding this comment.
"you touch it you own it", it was not in the right place before you added to it, and it's not in it's right place now. Since you added to it, can you move it to the currect place ?
p.s.
you will also need to rename to fit the validation methods naming convention
There was a problem hiding this comment.
I'm ok with a folow up PR to fix in all places this method is used out of place
| <Trans t={t} ns="plugin__forklift-console-plugin"> | ||
| { | ||
| "Note: If 'Skip certificate validation' is selected, migrations from this provider will not be secure.<br><br> Insecure migration means that the transferred data is sent over an insecure connection and potentially sensitive data could be exposed." | ||
| } | ||
| </Trans> |
There was a problem hiding this comment.
| <Trans t={t} ns="plugin__forklift-console-plugin"> | |
| { | |
| "Note: If 'Skip certificate validation' is selected, migrations from this provider will not be secure.<br><br> Insecure migration means that the transferred data is sent over an insecure connection and potentially sensitive data could be exposed." | |
| } | |
| </Trans> | |
| <Trans t={t} ns="plugin__forklift-console-plugin"> | |
| Note: If 'Skip certificate validation' is selected, migrations from this provider will be insecure.<br /> | |
| <br /> | |
| Insecure migration means that the transferred data is sent over an insecure connection and potentially | |
| sensitive data could be exposed. | |
| </Trans> |
| ? t('CA certificate - disabled when skip certificate validation is checked') | ||
| : t('CA certificate - leave empty to use system certificates') | ||
| ? t( | ||
| "CA certificate - disabled and ignored when 'Skip certificate validation' is checked", |
There was a problem hiding this comment.
| "CA certificate - disabled and ignored when 'Skip certificate validation' is checked", | |
| "CA certificate - disabled when 'Skip certificate validation' is checked", |
| <Trans t={t} ns="plugin__forklift-console-plugin"> | ||
| { | ||
| 'URL of the OpenStack Identity (Keystone) endpoint. For example: <strong>https://identity_service.com:5000/v3</strong>.' | ||
| } | ||
| </Trans> |
There was a problem hiding this comment.
| <Trans t={t} ns="plugin__forklift-console-plugin"> | |
| { | |
| 'URL of the OpenStack Identity (Keystone) endpoint. For example: <strong>https://identity_service.com:5000/v3</strong>.' | |
| } | |
| </Trans> | |
| <Trans t={t} ns="plugin__forklift-console-plugin"> | |
| [ reactnode ] | |
| </Trans> |
can you replace all the places you send a string to Trans ?
There was a problem hiding this comment.
There are few places left to replace for other providers as well. I'll fix it in a follow up PR, together with doc review fixes.
Reference: kubev2v#646 Enhance and rephrase the help text fields and validations for following OpenStack dialogs: 1. Create OpenStack provider. 2. OpenStack Credentials view/edit modes. 3. OpenStack details - URL field 4. OpenStack details - URL icon help text Changes include supporting formatted help text. Signed-off-by: Sharon Gratch <[email protected]>
sgratch
force-pushed
the
enhance-openstack-provider-help-text-fields
branch
from
10fd83c to
c65e5a2
Compare
|
…texts to Obj Reference: kubev2v#646 Reference: kubev2v#854 (comment) Reference: kubev2v#861 (comment) A follow up for kubev2v#854 and kubev2v#861 that includes the following fixes: 1. Fix vSphere and oVirt help text fields and code comments, based on doc team review. 2. Replace all reminded <Trans> text appearances from String to React Obj. Signed-off-by: Sharon Gratch <[email protected]>
…texts to Obj Reference: kubev2v#646 Reference: kubev2v#854 (comment) Reference: kubev2v#861 (comment) A follow up for kubev2v#854 and kubev2v#861 that includes the following fixes: 1. Fix vSphere and oVirt help text fields and code comments, based on doc team review. 2. Replace all reminded <Trans> text appearances from String to React Obj. Signed-off-by: Sharon Gratch <[email protected]>
…texts to Obj Reference: kubev2v#646 Reference: kubev2v#854 (comment) Reference: kubev2v#861 (comment) A follow up for kubev2v#854 and kubev2v#861 that includes the following fixes: 1. Fix vSphere and oVirt help text fields and code comments, based on doc team review. 2. Replace all reminded <Trans> text appearances from String to React Obj. Signed-off-by: Sharon Gratch <[email protected]>
…texts to Obj Reference: kubev2v#646 Reference: kubev2v#854 (comment) Reference: kubev2v#861 (comment) A follow up for kubev2v#854 and kubev2v#861 that includes the following fixes: 1. Fix vSphere and oVirt help text fields and code comments, based on doc team review. 2. Replace all reminded <Trans> text appearances from String to React Obj. Signed-off-by: Sharon Gratch <[email protected]>
|
|
||
| const cacertHelperTextMsgs = { | ||
| error: t( | ||
| 'Error: The format of the provided CA Certificate is invalid. Ensure the CA certificate format is valid.', |
There was a problem hiding this comment.
First ooccurance: please change to "CA certificate"
| ), | ||
| success: t( | ||
| 'A CA certificate to be trusted when connecting to the OpenStack Identity (Keystone) endpoint. Ensure the CA certificate format is valid. To use a CA certificate, drag the file to the text box or browse for it. To use the system CA certificates, leave the field empty.', | ||
| ), |
There was a problem hiding this comment.
Is system CA certificates supposed to be plural? [I don't know]
There was a problem hiding this comment.
both works for me, it's one certificate per our provider, but the system may have more then one for other servers, so both singular and plural are ok with me.
There was a problem hiding this comment.
I'll change to a singular phrasing since the current text refers to the ca assigned to our provider:
"To use the system CA certificate, leave the field empty"
| <Trans t={t} ns="plugin__forklift-console-plugin"> | ||
| Note: If {"'"}Skip certificate validation{"'"} is selected, migrations from this provider will | ||
| not be secure.{'<br><br>'}Insecure migration means that the transferred data is sent over an | ||
| insecure connection and potentially sensitive data could be exposed. |
There was a problem hiding this comment.
"not be secure[tags], meaning that the transferred data..."
There was a problem hiding this comment.
not be secure => be inscure
when I talkd to people i say the connection is inscure, "the connnection is not secure" sounds strange to my ears
| @@ -231,15 +282,13 @@ export const OpenstackCredentialsEdit: React.FC<EditComponentProps> = ({ secret, | |||
| <FormGroup | |||
| label={ | |||
| insecureSkipVerify | |||
| ? t('CA certificate - disabled when skip certificate validation is checked') | |||
| : t('CA certificate - leave empty to use system certificates') | |||
| ? t("CA certificate - disabled when 'Skip certificate validation' is checked") | |||
There was a problem hiding this comment.
@RichardHoch This is a Switch component for selecting the options, so I wonder which action is commonly used for a Switch clicking. I checked other documentations and saw that they use checked but I guess that select is ok as well....
| ? t('CA certificate - disabled when skip certificate validation is checked') | ||
| : t('CA certificate - leave empty to use system certificates') | ||
| ? t("CA certificate - disabled when 'Skip certificate validation' is checked") | ||
| : t('CA certificate - leave empty to use system CA certificates') | ||
| } |
There was a problem hiding this comment.
here I'll leave the plural phrasing...
| )} | ||
| helperTextInvalid={t( | ||
| "Error: The format of the provided application credential name is invalid. Ensure the name doesn't include whitespace characters.", | ||
| )} |
There was a problem hiding this comment.
Maybe "spaces" instead of "whitespace characters" ? This is a "global" issue; I did not mark it every time. If I should mark every time, let me know.
| )} | ||
| helperTextInvalid={t( | ||
| "Error: The format of the provided application credential Secret is invalid. Ensure the secret doesn't include whitespace characters.", | ||
| )} |
There was a problem hiding this comment.
Should be Secret in both places.
| helperText={t('A user name for connecting to the OpenStack Identity (Keystone) endpoint.')} | ||
| helperTextInvalid={t( | ||
| "Error: The format of the provided user name is invalid. Ensure the user name doesn't include whitespace characters.", | ||
| )} |
There was a problem hiding this comment.
Either "user name" or "username" is OK -- but please use the same for the UI and the messages, Help texts, etc.
| ? t( | ||
| "CA certificate - disabled and ignored when 'Skip certificate validation' is checked", | ||
| ) | ||
| ? t("CA certificate - disabled when 'Skip certificate validation' is checked") |
| ? t( | ||
| "CA certificate - disabled and ignored when 'Skip certificate validation' is checked", | ||
| ) | ||
| ? t("CA certificate - disabled when 'Skip certificate validation' is checked") | ||
| : t('CA certificate - leave empty to use system CA certificates') | ||
| } |
There was a problem hiding this comment.
here I'll leave the plural phrasing...
| insecureSkipVerify: { | ||
| label: t('Skip certificate validation'), | ||
| description: t("If true, the provider's REST API TLS certificate won't be validated."), | ||
| description: t( | ||
| "If true (check box is checked), the provider's CA certificate won't be validated.", |
| }, | ||
| cacert: { | ||
| label: t('CA certificate'), | ||
| description: t( | ||
| 'Custom certification used to verify the OpenStack REST API server, when empty use system certificate.', | ||
| 'A CA certificate to be trusted when connecting to the OpenStack Identity (Keystone) endpoint. Ensure the CA certificate format is valid. To use a CA certificate, drag the file to the text box or browse for it. To use the system CA certificates, leave the field empty.', | ||
| ), |
There was a problem hiding this comment.
systen CA certifucates or certificate
There was a problem hiding this comment.
changed to a singular phrasing
| description: t("If true, the provider's REST API TLS certificate won't be validated."), | ||
| description: t( | ||
| "If true (check box is checked), the provider's CA certificate won't be validated.", | ||
| ), |
| helperTextPopover: ( | ||
| <Trans t={t} ns="plugin__forklift-console-plugin"> | ||
| Note: If this field is checked/true, the migration from this provider will be insecure. | ||
| {'<br><br>'} Insecure migration means that the transferred data is sent over an insecure |
| insecureSkipVerify: { | ||
| label: t('Skip certificate validation'), | ||
| description: t("If true, the provider's REST API TLS certificate won't be validated."), | ||
| description: t( | ||
| "If true (check box is checked), the provider's CA certificate won't be validated.", |
There was a problem hiding this comment.
Lines 128-134: See examples above.
| description: t("If true, the provider's REST API TLS certificate won't be validated."), | ||
| description: t( | ||
| "If true (check box is checked), the provider's CA certificate won't be validated.", | ||
| ), |
| insecureSkipVerify: { | ||
| label: t('Skip certificate validation'), | ||
| description: t("If true, the provider's REST API TLS certificate won't be validated."), | ||
| description: t( | ||
| "If true (check box is checked), the provider's CA certificate won't be validated.", |
…texts to Obj Reference: kubev2v#646 Reference: kubev2v#854 (comment) Reference: kubev2v#861 (comment) A follow up for kubev2v#854 and kubev2v#861 that includes the following fixes: 1. Fix vSphere and oVirt help text fields and code comments, based on doc team review. 2. Replace all reminded <Trans> text appearances from String to React Obj. Signed-off-by: Sharon Gratch <[email protected]>
… provider Reference: kubev2v#646 Reference: kubev2v#861 (comment) Fix all comments mentioned by the doc team in kubev2v#861 and applied it to all relevant providers. Signed-off-by: Sharon Gratch <[email protected]>
Reference: #646
Enhance and rephrase the help text fields and validations for following OpenStack dialogs:
Changes include adding tooltips to skip ca cert field and supporting formatted help text.
Few Screenshots