Fix ilm navigation (#81664)

* Fix edit policy page navigation

* Fix edit policy page navigation

* Add links to PR for explanation

* Added more tests and linked to a github issue about navigation issues

* Fix decoding function for undefined values

* Fix type check issues

* Renamed dollar sign to percent sign, added a method for (double) encoded paths and better description in test names

* Deleted Index Management from required bundles in ILM

* Fixed merge conflicts

* Revert "Deleted Index Management from required bundles in ILM"

This reverts commit 5a735dfe

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Yulia Čech 2020-11-10 11:09:37 +01:00 committed by GitHub
parent bafe9dfea1
commit 1de3a02a46
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
22 changed files with 448 additions and 43 deletions

View file

@ -19,14 +19,51 @@
import { attemptToURIDecode } from './attempt_to_uri_decode';
// this function doesn't work for % with other special chars or sequence %25
// known issue https://github.com/elastic/kibana/issues/82440
test('decodes an encoded string', () => {
const encodedString = 'test%3F';
expect(attemptToURIDecode(encodedString)).toBe('test?');
const originalName = 'test;,/?:@&=+$#';
const encodedName = encodeURIComponent(originalName);
// react router v5 automatically decodes route match params
const reactRouterDecoded = decodeURI(encodedName);
expect(attemptToURIDecode(encodedName)).toBe(originalName);
expect(attemptToURIDecode(reactRouterDecoded)).toBe(originalName);
});
// react router partially decodes %25 sequence to % in match params
// https://github.com/elastic/kibana/pull/81664
test('ignores the error if a string is already decoded', () => {
const decodedString = 'test%';
expect(attemptToURIDecode(decodedString)).toBe(decodedString);
const originalName = 'test%';
const encodedName = encodeURIComponent(originalName);
// react router v5 automatically decodes route match params
const reactRouterDecoded = decodeURI(encodedName);
expect(attemptToURIDecode(encodedName)).toBe(originalName);
expect(attemptToURIDecode(reactRouterDecoded)).toBe(originalName);
});
test('returns wrong decoded value for %25 sequence', () => {
const originalName = 'test%25';
const encodedName = encodeURIComponent(originalName);
// react router v5 automatically decodes route match params
const reactRouterDecoded = decodeURI(encodedName);
expect(attemptToURIDecode(encodedName)).toBe(originalName);
expect(attemptToURIDecode(reactRouterDecoded)).not.toBe(originalName);
});
test('returns wrong decoded value for % with other escaped characters', () => {
const originalName = 'test%?#';
const encodedName = encodeURIComponent(originalName);
// react router v5 automatically decodes route match params
const reactRouterDecoded = decodeURI(encodedName);
expect(attemptToURIDecode(encodedName)).toBe(originalName);
expect(attemptToURIDecode(reactRouterDecoded)).not.toBe(originalName);
});
test("doesn't convert undefined to a string", () => {
expect(attemptToURIDecode(undefined)).toBeUndefined();
});

View file

@ -19,12 +19,14 @@
/*
* Use this function with any match params coming from react router to safely decode values.
* https://github.com/elastic/kibana/pull/81664
* After an update to react router v6, this functions should be deprecated.
* Known issue for navigation with special characters in paths
* https://github.com/elastic/kibana/issues/82440
*/
export const attemptToURIDecode = (value: string) => {
export const attemptToURIDecode = (value?: string): string | undefined => {
let result = value;
try {
result = decodeURIComponent(value);
result = value ? decodeURIComponent(value) : value;
} catch (e) {
// do nothing
}

View file

@ -0,0 +1,74 @@
/*
* 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 React from 'react';
import { act } from 'react-dom/test-utils';
import { registerTestBed, TestBed, TestBedConfig } from '../../../../../test_utils';
import { App } from '../../../public/application/app';
import { TestSubjects } from '../helpers';
import { createBreadcrumbsMock } from '../../../public/application/services/breadcrumbs.mock';
import { KibanaContextProvider } from '../../../../../../src/plugins/kibana_react/public/context';
const breadcrumbService = createBreadcrumbsMock();
const AppWithContext = (props: any) => {
return (
<KibanaContextProvider services={{ breadcrumbService }}>
<App {...props} />
</KibanaContextProvider>
);
};
const getTestBedConfig = (initialEntries: string[]): TestBedConfig => ({
memoryRouter: {
initialEntries,
},
defaultProps: {
getUrlForApp: () => {},
navigateToApp: () => {},
},
});
const initTestBed = (initialEntries: string[]) =>
registerTestBed(AppWithContext, getTestBedConfig(initialEntries))();
export interface AppTestBed extends TestBed<TestSubjects> {
actions: {
clickPolicyNameLink: () => void;
clickCreatePolicyButton: () => void;
};
}
export const setup = async (initialEntries: string[]): Promise<AppTestBed> => {
const testBed = await initTestBed(initialEntries);
const clickPolicyNameLink = async () => {
const { component, find } = testBed;
await act(async () => {
find('policyTablePolicyNameLink').simulate('click', { button: 0 });
});
component.update();
};
const clickCreatePolicyButton = async () => {
const { component, find } = testBed;
await act(async () => {
find('createPolicyButton').simulate('click', { button: 0 });
});
component.update();
};
return {
...testBed,
actions: { clickPolicyNameLink, clickCreatePolicyButton },
};
};
export const getEncodedPolicyEditPath = (policyName: string): string =>
`/policies/edit/${encodeURIComponent(policyName)}`;
export const getDoubleEncodedPolicyEditPath = (policyName: string): string =>
encodeURI(getEncodedPolicyEditPath(policyName));

View file

@ -0,0 +1,252 @@
/*
* 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 {
AppTestBed,
getDoubleEncodedPolicyEditPath,
getEncodedPolicyEditPath,
setup,
} from './app.helpers';
import { setupEnvironment } from '../helpers/setup_environment';
import { getDefaultHotPhasePolicy, POLICY_NAME } from '../edit_policy/constants';
import { act } from 'react-dom/test-utils';
const SPECIAL_CHARS_NAME = 'test?#$+=&@:';
const PERCENT_SIGN_NAME = 'test%';
// navigation doesn't work for % with other special chars or sequence %25
// known issue https://github.com/elastic/kibana/issues/82440
const PERCENT_SIGN_WITH_OTHER_CHARS_NAME = 'test%#';
const PERCENT_SIGN_25_SEQUENCE = 'test%25';
window.scrollTo = jest.fn();
describe('<App />', () => {
let testBed: AppTestBed;
const { server, httpRequestsMockHelpers } = setupEnvironment();
afterAll(() => {
server.restore();
});
describe('new policy creation', () => {
test('when there are no policies', async () => {
httpRequestsMockHelpers.setLoadPolicies([]);
await act(async () => {
testBed = await setup(['/']);
});
const { component, actions } = testBed;
component.update();
await actions.clickCreatePolicyButton();
component.update();
expect(testBed.find('policyTitle').text()).toBe(`Create an index lifecycle policy`);
expect(testBed.find('policyNameField').props().value).toBe('');
});
test('when there are policies', async () => {
httpRequestsMockHelpers.setLoadPolicies([getDefaultHotPhasePolicy(POLICY_NAME)]);
await act(async () => {
testBed = await setup(['/']);
});
const { component, actions } = testBed;
component.update();
await actions.clickCreatePolicyButton();
component.update();
expect(testBed.find('policyTitle').text()).toBe(`Create an index lifecycle policy`);
expect(testBed.find('policyNameField').props().value).toBe('');
});
});
describe('navigation with special characters', () => {
beforeAll(async () => {
httpRequestsMockHelpers.setLoadPolicies([getDefaultHotPhasePolicy(SPECIAL_CHARS_NAME)]);
});
test('clicking policy name in the table works', async () => {
await act(async () => {
testBed = await setup(['/']);
});
const { component, actions } = testBed;
component.update();
await actions.clickPolicyNameLink();
component.update();
expect(testBed.find('policyTitle').text()).toBe(
`Edit index lifecycle policy ${SPECIAL_CHARS_NAME}`
);
});
test('loading edit policy page url works', async () => {
await act(async () => {
testBed = await setup([getEncodedPolicyEditPath(SPECIAL_CHARS_NAME)]);
});
const { component } = testBed;
component.update();
expect(testBed.find('policyTitle').text()).toBe(
`Edit index lifecycle policy ${SPECIAL_CHARS_NAME}`
);
});
// using double encoding to counteract react-router's v5 internal decodeURI call
// when those links are open in a new tab, address bar contains double encoded url
test('loading edit policy page url with double encoding works', async () => {
await act(async () => {
testBed = await setup([getDoubleEncodedPolicyEditPath(SPECIAL_CHARS_NAME)]);
});
const { component } = testBed;
component.update();
expect(testBed.find('policyTitle').text()).toBe(
`Edit index lifecycle policy ${SPECIAL_CHARS_NAME}`
);
});
});
describe('navigation with percent sign', () => {
beforeAll(async () => {
httpRequestsMockHelpers.setLoadPolicies([getDefaultHotPhasePolicy(PERCENT_SIGN_NAME)]);
});
test('loading edit policy page url works', async () => {
await act(async () => {
testBed = await setup([getEncodedPolicyEditPath(PERCENT_SIGN_NAME)]);
});
const { component } = testBed;
component.update();
expect(testBed.find('policyTitle').text()).toBe(
`Edit index lifecycle policy ${PERCENT_SIGN_NAME}`
);
});
test('loading edit policy page url with double encoding works', async () => {
await act(async () => {
testBed = await setup([getDoubleEncodedPolicyEditPath(PERCENT_SIGN_NAME)]);
});
const { component } = testBed;
component.update();
expect(testBed.find('policyTitle').text()).toBe(
`Edit index lifecycle policy ${PERCENT_SIGN_NAME}`
);
});
});
describe('navigation with percent sign with other special characters', () => {
beforeAll(async () => {
httpRequestsMockHelpers.setLoadPolicies([
getDefaultHotPhasePolicy(PERCENT_SIGN_WITH_OTHER_CHARS_NAME),
]);
});
test('clicking policy name in the table works', async () => {
await act(async () => {
testBed = await setup(['/']);
});
const { component, actions } = testBed;
component.update();
await actions.clickPolicyNameLink();
component.update();
expect(testBed.find('policyTitle').text()).toBe(
`Edit index lifecycle policy ${PERCENT_SIGN_WITH_OTHER_CHARS_NAME}`
);
});
test("loading edit policy page url doesn't work", async () => {
await act(async () => {
testBed = await setup([getEncodedPolicyEditPath(PERCENT_SIGN_WITH_OTHER_CHARS_NAME)]);
});
const { component } = testBed;
component.update();
// known issue https://github.com/elastic/kibana/issues/82440
expect(testBed.find('policyTitle').text()).not.toBe(
`Edit index lifecycle policy ${PERCENT_SIGN_WITH_OTHER_CHARS_NAME}`
);
});
// using double encoding to counteract react-router's v5 internal decodeURI call
// when those links are open in a new tab, address bar contains double encoded url
test('loading edit policy page url with double encoding works', async () => {
await act(async () => {
testBed = await setup([getDoubleEncodedPolicyEditPath(PERCENT_SIGN_WITH_OTHER_CHARS_NAME)]);
});
const { component } = testBed;
component.update();
expect(testBed.find('policyTitle').text()).toBe(
`Edit index lifecycle policy ${PERCENT_SIGN_WITH_OTHER_CHARS_NAME}`
);
});
});
describe('navigation with %25 sequence', () => {
beforeAll(async () => {
httpRequestsMockHelpers.setLoadPolicies([getDefaultHotPhasePolicy(PERCENT_SIGN_25_SEQUENCE)]);
});
test('clicking policy name in the table works correctly', async () => {
await act(async () => {
testBed = await setup(['/']);
});
const { component, actions } = testBed;
component.update();
await actions.clickPolicyNameLink();
component.update();
expect(testBed.find('policyTitle').text()).toBe(
`Edit index lifecycle policy ${PERCENT_SIGN_25_SEQUENCE}`
);
});
test("loading edit policy page url doesn't work", async () => {
await act(async () => {
testBed = await setup([getEncodedPolicyEditPath(PERCENT_SIGN_25_SEQUENCE)]);
});
const { component } = testBed;
component.update();
// known issue https://github.com/elastic/kibana/issues/82440
expect(testBed.find('policyTitle').text()).not.toBe(
`Edit index lifecycle policy ${PERCENT_SIGN_25_SEQUENCE}`
);
});
// using double encoding to counteract react-router's v5 internal decodeURI call
// when those links are open in a new tab, address bar contains double encoded url
test('loading edit policy page url with double encoding works', async () => {
await act(async () => {
testBed = await setup([getDoubleEncodedPolicyEditPath(PERCENT_SIGN_25_SEQUENCE)]);
});
const { component } = testBed;
component.update();
expect(testBed.find('policyTitle').text()).toBe(
`Edit index lifecycle policy ${PERCENT_SIGN_25_SEQUENCE}`
);
});
});
});

View file

@ -121,6 +121,26 @@ export const DELETE_PHASE_POLICY: PolicyFromES = {
name: POLICY_NAME,
};
export const getDefaultHotPhasePolicy = (policyName: string): PolicyFromES => ({
version: 1,
modified_date: Date.now().toString(),
policy: {
name: policyName,
phases: {
hot: {
min_age: '0ms',
actions: {
rollover: {
max_age: '30d',
max_size: '50gb',
},
},
},
},
},
name: policyName,
});
export const POLICY_WITH_NODE_ATTR_AND_OFF_ALLOCATION: PolicyFromES = {
version: 1,
modified_date: Date.now().toString(),

View file

@ -19,6 +19,7 @@ import {
POLICY_WITH_INCLUDE_EXCLUDE,
POLICY_WITH_NODE_ATTR_AND_OFF_ALLOCATION,
POLICY_WITH_NODE_ROLE_ALLOCATION,
getDefaultHotPhasePolicy,
} from './constants';
window.scrollTo = jest.fn();
@ -33,7 +34,7 @@ describe('<EditPolicy />', () => {
describe('hot phase', () => {
describe('serialization', () => {
beforeEach(async () => {
httpRequestsMockHelpers.setLoadPolicies([DEFAULT_POLICY]);
httpRequestsMockHelpers.setLoadPolicies([getDefaultHotPhasePolicy('my_policy')]);
httpRequestsMockHelpers.setLoadSnapshotPolicies([]);
await act(async () => {

View file

@ -17,5 +17,8 @@ export type TestSubjects =
| 'hot-selectedMaxDocuments'
| 'hot-selectedMaxAge'
| 'hot-selectedMaxAgeUnits'
| 'policyTablePolicyNameLink'
| 'policyTitle'
| 'createPolicyButton'
| 'freezeSwitch'
| string;

View file

@ -15,7 +15,7 @@ import { PolicyTable } from './sections/policy_table';
import { trackUiMetric } from './services/ui_metric';
import { ROUTES } from './services/navigation';
export const App = ({
export const AppWithRouter = ({
history,
navigateToApp,
getUrlForApp,
@ -23,23 +23,33 @@ export const App = ({
history: ScopedHistory;
navigateToApp: ApplicationStart['navigateToApp'];
getUrlForApp: ApplicationStart['getUrlForApp'];
}) => (
<Router history={history}>
<App navigateToApp={navigateToApp} getUrlForApp={getUrlForApp} />
</Router>
);
export const App = ({
navigateToApp,
getUrlForApp,
}: {
navigateToApp: ApplicationStart['navigateToApp'];
getUrlForApp: ApplicationStart['getUrlForApp'];
}) => {
useEffect(() => trackUiMetric(METRIC_TYPE.LOADED, UIM_APP_LOAD), []);
return (
<Router history={history}>
<Switch>
<Redirect exact from="/" to={ROUTES.list} />
<Route
exact
path={ROUTES.list}
render={(props) => <PolicyTable {...props} navigateToApp={navigateToApp} />}
/>
<Route
path={ROUTES.edit}
render={(props) => <EditPolicy {...props} getUrlForApp={getUrlForApp} />}
/>
</Switch>
</Router>
<Switch>
<Redirect exact from="/" to={ROUTES.list} />
<Route
exact
path={ROUTES.list}
render={(props) => <PolicyTable {...props} navigateToApp={navigateToApp} />}
/>
<Route
path={ROUTES.edit}
render={(props) => <EditPolicy {...props} getUrlForApp={getUrlForApp} />}
/>
</Switch>
);
};

View file

@ -12,7 +12,7 @@ import { CloudSetup } from '../../../cloud/public';
import { KibanaContextProvider } from '../shared_imports';
import { App } from './app';
import { AppWithRouter } from './app';
import { BreadcrumbService } from './services/breadcrumbs';
@ -28,7 +28,11 @@ export const renderApp = (
render(
<I18nContext>
<KibanaContextProvider services={{ cloud, breadcrumbService }}>
<App history={history} navigateToApp={navigateToApp} getUrlForApp={getUrlForApp} />
<AppWithRouter
history={history}
navigateToApp={navigateToApp}
getUrlForApp={getUrlForApp}
/>
</KibanaContextProvider>
</I18nContext>,
element

View file

@ -9,7 +9,7 @@ import { RouteComponentProps } from 'react-router-dom';
import { EuiButton, EuiEmptyPrompt, EuiLoadingSpinner } from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n/react';
import { useKibana } from '../../../shared_imports';
import { useKibana, attemptToURIDecode } from '../../../shared_imports';
import { useLoadPoliciesList } from '../../services/api';
import { getPolicyByName } from '../../lib/policies';
@ -90,13 +90,13 @@ export const EditPolicy: React.FunctionComponent<Props & RouteComponentProps<Rou
);
}
const existingPolicy = getPolicyByName(policies, policyName);
const existingPolicy = getPolicyByName(policies, attemptToURIDecode(policyName));
return (
<EditPolicyContextProvider
value={{
isNewPolicy: !existingPolicy?.policy,
policyName,
policyName: attemptToURIDecode(policyName),
policy: existingPolicy?.policy ?? defaultPolicy,
existingPolicies: policies,
getUrlForApp,

View file

@ -135,7 +135,7 @@ export const EditPolicy: React.FunctionComponent<Props> = ({ history }) => {
verticalPosition="center"
horizontalPosition="center"
>
<EuiTitle size="l">
<EuiTitle size="l" data-test-subj="policyTitle">
<h1>
{isNewPolicy
? i18n.translate('xpack.indexLifecycleMgmt.editPolicy.createPolicyMessage', {

View file

@ -32,6 +32,8 @@ export {
TextField,
} from '../../../../src/plugins/es_ui_shared/static/forms/components';
export { attemptToURIDecode } from '../../../../src/plugins/es_ui_shared/public';
export { KibanaContextProvider } from '../../../../src/plugins/kibana_react/public';
export const useKibana = () => _useKibana<AppServicesContext>();

View file

@ -52,7 +52,7 @@ export const ComponentTemplateDetailsFlyoutContent: React.FunctionComponent<Prop
}) => {
const { api } = useComponentTemplatesContext();
const decodedComponentTemplateName = attemptToURIDecode(componentTemplateName);
const decodedComponentTemplateName = attemptToURIDecode(componentTemplateName)!;
const { data: componentTemplateDetails, isLoading, error } = api.useLoadComponentTemplate(
decodedComponentTemplateName

View file

@ -84,7 +84,7 @@ export const ComponentTemplateList: React.FunctionComponent<Props> = ({
}),
icon: 'pencil',
handleActionClick: () =>
goToEditComponentTemplate(attemptToURIDecode(componentTemplateName)),
goToEditComponentTemplate(attemptToURIDecode(componentTemplateName)!),
},
{
name: i18n.translate('xpack.idxMgmt.componentTemplateDetails.cloneActionLabel', {
@ -92,7 +92,7 @@ export const ComponentTemplateList: React.FunctionComponent<Props> = ({
}),
icon: 'copy',
handleActionClick: () =>
goToCloneComponentTemplate(attemptToURIDecode(componentTemplateName)),
goToCloneComponentTemplate(attemptToURIDecode(componentTemplateName)!),
},
{
name: i18n.translate('xpack.idxMgmt.componentTemplateDetails.deleteButtonLabel', {
@ -103,7 +103,7 @@ export const ComponentTemplateList: React.FunctionComponent<Props> = ({
details._kbnMeta.usedBy.length > 0,
closePopoverOnClick: true,
handleActionClick: () => {
setComponentTemplatesToDelete([attemptToURIDecode(componentTemplateName)]);
setComponentTemplatesToDelete([attemptToURIDecode(componentTemplateName)!]);
},
},
];

View file

@ -19,7 +19,7 @@ export interface Params {
export const ComponentTemplateClone: FunctionComponent<RouteComponentProps<Params>> = (props) => {
const { sourceComponentTemplateName } = props.match.params;
const decodedSourceName = attemptToURIDecode(sourceComponentTemplateName);
const decodedSourceName = attemptToURIDecode(sourceComponentTemplateName)!;
const { toasts, api } = useComponentTemplatesContext();

View file

@ -31,7 +31,7 @@ export const ComponentTemplateEdit: React.FunctionComponent<RouteComponentProps<
const [isSaving, setIsSaving] = useState<boolean>(false);
const [saveError, setSaveError] = useState<any>(null);
const decodedName = attemptToURIDecode(name);
const decodedName = attemptToURIDecode(name)!;
const { error, data: componentTemplate, isLoading } = api.useLoadComponentTemplate(decodedName);

View file

@ -231,7 +231,7 @@ export const DataStreamList: React.FunctionComponent<RouteComponentProps<MatchPa
*/}
{dataStreamName && (
<DataStreamDetailPanel
dataStreamName={attemptToURIDecode(dataStreamName)}
dataStreamName={attemptToURIDecode(dataStreamName)!}
onClose={(shouldReload?: boolean) => {
history.push(`/${Section.DataStreams}`);

View file

@ -101,7 +101,7 @@ export const TemplateList: React.FunctionComponent<RouteComponentProps<MatchPara
const selectedTemplate = Boolean(templateName)
? {
name: attemptToURIDecode(templateName!),
name: attemptToURIDecode(templateName!)!,
isLegacy: getIsLegacyFromQueryParams(location),
}
: null;

View file

@ -27,7 +27,7 @@ export const TemplateClone: React.FunctionComponent<RouteComponentProps<MatchPar
location,
history,
}) => {
const decodedTemplateName = attemptToURIDecode(name);
const decodedTemplateName = attemptToURIDecode(name)!;
const isLegacy = getIsLegacyFromQueryParams(location);
const [isSaving, setIsSaving] = useState<boolean>(false);

View file

@ -27,7 +27,7 @@ export const TemplateEdit: React.FunctionComponent<RouteComponentProps<MatchPara
location,
history,
}) => {
const decodedTemplateName = attemptToURIDecode(name);
const decodedTemplateName = attemptToURIDecode(name)!;
const isLegacy = getIsLegacyFromQueryParams(location);
const [isSaving, setIsSaving] = useState<boolean>(false);

View file

@ -25,7 +25,7 @@ export const PipelinesClone: FunctionComponent<RouteComponentProps<ParamProps>>
const { sourceName } = props.match.params;
const { services } = useKibana();
const decodedSourceName = attemptToURIDecode(sourceName);
const decodedSourceName = attemptToURIDecode(sourceName)!;
const { error, data: pipeline, isLoading, isInitialRequest } = services.api.useLoadPipeline(
decodedSourceName
);

View file

@ -38,7 +38,7 @@ export const PipelinesEdit: React.FunctionComponent<RouteComponentProps<MatchPar
const [isSaving, setIsSaving] = useState<boolean>(false);
const [saveError, setSaveError] = useState<any>(null);
const decodedPipelineName = attemptToURIDecode(name);
const decodedPipelineName = attemptToURIDecode(name)!;
const { error, data: pipeline, isLoading } = services.api.useLoadPipeline(decodedPipelineName);