Skip to content

Commit

Permalink
[Enterprise Search] Refactor Role mappings to use single endpoint (#1…
Browse files Browse the repository at this point in the history 
…02096)

* Add tooltip back to table row

This was missed when refactoring the table to an EUI component. Built-in mappings have tooltips and don’t have IDs and need to show tooltips instead of actions.

* Fix roleType display

Also missed in the refactor. Made a mistake when copying/pasting

* Refactor logic files to use single endpoint for UI props

As a part of this feature, we are now passing all props needed for the UI in the list endpoint. Previously, whether creating a new mapping, or updating an existing mapping, an endpoint had to be called to fetch the data needed for display. Now all this data comes from the initial fetching of mappings and the other endpoints are no longer needed.

* Refactor WS test to match AS

There was an issue where 100% test coverage was not achieved in Workplace Search. This had already been fixed in App Search by refactoring. This changes the code and test in Workplace Search to match

* Remove server routes
  • Loading branch information
@scottybollinger
scottybollinger committed Jun 15, 2021
1 parent 7dfe203 commit 42aa7f5
Show file tree
Hide file tree
Showing 9 changed files with 191 additions and 472 deletions.
178 changes: 71 additions & 107 deletions ...earch/public/applications/app_search/components/role_mappings/role_mappings_logic.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,13 @@ import { engines } from '../../__mocks__/engines.mock';
import { nextTick } from '@kbn/test/jest';

import { asRoleMapping } from '../../../shared/role_mapping/__mocks__/roles';
import { ANY_AUTH_PROVIDER, ROLE_MAPPING_NOT_FOUND } from '../../../shared/role_mapping/constants';
import { ANY_AUTH_PROVIDER } from '../../../shared/role_mapping/constants';

import { RoleMappingsLogic } from './role_mappings_logic';

describe('RoleMappingsLogic', () => {
const { http } = mockHttpValues;
const {
clearFlashMessages,
flashAPIErrors,
setSuccessMessage,
setErrorMessage,
} = mockFlashMessageHelpers;
const { clearFlashMessages, flashAPIErrors, setSuccessMessage } = mockFlashMessageHelpers;
const { mount } = new LogicMounter(RoleMappingsLogic);
const DEFAULT_VALUES = {
attributes: [],
Expand All @@ -50,15 +45,14 @@ describe('RoleMappingsLogic', () => {
roleMappingErrors: [],
};

const mappingsServerProps = { multipleAuthProvidersConfig: true, roleMappings: [asRoleMapping] };
const mappingServerProps = {
const mappingsServerProps = {
multipleAuthProvidersConfig: true,
roleMappings: [asRoleMapping],
attributes: ['email', 'metadata', 'username', 'role'],
authProviders: [ANY_AUTH_PROVIDER],
availableEngines: engines,
elasticsearchRoles: [],
hasAdvancedRoles: false,
multipleAuthProvidersConfig: false,
roleMapping: asRoleMapping,
};

beforeEach(() => {
Expand All @@ -75,48 +69,20 @@ describe('RoleMappingsLogic', () => {
it('sets data based on server response from the `mappings` (plural) endpoint', () => {
RoleMappingsLogic.actions.setRoleMappingsData(mappingsServerProps);

expect(RoleMappingsLogic.values.roleMappings).toEqual([asRoleMapping]);
expect(RoleMappingsLogic.values.dataLoading).toEqual(false);
expect(RoleMappingsLogic.values.multipleAuthProvidersConfig).toEqual(true);
});
});

describe('setRoleMappingData', () => {
it('sets state based on server response from the `mapping` (singular) endpoint', () => {
RoleMappingsLogic.actions.setRoleMappingData(mappingServerProps);

expect(RoleMappingsLogic.values).toEqual({
...DEFAULT_VALUES,
roleMapping: asRoleMapping,
roleMappings: [asRoleMapping],
dataLoading: false,
attributes: mappingServerProps.attributes,
availableAuthProviders: mappingServerProps.authProviders,
availableEngines: mappingServerProps.availableEngines,
attributes: mappingsServerProps.attributes,
availableAuthProviders: mappingsServerProps.authProviders,
availableEngines: mappingsServerProps.availableEngines,
accessAllEngines: true,
attributeName: 'role',
attributeValue: 'superuser',
elasticsearchRoles: mappingServerProps.elasticsearchRoles,
selectedEngines: new Set(engines.map((e) => e.name)),
selectedOptions: [
{ label: engines[0].name, value: engines[0].name },
{ label: engines[1].name, value: engines[1].name },
],
});
});

it('will remove all selected engines if no roleMapping was returned from the server', () => {
RoleMappingsLogic.actions.setRoleMappingData({
...mappingServerProps,
roleMapping: undefined,
});

expect(RoleMappingsLogic.values).toEqual({
...DEFAULT_VALUES,
dataLoading: false,
multipleAuthProvidersConfig: true,
attributeName: 'username',
attributeValue: '',
elasticsearchRoles: mappingsServerProps.elasticsearchRoles,
selectedEngines: new Set(),
attributes: mappingServerProps.attributes,
availableAuthProviders: mappingServerProps.authProviders,
availableEngines: mappingServerProps.availableEngines,
selectedOptions: [],
});
});
});
Expand All @@ -135,11 +101,13 @@ describe('RoleMappingsLogic', () => {
const engine = engines[0];
const otherEngine = engines[1];
const mountedValues = {
...mappingServerProps,
roleMapping: {
...asRoleMapping,
engines: [engine, otherEngine],
},
...mappingsServerProps,
roleMappings: [
{
...asRoleMapping,
engines: [engine, otherEngine],
},
],
selectedEngines: new Set([engine.name]),
};

Expand All @@ -153,11 +121,18 @@ describe('RoleMappingsLogic', () => {
expect(RoleMappingsLogic.values.selectedEngines).toEqual(
new Set([engine.name, otherEngine.name])
);
expect(RoleMappingsLogic.values.selectedOptions).toEqual([
{ label: engine.name, value: engine.name },
{ label: otherEngine.name, value: otherEngine.name },
]);
});
it('handles removing an engine from selected engines', () => {
RoleMappingsLogic.actions.handleEngineSelectionChange([engine.name]);

expect(RoleMappingsLogic.values.selectedEngines).toEqual(new Set([engine.name]));
expect(RoleMappingsLogic.values.selectedOptions).toEqual([
{ label: engine.name, value: engine.name },
]);
});
});

Expand All @@ -175,17 +150,19 @@ describe('RoleMappingsLogic', () => {

it('sets values correctly', () => {
mount({
...mappingServerProps,
...mappingsServerProps,
elasticsearchRoles,
});
RoleMappingsLogic.actions.handleAttributeSelectorChange('role', elasticsearchRoles[0]);

expect(RoleMappingsLogic.values).toEqual({
...DEFAULT_VALUES,
multipleAuthProvidersConfig: true,
attributeValue: elasticsearchRoles[0],
roleMapping: asRoleMapping,
attributes: mappingServerProps.attributes,
availableEngines: mappingServerProps.availableEngines,
roleMappings: [asRoleMapping],
roleMapping: null,
attributes: mappingsServerProps.attributes,
availableEngines: mappingsServerProps.availableEngines,
accessAllEngines: true,
attributeName: 'role',
elasticsearchRoles,
Expand Down Expand Up @@ -215,11 +192,13 @@ describe('RoleMappingsLogic', () => {
describe('handleAuthProviderChange', () => {
beforeEach(() => {
mount({
...mappingServerProps,
roleMapping: {
...asRoleMapping,
authProvider: ['foo'],
},
...mappingsServerProps,
roleMappings: [
{
...asRoleMapping,
authProvider: ['foo'],
},
],
});
});
const providers = ['bar', 'baz'];
Expand All @@ -244,11 +223,13 @@ describe('RoleMappingsLogic', () => {

it('handles "any" auth in previous state', () => {
mount({
...mappingServerProps,
roleMapping: {
...asRoleMapping,
authProvider: [ANY_AUTH_PROVIDER],
},
...mappingsServerProps,
roleMappings: [
{
...asRoleMapping,
authProvider: [ANY_AUTH_PROVIDER],
},
],
});
RoleMappingsLogic.actions.handleAuthProviderChange(providerWithAny);

Expand All @@ -258,15 +239,14 @@ describe('RoleMappingsLogic', () => {

it('resetState', () => {
mount(mappingsServerProps);
mount(mappingServerProps);
RoleMappingsLogic.actions.resetState();

expect(RoleMappingsLogic.values).toEqual(DEFAULT_VALUES);
expect(clearFlashMessages).toHaveBeenCalled();
});

it('openRoleMappingFlyout', () => {
mount(mappingServerProps);
mount(mappingsServerProps);
RoleMappingsLogic.actions.openRoleMappingFlyout();

expect(RoleMappingsLogic.values.roleMappingFlyoutOpen).toEqual(true);
Expand All @@ -275,7 +255,7 @@ describe('RoleMappingsLogic', () => {

it('closeRoleMappingFlyout', () => {
mount({
...mappingServerProps,
...mappingsServerProps,
roleMappingFlyoutOpen: true,
});
RoleMappingsLogic.actions.closeRoleMappingFlyout();
Expand Down Expand Up @@ -307,40 +287,20 @@ describe('RoleMappingsLogic', () => {
});

describe('initializeRoleMapping', () => {
it('calls API and sets values for new mapping', async () => {
const setRoleMappingDataSpy = jest.spyOn(RoleMappingsLogic.actions, 'setRoleMappingData');
http.get.mockReturnValue(Promise.resolve(mappingServerProps));
RoleMappingsLogic.actions.initializeRoleMapping();

expect(http.get).toHaveBeenCalledWith('/api/app_search/role_mappings/new');
await nextTick();
expect(setRoleMappingDataSpy).toHaveBeenCalledWith(mappingServerProps);
});

it('calls API and sets values for existing mapping', async () => {
const setRoleMappingDataSpy = jest.spyOn(RoleMappingsLogic.actions, 'setRoleMappingData');
http.get.mockReturnValue(Promise.resolve(mappingServerProps));
RoleMappingsLogic.actions.initializeRoleMapping('123');

expect(http.get).toHaveBeenCalledWith('/api/app_search/role_mappings/123');
await nextTick();
expect(setRoleMappingDataSpy).toHaveBeenCalledWith(mappingServerProps);
});

it('handles error', async () => {
http.get.mockReturnValue(Promise.reject('this is an error'));
RoleMappingsLogic.actions.initializeRoleMapping();
await nextTick();
it('sets values for existing mapping', () => {
const setRoleMappingDataSpy = jest.spyOn(RoleMappingsLogic.actions, 'setRoleMapping');
RoleMappingsLogic.actions.setRoleMappingsData(mappingsServerProps);
RoleMappingsLogic.actions.initializeRoleMapping(asRoleMapping.id);

expect(flashAPIErrors).toHaveBeenCalledWith('this is an error');
expect(setRoleMappingDataSpy).toHaveBeenCalledWith(asRoleMapping);
});

it('shows error when there is a 404 status', async () => {
http.get.mockReturnValue(Promise.reject({ status: 404 }));
it('does not set data for new mapping', async () => {
const setRoleMappingDataSpy = jest.spyOn(RoleMappingsLogic.actions, 'setRoleMapping');
RoleMappingsLogic.actions.setRoleMappingsData(mappingsServerProps);
RoleMappingsLogic.actions.initializeRoleMapping();
await nextTick();

expect(setErrorMessage).toHaveBeenCalledWith(ROLE_MAPPING_NOT_FOUND);
expect(setRoleMappingDataSpy).not.toHaveBeenCalledWith(asRoleMapping);
});
});

Expand All @@ -362,7 +322,7 @@ describe('RoleMappingsLogic', () => {
'initializeRoleMappings'
);

http.post.mockReturnValue(Promise.resolve(mappingServerProps));
http.post.mockReturnValue(Promise.resolve(mappingsServerProps));
RoleMappingsLogic.actions.handleSaveMapping();

expect(http.post).toHaveBeenCalledWith('/api/app_search/role_mappings', {
Expand All @@ -374,13 +334,16 @@ describe('RoleMappingsLogic', () => {
});

it('calls API and refreshes list when existing mapping', async () => {
mount(mappingServerProps);
mount({
...mappingsServerProps,
roleMapping: asRoleMapping,
});
const initializeRoleMappingsSpy = jest.spyOn(
RoleMappingsLogic.actions,
'initializeRoleMappings'
);

http.put.mockReturnValue(Promise.resolve(mappingServerProps));
http.put.mockReturnValue(Promise.resolve(mappingsServerProps));
RoleMappingsLogic.actions.handleSaveMapping();

expect(http.put).toHaveBeenCalledWith(`/api/app_search/role_mappings/${asRoleMapping.id}`, {
Expand All @@ -396,12 +359,13 @@ describe('RoleMappingsLogic', () => {
const engine = engines[0];

mount({
...mappingServerProps,
...mappingsServerProps,
accessAllEngines: false,
selectedEngines: new Set([engine.name]),
roleMapping: asRoleMapping,
});

http.put.mockReturnValue(Promise.resolve(mappingServerProps));
http.put.mockReturnValue(Promise.resolve(mappingsServerProps));
RoleMappingsLogic.actions.handleSaveMapping();

expect(http.put).toHaveBeenCalledWith(`/api/app_search/role_mappings/${asRoleMapping.id}`, {
Expand Down Expand Up @@ -449,7 +413,7 @@ describe('RoleMappingsLogic', () => {
});

it('calls API and refreshes list', async () => {
mount(mappingServerProps);
mount(mappingsServerProps);
const initializeRoleMappingsSpy = jest.spyOn(
RoleMappingsLogic.actions,
'initializeRoleMappings'
Expand All @@ -465,7 +429,7 @@ describe('RoleMappingsLogic', () => {
});

it('handles error', async () => {
mount(mappingServerProps);
mount(mappingsServerProps);
http.delete.mockReturnValue(Promise.reject('this is an error'));
RoleMappingsLogic.actions.handleDeleteMapping(roleMappingId);
await nextTick();
Expand All @@ -474,7 +438,7 @@ describe('RoleMappingsLogic', () => {
});

it('will do nothing if not confirmed', () => {
mount(mappingServerProps);
mount(mappingsServerProps);
jest.spyOn(window, 'confirm').mockReturnValueOnce(false);
RoleMappingsLogic.actions.handleDeleteMapping(roleMappingId);

Expand Down
Loading

0 comments on commit 42aa7f5

Please sign in to comment.