Skip to content

Translate security/roles component#23984

Merged
pavel06081991 merged 35 commits intoelastic:masterfrom
tibmt:feature/translations/security/roles
Nov 20, 2018
Merged

Translate security/roles component#23984
pavel06081991 merged 35 commits intoelastic:masterfrom
tibmt:feature/translations/security/roles

Conversation

@tibmt
Copy link
Copy Markdown
Contributor

@tibmt tibmt commented Oct 12, 2018

Translation of Security Roles visualization components

@tibmt tibmt self-assigned this Oct 12, 2018
@tibmt tibmt requested a review from pavel06081991 October 12, 2018 10:49
@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@tibmt
Copy link
Copy Markdown
Contributor Author

tibmt commented Oct 12, 2018

retest

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

<EuiLink onClick={this.toggleCollapsed}>{this.state.collapsed ? 'show' : 'hide'}</EuiLink>
<EuiLink onClick={this.toggleCollapsed}>
{this.state.collapsed
? intl.formatMessage({
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can use FormattedMessage here. So you do not need to use injectI18n

{this.state.collapsed
? intl.formatMessage({
id:
'xpack.security.views.management.editRoles.components.collapsiblePanel.showTitle',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

showTitle => showLinkText

})
: intl.formatMessage({
id:
'xpack.security.views.management.editRoles.components.collapsiblePanel.hideTitle',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

hideTitle => hideLinkText

<EuiOverlayMask>
<EuiConfirmModal
title={'Delete Role'}
title={intl.formatMessage({
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Check if title property supports JSX. I mean try to use FormattedMessage. If it works, then do not use injectI18n

<EuiButtonEmpty color={'danger'} onClick={this.showModal}>
Delete role
<FormattedMessage
id="xpack.security.views.management.editRoles.components.deleteRoleButton.deleteRoleButton"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

=> deleteRoleButtonLabel

i18n('xpack.security.views.management.roles.deleteRoleTitle', {
values: {
valueText: $scope.selectedRoles.length > 1 ?
i18n('xpack.security.views.management.roles.deleteRoleRolesTitle', { defaultMessage: 'roles' })
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

deleteRoleRolesTitle => rolesSuccessfullyDeletedNotificationMessage

.then(() => toastNotifications.addSuccess(
i18n('xpack.security.views.management.roles.deleteRoleTitle', {
values: {
valueText: $scope.selectedRoles.length > 1 ?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use plural from ICU format for such cases. Come to me and I'll show you how to do it

};
const confirmModalOptions = {
confirmButtonText: 'Delete role(s)',
confirmButtonText: i18n('xpack.security.views.management.roles.deleteRoleButton', { defaultMessage: 'Delete role(s)' }),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

deleteRoleButton => deleteRoleConfirmButtonLabel

confirmModalOptions
confirmModal(
i18n('xpack.security.views.management.roles.sureToDeleteRoleTitle', {
defaultMessage: 'Are you sure you want to delete the selected role(s)? This action is irreversible!'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sureToDeleteRoleTitle => deletingRolesWarningMessage

return $scope.roles.filter((role) => !role.metadata._reserved);
}

$scope.noFoundMatches = i18n('xpack.security.views.management.roles.noMatches', {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

noMatches => matchingText

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

this.props.editable
? intl.formatMessage({
id:
'xpack.security.views.management.editRoles.components.privileges.es.elasticSearchPrivileges.addUserTitle',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

addUserTitle => addUserPlaceholder

This role always grants read access to all spaces. To customize privileges for
individual spaces, you must create a new role.
<FormattedMessage
id="xpack.security.views.management.editRoles.components.privileges.kibana.privilegeCalloutWarning.alwaysGrantReadRoleTitle"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not fixed

This role never grants access to any spaces within Kibana. To customize privileges for
individual spaces, you must create a new role.
<FormattedMessage
id="xpack.security.views.management.editRoles.components.privileges.kibana.privilegeCalloutWarning.roleNeverAccessTitle"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not fixed

class="kuiTableHeaderCell__liner"
i18n-id="xpack.security.views.management.roles.roleTitle"
i18n-default-message="Role {icon}"
i18n-values="{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

after #23684 is merged we will need to name properties which contain angular directives with unsafe_ prefix. So please check all places in this MR where you use i18n-values and if property contains directives - use prefix unsafe_.
for example icon => unsafe_icon

.then(() =>
toastNotifications.addSuccess(
i18n('xpack.security.views.management.roles.deleteRoleTitle', {
defaultMessage: 'Deleted {value, plural, one {# role} other {# roles}}',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove #, just {role} other {roles}

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

Copy link
Copy Markdown
Contributor

@pavel06081991 pavel06081991 left a comment

Choose a reason for hiding this comment

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

LGTM

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

helptext = 'View, edit, and share objects and apps within all spaces';
helptext = intl.formatMessage({
id:
'xpack.security.views.management.editRoles.components.privileges.kibana.spaceAwarePrivilegeForm.viewEditShareAppsWithinAllSpacesHelpText',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@azasypkin it'd be neat if the first part of these keys could be inferred by the component tree context so you could do something like:

intl.formatMessage({
  id: `spaceAwarePrivilegeForm.viewEditShareAppsWithinAllSpacesHelpText`
  defaultMessage: '...'
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

xpack.security.views.management.editRoles.components.privileges.kibana.spaceAwarePrivilegeForm.viewEditShareAppsWithinAllSpacesHelpText

Ugh, I believe it's a bad id: it's too detailed and contains a bunch of useless info (views?, editRoles implies both views and management, components is useless and doesn't say anything, privilges.kibana - seems to be useless since we have spaceAwarePrivilegeForm), we should have more laconic and useful ids. Why not just something like this?

Rule: {plugin}.{area}.[{sub-area}].{element}

Example: {xpack.security}.{editRoles}.{spaceAwarePrivilegeForm}.{viewEditShareAppsWithinAllSpacesHelpText}

I doubt that we'll have collision with this shorter form. @pavel06081991 @maryia-lapata thoughts?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yeah, agree. we will use it for all new ids

Copy link
Copy Markdown
Contributor

@joshdover joshdover left a comment

Choose a reason for hiding this comment

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

LGTM after removing these lines

return valid();
}
return invalid('Privilege is required');
// return invalid('Privilege is required');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove?

if (!this.props.readonly) {
columns.push({
name: 'Actions',
// name: 'Actions',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove?

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@pavel06081991
Copy link
Copy Markdown
Contributor

retest

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@pavel06081991
Copy link
Copy Markdown
Contributor

retest

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@pavel06081991
Copy link
Copy Markdown
Contributor

retest

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@pavel06081991 pavel06081991 merged commit cf64825 into elastic:master Nov 20, 2018
pavel06081991 pushed a commit to pavel06081991/kibana that referenced this pull request Nov 20, 2018
Translate security/roles component
pavel06081991 added a commit that referenced this pull request Nov 20, 2018
Translate security/roles component
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants