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

Localize strings in Rollup and ILM apps. #29034

Merged
merged 8 commits into from
Jan 24, 2019

Conversation

cjcenizal
Copy link
Contributor

No description provided.

@cjcenizal cjcenizal added Project:i18n v7.0.0 Feature:ILM Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v6.7.0 labels Jan 19, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui

@@ -314,7 +314,6 @@ export class PolicyTableUi extends Component {
const button = (
<EuiButtonEmpty
data-test-subj="policyActionsContextMenuButton"
aria-label="Policy options"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bmcconaghy Do you think we still need this? I figure the text in the button itself is explanatory ("Actions"). Otherwise, I'd suggest we change this to be "Policy actions".

        aria-label={intl.formatMessage({
          id: 'xpack.indexLifecycleMgmt.policyTable.actionsButtonAriaLabel',
          defaultMessage: 'Policy actions',
        })}

@@ -410,8 +409,11 @@ export class PolicyTableUi extends Component {
color="danger"
onClick={() => this.setState({ showDeleteConfirmation: true })}
>
Delete {numSelected} polic
{numSelected > 1 ? 'ies' : 'y'}
<FormattedMessage
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bmcconaghy This code implies you should be able to select multiple policies, but I'm not seeing checkboxes in the table. Did this bug ship originally, or is it a regression, or am I just missing something?

image

@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Contributor

@maryia-lapata maryia-lapata left a comment

Choose a reason for hiding this comment

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

@cjcenizal I just noticed that ILM wasn't added to .i18nrc.json config. It means that translations for ILM were not validated i18n tool (during CI) and labels weren't send to translation vendors.
We'll send labels to vendor on the 23rd of January. It'll the last time for v6.7.0.

Could you please add "xpack.indexLifecycleMgmt": "x-pack/plugins/index_lifecycle_management", to paths object of .i18nrc.json? It will required to fix some issue of i18n validation tool. To run it: node scripts/i18n_check.js --path x-pack/plugins/index_lifecycle_management/

{numSelected > 1 ? 'ies' : 'y'}
<FormattedMessage
id="xpack.indexLifecycleMgmt.policyTable.sectionHeading"
defaultMessage="Deleted {numSelected} {selectedIndexCount, plural, one {policy} other {policies}}"
Copy link
Contributor

Choose a reason for hiding this comment

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

selectedIndexCount wasn't defined:

Suggested change
defaultMessage="Deleted {numSelected} {selectedIndexCount, plural, one {policy} other {policies}}"
defaultMessage="Deleted {numSelected, plural, one {policy} other {policies}}"

@cjcenizal
Copy link
Contributor Author

Retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Contributor Author

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

@maryia-lapata Thanks for catching that! I've added index lifecycle management to the i18nrc file and fixed the validation errors. I also caught a few other things which required internationalization. Could you please take another look?

@@ -37,7 +37,7 @@ export class AddPolicyToTemplateConfirmModalUi extends Component {
const policyName = policy.name;
if (!templateName) {
this.setState({ templateError: i18n.translate(
'xpack.indexLifecycleMgmt.policyTable.addLifecyclePolicyConfirmModal.noTemplateSelectedErrorMessage',
'xpack.indexLifecycleMgmt.policyTable.addLifecyclePolicyToTemplateConfirmModal.noTemplateSelectedErrorMessage',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@maryia-lapata Please note I've changed a lot of the IDs in this file. Will this impact the translations?

Copy link
Contributor

Choose a reason for hiding this comment

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

Since all xpack.indexLifecycleMgmt message were not sent to translation vendors, so there is no impact.

values: { pluginName }
})
message: i18n.translate(
'xpack.indexLifecycleMgmt.checkLicense.errorUnavailableMessage',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The IDs in this file have also changed.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@maryia-lapata maryia-lapata left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -14,7 +16,13 @@ export function checkLicense(xpackLicenseInfo) {
isAvailable: false,
showLinks: true,
enableLinks: false,
message: `You cannot use ${pluginName} because license information is not available at this time.`,
message: i18n.translate(
Copy link
Contributor

Choose a reason for hiding this comment

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

Error messages from this file are not displayed on UI. Ordinary we don't translate such labels, but maybe these labels will be displayed on UI in the future...

@@ -37,7 +37,7 @@ export class AddPolicyToTemplateConfirmModalUi extends Component {
const policyName = policy.name;
if (!templateName) {
this.setState({ templateError: i18n.translate(
'xpack.indexLifecycleMgmt.policyTable.addLifecyclePolicyConfirmModal.noTemplateSelectedErrorMessage',
'xpack.indexLifecycleMgmt.policyTable.addLifecyclePolicyToTemplateConfirmModal.noTemplateSelectedErrorMessage',
Copy link
Contributor

Choose a reason for hiding this comment

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

Since all xpack.indexLifecycleMgmt message were not sent to translation vendors, so there is no impact.

{numSelected > 1 ? 'ies' : 'y'}
<FormattedMessage
id="xpack.indexLifecycleMgmt.policyTable.deletedPoliciesText"
defaultMessage="Deleted {numSelected} {numSelected, plural, one {policy} other {policies}}"
Copy link
Contributor

Choose a reason for hiding this comment

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

The sorter version:

Suggested change
defaultMessage="Deleted {numSelected} {numSelected, plural, one {policy} other {policies}}"
defaultMessage="Deleted {numSelected, plural, one {# policy} other {# policies}}"

# means the number before plural

@maryia-lapata
Copy link
Contributor

@cj we'll send the next bunch of labels to vendors on 24 January at 8 AM (GMT+3). It would be most appreciated, if this PR is merged and backported before this time. Please do not worry about changed IDs. Since xpack.indexLifecycleMgmt labels haven't got in json file earlier(because this plugin hasn't been configured correct), vendors will receive this plugin to translate for the first time.

Copy link
Contributor

@bmcconaghy bmcconaghy left a comment

Choose a reason for hiding this comment

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

LGTM

@cjcenizal cjcenizal merged commit bfa417c into elastic:master Jan 24, 2019
@cjcenizal cjcenizal deleted the i18n/localize-strings branch January 24, 2019 00:36
cjcenizal added a commit to cjcenizal/kibana that referenced this pull request Jan 24, 2019
* Add ILM to i18nrc.json. Fix validation errors.
* Localize CCR and Rollups checkLicense errors.
* Fix bug in Remote Clusters checkLicense error.
* Use rollupJobs namespace in checkLicense.
azasypkin pushed a commit that referenced this pull request Jan 24, 2019
* Add ILM to i18nrc.json. Fix validation errors.
* Localize CCR and Rollups checkLicense errors.
* Fix bug in Remote Clusters checkLicense error.
* Use rollupJobs namespace in checkLicense.
cjcenizal added a commit that referenced this pull request Jan 24, 2019
* Add ILM to i18nrc.json. Fix validation errors.
* Localize CCR and Rollups checkLicense errors.
* Fix bug in Remote Clusters checkLicense error.
* Use rollupJobs namespace in checkLicense.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:ILM Project:i18n Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v6.7.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants