Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[App Search] Convert Settings & Credentials pages to new page template #102671

Merged
merged 6 commits into from
Jun 21, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,15 @@ import React from 'react';

import { shallow } from 'enzyme';

import { EuiCopy, EuiLoadingContent, EuiPageContentBody } from '@elastic/eui';
import { EuiCopy, EuiLoadingContent } from '@elastic/eui';

import { DEFAULT_META } from '../../../shared/constants';
import { externalUrl } from '../../../shared/enterprise_search_url';

import { Credentials } from './credentials';

import { CredentialsFlyout } from './credentials_flyout';
import { CredentialsList } from './credentials_list';

describe('Credentials', () => {
// Kea mocks
Expand All @@ -42,7 +43,7 @@ describe('Credentials', () => {

it('renders', () => {
const wrapper = shallow(<Credentials />);
expect(wrapper.find(EuiPageContentBody)).toHaveLength(1);
expect(wrapper.find(CredentialsList)).toHaveLength(1);
});

it('fetches data on mount', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@ import React, { useEffect } from 'react';
import { useActions, useValues } from 'kea';

import {
EuiPageHeader,
EuiTitle,
EuiPageContentBody,
EuiPanel,
EuiCopy,
EuiButtonIcon,
Expand All @@ -25,8 +23,7 @@ import {
import { i18n } from '@kbn/i18n';

import { externalUrl } from '../../../shared/enterprise_search_url/external_url';
import { FlashMessages } from '../../../shared/flash_messages';
import { SetAppSearchChrome as SetPageChrome } from '../../../shared/kibana_chrome';
import { AppSearchPageTemplate } from '../layout';

import { CREDENTIALS_TITLE } from './constants';
import { CredentialsFlyout } from './credentials_flyout';
Expand All @@ -52,74 +49,72 @@ export const Credentials: React.FC = () => {
}, []);

return (
<>
<SetPageChrome trail={[CREDENTIALS_TITLE]} />
<EuiPageHeader pageTitle={CREDENTIALS_TITLE} />
<EuiPageContentBody>
{shouldShowCredentialsForm && <CredentialsFlyout />}
<EuiPanel hasBorder className="eui-textCenter">
<EuiTitle size="s">
<AppSearchPageTemplate
pageChrome={[CREDENTIALS_TITLE]}
pageHeader={{ pageTitle: CREDENTIALS_TITLE }}
>
{shouldShowCredentialsForm && <CredentialsFlyout />}
<EuiPanel color="subdued" className="eui-textCenter">
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 quick notes,

  1. This large diff block is mostly whitespace diff shenanigans, so don't forget to disable those
  2. I did a general panel pass with Davey in light of the new page template header having a bottom border, and he approved converting this panel from bordered to subdued

<EuiTitle size="s">
<h2>
{i18n.translate('xpack.enterpriseSearch.appSearch.credentials.apiEndpoint', {
defaultMessage: 'Endpoint',
})}
</h2>
</EuiTitle>
<EuiCopy
textToCopy={externalUrl.enterpriseSearchUrl}
afterMessage={i18n.translate('xpack.enterpriseSearch.appSearch.credentials.copied', {
defaultMessage: 'Copied',
})}
>
{(copy) => (
<>
<EuiButtonIcon
onClick={copy}
iconType="copyClipboard"
aria-label={i18n.translate(
'xpack.enterpriseSearch.appSearch.credentials.copyApiEndpoint',
{
defaultMessage: 'Copy API Endpoint to clipboard.',
}
)}
/>
{externalUrl.enterpriseSearchUrl}
</>
)}
</EuiCopy>
</EuiPanel>
<EuiSpacer size="xxl" />
<EuiPageContentHeader responsive={false}>
<EuiPageContentHeaderSection>
<EuiTitle size="m">
<h2>
{i18n.translate('xpack.enterpriseSearch.appSearch.credentials.apiEndpoint', {
defaultMessage: 'Endpoint',
{i18n.translate('xpack.enterpriseSearch.appSearch.credentials.apiKeys', {
defaultMessage: 'API Keys',
})}
</h2>
</EuiTitle>
<EuiCopy
textToCopy={externalUrl.enterpriseSearchUrl}
afterMessage={i18n.translate('xpack.enterpriseSearch.appSearch.credentials.copied', {
defaultMessage: 'Copied',
})}
>
{(copy) => (
<>
<EuiButtonIcon
onClick={copy}
iconType="copyClipboard"
aria-label={i18n.translate(
'xpack.enterpriseSearch.appSearch.credentials.copyApiEndpoint',
{
defaultMessage: 'Copy API Endpoint to clipboard.',
}
)}
/>
{externalUrl.enterpriseSearchUrl}
</>
)}
</EuiCopy>
</EuiPanel>
<EuiSpacer size="xxl" />
<EuiPageContentHeader responsive={false}>
<EuiPageContentHeaderSection>
<EuiTitle size="m">
<h2>
{i18n.translate('xpack.enterpriseSearch.appSearch.credentials.apiKeys', {
defaultMessage: 'API Keys',
})}
</h2>
</EuiTitle>
</EuiPageContentHeaderSection>
<EuiPageContentHeaderSection>
{!dataLoading && (
<EuiButton
color="primary"
data-test-subj="CreateAPIKeyButton"
fill
onClick={() => showCredentialsForm()}
>
{i18n.translate('xpack.enterpriseSearch.appSearch.credentials.createKey', {
defaultMessage: 'Create a key',
})}
</EuiButton>
)}
</EuiPageContentHeaderSection>
</EuiPageContentHeader>
<EuiSpacer size="m" />
<FlashMessages />
<EuiPanel hasBorder>
{!!dataLoading ? <EuiLoadingContent lines={3} /> : <CredentialsList />}
</EuiPanel>
</EuiPageContentBody>
</>
</EuiPageContentHeaderSection>
<EuiPageContentHeaderSection>
{!dataLoading && (
<EuiButton
color="primary"
data-test-subj="CreateAPIKeyButton"
fill
onClick={() => showCredentialsForm()}
>
{i18n.translate('xpack.enterpriseSearch.appSearch.credentials.createKey', {
defaultMessage: 'Create a key',
})}
</EuiButton>
)}
</EuiPageContentHeaderSection>
</EuiPageContentHeader>
<EuiSpacer size="m" />
<EuiPanel hasBorder>
{!!dataLoading ? <EuiLoadingContent lines={3} /> : <CredentialsList />}
</EuiPanel>
</AppSearchPageTemplate>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,15 @@ import React, { useEffect } from 'react';

import { useActions, useValues } from 'kea';

import { EuiLink, EuiSpacer, EuiSwitch, EuiText, EuiTextColor, EuiTitle } from '@elastic/eui';
import {
EuiPanel,
EuiLink,
EuiSpacer,
EuiSwitch,
EuiText,
EuiTextColor,
EuiTitle,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';

import { DOCS_PREFIX } from '../../../routes';
Expand All @@ -30,7 +38,7 @@ export const LogRetentionPanel: React.FC = () => {
}, []);

return (
<div data-test-subj="LogRetentionPanel">
<EuiPanel hasBorder data-test-subj="LogRetentionPanel">
<EuiTitle size="s">
<h2>
{i18n.translate('xpack.enterpriseSearch.appSearch.settings.logRetention.title', {
Expand Down Expand Up @@ -104,6 +112,6 @@ export const LogRetentionPanel: React.FC = () => {
data-test-subj="LogRetentionPanelAPISwitch"
/>
</EuiText>
</div>
</EuiPanel>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ import React from 'react';

import { shallow } from 'enzyme';

import { EuiPageContentBody } from '@elastic/eui';
import { LogRetentionPanel } from './log_retention';

import { Settings } from './settings';

describe('Settings', () => {
it('renders', () => {
const wrapper = shallow(<Settings />);
expect(wrapper.find(EuiPageContentBody)).toHaveLength(1);
expect(wrapper.find(LogRetentionPanel)).toHaveLength(1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,17 @@

import React from 'react';

import { EuiPageHeader, EuiPageContent, EuiPageContentBody } from '@elastic/eui';

import { FlashMessages } from '../../../shared/flash_messages';
import { SetAppSearchChrome as SetPageChrome } from '../../../shared/kibana_chrome';
import { AppSearchPageTemplate } from '../layout';

import { LogRetentionPanel, LogRetentionConfirmationModal } from './log_retention';

import { SETTINGS_TITLE } from './';

export const Settings: React.FC = () => {
return (
<>
<SetPageChrome trail={[SETTINGS_TITLE]} />
<EuiPageHeader pageTitle={SETTINGS_TITLE} />
<EuiPageContent hasBorder>
<EuiPageContentBody>
<FlashMessages />
<LogRetentionConfirmationModal />
<LogRetentionPanel />
</EuiPageContentBody>
</EuiPageContent>
</>
<AppSearchPageTemplate pageChrome={[SETTINGS_TITLE]} pageHeader={{ pageTitle: SETTINGS_TITLE }}>
<LogRetentionConfirmationModal />
<LogRetentionPanel />
</AppSearchPageTemplate>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,15 @@ import { rerender } from '../test_helpers';
jest.mock('./app_logic', () => ({ AppLogic: jest.fn() }));
import { AppLogic } from './app_logic';

import { Credentials } from './components/credentials';
import { EngineRouter, EngineNav } from './components/engine';
import { EngineCreation } from './components/engine_creation';
import { EnginesOverview } from './components/engines';
import { ErrorConnecting } from './components/error_connecting';
import { Library } from './components/library';
import { MetaEngineCreation } from './components/meta_engine_creation';
import { RoleMappings } from './components/role_mappings';
import { Settings } from './components/settings';
import { SetupGuide } from './components/setup_guide';

import { AppSearch, AppSearchUnconfigured, AppSearchConfigured, AppSearchNav } from './';
Expand Down Expand Up @@ -104,6 +106,34 @@ describe('AppSearchConfigured', () => {
});

describe('ability checks', () => {
describe('canViewSettings', () => {
it('renders Settings when user canViewSettings is true', () => {
setMockValues({ myRole: { canViewSettings: true } });
rerender(wrapper);
expect(wrapper.find(Settings)).toHaveLength(1);
});

it('does not render Settings when user canViewSettings is false', () => {
setMockValues({ myRole: { canViewSettings: false } });
rerender(wrapper);
expect(wrapper.find(Settings)).toHaveLength(0);
});
});

describe('canViewAccountCredentials', () => {
it('renders Credentials when user canViewAccountCredentials is true', () => {
setMockValues({ myRole: { canViewAccountCredentials: true } });
rerender(wrapper);
expect(wrapper.find(Credentials)).toHaveLength(1);
});

it('does not render Credentials when user canViewAccountCredentials is false', () => {
setMockValues({ myRole: { canViewAccountCredentials: false } });
rerender(wrapper);
expect(wrapper.find(Credentials)).toHaveLength(0);
});
});

describe('canViewRoleMappings', () => {
it('renders RoleMappings when canViewRoleMappings is true', () => {
setMockValues({ myRole: { canViewRoleMappings: true } });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,13 @@ export const AppSearchUnconfigured: React.FC = () => (

export const AppSearchConfigured: React.FC<Required<InitialAppData>> = (props) => {
const {
myRole: { canManageEngines, canManageMetaEngines, canViewRoleMappings },
myRole: {
canManageEngines,
canManageMetaEngines,
canViewSettings,
canViewAccountCredentials,
canViewRoleMappings,
},
} = useValues(AppLogic(props));
const { renderHeaderActions } = useValues(KibanaLogic);
const { readOnlyMode } = useValues(HttpLogic);
Expand All @@ -92,6 +98,16 @@ export const AppSearchConfigured: React.FC<Required<InitialAppData>> = (props) =
<Library />
</Route>
)}
{canViewSettings && (
<Route exact path={SETTINGS_PATH}>
<Settings />
</Route>
)}
{canViewAccountCredentials && (
<Route exact path={CREDENTIALS_PATH}>
<Credentials />
</Route>
)}
{canViewRoleMappings && (
<Route path={ROLE_MAPPINGS_PATH}>
<RoleMappings />
Expand All @@ -109,12 +125,6 @@ export const AppSearchConfigured: React.FC<Required<InitialAppData>> = (props) =
<Route path={ENGINE_PATH}>
<EngineRouter />
</Route>
<Route exact path={SETTINGS_PATH}>
<Settings />
</Route>
<Route exact path={CREDENTIALS_PATH}>
<Credentials />
</Route>
{canManageEngines && (
<Route exact path={ENGINE_CREATION_PATH}>
<EngineCreation />
Expand Down