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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
48b9c1e
Convert Settings to new page template
cee-chen Jun 18, 2021
bf5aee3
Convert Credentials to new page template
cee-chen Jun 18, 2021
b7009bc
[Tests refactor] DRY out repeated ability tests to a helper
cee-chen Jun 18, 2021
1fc987e
Merge branch 'master' into kibana-page-template-as-1
kibanamachine Jun 21, 2021
eea67d6
Merge branch 'master' into kibana-page-template-as-1
constancecchen Jun 21, 2021
70f776c
Merge branch 'master' into kibana-page-template-as-1
constancecchen Jun 21, 2021
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
5 changes: 3 additions & 2 deletions ...erprise_search/public/applications/app_search/components/credentials/credentials.test.tsx
View file
Open in desktop
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
133 changes: 64 additions & 69 deletions ...s/enterprise_search/public/applications/app_search/components/credentials/credentials.tsx
View file
Open in desktop
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
Contributor 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>
);
};
14 changes: 11 additions & 3 deletions .../public/applications/app_search/components/settings/log_retention/log_retention_panel.tsx
View file
Open in desktop
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>
);
};
4 changes: 2 additions & 2 deletions ...ns/enterprise_search/public/applications/app_search/components/settings/settings.test.tsx
View file
Open in desktop
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);
});
});
20 changes: 5 additions & 15 deletions ...plugins/enterprise_search/public/applications/app_search/components/settings/settings.tsx
View file
Open in desktop
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>
);
};
66 changes: 22 additions & 44 deletions x-pack/plugins/enterprise_search/public/applications/app_search/index.test.tsx
View file
Open in desktop
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 @@ -103,52 +105,28 @@ describe('AppSearchConfigured', () => {
expect(wrapper.find(Layout).first().prop('readOnlyMode')).toEqual(true);
});

describe('ability checks', () => {
describe('canViewRoleMappings', () => {
it('renders RoleMappings when canViewRoleMappings is true', () => {
setMockValues({ myRole: { canViewRoleMappings: true } });
rerender(wrapper);
expect(wrapper.find(RoleMappings)).toHaveLength(1);
describe('routes with ability checks', () => {
const runRouteAbilityCheck = (routeAbility: string, View: React.FC) => {
describe(View.name, () => {
it(`renders ${View.name} when user ${routeAbility} is true`, () => {
setMockValues({ myRole: { [routeAbility]: true } });
rerender(wrapper);
expect(wrapper.find(View)).toHaveLength(1);
});

it(`does not render ${View.name} when user ${routeAbility} is false`, () => {
setMockValues({ myRole: { [routeAbility]: false } });
rerender(wrapper);
expect(wrapper.find(View)).toHaveLength(0);
});
});
};

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

describe('canManageEngines', () => {
it('renders EngineCreation when user canManageEngines is true', () => {
setMockValues({ myRole: { canManageEngines: true } });
rerender(wrapper);

expect(wrapper.find(EngineCreation)).toHaveLength(1);
});

it('does not render EngineCreation when user canManageEngines is false', () => {
setMockValues({ myRole: { canManageEngines: false } });
rerender(wrapper);

expect(wrapper.find(EngineCreation)).toHaveLength(0);
});
});

describe('canManageMetaEngines', () => {
it('renders MetaEngineCreation when user canManageMetaEngines is true', () => {
setMockValues({ myRole: { canManageMetaEngines: true } });
rerender(wrapper);

expect(wrapper.find(MetaEngineCreation)).toHaveLength(1);
});

it('does not render MetaEngineCreation when user canManageMetaEngines is false', () => {
setMockValues({ myRole: { canManageMetaEngines: false } });
rerender(wrapper);

expect(wrapper.find(MetaEngineCreation)).toHaveLength(0);
});
});
runRouteAbilityCheck('canViewSettings', Settings);
runRouteAbilityCheck('canViewAccountCredentials', Credentials);
runRouteAbilityCheck('canViewRoleMappings', RoleMappings);
runRouteAbilityCheck('canManageEngines', EngineCreation);
runRouteAbilityCheck('canManageMetaEngines', MetaEngineCreation);
});

describe('library', () => {
Expand Down
Loading