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.

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

NP Migration: Move doc views registry and existing doc views into discover plugin #53465

Merged
merged 11 commits into from
Jan 7, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
1 change: 1 addition & 0 deletions src/core/MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -1173,6 +1173,7 @@ import { setup, start } from '../core_plugins/visualizations/public/legacy';
| `ui/registry/field_formats` | `data.fieldFormats` | |
| `ui/registry/feature_catalogue` | `home.featureCatalogue.register` | Must add `home` as a dependency in your kibana.json. |
| `ui/registry/vis_types` | `visualizations.types` | -- |
| `ui/registry/doc_views` | `discover.addDocView` | Currently shimmed in the old platform - use `import { setup } from '../kibana/public/discover'; setup.addDocView(...)` |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it ready to be consumed by new platform plugins? it means no webpack imports, no global angular

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it's still in the legacy world for now, good point. I will remove it.

| `ui/vis` | `visualizations.types` | -- |
| `ui/share` | `share` | `showShareContextMenu` is now called `toggleShareContextMenu`, `ShareContextMenuExtensionsRegistryProvider` is now called `register` |
| `ui/vis/vis_factory` | `visualizations.types` | -- |
Expand Down
26 changes: 0 additions & 26 deletions src/legacy/core_plugins/kbn_doc_views/index.js

This file was deleted.

4 changes: 0 additions & 4 deletions src/legacy/core_plugins/kbn_doc_views/package.json

This file was deleted.

21 changes: 0 additions & 21 deletions src/legacy/core_plugins/kbn_doc_views/public/kbn_doc_views.js

This file was deleted.

33 changes: 0 additions & 33 deletions src/legacy/core_plugins/kbn_doc_views/public/views/json.tsx

This file was deleted.

31 changes: 0 additions & 31 deletions src/legacy/core_plugins/kbn_doc_views/public/views/table.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { findTestSubject } from '@elastic/eui/lib/test';
import { Doc, DocProps } from './doc';

jest.mock('../doc_viewer/doc_viewer', () => ({
DocViewer: 'test',
DocViewer: () => null,
Copy link
Contributor Author

@flash1293 flash1293 Dec 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the doc viewer is actually rendered (which happens now because of async act), test throws another warning because it's not a valid react component

}));

jest.mock('../../kibana_services', () => {
Expand Down Expand Up @@ -67,7 +67,7 @@ async function mountDoc(search: () => void, update = false, indexPatternGetter:
indexPatternService,
} as DocProps;
let comp!: ReactWrapper;
act(() => {
await act(async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some here - getting rid of react warnings during test run

comp = mountWithIntl(<Doc {...props} />);
if (update) comp.update();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe('Test of <Doc /> helper / hook', () => {
`);
});

test('useEsDocSearch', () => {
test('useEsDocSearch', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a small cleanup getting rid of the async state update outside of act(...) warnings.
The latest react update made it possible to wait for the async updates by using asynchronous act. In the case of this test the assertions assumed no async functions would be called, so the mocks had to be slightly adjusted.

const indexPattern = {
getComputedFields: () => [],
};
Expand All @@ -53,16 +53,16 @@ describe('Test of <Doc /> helper / hook', () => {
const props = {
id: '1',
index: 'index1',
esClient: { search: jest.fn() },
esClient: { search: jest.fn(() => new Promise(() => {})) },
indexPatternId: 'xyz',
indexPatternService,
} as DocProps;
let hook;
act(() => {
await act(async () => {
hook = renderHook((p: DocProps) => useEsDocSearch(p), { initialProps: props });
});
// @ts-ignore
expect(hook.result.current).toEqual([ElasticRequestState.Loading, null, null]);
expect(hook.result.current).toEqual([ElasticRequestState.Loading, null, indexPattern]);
expect(indexPatternService.get).toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -21,34 +21,33 @@ import { mount, shallow } from 'enzyme';
import { DocViewer } from './doc_viewer';
// @ts-ignore
import { findTestSubject } from '@elastic/eui/lib/test';
import {
addDocView,
emptyDocViews,
DocViewRenderProps,
getDocViewsSorted as mockGetDocViewsSorted,
} from 'ui/registry/doc_views';
import { DocViewRenderProps } from '../../doc_views/doc_views_registry';
import { getServices } from '../../kibana_services';

jest.mock('../../kibana_services', () => {
// eslint-disable-next-line @typescript-eslint/no-var-requires
const { DocViewsRegistry } = require('../../doc_views/doc_views_registry');
let registry = new DocViewsRegistry();
return {
getServices: () => ({
docViewsRegistry: {
getDocViewsSorted: (hit: any) => {
return mockGetDocViewsSorted(hit);
},
docViewsRegistry: registry,
resetRegistry: () => {
registry = new DocViewsRegistry();
},
}),
};
});

beforeEach(() => {
emptyDocViews();
(getServices() as any).resetRegistry();
jest.clearAllMocks();
});

test('Render <DocViewer/> with 3 different tabs', () => {
addDocView({ order: 20, title: 'React component', component: () => <div>test</div> });
addDocView({ order: 10, title: 'Render function', render: jest.fn() });
addDocView({ order: 30, title: 'Invalid doc view' });
const registry = getServices().docViewsRegistry;
registry.addDocView({ order: 20, title: 'React component', component: () => <div>test</div> });
registry.addDocView({ order: 10, title: 'Render function', render: jest.fn() });
registry.addDocView({ order: 30, title: 'Invalid doc view' });

const renderProps = { hit: {} } as DocViewRenderProps;

Expand All @@ -63,7 +62,8 @@ test('Render <DocViewer/> with 1 tab displaying error message', () => {
return null;
}

addDocView({
const registry = getServices().docViewsRegistry;
registry.addDocView({
order: 10,
title: 'React component',
component: SomeComponent,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
* under the License.
*/
import React from 'react';
import { DocView } from 'ui/registry/doc_views_types';
import { EuiTabbedContent } from '@elastic/eui';
import { getServices, DocViewRenderProps } from '../../kibana_services';
import { DocView, DocViewRenderProps } from '../../doc_views/doc_views_types';
import { getServices } from '../../kibana_services';
import { DocViewerTab } from './doc_viewer_tab';

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import React from 'react';
import { mount } from 'enzyme';
import { DocViewRenderTab } from './doc_viewer_render_tab';
import { DocViewRenderProps } from '../../kibana_services';
import { DocViewRenderProps } from '../../doc_views/doc_views_types';

test('Mounting and unmounting DocViewerRenderTab', () => {
const unmountFn = jest.fn();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/
import React, { useRef, useEffect } from 'react';
import { DocViewRenderFn, DocViewRenderProps } from '../../kibana_services';
import { DocViewRenderFn, DocViewRenderProps } from '../../doc_views/doc_views_types';

interface Props {
render: DocViewRenderFn;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
*/
import React from 'react';
import { I18nProvider } from '@kbn/i18n/react';
import { DocViewRenderProps, DocViewRenderFn } from '../../kibana_services';
import { DocViewRenderTab } from './doc_viewer_render_tab';
import { DocViewerError } from './doc_viewer_render_error';
import { DocViewRenderFn, DocViewRenderProps } from '../../doc_views/doc_views_types';

interface Props {
component?: React.ComponentType<DocViewRenderProps>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@
import React from 'react';
import { EuiCodeBlock } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { DocViewRenderProps } from 'ui/registry/doc_views';
import { DocViewRenderProps } from '../../doc_views/doc_views_types';

export function JsonCodeBlock({ hit }: DocViewRenderProps) {
const label = i18n.translate('kbnDocViews.json.codeEditorAriaLabel', {
const label = i18n.translate('kbn.discover.docViews.json.codeEditorAriaLabel', {
defaultMessage: 'Read only JSON view of an elasticsearch document',
});
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { mount } from 'enzyme';
import { IndexPattern } from 'ui/index_patterns';
// @ts-ignore
import { findTestSubject } from '@elastic/eui/lib/test';
import { flattenHitWrapper } from '../../../../data/public/';
import { flattenHitWrapper } from '../../../../../data/public';
import { DocViewTable } from './table';

// @ts-ignore
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
* under the License.
*/
import React, { useState } from 'react';
import { DocViewRenderProps } from 'ui/registry/doc_views';
import { DocViewTableRow } from './table_row';
import { arrayContainsObjects, trimAngularSpan } from './table_helper';
import { DocViewRenderProps } from '../../doc_views/doc_views_types';

const COLLAPSE_LINE_LENGTH = 350;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@
* specific language governing permissions and limitations
* under the License.
*/
import classNames from 'classnames';
import React, { ReactNode } from 'react';
import { FieldName } from 'ui/directives/field_name/field_name';
import { FieldMapping, DocViewFilterFn } from 'ui/registry/doc_views_types';
import classNames from 'classnames';
import { FieldMapping, DocViewFilterFn } from '../../doc_views/doc_views_types';
import { DocViewTableRowBtnFilterAdd } from './table_row_btn_filter_add';
import { DocViewTableRowBtnFilterRemove } from './table_row_btn_filter_remove';
import { DocViewTableRowBtnToggleColumn } from './table_row_btn_toggle_column';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export interface Props {
}

export function DocViewTableRowBtnCollapse({ onClick, isCollapsed }: Props) {
const label = i18n.translate('kbnDocViews.table.toggleFieldDetails', {
const label = i18n.translate('kbn.discover.docViews.table.toggleFieldDetails', {
defaultMessage: 'Toggle field details',
});
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,20 @@ export interface Props {
export function DocViewTableRowBtnFilterAdd({ onClick, disabled = false }: Props) {
const tooltipContent = disabled ? (
<FormattedMessage
id="kbnDocViews.table.unindexedFieldsCanNotBeSearchedTooltip"
id="kbn.discover.docViews.table.unindexedFieldsCanNotBeSearchedTooltip"
defaultMessage="Unindexed fields can not be searched"
/>
) : (
<FormattedMessage
id="kbnDocViews.table.filterForValueButtonTooltip"
id="kbn.discover.docViews.table.filterForValueButtonTooltip"
defaultMessage="Filter for value"
/>
);

return (
<EuiToolTip content={tooltipContent}>
<EuiButtonIcon
aria-label={i18n.translate('kbnDocViews.table.filterForValueButtonAriaLabel', {
aria-label={i18n.translate('kbn.discover.docViews.table.filterForValueButtonAriaLabel', {
defaultMessage: 'Filter for value',
})}
className="kbnDocViewer__actionButton"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,28 +35,31 @@ export function DocViewTableRowBtnFilterExists({
const tooltipContent = disabled ? (
scripted ? (
<FormattedMessage
id="kbnDocViews.table.unableToFilterForPresenceOfScriptedFieldsTooltip"
id="kbn.discover.docViews.table.unableToFilterForPresenceOfScriptedFieldsTooltip"
defaultMessage="Unable to filter for presence of scripted fields"
/>
) : (
<FormattedMessage
id="kbnDocViews.table.unableToFilterForPresenceOfMetaFieldsTooltip"
id="kbn.discover.docViews.table.unableToFilterForPresenceOfMetaFieldsTooltip"
defaultMessage="Unable to filter for presence of meta fields"
/>
)
) : (
<FormattedMessage
id="kbnDocViews.table.filterForFieldPresentButtonTooltip"
id="kbn.discover.docViews.table.filterForFieldPresentButtonTooltip"
defaultMessage="Filter for field present"
/>
);

return (
<EuiToolTip content={tooltipContent}>
<EuiButtonIcon
aria-label={i18n.translate('kbnDocViews.table.filterForFieldPresentButtonAriaLabel', {
defaultMessage: 'Filter for field present',
})}
aria-label={i18n.translate(
'kbn.discover.docViews.table.filterForFieldPresentButtonAriaLabel',
{
defaultMessage: 'Filter for field present',
}
)}
onClick={onClick}
className="kbnDocViewer__actionButton"
data-test-subj="addExistsFilterButton"
Expand Down
Loading