Skip to content

Commit

Permalink
[Workplace Search] Initial rendering of Org Sources (#84164)
Browse files Browse the repository at this point in the history
* Fix broken routes

Didn’t have a way to test these when created

* Get context from global state

No need to do this in 2 places now. There was a race condition where the default logic value for `isOrganization` was set to `false` We don’t need a useEffect call here because the value is synchronous and has no side effects. Calling the method directly fixes the race condition.

* Add the ‘path’ to the logic files for easier debugging

* Add SourceSubNav component

* Flip routes to match new convention

It was decided by Product that instead of keying off of `/org` to determine context, that we would now flip it where we key of provate with `/p`.

This means that /sources is now organization where before it was personal

* Convert routers to use children instead of props

This aligns with App Search and allows for easier telemtry and breadcrumbs

* Add breadcrumbs and basic telemetry

* Add in and refactor subnavigation

As a part of this commit, the approach for rendering subnavs was refactored to align with App Search.

There was a bug where some components weren’t rendering properly because the SourceLogic and GroupsLogic files were never unmounting. The reason for this is the subnav components use their respective logic files to get the IDs needed for rendering the subnav links. That is, SourceSubNav would call SourceLogic to get the ID to render the links and would stay rendered for the duration of the user’s time in the app. The result is that users would leave the source details page and navigate to add a new source and the logic file would never reset to a loading state and the UI would break.

The fix was to borrow from the pattern App Search uses and pass the subnavs as props. Because App Search only uses a single engines subnav, they only needed one prop. We use multiple props for each subnav.

Also, the subnav should not be rendered on the root routes (/sources, /p/sources, and /groups) so conditionals were added to only render the subnavs when not on those root routes.

* Add FlashMessages

* Fix some failed tests

Missed this in first commit

* Update SourceIcon to use EuiIcon

Before this change, the legacy styles were not ported over. This gives a uniform size for both wrapped and unwrapped icons. The icons are a bit smaller on the add source page but Eui has lowered it’s largest size ‘xxl’ and we would need to write manual overrides. IMO the change is not significant enough to override.

* Fix broken icons

* Replace legacy div with component

The eui.css file in ent-search is no longer up to date with current EUI and this was broken. The best fix was to use the component that renders as expected

* Add base styles for Sources

More in a future PR but this makes the majority of things look correct.

* Cleanup

Fix some type errors and rename constants

* Couple more failing tests

We have multiple `Layouts` now with the new subnavs

* Fix prepare routes

Like the first commit, missed these when porting over routes with no UI.

* Clean up the desgin of the source connect screen

The columns were way off in Kibana

* Remove ORG_PATH const

No longer needed since ‘/org’ is gone
  • Loading branch information
scottybollinger authored Nov 24, 2020
1 parent 5f844bf commit a12bb04
Show file tree
Hide file tree
Showing 25 changed files with 332 additions and 157 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,29 +11,32 @@ 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 { NAV } from '../../constants';

import {
ORG_SOURCES_PATH,
SOURCES_PATH,
SECURITY_PATH,
ROLE_MAPPINGS_PATH,
GROUPS_PATH,
ORG_SETTINGS_PATH,
} from '../../routes';

export const WorkplaceSearchNav: React.FC = () => {
interface Props {
sourcesSubNav?: React.ReactNode;
groupsSubNav?: React.ReactNode;
}

export const WorkplaceSearchNav: React.FC<Props> = ({ sourcesSubNav, groupsSubNav }) => {
// TODO: icons
return (
<SideNav product={WORKPLACE_SEARCH_PLUGIN}>
<SideNavLink to="/" isRoot>
{NAV.OVERVIEW}
</SideNavLink>
<SideNavLink isExternal to={getWorkplaceSearchUrl(ORG_SOURCES_PATH)}>
<SideNavLink to={SOURCES_PATH} subNav={sourcesSubNav}>
{NAV.SOURCES}
</SideNavLink>
<SideNavLink to={GROUPS_PATH} subNav={<GroupSubNav />}>
<SideNavLink to={GROUPS_PATH} subNav={groupsSubNav}>
{NAV.GROUPS}
</SideNavLink>
<SideNavLink isExternal to={getWorkplaceSearchUrl(`#${ROLE_MAPPINGS_PATH}`)}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,27 @@ import zendesk from './zendesk.svg';
export const images = {
box,
confluence,
confluenceCloud: confluence,
confluenceServer: confluence,
crawler,
custom,
drive,
dropbox,
github,
githubEnterpriseServer: github,
gmail,
googleDrive,
google,
jira,
jiraServer,
jiraCloud: jira,
loadingSmall,
office365,
oneDrive,
outlook,
people,
salesforce,
salesforceSandbox: salesforce,
serviceNow,
sharePoint,
slack,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* 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.
*/

.wrapped-icon {
width: 30px;
height: 30px;
overflow: hidden;
margin-right: 4px;
position: relative;
display: flex;
justify-content: center;
align-items: center;

img {
max-width: 100%;
max-height: 100%;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,21 @@
import React from 'react';
import { shallow } from 'enzyme';

import { EuiIcon } from '@elastic/eui';

import { SourceIcon } from './';

describe('SourceIcon', () => {
it('renders unwrapped icon', () => {
const wrapper = shallow(<SourceIcon name="foo" serviceType="custom" />);

expect(wrapper.find('img')).toHaveLength(1);
expect(wrapper.find(EuiIcon)).toHaveLength(1);
expect(wrapper.find('.user-group-source')).toHaveLength(0);
});

it('renders wrapped icon', () => {
const wrapper = shallow(<SourceIcon name="foo" wrapped serviceType="custom" />);

expect(wrapper.find('.user-group-source')).toHaveLength(1);
expect(wrapper.find('.wrapped-icon')).toHaveLength(1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ import React from 'react';

import { camelCase } from 'lodash';

import { EuiIcon } from '@elastic/eui';

import './source_icon.scss';

import { images } from '../assets/source_icons';
import { imagesFull } from '../assets/sources_full_bleed';

Expand All @@ -27,14 +31,15 @@ export const SourceIcon: React.FC<SourceIconProps> = ({
fullBleed = false,
}) => {
const icon = (
<img
src={fullBleed ? imagesFull[camelCase(serviceType)] : images[camelCase(serviceType)]}
alt={name}
<EuiIcon
type={fullBleed ? imagesFull[camelCase(serviceType)] : images[camelCase(serviceType)]}
title={name}
className={className}
size="xxl"
/>
);
return wrapped ? (
<div className="user-group-source" title={name}>
<div className="wrapped-icon" title={name}>
{icon}
</div>
) : (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,27 @@ export const NAV = {
'xpack.enterpriseSearch.workplaceSearch.nav.groups.sourcePrioritization',
{ defaultMessage: 'Source Prioritization' }
),
CONTENT: i18n.translate('xpack.enterpriseSearch.workplaceSearch.nav.content', {
defaultMessage: 'Content',
}),
ROLE_MAPPINGS: i18n.translate('xpack.enterpriseSearch.workplaceSearch.nav.roleMappings', {
defaultMessage: 'Role Mappings',
}),
SECURITY: i18n.translate('xpack.enterpriseSearch.workplaceSearch.nav.security', {
defaultMessage: 'Security',
}),
SCHEMA: i18n.translate('xpack.enterpriseSearch.workplaceSearch.nav.schema', {
defaultMessage: 'Schema',
}),
DISPLAY_SETTINGS: i18n.translate('xpack.enterpriseSearch.workplaceSearch.nav.displaySettings', {
defaultMessage: 'Display Settings',
}),
SETTINGS: i18n.translate('xpack.enterpriseSearch.workplaceSearch.nav.settings', {
defaultMessage: 'Settings',
}),
ADD_SOURCE: i18n.translate('xpack.enterpriseSearch.workplaceSearch.nav.addSource', {
defaultMessage: 'Add Source',
}),
PERSONAL_DASHBOARD: i18n.translate(
'xpack.enterpriseSearch.workplaceSearch.nav.personalDashboard',
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ describe('WorkplaceSearchConfigured', () => {
it('renders layout and header actions', () => {
const wrapper = shallow(<WorkplaceSearchConfigured />);

expect(wrapper.find(Layout).prop('readOnlyMode')).toBeFalsy();
expect(wrapper.find(Layout).first().prop('readOnlyMode')).toBeFalsy();
expect(wrapper.find(Overview)).toHaveLength(1);
expect(mockKibanaValues.renderHeaderActions).toHaveBeenCalledWith(WorkplaceSearchHeaderActions);
});
Expand Down Expand Up @@ -90,6 +90,6 @@ describe('WorkplaceSearchConfigured', () => {

const wrapper = shallow(<WorkplaceSearchConfigured />);

expect(wrapper.find(Layout).prop('readOnlyMode')).toEqual(true);
expect(wrapper.find(Layout).first().prop('readOnlyMode')).toEqual(true);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,17 @@ import { AppLogic } from './app_logic';
import { Layout } from '../shared/layout';
import { WorkplaceSearchNav, WorkplaceSearchHeaderActions } from './components/layout';

import { GROUPS_PATH, SETUP_GUIDE_PATH } from './routes';
import { GROUPS_PATH, SETUP_GUIDE_PATH, SOURCES_PATH, PERSONAL_SOURCES_PATH } from './routes';

import { SetupGuide } from './views/setup_guide';
import { ErrorState } from './views/error_state';
import { NotFound } from '../shared/not_found';
import { Overview } from './views/overview';
import { GroupsRouter } from './views/groups';
import { SourcesRouter } from './views/content_sources';

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

export const WorkplaceSearch: React.FC<InitialAppData> = (props) => {
const { config } = useValues(KibanaLogic);
Expand All @@ -37,6 +41,10 @@ export const WorkplaceSearchConfigured: React.FC<InitialAppData> = (props) => {

const { pathname } = useLocation();

// We don't want so show the subnavs on the container root pages.
const showSourcesSubnav = pathname !== SOURCES_PATH && pathname !== PERSONAL_SOURCES_PATH;
const showGroupsSubnav = pathname !== GROUPS_PATH;

/**
* Personal dashboard urls begin with /p/
* EX: http://localhost:5601/app/enterprise_search/workplace_search/p/sources
Expand All @@ -45,6 +53,7 @@ export const WorkplaceSearchConfigured: React.FC<InitialAppData> = (props) => {

// TODO: Once auth is figured out, we need to have a check for the equivilent of `isAdmin`.
const isOrganization = !pathname.match(personalSourceUrlRegex);
setContext(isOrganization);

useEffect(() => {
if (!hasInitialized) {
Expand All @@ -53,10 +62,6 @@ export const WorkplaceSearchConfigured: React.FC<InitialAppData> = (props) => {
}
}, [hasInitialized]);

useEffect(() => {
setContext(isOrganization);
}, [isOrganization]);

return (
<Switch>
<Route path={SETUP_GUIDE_PATH}>
Expand All @@ -65,19 +70,32 @@ export const WorkplaceSearchConfigured: React.FC<InitialAppData> = (props) => {
<Route exact path="/">
{errorConnecting ? <ErrorState /> : <Overview />}
</Route>
<Route path={SOURCES_PATH}>
<Layout
navigation={<WorkplaceSearchNav sourcesSubNav={showSourcesSubnav && <SourceSubNav />} />}
restrictWidth
readOnlyMode={readOnlyMode}
>
<SourcesRouter />
</Layout>
</Route>
<Route path={GROUPS_PATH}>
<Layout
navigation={<WorkplaceSearchNav groupsSubNav={showGroupsSubnav && <GroupSubNav />} />}
restrictWidth
readOnlyMode={readOnlyMode}
>
<GroupsRouter />
</Layout>
</Route>
<Route>
<Layout navigation={<WorkplaceSearchNav />} restrictWidth readOnlyMode={readOnlyMode}>
{errorConnecting ? (
<ErrorState />
) : (
<Switch>
<Route path={GROUPS_PATH}>
<GroupsRouter />
</Route>
<Route>
<NotFound product={WORKPLACE_SEARCH_PLUGIN} />
</Route>
</Switch>
<Route>
<NotFound product={WORKPLACE_SEARCH_PLUGIN} />
</Route>
)}
</Layout>
</Route>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { EuiLink } from '@elastic/eui';
import {
getContentSourcePath,
SOURCES_PATH,
ORG_SOURCES_PATH,
PERSONAL_SOURCES_PATH,
SOURCE_DETAILS_PATH,
} from './routes';

Expand All @@ -26,13 +26,13 @@ describe('getContentSourcePath', () => {
const wrapper = shallow(<TestComponent id="123" isOrg />);
const path = wrapper.find(EuiLink).prop('href');

expect(path).toEqual(`${ORG_SOURCES_PATH}/123`);
expect(path).toEqual(`${SOURCES_PATH}/123`);
});

it('should format user route', () => {
const wrapper = shallow(<TestComponent id="123" />);
const path = wrapper.find(EuiLink).prop('href');

expect(path).toEqual(`${SOURCES_PATH}/123`);
expect(path).toEqual(`${PERSONAL_SOURCES_PATH}/123`);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -44,21 +44,21 @@ export const CUSTOM_API_DOCS_URL = `${DOCS_PREFIX}/workplace-search-custom-sourc
export const CUSTOM_API_DOCUMENT_PERMISSIONS_DOCS_URL = `${CUSTOM_SOURCE_DOCS_URL}#custom-api-source-document-level-access-control`;
export const ENT_SEARCH_LICENSE_MANAGEMENT = `${ENT_SEARCH_DOCS_PREFIX}/license-management.html`;

export const ORG_PATH = '/org';
export const PERSONAL_PATH = '/p';

export const ROLE_MAPPINGS_PATH = `${ORG_PATH}/role-mappings`;
export const ROLE_MAPPINGS_PATH = '/role-mappings';
export const ROLE_MAPPING_PATH = `${ROLE_MAPPINGS_PATH}/:roleId`;
export const ROLE_MAPPING_NEW_PATH = `${ROLE_MAPPINGS_PATH}/new`;

export const USERS_PATH = `${ORG_PATH}/users`;
export const SECURITY_PATH = `${ORG_PATH}/security`;
export const USERS_PATH = '/users';
export const SECURITY_PATH = '/security';

export const GROUPS_PATH = '/groups';
export const GROUP_PATH = `${GROUPS_PATH}/:groupId`;
export const GROUP_SOURCE_PRIORITIZATION_PATH = `${GROUPS_PATH}/:groupId/source_prioritization`;

export const SOURCES_PATH = '/sources';
export const ORG_SOURCES_PATH = `${ORG_PATH}${SOURCES_PATH}`;
export const PERSONAL_SOURCES_PATH = `${PERSONAL_PATH}${SOURCES_PATH}`;

export const SOURCE_ADDED_PATH = `${SOURCES_PATH}/added`;
export const ADD_SOURCE_PATH = `${SOURCES_PATH}/add`;
Expand All @@ -81,7 +81,7 @@ export const ADD_SLACK_PATH = `${SOURCES_PATH}/add/slack`;
export const ADD_ZENDESK_PATH = `${SOURCES_PATH}/add/zendesk`;
export const ADD_CUSTOM_PATH = `${SOURCES_PATH}/add/custom`;

export const PERSONAL_SETTINGS_PATH = '/settings';
export const PERSONAL_SETTINGS_PATH = `${PERSONAL_PATH}/settings`;

export const SOURCE_DETAILS_PATH = `${SOURCES_PATH}/:sourceId`;
export const SOURCE_CONTENT_PATH = `${SOURCES_PATH}/:sourceId/content`;
Expand All @@ -93,7 +93,7 @@ export const REINDEX_JOB_PATH = `${SOURCES_PATH}/:sourceId/schema-errors/:active
export const DISPLAY_SETTINGS_SEARCH_RESULT_PATH = `${SOURCE_DISPLAY_SETTINGS_PATH}/`;
export const DISPLAY_SETTINGS_RESULT_DETAIL_PATH = `${SOURCE_DISPLAY_SETTINGS_PATH}/result-detail`;

export const ORG_SETTINGS_PATH = `${ORG_PATH}/settings`;
export const ORG_SETTINGS_PATH = '/settings';
export const ORG_SETTINGS_CUSTOMIZE_PATH = `${ORG_SETTINGS_PATH}/customize`;
export const ORG_SETTINGS_CONNECTORS_PATH = `${ORG_SETTINGS_PATH}/connectors`;
export const ORG_SETTINGS_OAUTH_APPLICATION_PATH = `${ORG_SETTINGS_PATH}/oauth`;
Expand All @@ -120,9 +120,9 @@ export const getContentSourcePath = (
path: string,
sourceId: string,
isOrganization: boolean
): string => generatePath(isOrganization ? ORG_PATH + path : path, { sourceId });
export const getGroupPath = (groupId: string) => generatePath(GROUP_PATH, { groupId });
export const getGroupSourcePrioritizationPath = (groupId: string) =>
): string => generatePath(isOrganization ? path : `${PERSONAL_PATH}${path}`, { sourceId });
export const getGroupPath = (groupId: string): string => generatePath(GROUP_PATH, { groupId });
export const getGroupSourcePrioritizationPath = (groupId: string): string =>
`${GROUPS_PATH}/${groupId}/source_prioritization`;
export const getSourcesPath = (path: string, isOrganization: boolean) =>
isOrganization ? `${ORG_PATH}${path}` : path;
export const getSourcesPath = (path: string, isOrganization: boolean): string =>
isOrganization ? path : `${PERSONAL_PATH}${path}`;
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
EuiFlexGrid,
EuiFlexGroup,
EuiFlexItem,
EuiPanel,
EuiSpacer,
EuiText,
EuiTitle,
Expand Down Expand Up @@ -57,7 +58,7 @@ export const ConfiguredSourcesList: React.FC<ConfiguredSourcesProps> = ({
{sources.map(({ name, serviceType, addPath, connected, accountContextOnly }, i) => (
<React.Fragment key={i}>
<EuiFlexItem>
<div className="source-card-configured euiCard">
<EuiPanel paddingSize="s">
<EuiFlexGroup alignItems="center" gutterSize="none" responsive={false}>
<EuiFlexItem>
<EuiFlexGroup
Expand Down Expand Up @@ -95,7 +96,7 @@ export const ConfiguredSourcesList: React.FC<ConfiguredSourcesProps> = ({
</EuiFlexItem>
)}
</EuiFlexGroup>
</div>
</EuiPanel>
</EuiFlexItem>
</React.Fragment>
))}
Expand Down
Loading

0 comments on commit a12bb04

Please sign in to comment.