Skip to content

Commit

Permalink
Add Kea.js support to Enterprise Search plugin (#72160) (#73372)
Browse files Browse the repository at this point in the history
* Add Kea packages

- kea and kea-waitfor

* Add Kea declarations and types

Hopefully TypeScript support coming soon from author

* Add Kea to entry point

* Add logic for overview

* Update components to use Kea

* Fix a couple of tests that weren’t getting complete coverage

* Remove kea-waitfor

Turns out we don’t need it

* Remove unused declaration

* Update x-pack/plugins/enterprise_search/public/applications/workplace_search/components/overview/overview_logic.ts

Co-authored-by: Constance <constancecchen@users.noreply.github.com>

* Update x-pack/plugins/enterprise_search/public/applications/workplace_search/components/overview/overview_logic.ts

Co-authored-by: Constance <constancecchen@users.noreply.github.com>

* Update x-pack/plugins/enterprise_search/public/applications/workplace_search/components/overview/__mocks__/overview_logic.mock.ts

Co-authored-by: Constance <constancecchen@users.noreply.github.com>

* Update x-pack/plugins/enterprise_search/public/applications/workplace_search/components/overview/overview_logic.test.ts

Co-authored-by: Constance <constancecchen@users.noreply.github.com>

* [Opinionated] Remove extra actions defs

- they're already being defined in IOverviewActions, so no need to repeat them

* DRY out a new reusable/generics IKeaLogic/Listeners interface

- Multiple logic files can now do IKeaListeners<SomeLogicActions> and not have to declare their own IListenerParams!

+ bonus IKeaSelectors just for consistency

* DRY out Kea reducers definitions to generics interface

* [Refactor] Improve KeaReducers generic to actually type-check/check key names

- Typescript will now throw an error if you put in a key name that isn't declared in your actions/values interface
- default & new states now will be type checked!! 🎉

* [Refactor] Update selectors() and listeners() to also check types and keys

* [Refactor] Move param defs to bottom of file instead of inline

- so that inline definitions mostly focus on type checks, and more boilerplate defs are DRYed out
- I played around with 2.1 obj definitions and got terrible results here :(

* Update tests and remove selectors per code review

* Remove last statsColumns instance

* Remove last instance of hideOnboarding

Co-authored-by: Constance <constancecchen@users.noreply.github.com>
Co-authored-by: Constance Chen <constance.chen.3@gmail.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Constance <constancecchen@users.noreply.github.com>
Co-authored-by: Constance Chen <constance.chen.3@gmail.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
  • Loading branch information
4 people authored Jul 27, 2020
1 parent 7c8706b commit 9c4b3e0
Show file tree
Hide file tree
Showing 20 changed files with 633 additions and 236 deletions.
1 change: 1 addition & 0 deletions x-pack/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@
"json-stable-stringify": "^1.0.1",
"jsonwebtoken": "^8.5.1",
"jsts": "^1.6.2",
"kea": "^2.0.1",
"lodash": "^4.17.15",
"lz-string": "^1.4.4",
"mapbox-gl": "^1.10.0",
Expand Down
13 changes: 13 additions & 0 deletions x-pack/plugins/enterprise_search/public/applications/kea.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

declare module 'kea' {
export function useValues(logic?: object): object;
export function useActions(logic?: object): object;
export function getContext(): { store: object };
export function resetContext(context: object): object;
export function kea(logic: object): object;
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,59 @@ export interface IFlashMessagesProps {
isWrapped?: boolean;
children?: React.ReactNode;
}

export interface IKeaLogic<IKeaValues, IKeaActions> {
mount(): void;
values: IKeaValues;
actions: IKeaActions;
}

/**
* This reusable interface mostly saves us a few characters / allows us to skip
* defining params inline. Unfortunately, the return values *do not work* as
* expected (hence the voids). While I can tell selectors to use TKeaSelectors,
* the return value is *not* properly type checked if it's not declared inline. :/
*
* Also note that if you switch to Kea 2.1's plain object notation -
* `selectors: {}` vs. `selectors: () => ({})`
* - type checking also stops working and type errors become significantly less
* helpful - showing less specific error messages and highlighting. 👎
*/
export interface IKeaParams<IKeaValues, IKeaActions> {
selectors?(params: { selectors: IKeaValues }): void;
listeners?(params: { actions: IKeaActions; values: IKeaValues }): void;
}

/**
* This reducers() type checks that:
*
* 1. The value object keys are defined within IKeaValues
* 2. The default state (array[0]) matches the type definition within IKeaValues
* 3. The action object keys (array[1]) are defined within IKeaActions
* 3. The new state returned by the action matches the type definition within IKeaValues
*/
export type TKeaReducers<IKeaValues, IKeaActions> = {
[Value in keyof IKeaValues]?: [
IKeaValues[Value],
{
[Action in keyof IKeaActions]?: (state: IKeaValues, payload: IKeaValues) => IKeaValues[Value];
}
];
};

/**
* This selectors() type checks that:
*
* 1. The object keys are defined within IKeaValues
* 2. The selected values are defined within IKeaValues
* 3. The returned value match the type definition within IKeaValues
*
* The unknown[] and any[] are unfortunately because I have no idea how to
* assert for arbitrary type/values as an array
*/
export type TKeaSelectors<IKeaValues> = {
[Value in keyof IKeaValues]?: [
(selectors: IKeaValues) => unknown[],
(...args: any[]) => IKeaValues[Value] // eslint-disable-line @typescript-eslint/no-explicit-any
];
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

export { setMockValues, mockLogicValues, mockLogicActions } from './overview_logic.mock';
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { IOverviewValues } from '../overview_logic';
import { IAccount, IOrganization, IUser } from '../../../types';

export const mockLogicValues = {
accountsCount: 0,
activityFeed: [],
canCreateContentSources: false,
canCreateInvitations: false,
currentUser: {} as IUser,
fpAccount: {} as IAccount,
hasOrgSources: false,
hasUsers: false,
isFederatedAuth: true,
isOldAccount: false,
organization: {} as IOrganization,
pendingInvitationsCount: 0,
personalSourcesCount: 0,
sourcesCount: 0,
dataLoading: true,
hasErrorConnecting: false,
flashMessages: {},
} as IOverviewValues;

export const mockLogicActions = {
initializeOverview: jest.fn(() => ({})),
};

jest.mock('kea', () => ({
...(jest.requireActual('kea') as object),
useActions: jest.fn(() => ({ ...mockLogicActions })),
useValues: jest.fn(() => ({ ...mockLogicValues })),
}));

import { useValues } from 'kea';

export const setMockValues = (values: object) => {
(useValues as jest.Mock).mockImplementationOnce(() => ({
...mockLogicValues,
...values,
}));
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
*/

import '../../../__mocks__/shallow_usecontext.mock';
import './__mocks__/overview_logic.mock';
import { setMockValues } from './__mocks__';

import React from 'react';
import { shallow } from 'enzyme';
Expand All @@ -16,7 +18,6 @@ import { sendTelemetry } from '../../../shared/telemetry';

import { OnboardingSteps, OrgNameOnboarding } from './onboarding_steps';
import { OnboardingCard } from './onboarding_card';
import { defaultServerData } from './overview';

const account = {
id: '1',
Expand All @@ -30,7 +31,8 @@ const account = {
describe('OnboardingSteps', () => {
describe('Shared Sources', () => {
it('renders 0 sources state', () => {
const wrapper = shallow(<OnboardingSteps {...defaultServerData} />);
setMockValues({ canCreateContentSources: true });
const wrapper = shallow(<OnboardingSteps />);

expect(wrapper.find(OnboardingCard)).toHaveLength(1);
expect(wrapper.find(OnboardingCard).prop('actionPath')).toBe(ORG_SOURCES_PATH);
Expand All @@ -40,35 +42,32 @@ describe('OnboardingSteps', () => {
});

it('renders completed sources state', () => {
const wrapper = shallow(
<OnboardingSteps {...defaultServerData} sourcesCount={2} hasOrgSources />
);
setMockValues({ sourcesCount: 2, hasOrgSources: true });
const wrapper = shallow(<OnboardingSteps />);

expect(wrapper.find(OnboardingCard).prop('description')).toEqual(
'You have added 2 shared sources. Happy searching.'
);
});

it('disables link when the user cannot create sources', () => {
const wrapper = shallow(
<OnboardingSteps {...defaultServerData} canCreateContentSources={false} />
);
setMockValues({ canCreateContentSources: false });
const wrapper = shallow(<OnboardingSteps />);

expect(wrapper.find(OnboardingCard).prop('actionPath')).toBe(undefined);
});
});

describe('Users & Invitations', () => {
it('renders 0 users when not on federated auth', () => {
const wrapper = shallow(
<OnboardingSteps
{...defaultServerData}
isFederatedAuth={false}
fpAccount={account}
accountsCount={0}
hasUsers={false}
/>
);
setMockValues({
canCreateInvitations: true,
isFederatedAuth: false,
fpAccount: account,
accountsCount: 0,
hasUsers: false,
});
const wrapper = shallow(<OnboardingSteps />);

expect(wrapper.find(OnboardingCard)).toHaveLength(2);
expect(wrapper.find(OnboardingCard).last().prop('actionPath')).toBe(USERS_PATH);
Expand All @@ -78,37 +77,29 @@ describe('OnboardingSteps', () => {
});

it('renders completed users state', () => {
const wrapper = shallow(
<OnboardingSteps
{...defaultServerData}
isFederatedAuth={false}
fpAccount={account}
accountsCount={1}
hasUsers
/>
);
setMockValues({
isFederatedAuth: false,
fpAccount: account,
accountsCount: 1,
hasUsers: true,
});
const wrapper = shallow(<OnboardingSteps />);

expect(wrapper.find(OnboardingCard).last().prop('description')).toEqual(
'Nice, you’ve invited colleagues to search with you.'
);
});

it('disables link when the user cannot create invitations', () => {
const wrapper = shallow(
<OnboardingSteps
{...defaultServerData}
isFederatedAuth={false}
canCreateInvitations={false}
/>
);

setMockValues({ isFederatedAuth: false, canCreateInvitations: false });
const wrapper = shallow(<OnboardingSteps />);
expect(wrapper.find(OnboardingCard).last().prop('actionPath')).toBe(undefined);
});
});

describe('Org Name', () => {
it('renders button to change name', () => {
const wrapper = shallow(<OnboardingSteps {...defaultServerData} />);
const wrapper = shallow(<OnboardingSteps />);

const button = wrapper
.find(OrgNameOnboarding)
Expand All @@ -120,15 +111,13 @@ describe('OnboardingSteps', () => {
});

it('hides card when name has been changed', () => {
const wrapper = shallow(
<OnboardingSteps
{...defaultServerData}
organization={{
name: 'foo',
defaultOrgName: 'bar',
}}
/>
);
setMockValues({
organization: {
name: 'foo',
defaultOrgName: 'bar',
},
});
const wrapper = shallow(<OnboardingSteps />);

expect(wrapper.find(OrgNameOnboarding)).toHaveLength(0);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import React, { useContext } from 'react';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n/react';
import { useValues } from 'kea';

import {
EuiSpacer,
Expand All @@ -28,7 +29,7 @@ import { ORG_SOURCES_PATH, USERS_PATH, ORG_SETTINGS_PATH } from '../../routes';

import { ContentSection } from '../shared/content_section';

import { IAppServerData } from './overview';
import { OverviewLogic, IOverviewValues } from './overview_logic';

import { OnboardingCard } from './onboarding_card';

Expand Down Expand Up @@ -57,17 +58,19 @@ const ONBOARDING_USERS_CARD_DESCRIPTION = i18n.translate(
{ defaultMessage: 'Invite your colleagues into this organization to search with you.' }
);

export const OnboardingSteps: React.FC<IAppServerData> = ({
hasUsers,
hasOrgSources,
canCreateContentSources,
canCreateInvitations,
accountsCount,
sourcesCount,
fpAccount: { isCurated },
organization: { name, defaultOrgName },
isFederatedAuth,
}) => {
export const OnboardingSteps: React.FC = () => {
const {
hasUsers,
hasOrgSources,
canCreateContentSources,
canCreateInvitations,
accountsCount,
sourcesCount,
fpAccount: { isCurated },
organization: { name, defaultOrgName },
isFederatedAuth,
} = useValues(OverviewLogic) as IOverviewValues;

const accountsPath =
!isFederatedAuth && (canCreateInvitations || isCurated) ? USERS_PATH : undefined;
const sourcesPath = canCreateContentSources || isCurated ? ORG_SOURCES_PATH : undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,27 @@
*/

import '../../../__mocks__/shallow_usecontext.mock';
import './__mocks__/overview_logic.mock';
import { setMockValues } from './__mocks__';

import React from 'react';
import { shallow } from 'enzyme';
import { EuiFlexGrid } from '@elastic/eui';

import { OrganizationStats } from './organization_stats';
import { StatisticCard } from './statistic_card';
import { defaultServerData } from './overview';

describe('OrganizationStats', () => {
it('renders', () => {
const wrapper = shallow(<OrganizationStats {...defaultServerData} />);
const wrapper = shallow(<OrganizationStats />);

expect(wrapper.find(StatisticCard)).toHaveLength(2);
expect(wrapper.find(EuiFlexGrid).prop('columns')).toEqual(2);
});

it('renders additional cards for federated auth', () => {
const wrapper = shallow(<OrganizationStats {...defaultServerData} isFederatedAuth={false} />);
setMockValues({ isFederatedAuth: false });
const wrapper = shallow(<OrganizationStats />);

expect(wrapper.find(StatisticCard)).toHaveLength(4);
expect(wrapper.find(EuiFlexGrid).prop('columns')).toEqual(4);
Expand Down
Loading

0 comments on commit 9c4b3e0

Please sign in to comment.