Skip to content

Commit

Permalink
[Discover] Improve doc viewer code in Discover (elastic#114759)
Browse files Browse the repository at this point in the history
Co-authored-by: Dmitry Tomashevich <[email protected]>
  • Loading branch information
2 people authored and kibanamachine committed Oct 19, 2021
1 parent 8e6ba27 commit 6f47065
Show file tree
Hide file tree
Showing 16 changed files with 135 additions and 101 deletions.
8 changes: 5 additions & 3 deletions src/plugins/discover/public/__mocks__/index_patterns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@ import { IndexPatternsService } from '../../../data/common';
import { indexPatternMock } from './index_pattern';

export const indexPatternsMock = {
getCache: () => {
getCache: async () => {
return [indexPatternMock];
},
get: (id: string) => {
get: async (id: string) => {
if (id === 'the-index-pattern-id') {
return indexPatternMock;
return Promise.resolve(indexPatternMock);
} else if (id === 'invalid-index-pattern-id') {
return Promise.reject('Invald');
}
},
updateSavedObject: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import React, { useEffect } from 'react';
import { useParams } from 'react-router-dom';
import { i18n } from '@kbn/i18n';
import { EuiEmptyPrompt } from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n/react';
import { DiscoverServices } from '../../../build_services';
import { ContextApp } from './context_app';
import { getRootBreadcrumbs } from '../../helpers/breadcrumbs';
Expand Down Expand Up @@ -43,7 +45,29 @@ export function ContextAppRoute(props: ContextAppProps) {
]);
}, [chrome]);

const indexPattern = useIndexPattern(services.indexPatterns, indexPatternId);
const { indexPattern, error } = useIndexPattern(services.indexPatterns, indexPatternId);

if (error) {
return (
<EuiEmptyPrompt
iconType="alert"
iconColor="danger"
title={
<FormattedMessage
id="discover.singleDocRoute.errorTitle"
defaultMessage="An error occured"
/>
}
body={
<FormattedMessage
id="discover.singleDocRoute.errorMessage"
defaultMessage="No matching index pattern for id {indexPatternId}"
values={{ indexPatternId }}
/>
}
/>
);
}

if (!indexPattern) {
return <LoadingIndicator />;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { ReactWrapper } from 'enzyme';
import { findTestSubject } from '@elastic/eui/lib/test';
import { Doc, DocProps } from './doc';
import { SEARCH_FIELDS_FROM_SOURCE as mockSearchFieldsFromSource } from '../../../../../common';
import { indexPatternMock } from '../../../../__mocks__/index_pattern';

const mockSearchApi = jest.fn();

Expand Down Expand Up @@ -74,21 +75,11 @@ const waitForPromises = async () =>
* this works but logs ugly error messages until we're using React 16.9
* should be adapted when we upgrade
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
async function mountDoc(update = false, indexPatternGetter: any = null) {
const indexPattern = {
getComputedFields: () => [],
};
const indexPatternService = {
get: indexPatternGetter ? indexPatternGetter : jest.fn(() => Promise.resolve(indexPattern)),
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any;

async function mountDoc(update = false) {
const props = {
id: '1',
index: 'index1',
indexPatternId: 'xyz',
indexPatternService,
indexPattern: indexPatternMock,
} as DocProps;
let comp!: ReactWrapper;
await act(async () => {
Expand All @@ -108,12 +99,6 @@ describe('Test of <Doc /> of Discover', () => {
expect(findTestSubject(comp, 'doc-msg-loading').length).toBe(1);
});

test('renders IndexPattern notFound msg', async () => {
const indexPatternGetter = jest.fn(() => Promise.reject({ savedObjectId: '007' }));
const comp = await mountDoc(true, indexPatternGetter);
expect(findTestSubject(comp, 'doc-msg-notFoundIndexPattern').length).toBe(1);
});

test('renders notFound msg', async () => {
mockSearchApi.mockImplementation(() => throwError({ status: 404 }));
const comp = await mountDoc(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import React from 'react';
import { FormattedMessage } from '@kbn/i18n/react';
import { EuiCallOut, EuiLink, EuiLoadingSpinner, EuiPageContent, EuiPage } from '@elastic/eui';
import { IndexPatternsContract } from 'src/plugins/data/public';
import { IndexPattern } from 'src/plugins/data/public';
import { getServices } from '../../../../kibana_services';
import { DocViewer } from '../../../components/doc_viewer/doc_viewer';
import { ElasticRequestState } from '../types';
Expand All @@ -25,22 +25,18 @@ export interface DocProps {
*/
index: string;
/**
* IndexPattern ID used to get IndexPattern entity
* that's used for adding additional fields (stored_fields, script_fields, docvalue_fields)
* IndexPattern entity
*/
indexPatternId: string;
/**
* IndexPatternService to get a given index pattern by ID
*/
indexPatternService: IndexPatternsContract;
indexPattern: IndexPattern;
/**
* If set, will always request source, regardless of the global `fieldsFromSource` setting
*/
requestSource?: boolean;
}

export function Doc(props: DocProps) {
const [reqState, hit, indexPattern] = useEsDocSearch(props);
const { indexPattern } = props;
const [reqState, hit] = useEsDocSearch(props);
const indexExistsLink = getServices().docLinks.links.apis.indexExists;
return (
<EuiPage>
Expand All @@ -54,7 +50,7 @@ export function Doc(props: DocProps) {
<FormattedMessage
id="discover.doc.failedToLocateIndexPattern"
defaultMessage="No index pattern matches ID {indexPatternId}."
values={{ indexPatternId: props.indexPatternId }}
values={{ indexPatternId: indexPattern.id }}
/>
}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
*/
import React, { useEffect } from 'react';
import { useLocation, useParams } from 'react-router-dom';
import { EuiEmptyPrompt } from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n/react';
import { DiscoverServices } from '../../../build_services';
import { getRootBreadcrumbs } from '../../helpers/breadcrumbs';
import { Doc } from './components/doc';
Expand All @@ -31,7 +33,7 @@ function useQuery() {

export function SingleDocRoute(props: SingleDocRouteProps) {
const { services } = props;
const { chrome, timefilter, indexPatterns } = services;
const { chrome, timefilter } = services;

const { indexPatternId, index } = useParams<DocUrlParams>();

Expand All @@ -52,20 +54,37 @@ export function SingleDocRoute(props: SingleDocRouteProps) {
timefilter.disableTimeRangeSelector();
});

const indexPattern = useIndexPattern(services.indexPatterns, indexPatternId);
const { indexPattern, error } = useIndexPattern(services.indexPatterns, indexPatternId);

if (error) {
return (
<EuiEmptyPrompt
iconType="alert"
iconColor="danger"
title={
<FormattedMessage
id="discover.singleDocRoute.errorTitle"
defaultMessage="An error occured"
/>
}
body={
<FormattedMessage
id="discover.singleDocRoute.errorMessage"
defaultMessage="No matching index pattern for id {indexPatternId}"
values={{ indexPatternId }}
/>
}
/>
);
}

if (!indexPattern) {
return <LoadingIndicator />;
}

return (
<div className="app-container">
<Doc
id={docId}
index={index}
indexPatternId={indexPatternId}
indexPatternService={indexPatterns}
/>
<Doc id={docId} index={index} indexPattern={indexPattern} />
</div>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import React from 'react';
import { shallow } from 'enzyme';
import { DocViewerTab } from './doc_viewer_tab';
import { ElasticSearchHit } from '../../doc_views/doc_views_types';
import { indexPatternMock } from '../../../__mocks__/index_pattern';

describe('DocViewerTab', () => {
test('changing columns triggers an update', () => {
Expand All @@ -21,6 +22,7 @@ describe('DocViewerTab', () => {
renderProps: {
hit: {} as ElasticSearchHit,
columns: ['test'],
indexPattern: indexPatternMock,
},
};

Expand All @@ -31,6 +33,7 @@ describe('DocViewerTab', () => {
renderProps: {
hit: {} as ElasticSearchHit,
columns: ['test2'],
indexPattern: indexPatternMock,
},
};

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,13 @@ const mockIndexPatternService = {
}));
describe('Source Viewer component', () => {
test('renders loading state', () => {
jest.spyOn(hooks, 'useEsDocSearch').mockImplementation(() => [0, null, null, () => {}]);
jest.spyOn(hooks, 'useEsDocSearch').mockImplementation(() => [0, null, () => {}]);

const comp = mountWithIntl(
<SourceViewer
id={'1'}
index={'index1'}
indexPatternId={'xyz'}
indexPattern={mockIndexPattern}
width={123}
hasLineNumbers={true}
/>
Expand All @@ -60,13 +60,13 @@ describe('Source Viewer component', () => {
});

test('renders error state', () => {
jest.spyOn(hooks, 'useEsDocSearch').mockImplementation(() => [3, null, null, () => {}]);
jest.spyOn(hooks, 'useEsDocSearch').mockImplementation(() => [3, null, () => {}]);

const comp = mountWithIntl(
<SourceViewer
id={'1'}
index={'index1'}
indexPatternId={'xyz'}
indexPattern={mockIndexPattern}
width={123}
hasLineNumbers={true}
/>
Expand Down Expand Up @@ -97,17 +97,15 @@ describe('Source Viewer component', () => {
_underscore: 123,
},
} as never;
jest
.spyOn(hooks, 'useEsDocSearch')
.mockImplementation(() => [2, mockHit, mockIndexPattern, () => {}]);
jest.spyOn(hooks, 'useEsDocSearch').mockImplementation(() => [2, mockHit, () => {}]);
jest.spyOn(useUiSettingHook, 'useUiSetting').mockImplementation(() => {
return false;
});
const comp = mountWithIntl(
<SourceViewer
id={'1'}
index={'index1'}
indexPatternId={'xyz'}
indexPattern={mockIndexPattern}
width={123}
hasLineNumbers={true}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,31 +17,30 @@ import { getServices } from '../../../kibana_services';
import { SEARCH_FIELDS_FROM_SOURCE } from '../../../../common';
import { ElasticRequestState } from '../../apps/doc/types';
import { useEsDocSearch } from '../../services/use_es_doc_search';
import { IndexPattern } from '../../../../../data_views/common';

interface SourceViewerProps {
id: string;
index: string;
indexPatternId: string;
indexPattern: IndexPattern;
hasLineNumbers: boolean;
width?: number;
}

export const SourceViewer = ({
id,
index,
indexPatternId,
indexPattern,
width,
hasLineNumbers,
}: SourceViewerProps) => {
const [editor, setEditor] = useState<monaco.editor.IStandaloneCodeEditor>();
const [jsonValue, setJsonValue] = useState<string>('');
const indexPatternService = getServices().data.indexPatterns;
const useNewFieldsApi = !getServices().uiSettings.get(SEARCH_FIELDS_FROM_SOURCE);
const [reqState, hit, , requestData] = useEsDocSearch({
const [reqState, hit, requestData] = useEsDocSearch({
id,
index,
indexPatternId,
indexPatternService,
indexPattern,
requestSource: useNewFieldsApi,
});

Expand Down Expand Up @@ -106,11 +105,7 @@ export const SourceViewer = ({
<EuiEmptyPrompt iconType="alert" title={errorMessageTitle} body={errorMessage} />
);

if (
reqState === ElasticRequestState.Error ||
reqState === ElasticRequestState.NotFound ||
reqState === ElasticRequestState.NotFoundIndexPattern
) {
if (reqState === ElasticRequestState.Error || reqState === ElasticRequestState.NotFound) {
return errorState;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export interface DocViewerTableProps {
columns?: string[];
filter?: DocViewFilterFn;
hit: ElasticSearchHit;
indexPattern?: IndexPattern;
indexPattern: IndexPattern;
onAddColumn?: (columnName: string) => void;
onRemoveColumn?: (columnName: string) => void;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export interface DocViewRenderProps {
columns?: string[];
filter?: DocViewFilterFn;
hit: ElasticSearchHit;
indexPattern?: IndexPattern;
indexPattern: IndexPattern;
onAddColumn?: (columnName: string) => void;
onRemoveColumn?: (columnName: string) => void;
}
Expand Down
Loading

0 comments on commit 6f47065

Please sign in to comment.