github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-24 01:38:56 -04:00
Code Issues Projects Releases Packages Wiki Activity

[Enterprise Search] Fix edge case UI issues in Role Mapping flyouts (#101436)

Browse source 
* Add invalidation to Attribute Value field when empty

Also added some missed i18n strings for the form row labels

* Disable forms if attribute value is invalid

* Move error from saving role mapping to inline form error

Flash message was rendering behind flyover.
*Best to view this commit with whitespace changes hidden

* Fix i18n

Copy/Paste FTW

* Attempt at fixing lint issue

My local linter seems to be broken

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Scotty Bollinger 2021-06-07 12:13:02 -05:00 • committed by GitHub
parent 065e378c8f
commit 1e3f916cd9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 223 additions and 104 deletions
Download patch file Download diff file Expand all files Collapse all files

15
x-pack/plugins/enterprise_search/public/applications/app_search/components/role_mappings/role_mapping.test.tsx
View file

@ -17,7 +17,7 @@ import { shallow } from 'enzyme';
import { EuiComboBox, EuiComboBoxOptionOption, EuiRadioGroup } from '@elastic/eui';
import { AttributeSelector, RoleSelector } from '../../../shared/role_mapping';
import { AttributeSelector, RoleSelector, RoleMappingFlyout } from '../../../shared/role_mapping';
import { asRoleMapping } from '../../../shared/role_mapping/__mocks__/roles';
import { STANDARD_ROLE_TYPES } from './constants';
@ -58,6 +58,7 @@ describe('RoleMapping', () => {
myRole: {
availableRoleTypes: mockRole.ability.availableRoleTypes,
},
roleMappingErrors: [],
};
beforeEach(() => {
@ -106,4 +107,16 @@ describe('RoleMapping', () => {
expect(actions.handleEngineSelectionChange).toHaveBeenCalledWith([engines[0].name]);
});
it('enables flyout when attribute value is valid', () => {
setMockValues({
...mockValues,
attributeValue: 'foo',
attributeName: 'role',
accessAllEngines: true,
});
const wrapper = shallow(<RoleMapping />);
expect(wrapper.find(RoleMappingFlyout).prop('disabled')).toBe(false);
});
});

112
x-pack/plugins/enterprise_search/public/applications/app_search/components/role_mappings/role_mapping.tsx
View file

@ -9,7 +9,14 @@ import React from 'react';
import { useActions, useValues } from 'kea';
import { EuiComboBox, EuiFormRow, EuiHorizontalRule, EuiRadioGroup, EuiSpacer } from '@elastic/eui';
import {
EuiComboBox,
EuiForm,
EuiFormRow,
EuiHorizontalRule,
EuiRadioGroup,
EuiSpacer,
} from '@elastic/eui';
import {
AttributeSelector,
@ -63,10 +70,12 @@ export const RoleMapping: React.FC = () => {
selectedEngines,
selectedAuthProviders,
selectedOptions,
roleMappingErrors,
} = useValues(RoleMappingsLogic);
const isNew = !roleMapping;
const hasEngineAssignment = selectedEngines.size > 0 || accessAllEngines;
const attributeValueInvalid = attributeName !== 'role' && !attributeValue;
const mapRoleOptions = ({ id, description }: AdvanceRoleType) => ({
id,
@ -99,60 +108,63 @@ export const RoleMapping: React.FC = () => {
return (
<RoleMappingFlyout
disabled={!hasEngineAssignment}
disabled={attributeValueInvalid || !hasEngineAssignment}
isNew={isNew}
closeRoleMappingFlyout={closeRoleMappingFlyout}
handleSaveMapping={handleSaveMapping}
>
<AttributeSelector
attributeName={attributeName}
attributeValue={attributeValue}
attributes={attributes}
availableAuthProviders={availableAuthProviders}
elasticsearchRoles={elasticsearchRoles}
selectedAuthProviders={selectedAuthProviders}
disabled={!!roleMapping}
handleAttributeSelectorChange={handleAttributeSelectorChange}
handleAttributeValueChange={handleAttributeValueChange}
handleAuthProviderChange={handleAuthProviderChange}
multipleAuthProvidersConfig={multipleAuthProvidersConfig}
/>
<EuiSpacer size="m" />
<RoleSelector
roleType={roleType}
roleOptions={roleOptions}
onChange={handleRoleChange}
label="Role"
/>
<EuiForm isInvalid={roleMappingErrors.length > 0} error={roleMappingErrors}>
<AttributeSelector
attributeName={attributeName}
attributeValue={attributeValue}
attributeValueInvalid={attributeValueInvalid}
attributes={attributes}
availableAuthProviders={availableAuthProviders}
elasticsearchRoles={elasticsearchRoles}
selectedAuthProviders={selectedAuthProviders}
disabled={!!roleMapping}
handleAttributeSelectorChange={handleAttributeSelectorChange}
handleAttributeValueChange={handleAttributeValueChange}
handleAuthProviderChange={handleAuthProviderChange}
multipleAuthProvidersConfig={multipleAuthProvidersConfig}
/>
<EuiSpacer size="m" />
<RoleSelector
roleType={roleType}
roleOptions={roleOptions}
onChange={handleRoleChange}
label="Role"
/>
{hasAdvancedRoles && (
<>
<EuiHorizontalRule />
<EuiFormRow>
<EuiRadioGroup
options={engineOptions}
disabled={!roleHasScopedEngines(roleType)}
idSelected={accessAllEngines ? 'all' : 'specific'}
onChange={(id) => handleAccessAllEnginesChange(id === 'all')}
legend={{
children: <span>{ENGINE_ASSIGNMENT_LABEL}</span>,
}}
/>
</EuiFormRow>
<EuiFormRow isInvalid={!hasEngineAssignment} error={[ENGINE_REQUIRED_ERROR]}>
<EuiComboBox
data-test-subj="enginesSelect"
selectedOptions={selectedOptions}
options={availableEngines.map(({ name }) => ({ label: name, value: name }))}
onChange={(options) => {
handleEngineSelectionChange(options.map(({ value }) => value as string));
}}
fullWidth
isDisabled={accessAllEngines || !roleHasScopedEngines(roleType)}
/>
</EuiFormRow>
</>
)}
{hasAdvancedRoles && (
<>
<EuiHorizontalRule />
<EuiFormRow>
<EuiRadioGroup
options={engineOptions}
disabled={!roleHasScopedEngines(roleType)}
idSelected={accessAllEngines ? 'all' : 'specific'}
onChange={(id) => handleAccessAllEnginesChange(id === 'all')}
legend={{
children: <span>{ENGINE_ASSIGNMENT_LABEL}</span>,
}}
/>
</EuiFormRow>
<EuiFormRow isInvalid={!hasEngineAssignment} error={[ENGINE_REQUIRED_ERROR]}>
<EuiComboBox
data-test-subj="enginesSelect"
selectedOptions={selectedOptions}
options={availableEngines.map(({ name }) => ({ label: name, value: name }))}
onChange={(options) => {
handleEngineSelectionChange(options.map(({ value }) => value as string));
}}
fullWidth
isDisabled={accessAllEngines || !roleHasScopedEngines(roleType)}
/>
</EuiFormRow>
</>
)}
</EuiForm>
</RoleMappingFlyout>
);
};

18
x-pack/plugins/enterprise_search/public/applications/app_search/components/role_mappings/role_mappings_logic.test.ts
View file

@ -44,6 +44,7 @@ describe('RoleMappingsLogic', () => {
accessAllEngines: true,
selectedAuthProviders: [ANY_AUTH_PROVIDER],
selectedOptions: [],
roleMappingErrors: [],
};
const mappingsServerProps = { multipleAuthProvidersConfig: true, roleMappings: [asRoleMapping] };
@ -410,11 +411,24 @@ describe('RoleMappingsLogic', () => {
});
it('handles error', async () => {
http.post.mockReturnValue(Promise.reject('this is an error'));
const setRoleMappingErrorsSpy = jest.spyOn(
RoleMappingsLogic.actions,
'setRoleMappingErrors'
);
http.post.mockReturnValue(
Promise.reject({
body: {
attributes: {
errors: ['this is an error'],
},
},
})
);
RoleMappingsLogic.actions.handleSaveMapping();
await nextTick();
expect(flashAPIErrors).toHaveBeenCalledWith('this is an error');
expect(setRoleMappingErrorsSpy).toHaveBeenCalledWith(['this is an error']);
});
});

13
x-pack/plugins/enterprise_search/public/applications/app_search/components/role_mappings/role_mappings_logic.ts
View file

@ -68,6 +68,7 @@ interface RoleMappingsActions {
setRoleMappingsData(data: RoleMappingsServerDetails): RoleMappingsServerDetails;
openRoleMappingFlyout(): void;
closeRoleMappingFlyout(): void;
setRoleMappingErrors(errors: string[]): { errors: string[] };
}
interface RoleMappingsValues {
@ -88,6 +89,7 @@ interface RoleMappingsValues {
selectedEngines: Set<string>;
roleMappingFlyoutOpen: boolean;
selectedOptions: EuiComboBoxOptionOption[];
roleMappingErrors: string[];
}
export const RoleMappingsLogic = kea<MakeLogicType<RoleMappingsValues, RoleMappingsActions>>({
@ -95,6 +97,7 @@ export const RoleMappingsLogic = kea<MakeLogicType<RoleMappingsValues, RoleMappi
actions: {
setRoleMappingsData: (data: RoleMappingsServerDetails) => data,
setRoleMappingData: (data: RoleMappingServerDetails) => data,
setRoleMappingErrors: (errors: string[]) => ({ errors }),
handleAuthProviderChange: (value: string) => ({ value }),
handleRoleChange: (roleType: RoleTypes) => ({ roleType }),
handleEngineSelectionChange: (engineNames: string[]) => ({ engineNames }),
@ -256,6 +259,14 @@ export const RoleMappingsLogic = kea<MakeLogicType<RoleMappingsValues, RoleMappi
initializeRoleMapping: () => true,
},
],
roleMappingErrors: [
[],
{
setRoleMappingErrors: (_, { errors }) => errors,
handleSaveMapping: () => [],
closeRoleMappingFlyout: () => [],
},
],
},
selectors: ({ selectors }) => ({
selectedOptions: [
@ -347,7 +358,7 @@ export const RoleMappingsLogic = kea<MakeLogicType<RoleMappingsValues, RoleMappi
actions.initializeRoleMappings();
setSuccessMessage(SUCCESS_MESSAGE);
} catch (e) {
flashAPIErrors(e);
actions.setRoleMappingErrors(e?.body?.attributes?.errors);
}
},
resetState: () => {

1
x-pack/plugins/enterprise_search/public/applications/shared/role_mapping/attribute_selector.test.tsx
View file

@ -23,6 +23,7 @@ const handleAuthProviderChange = jest.fn();
const baseProps = {
attributeName: 'username' as AttributeName,
attributeValue: 'Something',
attributeValueInvalid: false,
attributes: ['a', 'b', 'c'],
availableAuthProviders: ['ees_saml', 'kbn_saml'],
selectedAuthProviders: ['ees_saml'],

17
x-pack/plugins/enterprise_search/public/applications/shared/role_mapping/attribute_selector.tsx
View file

@ -20,13 +20,18 @@ import { AttributeName, AttributeExamples } from '../types';
import {
ANY_AUTH_PROVIDER,
ANY_AUTH_PROVIDER_OPTION_LABEL,
ATTRIBUTE_VALUE_LABEL,
ATTRIBUTE_VALUE_ERROR,
AUTH_ANY_PROVIDER_LABEL,
AUTH_INDIVIDUAL_PROVIDER_LABEL,
AUTH_PROVIDER_LABEL,
EXTERNAL_ATTRIBUTE_LABEL,
} from './constants';
interface Props {
attributeName: AttributeName;
attributeValue?: string;
attributeValueInvalid: boolean;
attributes: string[];
selectedAuthProviders?: string[];
availableAuthProviders?: string[];
@ -80,6 +85,7 @@ const getSelectedOptions = (selectedAuthProviders: string[], availableAuthProvid
export const AttributeSelector: React.FC<Props> = ({
attributeName,
attributeValue = '',
attributeValueInvalid,
attributes,
availableAuthProviders,
selectedAuthProviders = [ANY_AUTH_PROVIDER],
@ -93,7 +99,7 @@ export const AttributeSelector: React.FC<Props> = ({
return (
<div data-test-subj="AttributeSelector">
{availableAuthProviders && multipleAuthProvidersConfig && (
<EuiFormRow label="Auth Provider" fullWidth>
<EuiFormRow label={AUTH_PROVIDER_LABEL} fullWidth>
<EuiComboBox
data-test-subj="AuthProviderSelect"
selectedOptions={getSelectedOptions(selectedAuthProviders, availableAuthProviders)}
@ -106,7 +112,7 @@ export const AttributeSelector: React.FC<Props> = ({
/>
</EuiFormRow>
)}
<EuiFormRow label="External Attribute" fullWidth>
<EuiFormRow label={EXTERNAL_ATTRIBUTE_LABEL} fullWidth>
<EuiSelect
name="external-attribute"
data-test-subj="ExternalAttributeSelect"
@ -120,7 +126,12 @@ export const AttributeSelector: React.FC<Props> = ({
disabled={disabled}
/>
</EuiFormRow>
<EuiFormRow label="Attribute Value" fullWidth>
<EuiFormRow
label={ATTRIBUTE_VALUE_LABEL}
fullWidth
isInvalid={attributeValueInvalid}
error={[ATTRIBUTE_VALUE_ERROR]}
>
{attributeName === 'role' ? (
<EuiSelect
value={attributeValue}

7
x-pack/plugins/enterprise_search/public/applications/shared/role_mapping/constants.ts
View file

@ -73,6 +73,13 @@ export const ATTRIBUTE_VALUE_LABEL = i18n.translate(
}
);
export const ATTRIBUTE_VALUE_ERROR = i18n.translate(
'xpack.enterpriseSearch.roleMapping.attributeValueError',
{
defaultMessage: 'Attribute value is required',
}
);
export const DELETE_ROLE_MAPPING_TITLE = i18n.translate(
'xpack.enterpriseSearch.roleMapping.deleteRoleMappingTitle',
{

15
x-pack/plugins/enterprise_search/public/applications/workplace_search/views/role_mappings/role_mapping.test.tsx
View file

@ -15,7 +15,7 @@ import { shallow } from 'enzyme';
import { EuiComboBox, EuiComboBoxOptionOption, EuiRadioGroup } from '@elastic/eui';
import { AttributeSelector, RoleSelector } from '../../../shared/role_mapping';
import { AttributeSelector, RoleSelector, RoleMappingFlyout } from '../../../shared/role_mapping';
import { wsRoleMapping } from '../../../shared/role_mapping/__mocks__/roles';
import { RoleMapping } from './role_mapping';
@ -56,6 +56,7 @@ describe('RoleMapping', () => {
availableAuthProviders: [],
multipleAuthProvidersConfig: true,
selectedAuthProviders: [],
roleMappingErrors: [],
};
beforeEach(() => {
@ -109,4 +110,16 @@ describe('RoleMapping', () => {
expect(handleGroupSelectionChange).toHaveBeenCalledWith([groups[0].name]);
});
it('enables flyout when attribute value is valid', () => {
setMockValues({
...mockValues,
attributeValue: 'foo',
attributeName: 'role',
includeInAllGroups: true,
});
const wrapper = shallow(<RoleMapping />);
expect(wrapper.find(RoleMappingFlyout).prop('disabled')).toBe(false);
});
});

98
x-pack/plugins/enterprise_search/public/applications/workplace_search/views/role_mappings/role_mapping.tsx
View file

@ -9,7 +9,14 @@ import React from 'react';
import { useActions, useValues } from 'kea';
import { EuiComboBox, EuiFormRow, EuiHorizontalRule, EuiRadioGroup, EuiSpacer } from '@elastic/eui';
import {
EuiComboBox,
EuiForm,
EuiFormRow,
EuiHorizontalRule,
EuiRadioGroup,
EuiSpacer,
} from '@elastic/eui';
import {
AttributeSelector,
@ -88,62 +95,67 @@ export const RoleMapping: React.FC = () => {
selectedAuthProviders,
selectedOptions,
roleMapping,
roleMappingErrors,
} = useValues(RoleMappingsLogic);
const isNew = !roleMapping;
const hasGroupAssignment = selectedGroups.size > 0 || includeInAllGroups;
const attributeValueInvalid = attributeName !== 'role' && !attributeValue;
return (
<RoleMappingFlyout
disabled={!hasGroupAssignment}
disabled={attributeValueInvalid || !hasGroupAssignment}
isNew={isNew}
closeRoleMappingFlyout={closeRoleMappingFlyout}
handleSaveMapping={handleSaveMapping}
>
<AttributeSelector
attributeName={attributeName}
attributeValue={attributeValue}
attributes={attributes}
elasticsearchRoles={elasticsearchRoles}
disabled={!isNew}
handleAttributeSelectorChange={handleAttributeSelectorChange}
handleAttributeValueChange={handleAttributeValueChange}
availableAuthProviders={availableAuthProviders}
selectedAuthProviders={selectedAuthProviders}
multipleAuthProvidersConfig={multipleAuthProvidersConfig}
handleAuthProviderChange={handleAuthProviderChange}
/>
<EuiSpacer size="m" />
<RoleSelector
roleOptions={roleOptions}
roleType={roleType}
onChange={handleRoleChange}
label="Role"
/>
<EuiHorizontalRule />
<EuiFormRow>
<EuiRadioGroup
options={groupOptions}
idSelected={includeInAllGroups ? 'all' : 'specific'}
onChange={(id) => handleAllGroupsSelectionChange(id === 'all')}
legend={{
children: <span>{GROUP_ASSIGNMENT_LABEL}</span>,
}}
<EuiForm isInvalid={roleMappingErrors.length > 0} error={roleMappingErrors}>
<AttributeSelector
attributeName={attributeName}
attributeValue={attributeValue}
attributeValueInvalid={attributeValueInvalid}
attributes={attributes}
elasticsearchRoles={elasticsearchRoles}
disabled={!isNew}
handleAttributeSelectorChange={handleAttributeSelectorChange}
handleAttributeValueChange={handleAttributeValueChange}
availableAuthProviders={availableAuthProviders}
selectedAuthProviders={selectedAuthProviders}
multipleAuthProvidersConfig={multipleAuthProvidersConfig}
handleAuthProviderChange={handleAuthProviderChange}
/>
</EuiFormRow>
<EuiFormRow isInvalid={!hasGroupAssignment} error={[GROUP_ASSIGNMENT_INVALID_ERROR]}>
<EuiComboBox
data-test-subj="groupsSelect"
selectedOptions={selectedOptions}
options={availableGroups.map(({ name, id }) => ({ label: name, value: id }))}
onChange={(options) => {
handleGroupSelectionChange(options.map(({ value }) => value as string));
}}
fullWidth
isDisabled={includeInAllGroups}
<EuiSpacer size="m" />
<RoleSelector
roleOptions={roleOptions}
roleType={roleType}
onChange={handleRoleChange}
label="Role"
/>
</EuiFormRow>
<EuiHorizontalRule />
<EuiFormRow>
<EuiRadioGroup
options={groupOptions}
idSelected={includeInAllGroups ? 'all' : 'specific'}
onChange={(id) => handleAllGroupsSelectionChange(id === 'all')}
legend={{
children: <span>{GROUP_ASSIGNMENT_LABEL}</span>,
}}
/>
</EuiFormRow>
<EuiFormRow isInvalid={!hasGroupAssignment} error={[GROUP_ASSIGNMENT_INVALID_ERROR]}>
<EuiComboBox
data-test-subj="groupsSelect"
selectedOptions={selectedOptions}
options={availableGroups.map(({ name, id }) => ({ label: name, value: id }))}
onChange={(options) => {
handleGroupSelectionChange(options.map(({ value }) => value as string));
}}
fullWidth
isDisabled={includeInAllGroups}
/>
</EuiFormRow>
</EuiForm>
</RoleMappingFlyout>
);
};

18
x-pack/plugins/enterprise_search/public/applications/workplace_search/views/role_mappings/role_mappings_logic.test.ts
View file

@ -38,6 +38,7 @@ describe('RoleMappingsLogic', () => {
includeInAllGroups: false,
selectedAuthProviders: [ANY_AUTH_PROVIDER],
selectedOptions: [],
roleMappingErrors: [],
};
const roleGroup = {
id: '123',
@ -365,11 +366,24 @@ describe('RoleMappingsLogic', () => {
});
it('handles error', async () => {
http.post.mockReturnValue(Promise.reject('this is an error'));
const setRoleMappingErrorsSpy = jest.spyOn(
RoleMappingsLogic.actions,
'setRoleMappingErrors'
);
http.post.mockReturnValue(
Promise.reject({
body: {
attributes: {
errors: ['this is an error'],
},
},
})
);
RoleMappingsLogic.actions.handleSaveMapping();
await nextTick();
expect(flashAPIErrors).toHaveBeenCalledWith('this is an error');
expect(setRoleMappingErrorsSpy).toHaveBeenCalledWith(['this is an error']);
});
});

13
x-pack/plugins/enterprise_search/public/applications/workplace_search/views/role_mappings/role_mappings_logic.ts
View file

@ -66,6 +66,7 @@ interface RoleMappingsActions {
setRoleMappingsData(data: RoleMappingsServerDetails): RoleMappingsServerDetails;
openRoleMappingFlyout(): void;
closeRoleMappingFlyout(): void;
setRoleMappingErrors(errors: string[]): { errors: string[] };
}
interface RoleMappingsValues {
@ -85,6 +86,7 @@ interface RoleMappingsValues {
selectedGroups: Set<string>;
roleMappingFlyoutOpen: boolean;
selectedOptions: EuiComboBoxOptionOption[];
roleMappingErrors: string[];
}
export const RoleMappingsLogic = kea<MakeLogicType<RoleMappingsValues, RoleMappingsActions>>({
@ -92,6 +94,7 @@ export const RoleMappingsLogic = kea<MakeLogicType<RoleMappingsValues, RoleMappi
actions: {
setRoleMappingsData: (data: RoleMappingsServerDetails) => data,
setRoleMappingData: (data: RoleMappingServerDetails) => data,
setRoleMappingErrors: (errors: string[]) => ({ errors }),
handleAuthProviderChange: (value: string[]) => ({ value }),
handleRoleChange: (roleType: Role) => ({ roleType }),
handleGroupSelectionChange: (groupIds: string[]) => ({ groupIds }),
@ -247,6 +250,14 @@ export const RoleMappingsLogic = kea<MakeLogicType<RoleMappingsValues, RoleMappi
initializeRoleMapping: () => true,
},
],
roleMappingErrors: [
[],
{
setRoleMappingErrors: (_, { errors }) => errors,
handleSaveMapping: () => [],
closeRoleMappingFlyout: () => [],
},
],
},
selectors: ({ selectors }) => ({
selectedOptions: [
@ -337,7 +348,7 @@ export const RoleMappingsLogic = kea<MakeLogicType<RoleMappingsValues, RoleMappi
actions.initializeRoleMappings();
setSuccessMessage(SUCCESS_MESSAGE);
} catch (e) {
flashAPIErrors(e);
actions.setRoleMappingErrors(e?.body?.attributes?.errors);
}
},
resetState: () => {