Skip to content

Commit

Permalink
[Discover] [Log Explorer] Add tabs to Discover and Log Explorer (#171467
Browse files Browse the repository at this point in the history
)

## Summary

This PR adds tabs to navigate between Discover and Log Explorer in
serverless O11y projects. The Discover top nav should remain unchanged
in stateful deployments and all other serverless project types:

![tabs](https://github.com/elastic/kibana/assets/25592674/c68678be-ab1c-4323-bbd5-1f83828e8dce)

> [!IMPORTANT]  
> While writing tests for this, I encountered a few issues we'll
probably want to discuss and make decisions for before merging this PR.

1. When there are no data views in Kibana and a user navigates to
Discover, they are prevented from accessing it and shown a no data
screen. So if a user navigates to Discover in a serverless O11y project
in order to get to Log Explorer but there are no existing data views,
they will be unable to access the tabs since they will be blocked by the
no data screen. Is this a realistic scenario in serverless O11y that we
need to solve for, or will there always be at least one data view by
default?
2. When navigating from the Log Explorer tab to the Discover tab, we
convert the current dataset to an ad hoc data view to use in Discover.
This doesn't work in reverse since Log Explorer can't load an arbitrary
data view that might be selected in Discover, so instead I'm using the
"all logs" locator. I'll leave it to the O11y team to decide if this is
ok or if we need to take another approach here.
3. Since we are passing state between Discover and Log Explorer when a
user switches between tabs, the default columns in Log Explorer will be
overwritten by the current Discover grid columns (even if it's just the
Document column). I imagine the O11y team doesn't want this, but how
should we solve it? Should we avoid passing columns (and any other state
that might overwrite defaults) when navigating from Discover to Log
Explorer, or take another approach?

## Notes
- The sidebar navigation link has been changed from "Log Explorer" to
"Discover" and defaults to the Discover tab.
- "Log Explorer" has been renamed to "Logs Explorer" in the UI.
- The mockups used "Data Views" for the Discover tab title, but this was
later decided against, so the implementation uses "Discover".
- All of the same state that used to be passed from Log Explorer to
Discover through the top nav button is still being passed, but I also
added support for passing `sort` as well (I can revert this if it's
unwanted).
- In order to add tabs to the left side of the Discover top nav in
serverless, we had to stop relying on Unified Search for rendering the
top nav (in serverless only, stateful still uses it). Instead we are now
rendering the top nav directly in Discover in serverless, similar to
what Log Explorer does. This also required access to some of the
internal navigation plugin components we rely on for the Discover top
nav, so I exported them from the plugin since I figured it was better
than duplicating them.

Part of #169964. **In order to fully resolve this issue, there is still
a remaining task to remove the "Data Views" tab from the Log Explorer
dataset selector.**

Part of #171386. **In order to fully resolve this issue, it looks like
there may be some documentation work to do (especially since "Log
Explorer" has been named to "Logs Explorer" in the UI).**

### Checklist

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [ ] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
davismcphee and kibanamachine authored Dec 8, 2023
1 parent ef87bce commit cbb16b8
Show file tree
Hide file tree
Showing 51 changed files with 1,035 additions and 285 deletions.
3 changes: 2 additions & 1 deletion src/plugins/discover/kibana.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@
"savedObjectsTaggingOss",
"lens",
"noDataPage",
"globalSearch"
"globalSearch",
"serverless"
],
"requiredBundles": ["kibanaUtils", "kibanaReact", "unifiedSearch"],
"extraPublicDirs": ["common"]
Expand Down
4 changes: 4 additions & 0 deletions src/plugins/discover/public/__mocks__/discover_state.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ export function getDiscoverStateMock({
const container = getDiscoverStateContainer({
services: discoverServiceMock,
history,
customizationContext: {
displayMode: 'standalone',
showLogExplorerTabs: false,
},
});
container.savedSearchState.set(
savedSearch ? savedSearch : isTimeBased ? savedSearchMockWithTimeField : savedSearchMock
Expand Down
42 changes: 36 additions & 6 deletions src/plugins/discover/public/application/discover_router.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,19 @@ import { Redirect, RouteProps } from 'react-router-dom';
import { Route } from '@kbn/shared-ux-router';
import { createSearchSessionMock } from '../__mocks__/search_session';
import { discoverServiceMock as mockDiscoverServices } from '../__mocks__/services';
import { CustomDiscoverRoutes, DiscoverRouter, DiscoverRoutes } from './discover_router';
import {
CustomDiscoverRoutes,
DiscoverRouter,
DiscoverRoutes,
DiscoverRoutesProps,
} from './discover_router';
import { DiscoverMainRoute } from './main';
import { SingleDocRoute } from './doc';
import { ContextAppRoute } from './context';
import { createProfileRegistry } from '../customizations/profile_registry';
import { addProfile } from '../../common/customizations';
import { NotFoundRoute } from './not_found';
import type { DiscoverCustomizationContext } from './types';

let mockProfile: string | undefined;

Expand All @@ -43,9 +49,15 @@ const gatherRoutes = (wrapper: ShallowWrapper) => {
});
};

const props = {
const customizationContext: DiscoverCustomizationContext = {
displayMode: 'standalone',
showLogExplorerTabs: false,
};

const props: DiscoverRoutesProps = {
isDev: false,
customizationCallbacks: [],
customizationContext,
};

describe('DiscoverRoutes', () => {
Expand Down Expand Up @@ -147,12 +159,17 @@ describe('CustomDiscoverRoutes', () => {
it('should show DiscoverRoutes for a valid profile', () => {
mockProfile = 'test';
const component = shallow(
<CustomDiscoverRoutes profileRegistry={profileRegistry} isDev={props.isDev} />
<CustomDiscoverRoutes
profileRegistry={profileRegistry}
customizationContext={customizationContext}
isDev={props.isDev}
/>
);
expect(component.find(DiscoverRoutes).getElement()).toMatchObject(
<DiscoverRoutes
prefix={addProfile('', mockProfile)}
customizationCallbacks={callbacks}
customizationContext={customizationContext}
isDev={props.isDev}
/>
);
Expand All @@ -161,7 +178,11 @@ describe('CustomDiscoverRoutes', () => {
it('should show NotFoundRoute for an invalid profile', () => {
mockProfile = 'invalid';
const component = shallow(
<CustomDiscoverRoutes profileRegistry={profileRegistry} isDev={props.isDev} />
<CustomDiscoverRoutes
profileRegistry={profileRegistry}
customizationContext={customizationContext}
isDev={props.isDev}
/>
);
expect(component.find(NotFoundRoute).getElement()).toMatchObject(<NotFoundRoute />);
});
Expand All @@ -178,6 +199,7 @@ describe('DiscoverRouter', () => {
services={mockDiscoverServices}
history={history}
profileRegistry={profileRegistry}
customizationContext={customizationContext}
isDev={props.isDev}
/>
);
Expand All @@ -186,13 +208,21 @@ describe('DiscoverRouter', () => {

it('should show DiscoverRoutes component for / route', () => {
expect(pathMap['/']).toMatchObject(
<DiscoverRoutes customizationCallbacks={callbacks} isDev={props.isDev} />
<DiscoverRoutes
customizationCallbacks={callbacks}
customizationContext={customizationContext}
isDev={props.isDev}
/>
);
});

it(`should show CustomDiscoverRoutes component for ${profilePath} route`, () => {
expect(pathMap[profilePath]).toMatchObject(
<CustomDiscoverRoutes profileRegistry={profileRegistry} isDev={props.isDev} />
<CustomDiscoverRoutes
profileRegistry={profileRegistry}
customizationContext={customizationContext}
isDev={props.isDev}
/>
);
});
});
6 changes: 5 additions & 1 deletion src/plugins/discover/public/application/discover_router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@ import { ViewAlertRoute } from './view_alert';
import type { CustomizationCallback } from '../customizations';
import type { DiscoverProfileRegistry } from '../customizations/profile_registry';
import { addProfile } from '../../common/customizations';
import type { DiscoverCustomizationContext } from './types';

interface DiscoverRoutesProps {
export interface DiscoverRoutesProps {
prefix?: string;
customizationCallbacks: CustomizationCallback[];
customizationContext: DiscoverCustomizationContext;
isDev: boolean;
}

Expand Down Expand Up @@ -66,6 +68,7 @@ export const DiscoverRoutes = ({ prefix, ...mainRouteProps }: DiscoverRoutesProp

interface CustomDiscoverRoutesProps {
profileRegistry: DiscoverProfileRegistry;
customizationContext: DiscoverCustomizationContext;
isDev: boolean;
}

Expand All @@ -92,6 +95,7 @@ export const CustomDiscoverRoutes = ({ profileRegistry, ...props }: CustomDiscov
export interface DiscoverRouterProps {
services: DiscoverServices;
profileRegistry: DiscoverProfileRegistry;
customizationContext: DiscoverCustomizationContext;
history: History;
isDev: boolean;
}
Expand Down
11 changes: 10 additions & 1 deletion src/plugins/discover/public/application/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,23 @@ import { toMountPoint } from '@kbn/react-kibana-mount';
import { DiscoverRouter } from './discover_router';
import { DiscoverServices } from '../build_services';
import type { DiscoverProfileRegistry } from '../customizations/profile_registry';
import type { DiscoverCustomizationContext } from './types';

export interface RenderAppProps {
element: HTMLElement;
services: DiscoverServices;
profileRegistry: DiscoverProfileRegistry;
customizationContext: DiscoverCustomizationContext;
isDev: boolean;
}

export const renderApp = ({ element, services, profileRegistry, isDev }: RenderAppProps) => {
export const renderApp = ({
element,
services,
profileRegistry,
customizationContext,
isDev,
}: RenderAppProps) => {
const { history: getHistory, capabilities, chrome, data, core } = services;

const history = getHistory();
Expand All @@ -39,6 +47,7 @@ export const renderApp = ({ element, services, profileRegistry, isDev }: RenderA
<DiscoverRouter
services={services}
profileRegistry={profileRegistry}
customizationContext={customizationContext}
history={history}
isDev={isDev}
/>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { createHashHistory } from 'history';
import { SavedSearch } from '@kbn/saved-search-plugin/public';
import { buildDataTableRecordList } from '@kbn/discover-utils';
import { esHitsMock } from '@kbn/discover-utils/src/__mocks__';
import { FetchStatus } from '../../../../types';
import { DiscoverCustomizationContext, FetchStatus } from '../../../../types';
import {
AvailableFields$,
DataDocuments$,
Expand Down Expand Up @@ -124,11 +124,17 @@ function getSavedSearch(dataView: DataView) {
} as unknown as SavedSearch;
}

const customizationContext: DiscoverCustomizationContext = {
displayMode: 'standalone',
showLogExplorerTabs: false,
};

export function getDocumentsLayoutProps(dataView: DataView) {
const stateContainer = getDiscoverStateContainer({
history: createHashHistory(),
savedSearch: getSavedSearch(dataView),
services,
customizationContext,
});
stateContainer.appState.set({
columns: ['name', 'message', 'bytes'],
Expand All @@ -153,6 +159,7 @@ export const getPlainRecordLayoutProps = (dataView: DataView) => {
history: createHashHistory(),
savedSearch: getSavedSearch(dataView),
services,
customizationContext,
});
stateContainer.appState.set({
columns: ['name', 'message', 'bytes'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ discover-app {
.dscPage {
@include euiBreakpoint('m', 'l', 'xl') {
@include kibanaFullBodyHeight();

&.dscPage--serverless {
@include kibanaFullBodyHeight($euiSize * 3);
}
}

flex-direction: column;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import { DiscoverStateContainer } from '../../services/discover_state';
import { VIEW_MODE } from '../../../../../common/constants';
import { useInternalStateSelector } from '../../services/discover_internal_state_container';
import { useAppStateSelector } from '../../services/discover_app_state_container';
import { useInspector } from '../../hooks/use_inspector';
import { useDiscoverServices } from '../../../../hooks/use_discover_services';
import { DiscoverNoResults } from '../no_results';
import { LoadingSpinner } from '../loading_spinner/loading_spinner';
Expand Down Expand Up @@ -73,8 +72,8 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) {
filterManager,
history,
spaces,
inspector,
docLinks,
serverless,
} = useDiscoverServices();
const { euiTheme } = useEuiTheme();
const pageBackgroundColor = useEuiBackgroundColor('plain');
Expand Down Expand Up @@ -118,11 +117,6 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) {
[dataState.fetchStatus, dataState.foundDocuments]
);

const onOpenInspector = useInspector({
inspector,
stateContainer,
});

const {
columns: currentColumns,
onAddColumn,
Expand Down Expand Up @@ -243,7 +237,7 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) {

return (
<EuiPage
className="dscPage"
className={classNames('dscPage', { 'dscPage--serverless': serverless })}
data-fetch-counter={fetchCounter.current}
css={css`
background-color: ${pageBackgroundColor};
Expand All @@ -266,12 +260,9 @@ export function DiscoverLayout({ stateContainer }: DiscoverLayoutProps) {
})}
</h1>
<TopNavMemoized
onOpenInspector={onOpenInspector}
query={query}
savedQuery={savedQuery}
stateContainer={stateContainer}
updateQuery={stateContainer.actions.onUpdateQuery}
isPlainRecord={isPlainRecord}
textBasedLanguageModeErrors={textBasedLanguageModeErrors}
textBasedLanguageModeWarning={textBasedLanguageModeWarning}
onFieldEdited={onFieldEdited}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,20 @@ import { mountWithIntl } from '@kbn/test-jest-helpers';
import { dataViewMock } from '@kbn/discover-utils/src/__mocks__';
import { DiscoverTopNav, DiscoverTopNavProps } from './discover_topnav';
import { TopNavMenu, TopNavMenuData } from '@kbn/navigation-plugin/public';
import { Query } from '@kbn/es-query';
import { setHeaderActionMenuMounter } from '../../../../kibana_services';
import { discoverServiceMock as mockDiscoverService } from '../../../../__mocks__/services';
import { getDiscoverStateMock } from '../../../../__mocks__/discover_state.mock';
import { DiscoverMainProvider } from '../../services/discover_state_provider';
import type { SearchBarCustomization, TopNavCustomization } from '../../../../customizations';
import type { DiscoverCustomizationId } from '../../../../customizations/customization_service';
import { useDiscoverCustomization } from '../../../../customizations';
import { useKibana } from '@kbn/kibana-react-plugin/public';

setHeaderActionMenuMounter(jest.fn());

jest.mock('@kbn/kibana-react-plugin/public', () => ({
...jest.requireActual('@kbn/kibana-react-plugin/public'),
useKibana: () => ({
services: mockDiscoverService,
}),
useKibana: jest.fn(),
}));

const MockCustomSearchBar: typeof mockDiscoverService.navigation.ui.AggregateQueryTopNavMenu =
Expand Down Expand Up @@ -77,15 +75,14 @@ function getProps(

return {
stateContainer,
query: {} as Query,
savedQuery: '',
updateQuery: jest.fn(),
onOpenInspector: jest.fn(),
onFieldEdited: jest.fn(),
isPlainRecord: false,
};
}

const mockUseKibana = useKibana as jest.Mock;

describe('Discover topnav component', () => {
beforeEach(() => {
mockTopNavCustomization.defaultMenu = undefined;
Expand All @@ -107,6 +104,10 @@ describe('Discover topnav component', () => {
throw new Error(`Unknown customization id: ${id}`);
}
});

mockUseKibana.mockReturnValue({
services: mockDiscoverService,
});
});

test('generated config of TopNavMenu config is correct when discover save permissions are assigned', () => {
Expand Down Expand Up @@ -280,4 +281,38 @@ describe('Discover topnav component', () => {
expect(topNav.prop('dataViewPickerComponentProps')).toBeUndefined();
});
});

describe('serverless', () => {
it('should render top nav when serverless plugin is not defined', () => {
const props = getProps();
const component = mountWithIntl(
<DiscoverMainProvider value={props.stateContainer}>
<DiscoverTopNav {...props} />
</DiscoverMainProvider>
);
const searchBar = component.find(mockDiscoverService.navigation.ui.AggregateQueryTopNavMenu);
expect(searchBar.prop('badges')).toBeDefined();
expect(searchBar.prop('config')).toBeDefined();
expect(searchBar.prop('setMenuMountPoint')).toBeDefined();
});

it('should not render top nav when serverless plugin is defined', () => {
mockUseKibana.mockReturnValue({
services: {
...mockDiscoverService,
serverless: true,
},
});
const props = getProps();
const component = mountWithIntl(
<DiscoverMainProvider value={props.stateContainer}>
<DiscoverTopNav {...props} />
</DiscoverMainProvider>
);
const searchBar = component.find(mockDiscoverService.navigation.ui.AggregateQueryTopNavMenu);
expect(searchBar.prop('badges')).toBeUndefined();
expect(searchBar.prop('config')).toBeUndefined();
expect(searchBar.prop('setMenuMountPoint')).toBeUndefined();
});
});
});
Loading

0 comments on commit cbb16b8

Please sign in to comment.