[App Search] Refactor AppLogic to initialize data via props rather than action (#92841)
* [Misc cleanup] Move Access type to common - it was being duplicated in server/check_access and InitialAppData + add mock access data to DEFAULT_INITIAL_APP_DATA + update server/ tests to account for access in DEFAULT_INITIAL_APP_DATA * Update AppSearchConfigured to pass props to AppLogic vs calling initializeAppData + update tests to rerender a wrapper rather than doing {...DEFAULT_INITIAL_APP_DATA} repeatedly * Update AppLogic to set values from props rather than a listener - main goal of this PR is to prevent the flash of state between mount and initializeX being called - note: I recommend turning off whitespace changes in the test file * Update AppLogic typing so that app data is always expected - which it should be in any case in a production environment - note: I could have changed InitialAppData to remove the ? optional notation, but decided on this route instead since InitialAppData affects more than just App Search (e.g. server, WS), and I didn't want this to have potential far-reaching side effects * Update type scenarios where AppLogic values were previously thought potentially undefined - which is mostly just configuredLimits it looks like * [PR feedback] Type name
This commit is contained in:
parent
3f5473ef7d
commit
fd3b3eb8cd
|
@ -23,6 +23,10 @@ export const DEFAULT_INITIAL_APP_DATA = {
|
|||
},
|
||||
},
|
||||
},
|
||||
access: {
|
||||
hasAppSearchAccess: true,
|
||||
hasWorkplaceSearchAccess: true,
|
||||
},
|
||||
appSearch: {
|
||||
accountId: 'some-id-string',
|
||||
onboardingComplete: true,
|
||||
|
|
|
@ -19,10 +19,7 @@ export interface InitialAppData {
|
|||
ilmEnabled?: boolean;
|
||||
isFederatedAuth?: boolean;
|
||||
configuredLimits?: ConfiguredLimits;
|
||||
access?: {
|
||||
hasAppSearchAccess: boolean;
|
||||
hasWorkplaceSearchAccess: boolean;
|
||||
};
|
||||
access?: ProductAccess;
|
||||
appSearch?: AppSearchAccount;
|
||||
workplaceSearch?: WorkplaceSearchInitialData;
|
||||
}
|
||||
|
@ -32,6 +29,11 @@ export interface ConfiguredLimits {
|
|||
workplaceSearch: WorkplaceSearchConfiguredLimits;
|
||||
}
|
||||
|
||||
export interface ProductAccess {
|
||||
hasAppSearchAccess: boolean;
|
||||
hasWorkplaceSearchAccess: boolean;
|
||||
}
|
||||
|
||||
export interface MetaPage {
|
||||
current: number;
|
||||
size: number;
|
||||
|
|
|
@ -6,74 +6,68 @@
|
|||
*/
|
||||
|
||||
import { DEFAULT_INITIAL_APP_DATA } from '../../../common/__mocks__';
|
||||
import { LogicMounter } from '../__mocks__';
|
||||
|
||||
import { resetContext } from 'kea';
|
||||
|
||||
import { AppLogic } from './app_logic';
|
||||
|
||||
describe('AppLogic', () => {
|
||||
const { mount } = new LogicMounter(AppLogic);
|
||||
|
||||
beforeEach(() => {
|
||||
mount();
|
||||
});
|
||||
|
||||
const DEFAULT_VALUES = {
|
||||
hasInitialized: false,
|
||||
account: {},
|
||||
configuredLimits: {},
|
||||
ilmEnabled: false,
|
||||
myRole: {},
|
||||
const mount = (props = {}) => {
|
||||
AppLogic({ ...DEFAULT_INITIAL_APP_DATA, ...props });
|
||||
AppLogic.mount();
|
||||
};
|
||||
|
||||
it('has expected default values', () => {
|
||||
expect(AppLogic.values).toEqual(DEFAULT_VALUES);
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
resetContext({});
|
||||
});
|
||||
|
||||
describe('initializeAppData()', () => {
|
||||
it('sets values based on passed props', () => {
|
||||
AppLogic.actions.initializeAppData(DEFAULT_INITIAL_APP_DATA);
|
||||
it('sets values from props', () => {
|
||||
mount();
|
||||
|
||||
expect(AppLogic.values).toEqual({
|
||||
hasInitialized: true,
|
||||
ilmEnabled: true,
|
||||
configuredLimits: {
|
||||
engine: {
|
||||
maxDocumentByteSize: 102400,
|
||||
maxEnginesPerMetaEngine: 15,
|
||||
},
|
||||
expect(AppLogic.values).toEqual({
|
||||
ilmEnabled: true,
|
||||
configuredLimits: {
|
||||
engine: {
|
||||
maxDocumentByteSize: 102400,
|
||||
maxEnginesPerMetaEngine: 15,
|
||||
},
|
||||
account: {
|
||||
accountId: 'some-id-string',
|
||||
onboardingComplete: true,
|
||||
role: DEFAULT_INITIAL_APP_DATA.appSearch.role,
|
||||
},
|
||||
myRole: expect.objectContaining({
|
||||
id: 'account_id:somestring|user_oid:somestring',
|
||||
roleType: 'owner',
|
||||
availableRoleTypes: ['owner', 'admin'],
|
||||
credentialTypes: ['admin', 'private', 'search'],
|
||||
canAccessAllEngines: true,
|
||||
canViewAccountCredentials: true,
|
||||
// Truncated for brevity - see utils/role/index.test.ts for full output
|
||||
}),
|
||||
});
|
||||
},
|
||||
account: {
|
||||
accountId: 'some-id-string',
|
||||
onboardingComplete: true,
|
||||
role: DEFAULT_INITIAL_APP_DATA.appSearch.role,
|
||||
},
|
||||
myRole: expect.objectContaining({
|
||||
id: 'account_id:somestring|user_oid:somestring',
|
||||
roleType: 'owner',
|
||||
availableRoleTypes: ['owner', 'admin'],
|
||||
credentialTypes: ['admin', 'private', 'search'],
|
||||
canAccessAllEngines: true,
|
||||
canViewAccountCredentials: true,
|
||||
// Truncated for brevity - see utils/role/index.test.ts for full output
|
||||
}),
|
||||
});
|
||||
});
|
||||
|
||||
it('gracefully handles missing initial data', () => {
|
||||
AppLogic.actions.initializeAppData({});
|
||||
describe('actions', () => {
|
||||
describe('setOnboardingComplete()', () => {
|
||||
it('sets true', () => {
|
||||
mount({ appSearch: { onboardingComplete: false } });
|
||||
|
||||
expect(AppLogic.values).toEqual({
|
||||
...DEFAULT_VALUES,
|
||||
hasInitialized: true,
|
||||
AppLogic.actions.setOnboardingComplete();
|
||||
expect(AppLogic.values.account.onboardingComplete).toEqual(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('setOnboardingComplete()', () => {
|
||||
it('sets true', () => {
|
||||
expect(AppLogic.values.account.onboardingComplete).toBeFalsy();
|
||||
AppLogic.actions.setOnboardingComplete();
|
||||
expect(AppLogic.values.account.onboardingComplete).toEqual(true);
|
||||
describe('selectors', () => {
|
||||
describe('myRole', () => {
|
||||
it('falls back to an empty object if role is missing', () => {
|
||||
mount({ appSearch: {} });
|
||||
|
||||
expect(AppLogic.values.myRole).toEqual({});
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
@ -14,53 +14,33 @@ import { ConfiguredLimits, Account, Role } from './types';
|
|||
import { getRoleAbilities } from './utils/role';
|
||||
|
||||
interface AppValues {
|
||||
hasInitialized: boolean;
|
||||
ilmEnabled: boolean;
|
||||
configuredLimits: Partial<ConfiguredLimits>;
|
||||
account: Partial<Account>;
|
||||
myRole: Partial<Role>;
|
||||
configuredLimits: ConfiguredLimits;
|
||||
account: Account;
|
||||
myRole: Role;
|
||||
}
|
||||
interface AppActions {
|
||||
initializeAppData(props: InitialAppData): Required<InitialAppData>;
|
||||
setOnboardingComplete(): boolean;
|
||||
}
|
||||
|
||||
export const AppLogic = kea<MakeLogicType<AppValues, AppActions>>({
|
||||
export const AppLogic = kea<MakeLogicType<AppValues, AppActions, Required<InitialAppData>>>({
|
||||
path: ['enterprise_search', 'app_search', 'app_logic'],
|
||||
actions: {
|
||||
initializeAppData: (props) => props,
|
||||
setOnboardingComplete: () => true,
|
||||
},
|
||||
reducers: {
|
||||
hasInitialized: [
|
||||
false,
|
||||
{
|
||||
initializeAppData: () => true,
|
||||
},
|
||||
],
|
||||
reducers: ({ props }) => ({
|
||||
account: [
|
||||
{},
|
||||
props.appSearch,
|
||||
{
|
||||
initializeAppData: (_, { appSearch: account }) => account || {},
|
||||
setOnboardingComplete: (account) => ({
|
||||
...account,
|
||||
onboardingComplete: true,
|
||||
}),
|
||||
},
|
||||
],
|
||||
configuredLimits: [
|
||||
{},
|
||||
{
|
||||
initializeAppData: (_, { configuredLimits }) => configuredLimits?.appSearch || {},
|
||||
},
|
||||
],
|
||||
ilmEnabled: [
|
||||
false,
|
||||
{
|
||||
initializeAppData: (_, { ilmEnabled }) => !!ilmEnabled,
|
||||
},
|
||||
],
|
||||
},
|
||||
configuredLimits: [props.configuredLimits.appSearch, {}],
|
||||
ilmEnabled: [props.ilmEnabled, {}],
|
||||
}),
|
||||
selectors: {
|
||||
myRole: [
|
||||
(selectors) => [selectors.account],
|
||||
|
|
|
@ -55,8 +55,11 @@ export const FlyoutHeader: React.FC = () => {
|
|||
};
|
||||
|
||||
export const FlyoutBody: React.FC = () => {
|
||||
const { configuredLimits } = useValues(AppLogic);
|
||||
const maxDocumentByteSize = configuredLimits?.engine?.maxDocumentByteSize;
|
||||
const {
|
||||
configuredLimits: {
|
||||
engine: { maxDocumentByteSize },
|
||||
},
|
||||
} = useValues(AppLogic);
|
||||
|
||||
const { textInput, errors } = useValues(DocumentCreationLogic);
|
||||
const { setTextInput } = useActions(DocumentCreationLogic);
|
||||
|
|
|
@ -54,8 +54,11 @@ export const FlyoutHeader: React.FC = () => {
|
|||
};
|
||||
|
||||
export const FlyoutBody: React.FC = () => {
|
||||
const { configuredLimits } = useValues(AppLogic);
|
||||
const maxDocumentByteSize = configuredLimits?.engine?.maxDocumentByteSize;
|
||||
const {
|
||||
configuredLimits: {
|
||||
engine: { maxDocumentByteSize },
|
||||
},
|
||||
} = useValues(AppLogic);
|
||||
|
||||
const { isUploading, errors } = useValues(DocumentCreationLogic);
|
||||
const { setFileInput } = useActions(DocumentCreationLogic);
|
||||
|
|
|
@ -56,7 +56,7 @@ const comboBoxOptionToEngineName = (option: EuiComboBoxOptionOption<string>): st
|
|||
export const MetaEngineCreation: React.FC = () => {
|
||||
const {
|
||||
configuredLimits: {
|
||||
engine: { maxEnginesPerMetaEngine } = { maxEnginesPerMetaEngine: Infinity },
|
||||
engine: { maxEnginesPerMetaEngine },
|
||||
},
|
||||
} = useValues(AppLogic);
|
||||
|
||||
|
|
|
@ -5,18 +5,21 @@
|
|||
* 2.0.
|
||||
*/
|
||||
|
||||
import '../__mocks__/shallow_useeffect.mock';
|
||||
import { DEFAULT_INITIAL_APP_DATA } from '../../../common/__mocks__';
|
||||
import '../__mocks__/enterprise_search_url.mock';
|
||||
import { setMockValues, setMockActions } from '../__mocks__';
|
||||
import { setMockValues, rerender } from '../__mocks__';
|
||||
|
||||
import React from 'react';
|
||||
|
||||
import { Redirect } from 'react-router-dom';
|
||||
|
||||
import { shallow } from 'enzyme';
|
||||
import { shallow, ShallowWrapper } from 'enzyme';
|
||||
|
||||
import { Layout, SideNav, SideNavLink } from '../shared/layout';
|
||||
|
||||
jest.mock('./app_logic', () => ({ AppLogic: jest.fn() }));
|
||||
import { AppLogic } from './app_logic';
|
||||
|
||||
import { EngineRouter } from './components/engine';
|
||||
import { EngineCreation } from './components/engine_creation';
|
||||
import { EnginesOverview } from './components/engines';
|
||||
|
@ -52,52 +55,34 @@ describe('AppSearchUnconfigured', () => {
|
|||
});
|
||||
|
||||
describe('AppSearchConfigured', () => {
|
||||
beforeEach(() => {
|
||||
// Mock resets
|
||||
let wrapper: ShallowWrapper;
|
||||
|
||||
beforeAll(() => {
|
||||
setMockValues({ myRole: {} });
|
||||
setMockActions({ initializeAppData: () => {} });
|
||||
wrapper = shallow(<AppSearchConfigured {...DEFAULT_INITIAL_APP_DATA} />);
|
||||
});
|
||||
|
||||
it('renders with layout', () => {
|
||||
const wrapper = shallow(<AppSearchConfigured />);
|
||||
|
||||
expect(wrapper.find(Layout)).toHaveLength(2);
|
||||
expect(wrapper.find(Layout).last().prop('readOnlyMode')).toBeFalsy();
|
||||
expect(wrapper.find(EnginesOverview)).toHaveLength(1);
|
||||
expect(wrapper.find(EngineRouter)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('initializes app data with passed props', () => {
|
||||
const initializeAppData = jest.fn();
|
||||
setMockActions({ initializeAppData });
|
||||
|
||||
shallow(<AppSearchConfigured ilmEnabled />);
|
||||
|
||||
expect(initializeAppData).toHaveBeenCalledWith({ ilmEnabled: true });
|
||||
});
|
||||
|
||||
it('does not re-initialize app data', () => {
|
||||
const initializeAppData = jest.fn();
|
||||
setMockActions({ initializeAppData });
|
||||
setMockValues({ myRole: {}, hasInitialized: true });
|
||||
|
||||
shallow(<AppSearchConfigured />);
|
||||
|
||||
expect(initializeAppData).not.toHaveBeenCalled();
|
||||
it('mounts AppLogic with passed initial data props', () => {
|
||||
expect(AppLogic).toHaveBeenCalledWith(DEFAULT_INITIAL_APP_DATA);
|
||||
});
|
||||
|
||||
it('renders ErrorConnecting', () => {
|
||||
setMockValues({ myRole: {}, errorConnecting: true });
|
||||
|
||||
const wrapper = shallow(<AppSearchConfigured />);
|
||||
rerender(wrapper);
|
||||
|
||||
expect(wrapper.find(ErrorConnecting)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('passes readOnlyMode state', () => {
|
||||
setMockValues({ myRole: {}, readOnlyMode: true });
|
||||
|
||||
const wrapper = shallow(<AppSearchConfigured />);
|
||||
rerender(wrapper);
|
||||
|
||||
expect(wrapper.find(Layout).first().prop('readOnlyMode')).toEqual(true);
|
||||
});
|
||||
|
@ -106,14 +91,14 @@ describe('AppSearchConfigured', () => {
|
|||
describe('canManageEngines', () => {
|
||||
it('renders EngineCreation when user canManageEngines is true', () => {
|
||||
setMockValues({ myRole: { canManageEngines: true } });
|
||||
const wrapper = shallow(<AppSearchConfigured />);
|
||||
rerender(wrapper);
|
||||
|
||||
expect(wrapper.find(EngineCreation)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('does not render EngineCreation when user canManageEngines is false', () => {
|
||||
setMockValues({ myRole: { canManageEngines: false } });
|
||||
const wrapper = shallow(<AppSearchConfigured />);
|
||||
rerender(wrapper);
|
||||
|
||||
expect(wrapper.find(EngineCreation)).toHaveLength(0);
|
||||
});
|
||||
|
@ -122,14 +107,14 @@ describe('AppSearchConfigured', () => {
|
|||
describe('canManageMetaEngines', () => {
|
||||
it('renders MetaEngineCreation when user canManageMetaEngines is true', () => {
|
||||
setMockValues({ myRole: { canManageMetaEngines: true } });
|
||||
const wrapper = shallow(<AppSearchConfigured />);
|
||||
rerender(wrapper);
|
||||
|
||||
expect(wrapper.find(MetaEngineCreation)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('does not render MetaEngineCreation when user canManageMetaEngines is false', () => {
|
||||
setMockValues({ myRole: { canManageMetaEngines: false } });
|
||||
const wrapper = shallow(<AppSearchConfigured />);
|
||||
rerender(wrapper);
|
||||
|
||||
expect(wrapper.find(MetaEngineCreation)).toHaveLength(0);
|
||||
});
|
||||
|
|
|
@ -5,10 +5,10 @@
|
|||
* 2.0.
|
||||
*/
|
||||
|
||||
import React, { useEffect } from 'react';
|
||||
import React from 'react';
|
||||
import { Route, Redirect, Switch } from 'react-router-dom';
|
||||
|
||||
import { useActions, useValues } from 'kea';
|
||||
import { useValues } from 'kea';
|
||||
|
||||
import { APP_SEARCH_PLUGIN } from '../../../common/constants';
|
||||
import { InitialAppData } from '../../../common/types';
|
||||
|
@ -44,7 +44,11 @@ import {
|
|||
|
||||
export const AppSearch: React.FC<InitialAppData> = (props) => {
|
||||
const { config } = useValues(KibanaLogic);
|
||||
return !config.host ? <AppSearchUnconfigured /> : <AppSearchConfigured {...props} />;
|
||||
return !config.host ? (
|
||||
<AppSearchUnconfigured />
|
||||
) : (
|
||||
<AppSearchConfigured {...(props as Required<InitialAppData>)} />
|
||||
);
|
||||
};
|
||||
|
||||
export const AppSearchUnconfigured: React.FC = () => (
|
||||
|
@ -58,18 +62,12 @@ export const AppSearchUnconfigured: React.FC = () => (
|
|||
</Switch>
|
||||
);
|
||||
|
||||
export const AppSearchConfigured: React.FC<InitialAppData> = (props) => {
|
||||
const { initializeAppData } = useActions(AppLogic);
|
||||
export const AppSearchConfigured: React.FC<Required<InitialAppData>> = (props) => {
|
||||
const {
|
||||
hasInitialized,
|
||||
myRole: { canManageEngines, canManageMetaEngines },
|
||||
} = useValues(AppLogic);
|
||||
} = useValues(AppLogic(props));
|
||||
const { errorConnecting, readOnlyMode } = useValues(HttpLogic);
|
||||
|
||||
useEffect(() => {
|
||||
if (!hasInitialized) initializeAppData(props);
|
||||
}, [hasInitialized]);
|
||||
|
||||
return (
|
||||
<Switch>
|
||||
<Route exact path={SETUP_GUIDE_PATH}>
|
||||
|
|
|
@ -9,6 +9,7 @@ import { KibanaRequest, Logger } from 'src/core/server';
|
|||
|
||||
import { SecurityPluginSetup } from '../../../security/server';
|
||||
import { SpacesPluginStart } from '../../../spaces/server';
|
||||
import { ProductAccess } from '../../common/types';
|
||||
import { ConfigType } from '../index';
|
||||
|
||||
import { callEnterpriseSearchConfigAPI } from './enterprise_search_config_api';
|
||||
|
@ -20,10 +21,6 @@ interface CheckAccess {
|
|||
config: ConfigType;
|
||||
log: Logger;
|
||||
}
|
||||
export interface Access {
|
||||
hasAppSearchAccess: boolean;
|
||||
hasWorkplaceSearchAccess: boolean;
|
||||
}
|
||||
|
||||
const ALLOW_ALL_PLUGINS = {
|
||||
hasAppSearchAccess: true,
|
||||
|
@ -45,7 +42,7 @@ export const checkAccess = async ({
|
|||
spaces,
|
||||
request,
|
||||
log,
|
||||
}: CheckAccess): Promise<Access> => {
|
||||
}: CheckAccess): Promise<ProductAccess> => {
|
||||
const isRbacEnabled = security?.authz.mode.useRbacForRequest(request) ?? false;
|
||||
|
||||
// We can only retrieve the active space when either:
|
||||
|
|
|
@ -108,12 +108,12 @@ describe('callEnterpriseSearchConfigAPI', () => {
|
|||
});
|
||||
|
||||
expect(await callEnterpriseSearchConfigAPI(mockDependencies)).toEqual({
|
||||
...DEFAULT_INITIAL_APP_DATA,
|
||||
access: {
|
||||
hasAppSearchAccess: true,
|
||||
hasWorkplaceSearchAccess: false,
|
||||
},
|
||||
publicUrl: 'http://some.vanity.url',
|
||||
...DEFAULT_INITIAL_APP_DATA,
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
@ -14,15 +14,12 @@ import { stripTrailingSlash } from '../../common/strip_slashes';
|
|||
import { InitialAppData } from '../../common/types';
|
||||
import { ConfigType } from '../index';
|
||||
|
||||
import { Access } from './check_access';
|
||||
|
||||
interface Params {
|
||||
request: KibanaRequest;
|
||||
config: ConfigType;
|
||||
log: Logger;
|
||||
}
|
||||
interface Return extends InitialAppData {
|
||||
access?: Access;
|
||||
publicUrl?: string;
|
||||
}
|
||||
|
||||
|
|
|
@ -33,12 +33,8 @@ describe('Enterprise Search Config Data API', () => {
|
|||
describe('GET /api/enterprise_search/config_data', () => {
|
||||
it('returns an initial set of config data from Enterprise Search', async () => {
|
||||
const mockData = {
|
||||
access: {
|
||||
hasAppSearchAccess: true,
|
||||
hasWorkplaceSearchAccess: true,
|
||||
},
|
||||
publicUrl: 'http://localhost:3002',
|
||||
...DEFAULT_INITIAL_APP_DATA,
|
||||
publicUrl: 'http://localhost:3002',
|
||||
};
|
||||
|
||||
(callEnterpriseSearchConfigAPI as jest.Mock).mockImplementationOnce(() => {
|
||||
|
|
Loading…
Reference in a new issue