Skip to content

Commit

Permalink
[Log Explorer][Discover] Use DocViewsRegistry to add flyout Overview …
Browse files Browse the repository at this point in the history
…tab (#172364)

## 📓 Summary

Closes #171716 

To apply a new customization that allows consumers of the
`DiscoverContainer` to apply ad-hoc flyout tabs, this work refactors and
extends the current implementation of the `DocViewsRegistry` and
enhances the `flyout` extension point providing a new instance of the
base registry for customizations to be applied. Mainly, the changes
consist of:

### DocViewsRegistry
- Add a required `id` property when adding a `DocView`. Earlier the
identifier generation was purely based on the order on the list, with
this addition we now rely on unique strings for each doc view. This `id`
is also used to store and potentially remove the doc view.
- Move the sorting step when adding a new doc view. Previously the
sorting was performed on each retrieval, moving this operation at
insertion time we can only perform this once.
- Rename `addDocView` to `add`. In the context of the `DocViewsRegistry`
this name reflects better some native API names such as Set, Map etc.
- Add the `removeById` method. It will allow when necessary to remove
any doc view that matches the passed id.
- Add the `clone` method. Since we don't want to always mutate a
registry but we still want to use it as a base, we can clone it and
generate a new instance with this method. Used for generating a new
registry used by any customization profile.
- Removed filtering by the `shouldShow` property. Apart from not having
any consumer using this option, we agreed offline that is not ideal to
perform filtering on the doc views.

### UnifiedDocViewer plugin
- The `UnifiedDocViewer`component now accepts an optional
`docViewsRegistry` property. This can either be an instance of
`DocViewsRegistry` or a function that will provide a new copy of the
default registry created and exposed by the plugin.
This is important as we don't want consumers to update the base registry
but work on a clone that applies changes only for the current usage.
When passed, this registry will be used as a docViews source for the
`UnifiedDocViewer` component.
When instead is not passed, the source will be the default registry
instance.
- The plugin contract now exposes the whole registry instance instead of
the previous `addDocView ' method. This is necessary so that consumers
can clone this registry and work on the new instance using all the
features encapsulated in the `DocViewsRegistry` implementation, like in
the case described above.

### LogExplorer customization
- We moved the customization for the flyout content into a new doc view,
which is added to the Discover UnifiedDocViewer only for this app
through a new parameter on the `flyout` extension point, the
`docViewerRegistry` function which will be forwarded to the
UnifiedDocViewer component, following the logic described earlier in
this PR.

**Example**
```ts
customizations.set({
  id: 'flyout',
  docViewsRegistry: (registry) => {
    registry.add({
      id: 'doc_view_log_overview',
      title: 'Overview',
      order: 0,
      component: (props) => {
        return <FlyoutContent {...props} />;
      },
    });

    return registry;
  },
});

```

## 🎥 Demo


https://github.com/elastic/kibana/assets/34506779/894234cc-02e1-4880-949e-720f99793823

---------

Co-authored-by: Marco Antonio Ghiani <marcoantonio.ghiani@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
3 people authored Dec 18, 2023
1 parent d22560e commit b8a23eb
Show file tree
Hide file tree
Showing 30 changed files with 298 additions and 142 deletions.

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 @@ -17,16 +17,19 @@ import { DocViewsRegistry } from '../..';
describe('<DocViewer />', () => {
test('Render <DocViewer/> with 3 different tabs', () => {
const registry = new DocViewsRegistry();
registry.addDocView({ order: 10, title: 'Render function', render: jest.fn() });
registry.addDocView({ order: 20, title: 'React component', component: () => <div>test</div> });
registry.add({ id: 'function', order: 10, title: 'Render function', render: jest.fn() });
registry.add({
id: 'component',
order: 20,
title: 'React component',
component: () => <div>test</div>,
});
// @ts-expect-error This should be invalid and will throw an error when rendering
registry.addDocView({ order: 30, title: 'Invalid doc view' });
registry.add({ id: 'invalid', order: 30, title: 'Invalid doc view' });

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

const wrapper = shallow(
<DocViewer docViews={registry.getDocViewsSorted(renderProps.hit)} {...renderProps} />
);
const wrapper = shallow(<DocViewer docViews={registry.getAll()} {...renderProps} />);

expect(wrapper).toMatchSnapshot();
});
Expand All @@ -38,7 +41,8 @@ describe('<DocViewer />', () => {
}

const registry = new DocViewsRegistry();
registry.addDocView({
registry.add({
id: 'component',
order: 10,
title: 'React component',
component: SomeComponent,
Expand All @@ -49,9 +53,7 @@ describe('<DocViewer />', () => {
} as DocViewRenderProps;
const errorMsg = 'Catch me if you can!';

const wrapper = mount(
<DocViewer docViews={registry.getDocViewsSorted(renderProps.hit)} {...renderProps} />
);
const wrapper = mount(<DocViewer docViews={registry.getAll()} {...renderProps} />);
const error = new Error(errorMsg);
wrapper.find(SomeComponent).simulateError(error);
const errorMsgComponent = findTestSubject(wrapper, 'docViewerError');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,25 @@ export interface DocViewerProps extends DocViewRenderProps {
* a `render` function.
*/
export function DocViewer({ docViews, ...renderProps }: DocViewerProps) {
const tabs = docViews.map(({ title, render, component }: DocView, idx: number) => {
const tabs = docViews.map(({ id, title, render, component }: DocView) => {
return {
id: `kbn_doc_viewer_tab_${idx}`,
id: `kbn_doc_viewer_tab_${id}`,
name: title,
content: (
<DocViewerTab
id={idx}
id={id}
title={title}
component={component}
renderProps={renderProps}
render={render}
/>
),
['data-test-subj']: `docViewerTab-${idx}`,
['data-test-subj']: `docViewerTab-${id}`,
};
});

if (!tabs.length) {
// There there's a minimum of 2 tabs active in Discover.
// There's a minimum of 2 tabs active in Discover.
// This condition takes care of unit tests with 0 tabs.
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ describe('DocViewerTab', () => {
test('changing columns triggers an update', () => {
const hit = buildDataTableRecord({ _index: 'test', _id: '1' }, dataViewMock);
const props = {
id: 'doc_view_test',
title: 'test',
component: jest.fn(),
id: 1,
render: jest.fn(),
renderProps: {
hit,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { DocViewerError } from './doc_viewer_error';
import type { DocViewRenderFn, DocViewRenderProps } from '../../types';

interface Props {
id: number;
id: string;
renderProps: DocViewRenderProps;
title: string;
render?: DocViewRenderFn;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/*
* 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 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import React from 'react';
import { DocViewsRegistry } from './doc_views_registry';

const fnDocView = {
id: 'function-doc-view',
order: 10,
title: 'Render function',
render: jest.fn(),
};
const componentDocView = {
id: 'component-doc-view',
order: 20,
title: 'React component',
component: () => <div>test</div>,
};

describe('DocViewerRegistry', () => {
test('can be initialized from an array of doc views', () => {
const registry = new DocViewsRegistry([fnDocView, componentDocView]);

expect(registry.getAll()).toHaveLength(2);
});

test('can be initialized from another DocViewsRegistry instance', () => {
const registry = new DocViewsRegistry([fnDocView, componentDocView]);
const newRegistry = new DocViewsRegistry(registry);

expect(registry.getAll()).toHaveLength(2);
expect(newRegistry.getAll()).toHaveLength(2);
expect(registry).not.toBe(newRegistry);
});

describe('#add', () => {
test('should add a doc view to the registry in the correct order', () => {
const registry = new DocViewsRegistry([componentDocView]);

registry.add(fnDocView);

const docViews = registry.getAll();

expect(docViews[0]).toHaveProperty('id', 'function-doc-view');
expect(docViews[1]).toHaveProperty('id', 'component-doc-view');
});

test('should throw an error when the passed doc view already exists for the given id', () => {
const registry = new DocViewsRegistry([fnDocView]);

expect(() => registry.add(fnDocView)).toThrow(
'DocViewsRegistry#add: a DocView is already registered with id "function-doc-view".'
);
});
});

describe('#removeById', () => {
test('should remove a doc view given the passed id', () => {
const registry = new DocViewsRegistry([fnDocView, componentDocView]);

const docViews = registry.getAll();

expect(docViews[0]).toHaveProperty('id', 'function-doc-view');
expect(docViews[1]).toHaveProperty('id', 'component-doc-view');

registry.removeById('function-doc-view');

expect(registry.getAll()[0]).toHaveProperty('id', 'component-doc-view');
});
});

describe('#clone', () => {
test('should return a new DocViewRegistry instance starting from the current one', () => {
const registry = new DocViewsRegistry([fnDocView, componentDocView]);

const clonedRegistry = registry.clone();
const docViews = clonedRegistry.getAll();

expect(docViews[0]).toHaveProperty('id', 'function-doc-view');
expect(docViews[1]).toHaveProperty('id', 'component-doc-view');
expect(registry).not.toBe(clonedRegistry);
});
});
});
61 changes: 42 additions & 19 deletions packages/kbn-unified-doc-viewer/src/services/doc_views_registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import type { DataTableRecord } from '@kbn/discover-utils/types';
import { DocView, DocViewInput, DocViewInputFn } from './types';
import type { DocView, DocViewFactory } from './types';

export enum ElasticRequestState {
Loading,
Expand All @@ -18,24 +16,49 @@ export enum ElasticRequestState {
}

export class DocViewsRegistry {
private docViews: DocView[] = [];
private docViews: Map<string, DocView>;

constructor(initialValue?: DocViewsRegistry | DocView[]) {
if (initialValue instanceof DocViewsRegistry) {
this.docViews = new Map(initialValue.docViews);
} else if (Array.isArray(initialValue)) {
this.docViews = new Map(initialValue.map((docView) => [docView.id, docView]));
} else {
this.docViews = new Map();
}
}

/**
* Extends and adds the given doc view to the registry array
*/
addDocView(docViewRaw: DocViewInput | DocViewInputFn) {
getAll() {
return [...this.docViews.values()];
}

add(docViewRaw: DocView | DocViewFactory) {
const docView = typeof docViewRaw === 'function' ? docViewRaw() : docViewRaw;
this.docViews.push({
...docView,
shouldShow: docView.shouldShow ?? (() => true),
});

if (this.docViews.has(docView.id)) {
throw new Error(
`DocViewsRegistry#add: a DocView is already registered with id "${docView.id}".`
);
}

this.docViews.set(docView.id, docView);
// Sort the doc views at insertion time to perform this operation once and not on every retrieval.
this.sortDocViews();
}
/**
* Returns a sorted array of doc_views for rendering tabs
*/
getDocViewsSorted(hit: DataTableRecord) {
return this.docViews
.filter((docView) => docView.shouldShow(hit))
.sort((a, b) => (Number(a.order) > Number(b.order) ? 1 : -1));

removeById(id: string) {
this.docViews.delete(id);
}

clone() {
return new DocViewsRegistry(this);
}

private sortDocViews() {
const sortedEntries = [...this.docViews.entries()].sort(
([_currKey, curr], [_nextKey, next]) => curr.order - next.order
);

this.docViews = new Map(sortedEntries);
}
}
12 changes: 5 additions & 7 deletions packages/kbn-unified-doc-viewer/src/services/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import type { DataView, DataViewField } from '@kbn/data-views-plugin/public';
import type { AggregateQuery, Query } from '@kbn/es-query';
import type { DataTableRecord, IgnoredReason } from '@kbn/discover-utils/types';
import { DocViewsRegistry } from './doc_views_registry';

export interface FieldMapping {
filterable?: boolean;
Expand Down Expand Up @@ -40,6 +41,7 @@ export interface DocViewRenderProps {
filter?: DocViewFilterFn;
onAddColumn?: (columnName: string) => void;
onRemoveColumn?: (columnName: string) => void;
docViewsRegistry?: DocViewsRegistry | ((prevRegistry: DocViewsRegistry) => DocViewsRegistry);
}
export type DocViewerComponent = React.FC<DocViewRenderProps>;
export type DocViewRenderFn = (
Expand All @@ -48,8 +50,8 @@ export type DocViewRenderFn = (
) => () => void;

export interface BaseDocViewInput {
id: string;
order: number;
shouldShow?: (hit: DataTableRecord) => boolean;
title: string;
}

Expand All @@ -65,13 +67,9 @@ interface ComponentDocViewInput extends BaseDocViewInput {
directive?: undefined;
}

export type DocViewInput = ComponentDocViewInput | RenderDocViewInput;
export type DocView = ComponentDocViewInput | RenderDocViewInput;

export type DocView = DocViewInput & {
shouldShow: NonNullable<DocViewInput['shouldShow']>;
};

export type DocViewInputFn = () => DocViewInput;
export type DocViewFactory = () => DocView;

export interface FieldRecordLegacy {
action: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,11 +315,14 @@ describe('Discover flyout', function () {
it('should provide an actions prop collection to optionally update the grid content', async () => {
mockFlyoutCustomization.Content = ({ actions }) => (
<>
<button data-test-subj="addColumn" onClick={() => actions.addColumn('message')} />
<button data-test-subj="removeColumn" onClick={() => actions.removeColumn('message')} />
<button data-test-subj="addColumn" onClick={() => actions.onAddColumn?.('message')} />
<button
data-test-subj="removeColumn"
onClick={() => actions.onRemoveColumn?.('message')}
/>
<button
data-test-subj="addFilter"
onClick={() => actions.addFilter?.('_exists_', 'message', '+')}
onClick={() => actions.filter?.('_exists_', 'message', '+')}
/>
</>
);
Expand Down
Loading

0 comments on commit b8a23eb

Please sign in to comment.