Skip to content

Commit

Permalink
Add uri decode to es_ui_shared and fix navigation issues with special…
Browse files Browse the repository at this point in the history
… characters (#80835) (#81897)

* Add uri decode to es_ui_shared and fix data stream issue with % name

* Add a test for data streams tab opened for name with a dollar sign

* Import uri decode function from es_ui_shared and fix navigation issues for filters

* Add tests for data streams with special characters in name

* Revert react router navigate changes (is done in a separate PR)

* Reverting changes to dataManagement es client and get data stream api route

* Fix data stream name filter when activated from a url parameter

* Clean up for better consistency and fixes after
#81664

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
yuliacech and kibanamachine authored Oct 28, 2020
1 parent 188635f commit e22f2b4
Show file tree
Hide file tree
Showing 31 changed files with 220 additions and 165 deletions.
2 changes: 1 addition & 1 deletion src/plugins/es_ui_shared/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export {

export { Forms, ace, GlobalFlyout, XJson };

export { extractQueryParams } from './url';
export { extractQueryParams, attemptToURIDecode } from './url';

/** dummy plugin, we just want esUiShared to have its own bundle */
export function plugin() {
Expand Down
32 changes: 32 additions & 0 deletions src/plugins/es_ui_shared/public/url/attempt_to_uri_decode.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { attemptToURIDecode } from './attempt_to_uri_decode';

test('decodes an encoded string', () => {
const encodedString = 'test%3F';
expect(attemptToURIDecode(encodedString)).toBe('test?');
});

// 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);
});
32 changes: 32 additions & 0 deletions src/plugins/es_ui_shared/public/url/attempt_to_uri_decode.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

/*
* Use this function with any match params coming from react router to safely decode values.
* https://github.com/elastic/kibana/pull/81664
*/
export const attemptToURIDecode = (value: string) => {
let result = value;
try {
result = decodeURIComponent(value);
} catch (e) {
// do nothing
}
return result;
};
1 change: 1 addition & 0 deletions src/plugins/es_ui_shared/public/url/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@
*/

export { extractQueryParams } from './extract_query_params';
export { attemptToURIDecode } from './attempt_to_uri_decode';
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ export const TableContent: React.FunctionComponent<Props> = ({
icon: 'list',
onClick: () => {
navigateToApp('management', {
path: `/data/index_management${getIndexListUri(`ilm.policy:${policy.name}`, true)}`,
path: `/data/index_management${getIndexListUri(`ilm.policy:"${policy.name}"`, true)}`,
});
},
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,3 +186,28 @@ export const createDataStreamPayload = (name: string): DataStream => ({
storageSize: '1b',
maxTimeStamp: 420,
});

export const createDataStreamBackingIndex = (indexName: string, dataStreamName: string) => ({
health: '',
status: '',
primary: '',
replica: '',
documents: '',
documents_deleted: '',
size: '',
primary_size: '',
name: indexName,
data_stream: dataStreamName,
});

export const createNonDataStreamIndex = (name: string) => ({
health: 'green',
status: 'open',
primary: 1,
replica: 1,
documents: 10000,
documents_deleted: 100,
size: '156kb',
primary_size: '156kb',
name,
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@ import { act } from 'react-dom/test-utils';
import { API_BASE_PATH } from '../../../common/constants';
import { setupEnvironment } from '../helpers';

import { DataStreamsTabTestBed, setup, createDataStreamPayload } from './data_streams_tab.helpers';
import {
DataStreamsTabTestBed,
setup,
createDataStreamPayload,
createDataStreamBackingIndex,
createNonDataStreamIndex,
} from './data_streams_tab.helpers';

describe('Data Streams tab', () => {
const { server, httpRequestsMockHelpers } = setupEnvironment();
Expand Down Expand Up @@ -85,29 +91,8 @@ describe('Data Streams tab', () => {
} = httpRequestsMockHelpers;

setLoadIndicesResponse([
{
health: '',
status: '',
primary: '',
replica: '',
documents: '',
documents_deleted: '',
size: '',
primary_size: '',
name: 'data-stream-index',
data_stream: 'dataStream1',
},
{
health: 'green',
status: 'open',
primary: 1,
replica: 1,
documents: 10000,
documents_deleted: 100,
size: '156kb',
primary_size: '156kb',
name: 'non-data-stream-index',
},
createDataStreamBackingIndex('data-stream-index', 'dataStream1'),
createNonDataStreamIndex('non-data-stream-index'),
]);

const dataStreamForDetailPanel = createDataStreamPayload('dataStream1');
Expand Down Expand Up @@ -260,4 +245,46 @@ describe('Data Streams tab', () => {
});
});
});

describe('when there are special characters', () => {
beforeEach(async () => {
const {
setLoadIndicesResponse,
setLoadDataStreamsResponse,
setLoadDataStreamResponse,
} = httpRequestsMockHelpers;

setLoadIndicesResponse([
createDataStreamBackingIndex('data-stream-index', '%dataStream'),
createDataStreamBackingIndex('data-stream-index2', 'dataStream2'),
]);

const dataStreamDollarSign = createDataStreamPayload('%dataStream');
setLoadDataStreamsResponse([dataStreamDollarSign]);
setLoadDataStreamResponse(dataStreamDollarSign);

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

describe('detail panel', () => {
test('opens when the data stream name in the table is clicked', async () => {
const { actions, findDetailPanel, findDetailPanelTitle } = testBed;
await actions.clickNameAt(0);
expect(findDetailPanel().length).toBe(1);
expect(findDetailPanelTitle()).toBe('%dataStream');
});

test('clicking the indices count navigates to the backing indices', async () => {
const { table, actions } = testBed;
await actions.clickIndicesAt(0);
expect(table.getMetaData('indexTable').tableCellsValues).toEqual([
['', '', '', '', '', '', '', '%dataStream'],
]);
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,17 @@ import {
EuiBadge,
} from '@elastic/eui';

import { SectionLoading, TabSettings, TabAliases, TabMappings } from '../shared_imports';
import {
SectionLoading,
TabSettings,
TabAliases,
TabMappings,
attemptToURIDecode,
} from '../shared_imports';
import { useComponentTemplatesContext } from '../component_templates_context';
import { TabSummary } from './tab_summary';
import { ComponentTemplateTabs, TabType } from './tabs';
import { ManageButton, ManageAction } from './manage_button';
import { attemptToDecodeURI } from '../lib';

export interface Props {
componentTemplateName: string;
Expand All @@ -47,7 +52,7 @@ export const ComponentTemplateDetailsFlyoutContent: React.FunctionComponent<Prop
}) => {
const { api } = useComponentTemplatesContext();

const decodedComponentTemplateName = attemptToDecodeURI(componentTemplateName);
const decodedComponentTemplateName = attemptToURIDecode(componentTemplateName);

const { data: componentTemplateDetails, isLoading, error } = api.useLoadComponentTemplate(
decodedComponentTemplateName
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import { FormattedMessage } from '@kbn/i18n/react';
import { ScopedHistory } from 'kibana/public';
import { EuiLink, EuiText, EuiSpacer } from '@elastic/eui';

import { attemptToURIDecode } from '../../../../shared_imports';
import { SectionLoading, ComponentTemplateDeserialized, GlobalFlyout } from '../shared_imports';
import { UIM_COMPONENT_TEMPLATE_LIST_LOAD } from '../constants';
import { attemptToDecodeURI } from '../lib';
import { useComponentTemplatesContext } from '../component_templates_context';
import {
ComponentTemplateDetailsFlyoutContent,
Expand Down Expand Up @@ -84,15 +84,15 @@ export const ComponentTemplateList: React.FunctionComponent<Props> = ({
}),
icon: 'pencil',
handleActionClick: () =>
goToEditComponentTemplate(attemptToDecodeURI(componentTemplateName)),
goToEditComponentTemplate(attemptToURIDecode(componentTemplateName)),
},
{
name: i18n.translate('xpack.idxMgmt.componentTemplateDetails.cloneActionLabel', {
defaultMessage: 'Clone',
}),
icon: 'copy',
handleActionClick: () =>
goToCloneComponentTemplate(attemptToDecodeURI(componentTemplateName)),
goToCloneComponentTemplate(attemptToURIDecode(componentTemplateName)),
},
{
name: i18n.translate('xpack.idxMgmt.componentTemplateDetails.deleteButtonLabel', {
Expand All @@ -103,7 +103,7 @@ export const ComponentTemplateList: React.FunctionComponent<Props> = ({
details._kbnMeta.usedBy.length > 0,
closePopoverOnClick: true,
handleActionClick: () => {
setComponentTemplatesToDelete([attemptToDecodeURI(componentTemplateName)]);
setComponentTemplatesToDelete([attemptToURIDecode(componentTemplateName)]);
},
},
];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,8 @@ import { RouteComponentProps } from 'react-router-dom';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n/react';

import { SectionLoading } from '../../shared_imports';
import { SectionLoading, attemptToURIDecode } from '../../shared_imports';
import { useComponentTemplatesContext } from '../../component_templates_context';
import { attemptToDecodeURI } from '../../lib';
import { ComponentTemplateCreate } from '../component_template_create';

export interface Params {
Expand All @@ -20,7 +19,7 @@ export interface Params {

export const ComponentTemplateClone: FunctionComponent<RouteComponentProps<Params>> = (props) => {
const { sourceComponentTemplateName } = props.match.params;
const decodedSourceName = attemptToDecodeURI(sourceComponentTemplateName);
const decodedSourceName = attemptToURIDecode(sourceComponentTemplateName);

const { toasts, api } = useComponentTemplatesContext();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@ import { FormattedMessage } from '@kbn/i18n/react';
import { EuiPageBody, EuiPageContent, EuiTitle, EuiSpacer, EuiCallOut } from '@elastic/eui';

import { useComponentTemplatesContext } from '../../component_templates_context';
import { ComponentTemplateDeserialized, SectionLoading } from '../../shared_imports';
import { attemptToDecodeURI } from '../../lib';
import {
ComponentTemplateDeserialized,
SectionLoading,
attemptToURIDecode,
} from '../../shared_imports';
import { ComponentTemplateForm } from '../component_template_form';

interface MatchParams {
Expand All @@ -28,7 +31,7 @@ export const ComponentTemplateEdit: React.FunctionComponent<RouteComponentProps<
const [isSaving, setIsSaving] = useState<boolean>(false);
const [saveError, setSaveError] = useState<any>(null);

const decodedName = attemptToDecodeURI(name);
const decodedName = attemptToURIDecode(name);

const { error, data: componentTemplate, isLoading } = api.useLoadComponentTemplate(decodedName);

Expand All @@ -50,7 +53,9 @@ export const ComponentTemplateEdit: React.FunctionComponent<RouteComponentProps<
}

history.push({
pathname: encodeURI(`/component_templates/${encodeURIComponent(name)}`),
pathname: encodeURI(
`/component_templates/${encodeURIComponent(updatedComponentTemplate.name)}`
),
});
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,3 @@ export * from './request';
export * from './documentation';

export * from './breadcrumbs';

export { attemptToDecodeURI } from './utils';

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export {
NotAuthorizedSection,
Forms,
GlobalFlyout,
attemptToURIDecode,
} from '../../../../../../../src/plugins/es_ui_shared/public';

export {
Expand Down
Loading

0 comments on commit e22f2b4

Please sign in to comment.