Skip to content

Commit

Permalink
[Index Management] Fix encoding issue on index details page (elastic#…
Browse files Browse the repository at this point in the history
…166882)

## Summary
Fixes elastic#166100

This PR adds a workaround fix for the new index details page when
opening for index names with special characters, for example
`test_index%`. Because of how encoding/decoding works, we can't use the
index name as a part of the url like `/indices/${indexName}` (see for
more details). Instead we have to pass the index name in a query
parameter like `/indices/index_details?indexName=${indexName}. The
downside of this workaround is that the url semantics doesn't reflect
that the index name is mandatory for the page to work. Once
elastic#132600 is resolved, we should
revert this workaround and use the index name as a url segment again.

Note for reviewers: The jest tests for this fix are part of
elastic#165705

### How to test
1. Add `xpack.index_management.dev.enableIndexDetailsPage: true` to the
file `config/kibana.dev.yml` to enable the new index details page
2. Navigate to Index Management and use the "create index" button 
3. Type a name with special characters, for example `test%`
4. Click the new index name in the list and check that the details page
and all tabs work
5. Also reload the page completely and check that the page still loads
correctly

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Alison Goryachev <alisonmllr20@gmail.com>
  • Loading branch information
3 people authored and joemcelroy committed Sep 25, 2023
1 parent 86f7781 commit 7a4eea8
Show file tree
Hide file tree
Showing 15 changed files with 229 additions and 184 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,23 @@ import {
} from '@kbn/test-jest-helpers';
import { HttpSetup } from '@kbn/core/public';
import { act } from 'react-dom/test-utils';
import {
IndexDetailsPage,
IndexDetailsSection,
} from '../../../public/application/sections/home/index_list/details_page';

import { IndexDetailsSection } from '../../../common/constants';
import { IndexDetailsPage } from '../../../public/application/sections/home/index_list/details_page';
import { WithAppDependencies } from '../helpers';
import { testIndexName } from './mocks';

let routerMock: typeof reactRouterMock;
const testBedConfig: AsyncTestBedConfig = {
const getTestBedConfig = (initialEntry?: string): AsyncTestBedConfig => ({
memoryRouter: {
initialEntries: [`/indices/${testIndexName}`],
componentRoutePath: `/indices/:indexName/:indexDetailsSection?`,
initialEntries: [initialEntry ?? `/indices/index_details?indexName=${testIndexName}`],
componentRoutePath: `/indices/index_details`,
onRouter: (router) => {
routerMock = router;
},
},
doMountAsync: true,
};
});

export interface IndexDetailsPageTestBed extends TestBed {
routerMock: typeof reactRouterMock;
Expand Down Expand Up @@ -67,6 +66,7 @@ export interface IndexDetailsPageTestBed extends TestBed {
errorSection: {
isDisplayed: () => boolean;
clickReloadButton: () => Promise<void>;
noIndexNameMessageIsDisplayed: () => boolean;
};
stats: {
getCodeBlockContent: () => string;
Expand All @@ -85,13 +85,18 @@ export interface IndexDetailsPageTestBed extends TestBed {
};
}

export const setup = async (
httpSetup: HttpSetup,
overridingDependencies: any = {}
): Promise<IndexDetailsPageTestBed> => {
export const setup = async ({
httpSetup,
dependencies = {},
initialEntry,
}: {
httpSetup: HttpSetup;
dependencies?: any;
initialEntry?: string;
}): Promise<IndexDetailsPageTestBed> => {
const initTestBed = registerTestBed(
WithAppDependencies(IndexDetailsPage, httpSetup, overridingDependencies),
testBedConfig
WithAppDependencies(IndexDetailsPage, httpSetup, dependencies),
getTestBedConfig(initialEntry)
);
const testBed = await initTestBed();
const { find, component, exists } = testBed;
Expand All @@ -106,6 +111,9 @@ export const setup = async (
});
component.update();
},
noIndexNameMessageIsDisplayed: () => {
return exists('indexDetailsNoIndexNameError');
},
};
const getHeader = () => {
return component.find('[data-test-subj="indexDetailsHeader"] h1').text();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,14 @@
import { setupEnvironment } from '../helpers';
import { IndexDetailsPageTestBed, setup } from './index_details_page.helpers';
import { act } from 'react-dom/test-utils';

import React from 'react';
import { IndexDetailsSection } from '../../../common/constants';
import { API_BASE_PATH, INTERNAL_API_BASE_PATH } from '../../../common';
import {
breadcrumbService,
IndexManagementBreadcrumb,
} from '../../../public/application/services/breadcrumbs';
import { IndexDetailsSection } from '../../../public/application/sections/home/index_list/details_page';
import {
testIndexEditableSettings,
testIndexMappings,
Expand All @@ -21,8 +24,6 @@ import {
testIndexSettings,
testIndexStats,
} from './mocks';
import { API_BASE_PATH, INTERNAL_API_BASE_PATH } from '../../../common';
import React from 'react';

jest.mock('@kbn/kibana-react-plugin/public', () => {
const original = jest.requireActual('@kbn/kibana-react-plugin/public');
Expand Down Expand Up @@ -57,10 +58,13 @@ describe('<IndexDetailsPage />', () => {
httpRequestsMockHelpers.setLoadIndexSettingsResponse(testIndexName, testIndexSettings);

await act(async () => {
testBed = await setup(httpSetup, {
url: {
locators: {
get: () => ({ navigate: jest.fn() }),
testBed = await setup({
httpSetup,
dependencies: {
url: {
locators: {
get: () => ({ navigate: jest.fn() }),
},
},
},
});
Expand All @@ -75,7 +79,7 @@ describe('<IndexDetailsPage />', () => {
message: `Data for index ${testIndexName} was not found`,
});
await act(async () => {
testBed = await setup(httpSetup);
testBed = await setup({ httpSetup });
});

testBed.component.update();
Expand All @@ -91,6 +95,19 @@ describe('<IndexDetailsPage />', () => {
await testBed.actions.errorSection.clickReloadButton();
expect(httpSetup.get).toHaveBeenCalledTimes(numberOfRequests + 1);
});

it('renders an error section when no index name is provided', async () => {
// already sent 2 requests while setting up the component
const numberOfRequests = 2;
expect(httpSetup.get).toHaveBeenCalledTimes(numberOfRequests);
await act(async () => {
testBed = await setup({ httpSetup, initialEntry: '/indices/index_details' });
});
testBed.component.update();
expect(testBed.actions.errorSection.noIndexNameMessageIsDisplayed()).toBe(true);
// no extra http request was sent
expect(httpSetup.get).toHaveBeenCalledTimes(numberOfRequests);
});
});

describe('Stats tab', () => {
Expand Down Expand Up @@ -138,7 +155,7 @@ describe('<IndexDetailsPage />', () => {
);

await act(async () => {
testBed = await setup(httpSetup);
testBed = await setup({ httpSetup });
});
testBed.component.update();

Expand All @@ -148,8 +165,11 @@ describe('<IndexDetailsPage />', () => {

it('hides index stats tab if enableIndexStats===false', async () => {
await act(async () => {
testBed = await setup(httpSetup, {
config: { enableIndexStats: false },
testBed = await setup({
httpSetup,
dependencies: {
config: { enableIndexStats: false },
},
});
});
testBed.component.update();
Expand All @@ -164,7 +184,7 @@ describe('<IndexDetailsPage />', () => {
message: 'Error',
});
await act(async () => {
testBed = await setup(httpSetup);
testBed = await setup({ httpSetup });
});

testBed.component.update();
Expand Down Expand Up @@ -213,8 +233,11 @@ describe('<IndexDetailsPage />', () => {

it('hides index stats from detail panels if enableIndexStats===false', async () => {
await act(async () => {
testBed = await setup(httpSetup, {
config: { enableIndexStats: false },
testBed = await setup({
httpSetup,
dependencies: {
config: { enableIndexStats: false },
},
});
});
testBed.component.update();
Expand All @@ -226,10 +249,13 @@ describe('<IndexDetailsPage />', () => {
describe('extension service summary', () => {
it('renders all summaries added to the extension service', async () => {
await act(async () => {
testBed = await setup(httpSetup, {
services: {
extensionsService: {
summaries: [() => <span>test</span>, () => <span>test2</span>],
testBed = await setup({
httpSetup,
dependencies: {
services: {
extensionsService: {
summaries: [() => <span>test</span>, () => <span>test2</span>],
},
},
},
});
Expand All @@ -241,10 +267,13 @@ describe('<IndexDetailsPage />', () => {

it(`doesn't render empty panels if the summary renders null`, async () => {
await act(async () => {
testBed = await setup(httpSetup, {
services: {
extensionsService: {
summaries: [() => null],
testBed = await setup({
httpSetup,
dependencies: {
services: {
extensionsService: {
summaries: [() => null],
},
},
},
});
Expand All @@ -255,10 +284,13 @@ describe('<IndexDetailsPage />', () => {

it(`doesn't render anything when no summaries added to the extension service`, async () => {
await act(async () => {
testBed = await setup(httpSetup, {
services: {
extensionsService: {
summaries: [],
testBed = await setup({
httpSetup,
dependencies: {
services: {
extensionsService: {
summaries: [],
},
},
},
});
Expand All @@ -269,12 +301,6 @@ describe('<IndexDetailsPage />', () => {
});
});

it('documents tab', async () => {
await testBed.actions.clickIndexDetailsTab(IndexDetailsSection.Documents);
const tabContent = testBed.actions.getActiveTabContent();
expect(tabContent).toEqual('Documents');
});

describe('Mappings tab', () => {
it('updates the breadcrumbs to index details mappings', async () => {
await testBed.actions.clickIndexDetailsTab(IndexDetailsSection.Mappings);
Expand Down Expand Up @@ -315,7 +341,7 @@ describe('<IndexDetailsPage />', () => {
message: `Was not able to load mappings`,
});
await act(async () => {
testBed = await setup(httpSetup);
testBed = await setup({ httpSetup });
});

testBed.component.update();
Expand Down Expand Up @@ -377,7 +403,7 @@ describe('<IndexDetailsPage />', () => {
message: `Was not able to load settings`,
});
await act(async () => {
testBed = await setup(httpSetup);
testBed = await setup({ httpSetup });
});

testBed.component.update();
Expand Down Expand Up @@ -451,12 +477,6 @@ describe('<IndexDetailsPage />', () => {
});
});

it('pipelines tab', async () => {
await testBed.actions.clickIndexDetailsTab(IndexDetailsSection.Pipelines);
const tabContent = testBed.actions.getActiveTabContent();
expect(tabContent).toEqual('Pipelines');
});

it('navigates back to indices', async () => {
jest.spyOn(testBed.routerMock.history, 'push');
await testBed.actions.clickBackToIndicesButton();
Expand Down Expand Up @@ -496,7 +516,7 @@ describe('<IndexDetailsPage />', () => {
});

await act(async () => {
testBed = await setup(httpSetup);
testBed = await setup({ httpSetup });
});
testBed.component.update();

Expand Down Expand Up @@ -589,7 +609,7 @@ describe('<IndexDetailsPage />', () => {
});

await act(async () => {
testBed = await setup(httpSetup);
testBed = await setup({ httpSetup });
});
testBed.component.update();

Expand Down
21 changes: 21 additions & 0 deletions x-pack/plugins/index_management/common/constants/home_sections.ts
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
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

export enum Section {
Indices = 'indices',
DataStreams = 'data_streams',
IndexTemplates = 'templates',
ComponentTemplates = 'component_templates',
EnrichPolicies = 'enrich_policies',
}

export enum IndexDetailsSection {
Overview = 'overview',
Mappings = 'mappings',
Settings = 'settings',
Stats = 'stats',
}
2 changes: 2 additions & 0 deletions x-pack/plugins/index_management/common/constants/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,5 @@ export {
} from './ui_metric';

export { MAJOR_VERSION } from './plugin';

export { Section, IndexDetailsSection } from './home_sections';
4 changes: 2 additions & 2 deletions x-pack/plugins/index_management/public/application/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import { Redirect } from 'react-router-dom';
import { Router, Routes, Route } from '@kbn/shared-ux-router';
import { ScopedHistory } from '@kbn/core/public';

import { UIM_APP_LOAD } from '../../common/constants';
import { IndexManagementHome, homeSections, Section } from './sections/home';
import { UIM_APP_LOAD, Section } from '../../common/constants';
import { IndexManagementHome, homeSections } from './sections/home';
import { TemplateCreate } from './sections/template_create';
import { TemplateClone } from './sections/template_clone';
import { TemplateEdit } from './sections/template_edit';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ import {
APP_WRAPPER_CLASS,
useExecutionContext,
} from '../../../../shared_imports';
import { Section } from '../../../../../common/constants';
import { useAppContext } from '../../../app_context';
import { useLoadDataStreams } from '../../../services/api';
import { breadcrumbService, IndexManagementBreadcrumb } from '../../../services/breadcrumbs';
import { documentationService } from '../../../services/documentation';
import { Section } from '../home';
import { DataStreamTable } from './data_stream_table';
import { DataStreamDetailPanel } from './data_stream_detail_panel';
import { filterDataStreams, isSelectedDataStreamHidden } from '../../../lib/data_streams';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import { RouteComponentProps } from 'react-router-dom';
import { Routes, Route } from '@kbn/shared-ux-router';
import { FormattedMessage } from '@kbn/i18n-react';
import { EuiButtonEmpty, EuiPageHeader, EuiSpacer } from '@elastic/eui';

import { Section } from '../../../../common/constants';
import { documentationService } from '../../services/documentation';
import { useAppContext } from '../../app_context';
import { ComponentTemplateList } from '../../components/component_templates';
Expand All @@ -19,14 +21,6 @@ import { IndexDetailsPage } from './index_list/details_page';
import { DataStreamList } from './data_stream_list';
import { TemplateList } from './template_list';

export enum Section {
Indices = 'indices',
DataStreams = 'data_streams',
IndexTemplates = 'templates',
ComponentTemplates = 'component_templates',
EnrichPolicies = 'enrich_policies',
}

export const homeSections = [
Section.Indices,
Section.DataStreams,
Expand Down Expand Up @@ -157,10 +151,7 @@ export const IndexManagementHome: React.FunctionComponent<RouteComponentProps<Ma
return (
<>
<Routes>
<Route
path={`/${Section.Indices}/:indexName/:indexDetailsSection?`}
component={IndexDetailsPage}
/>
<Route path={`/${Section.Indices}/index_details`} component={IndexDetailsPage} />
<Route render={() => indexManagementTabs} />
</Routes>
</>
Expand Down
Loading

0 comments on commit 7a4eea8

Please sign in to comment.