Skip to content

Commit

Permalink
[Enterprise Search][Workplace Search] Migrate Groups to Kibana (#78679)
Browse files Browse the repository at this point in the history
* Initial copying of Groups component tree

This commit moves the base component tree from ent-search with the following changes to satisfy pre-commit hooks

- All file names changed to camel_case
- Copyright comment added to the top of each file
- Semicolons and formatting to match Kibana
- Default exports removed from components
- Placeholder keyboard listener functions added to non-interactive elements with click handlers

* Update paths, remove kea typecasting

This commit does the following:

- Updates the paths of the imports
- Removed the need to typecast logic i.e. "as IGroupsLogic" after the Kea hooks
- Fixed a few TypeScript errors with events and return values on functions

- Finally, this commit does away with the ConfirmModal component (was never copied over), as it only wraps 2 EUI components and we decided it was not needed

* Add constants and image

* Update types

- Adds new types
- Moves types needed by server and app to common

* Refactor ContentSection and ViewContentHeader

With the groups components, we needed to add a header action that was right-aligned to ContentSection.

After building what was needed, I realized that the header in the ContentSection was basically a ViewContentHeader with a different sized heading. I refactored ViewContentHeader to have variable sizes and for ContentSection to use that instead.

Also added styles from ent-search

* Add group routes

This is both the server routes and the frontend paths. After conversation with Jonas, we decided to not use the `/org` prefix. Will clean up the other routes as we build out the rest the migration

* Update logic files

This commit converts the logic files for use in Kibana. This includes:

- Using the new Kea 2.2 syntax that uses MakeLogicType to provide types
- Uses the Kibana HttpLogic instead of the rails routes/http
- Adds return types to Actions interfaces
- Adds `path` key to help with debuggin in dev tools
- Removes FashMessages in lieu of separate logic. Because of this, we have to manually clear the flash mesages with listeners where reducers used to do it when the messages were local to the logic file
- Preplaces promises with async/await & try/catch

- Also, in GroupsLogic’s getSearchResults method, we used Kea’s breakpoint functionality and replaced the useDebouce Hook that was used in the component (future commit)
- Uses global lodash per Kibana’s new directive

* Update routers an indexes

This PR configures the routers to work with Kibana

- Updates paths to imports
- Adds top-level styles

For GroupRouter
- Removes AppView
- Use global flash messages
- Remove sidebar and breadcrumbs

* Update GroupOverview

Adds some changes to facilitate the new design for Kibana

- Copy changes
- Layout changes to have buttons inline and not conditionally shown

* Various updates to components

Adds some changes to facilitate the new design for Kibana.

- Remove unnecessary TableHeader
- Adds pencil in lieu of manage button per design

* Update main groups component

A previous commit did this for the components, as the intention was to do this for components in one PR and the others an a separate PR. Unfortunately the build does not pass with all the missing imports.

This commit does the following:
- Updates the paths of the imports
- Removed the need to typecast logic i.e. "as IGroupsLogic" after the Kea hooks
- Fixed a few TypeScript errors with events and return values on functions
- Use global flash messages
- Remove debounces filderValue, as it’s now debounces in logic file
- Remove legacy isCurated props
- Remove legact AppView

* Add sub navigation to main nav

Also removes redundant search link in sidebar

* Update logic file to reset flashmessages correctly

Because we have separated concerns with global flash message state, we now have to manually trigger resets of flash messages with listeners in Kea where we used to be able to use a reducer to listen to changes and reset flash messages.

* Use navigateToUrl for navigation over history.push

Thanks to work by @constance, we can now use the KibanaLogic’s navigateToUrl value to change routes directly from logic files

* Fix failing test

A previous commit removed the redundant Search link from the sidebar nav because of the one in the header. This commit fixes a filing test and now assets on the number of items as the link addresses will be changing as we migrate more components over

* Convert React Router links to our wrappers

* Replace anchors with EuiButtonEmpty

The original pre-commit hooks failed because the anchors didn’t have key handlers. Pleaceholders were added with TODOs and these have been replaced with EuiButtonEmpty, which satisifies the UI needs and passes linting

* Fix a bug where header actions disappearing

There was a bug where changing routes would cause the header action, in the case of Workplace Search the “Go to search application” link, to disappear on route changes.

Turns out that we didn’t need it in the useEffect and that moving it out keeps the unmount from removing the link from the header.

* i18n top-level component and logic files

* i18n for components

* Fix failing test

This was from 3254c62

* Fix broken i18n

Had duplicate ids and misnamed value

* Fix a bug where manage group not in Flash message

There is a button that appears in the Flash Message when a new group is added so that the user can navigate to manage the group and, because the order of setting the new group was before the instantiation of the global flashmessages, the button was not appearing. This commit moves the action after the flash message is set.

* Refactor typings

* Refactor ContentSection spacing

The css was not being used and the bottom padding of 44px (xxl + xs spacer) was being generated with spacers. This has been changed to use only CSS

* Remove canCreateInvitations

* Remove hasMessages check rendering FlashMessages

* snake_case telemetry and path

Will convert other paths in a separate PR

* Fix failing tests

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
3 people authored Oct 5, 2020
1 parent 106ab7e commit 197510a
Show file tree
Hide file tree
Showing 49 changed files with 3,424 additions and 93 deletions.
46 changes: 46 additions & 0 deletions x-pack/plugins/enterprise_search/common/types/workplace_search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,49 @@ export interface IConfiguredLimits {
totalFields: number;
};
}

export interface IGroup {
id: string;
name: string;
createdAt: string;
updatedAt: string;
contentSources: IContentSource[];
users: IUser[];
usersCount: number;
color?: string;
}

export interface IGroupDetails extends IGroup {
contentSources: IContentSourceDetails[];
canEditGroup: boolean;
canDeleteGroup: boolean;
}

export interface IUser {
id: string;
name: string | null;
initials: string;
pictureUrl: string | null;
color: string;
email: string;
role?: string;
groupIds: string[];
}

export interface IContentSource {
id: string;
serviceType: string;
name: string;
}

export interface IContentSourceDetails extends IContentSource {
status: string;
statusMessage: string;
documentCount: string;
isFederatedSource: boolean;
searchable: boolean;
supportedByLicense: boolean;
errorReason: number;
allowsReauth: boolean;
boost: number;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/*
* 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 const DEFAULT_META = {
page: {
current: 1,
size: 10,
total_pages: 0,
total_results: 0,
},
};
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 { DEFAULT_META } from './default_meta';
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@ describe('WorkplaceSearchNav', () => {

expect(wrapper.find(SideNav)).toHaveLength(1);
expect(wrapper.find(SideNavLink).first().prop('to')).toEqual('/');
expect(wrapper.find(SideNavLink).last().prop('to')).toEqual('http://localhost:3002/ws/search');
expect(wrapper.find(SideNavLink)).toHaveLength(7);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import { WORKPLACE_SEARCH_PLUGIN } from '../../../../../common/constants';
import { getWorkplaceSearchUrl } from '../../../shared/enterprise_search_url';
import { SideNav, SideNavLink } from '../../../shared/layout';

import { GroupSubNav } from '../../views/groups/components/group_sub_nav';

import {
ORG_SOURCES_PATH,
SOURCES_PATH,
Expand All @@ -35,7 +37,7 @@ export const WorkplaceSearchNav: React.FC = () => {
defaultMessage: 'Sources',
})}
</SideNavLink>
<SideNavLink isExternal to={getWorkplaceSearchUrl(`#${GROUPS_PATH}`)}>
<SideNavLink to={GROUPS_PATH} subNav={<GroupSubNav />}>
{i18n.translate('xpack.enterpriseSearch.workplaceSearch.nav.groups', {
defaultMessage: 'Groups',
})}
Expand All @@ -61,11 +63,6 @@ export const WorkplaceSearchNav: React.FC = () => {
defaultMessage: 'View my personal dashboard',
})}
</SideNavLink>
<SideNavLink isExternal to={getWorkplaceSearchUrl('/search')}>
{i18n.translate('xpack.enterpriseSearch.workplaceSearch.nav.search', {
defaultMessage: 'Go to search application',
})}
</SideNavLink>
</SideNav>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* 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.
*/

.content-section {
padding-bottom: 44px;
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@

import React from 'react';
import { shallow } from 'enzyme';
import { EuiTitle, EuiSpacer } from '@elastic/eui';
import { EuiSpacer } from '@elastic/eui';

import { ContentSection } from './';
import { ViewContentHeader } from '../view_content_header';

const props = {
children: <div className="children" />,
Expand All @@ -20,15 +21,16 @@ describe('ContentSection', () => {
const wrapper = shallow(<ContentSection {...props} className="test" />);

expect(wrapper.prop('data-test-subj')).toEqual('contentSection');
expect(wrapper.prop('className')).toEqual('test');
expect(wrapper.prop('className')).toEqual('test content-section');
expect(wrapper.find('.children')).toHaveLength(1);
});

it('displays title and description', () => {
const wrapper = shallow(<ContentSection {...props} title="foo" description="bar" />);

expect(wrapper.find(EuiTitle)).toHaveLength(1);
expect(wrapper.find('p').text()).toEqual('bar');
expect(wrapper.find(ViewContentHeader)).toHaveLength(1);
expect(wrapper.find(ViewContentHeader).prop('title')).toEqual('foo');
expect(wrapper.find(ViewContentHeader).prop('description')).toEqual('bar');
});

it('displays header content', () => {
Expand All @@ -41,7 +43,8 @@ describe('ContentSection', () => {
/>
);

expect(wrapper.find(EuiSpacer).prop('size')).toEqual('s');
expect(wrapper.find(EuiSpacer).first().prop('size')).toEqual('s');
expect(wrapper.find(EuiSpacer)).toHaveLength(1);
expect(wrapper.find('.header')).toHaveLength(1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,20 @@

import React from 'react';

import { EuiSpacer, EuiTitle } from '@elastic/eui';
import { EuiSpacer } from '@elastic/eui';

import { TSpacerSize } from '../../../types';

import { ViewContentHeader } from '../view_content_header';

import './content_section.scss';

interface IContentSectionProps {
children: React.ReactNode;
className?: string;
title?: React.ReactNode;
description?: React.ReactNode;
action?: React.ReactNode;
headerChildren?: React.ReactNode;
headerSpacer?: TSpacerSize;
testSubj?: string;
Expand All @@ -25,17 +30,15 @@ export const ContentSection: React.FC<IContentSectionProps> = ({
className = '',
title,
description,
action,
headerChildren,
headerSpacer,
testSubj,
}) => (
<div className={className} data-test-subj={testSubj}>
<div className={`${className} content-section`} data-test-subj={testSubj}>
{title && (
<>
<EuiTitle size="s">
<h3>{title}</h3>
</EuiTitle>
{description && <p>{description}</p>}
<ViewContentHeader title={title} titleSize="s" description={description} action={action} />
{headerChildren}
{headerSpacer && <EuiSpacer size={headerSpacer} />}
</>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
.source-row {
&__icon {
width: 24px;
height: 24px;
}

&__name {
font-weight: 500;
}

&__actions {
width: 100px;
}

&__actions a {
opacity: 1.0;
pointer-events: auto;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import classNames from 'classnames';
// Prefer importing entire lodash library, e.g. import { get } from "lodash"
// eslint-disable-next-line no-restricted-imports
import _kebabCase from 'lodash/kebabCase';
import { Link } from 'react-router-dom';

import {
EuiFlexGroup,
Expand All @@ -24,12 +23,15 @@ import {
EuiToolTip,
} from '@elastic/eui';

import { EuiLink } from '../../../../shared/react_router_helpers';
import { SOURCE_STATUSES as statuses } from '../../../constants';
import { IContentSourceDetails } from '../../../types';
import { ADD_SOURCE_PATH, SOURCE_DETAILS_PATH, getContentSourcePath } from '../../../routes';

import { SourceIcon } from '../source_icon';

import './source_row.scss';

const CREDENTIALS_INVALID_ERROR_REASON = 1;

export interface ISourceRow {
Expand Down Expand Up @@ -75,14 +77,9 @@ export const SourceRow: React.FC<ISourceRowProps> = ({
const imageClass = classNames('source-row__icon', { 'source-row__icon--loading': isIndexing });

const fixLink = (
<Link
to={{
pathname: `${ADD_SOURCE_PATH}/${_kebabCase(serviceType)}/re-authenticate`,
search: `?sourceId=${id}`,
}}
>
<EuiLink to={`${ADD_SOURCE_PATH}/${_kebabCase(serviceType)}/re-authenticate?sourceId=${id}`}>
Fix
</Link>
</EuiLink>
);

const remoteTooltip = (
Expand All @@ -100,7 +97,12 @@ export const SourceRow: React.FC<ISourceRowProps> = ({
return (
<EuiTableRow data-test-subj="GroupsRow" className={rowClass}>
<EuiTableRowCell>
<EuiFlexGroup justifyContent="flexStart" alignItems="center" responsive={false}>
<EuiFlexGroup
justifyContent="flexStart"
alignItems="center"
gutterSize="xs"
responsive={false}
>
<EuiFlexItem grow={false}>
<SourceIcon
serviceType={isIndexing ? 'loadingSmall' : serviceType}
Expand Down Expand Up @@ -157,13 +159,13 @@ export const SourceRow: React.FC<ISourceRowProps> = ({
{showFix && <EuiFlexItem grow={false}>{fixLink}</EuiFlexItem>}
<EuiFlexItem grow={false}>
{showDetails && (
<Link
<EuiLink
className="eui-textNoWrap"
data-test-subj="SourceDetailsLink"
to={getContentSourcePath(SOURCE_DETAILS_PATH, id, !!isOrganization)}
>
Details
</Link>
</EuiLink>
)}
</EuiFlexItem>
</EuiFlexGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('ViewContentHeader', () => {
it('renders with title and alignItems', () => {
const wrapper = shallow(<ViewContentHeader {...props} />);

expect(wrapper.find('h2').text()).toEqual('Header');
expect(wrapper.find('h3').text()).toEqual('Header');
expect(wrapper.find(EuiFlexGroup).prop('alignItems')).toEqual('flexStart');
});

Expand All @@ -35,4 +35,20 @@ describe('ViewContentHeader', () => {

expect(wrapper.find('.action')).toHaveLength(1);
});

it('renders small heading', () => {
const wrapper = shallow(
<ViewContentHeader titleSize="s" {...props} action={<div className="action" />} />
);

expect(wrapper.find('h4')).toHaveLength(1);
});

it('renders large heading', () => {
const wrapper = shallow(
<ViewContentHeader titleSize="l" {...props} action={<div className="action" />} />
);

expect(wrapper.find('h2')).toHaveLength(1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -15,28 +15,44 @@ interface IViewContentHeaderProps {
description?: React.ReactNode;
action?: React.ReactNode;
alignItems?: FlexGroupAlignItems;
titleSize?: 's' | 'm' | 'l';
}

export const ViewContentHeader: React.FC<IViewContentHeaderProps> = ({
title,
titleSize = 'm',
description,
action,
alignItems = 'center',
}) => (
<>
<EuiFlexGroup alignItems={alignItems} justifyContent="spaceBetween">
<EuiFlexItem>
<EuiTitle size="m">
<h2>{title}</h2>
</EuiTitle>
{description && (
<EuiText grow={false}>
<p>{description}</p>
</EuiText>
)}
</EuiFlexItem>
{action && <EuiFlexItem grow={false}>{action}</EuiFlexItem>}
</EuiFlexGroup>
<EuiSpacer />
</>
);
}) => {
let titleElement;

switch (titleSize) {
case 's':
titleElement = <h4>{title}</h4>;
break;
case 'l':
titleElement = <h2>{title}</h2>;
break;
default:
titleElement = <h3>{title}</h3>;
break;
}

return (
<>
<EuiFlexGroup alignItems={alignItems} justifyContent="spaceBetween">
<EuiFlexItem>
<EuiTitle size={titleSize}>{titleElement}</EuiTitle>
{description && (
<EuiText grow={false} color="subdued">
<p>{description}</p>
</EuiText>
)}
</EuiFlexItem>
{action && <EuiFlexItem grow={false}>{action}</EuiFlexItem>}
</EuiFlexGroup>
<EuiSpacer />
</>
);
};
Loading

0 comments on commit 197510a

Please sign in to comment.