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.

Sign up for GitHub

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

Jump to bottom

[Lens][Dashboard] Adding Lens to Dashboard #53110

Merged
merged 23 commits into from
Jan 13, 2020
Merged

[Lens][Dashboard] Adding Lens to Dashboard #53110

Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
283ca5b
First version of adding Lens to dashboard
Dec 16, 2019
1842088
Fix failing unit test
Dec 16, 2019
57bef33
Replacing explicit Lens query param with a more generic one
Dec 17, 2019
75a3d26
Fixing failing unit test
Dec 17, 2019
449d5f4
Adding a unit test for redirect
Dec 18, 2019
e50a051
Do not show Save New if adding from Dashboard
Dec 18, 2019
3411d59
Adding functional test
Dec 18, 2019
a316315
Adding functional test
Dec 20, 2019
452b4dc
Fixing type issues
Dec 20, 2019
494b855
Renaming query params
Dec 20, 2019
12c3142
Fixing failing unit test
Dec 20, 2019
3a39424
Removing unused constants
Dec 23, 2019
d28733e
Fixing erroneous imports
Dec 23, 2019
e44ce7a
Fixing erroneous import
Dec 23, 2019
e335b91
Fixing import
Dec 23, 2019
1bfec16
Fix failing typecheck
Jan 2, 2020
bbfcc34
Removing timefilter from Dashboard URL
Jan 5, 2020
8022cea
Fixing type error
Jan 6, 2020
64e6ceb
Replacing time parsing with rison
Jan 8, 2020
fe9ab85
Merge branch 'master' into add-lens-to-dash
elasticmachine Jan 9, 2020
25010ff
Replacing URL regex parsing with legacy URLs
Jan 10, 2020
f78bdc7
Fixing failing test
Jan 11, 2020
9e15430
Merge branch 'master' into add-lens-to-dash
elasticmachine Jan 11, 2020
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
14 changes: 6 additions & 8 deletions src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_app_controller.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import {
KbnUrl,
SavedObjectSaveOpts,
unhashUrl,
VISUALIZE_EMBEDDABLE_TYPE,
} from '../legacy_imports';
import { FilterStateManager } from '../../../../data/public';
import {
Expand Down Expand Up @@ -334,13 +333,12 @@ export class DashboardAppController {
// This code needs to be replaced with a better mechanism for adding new embeddables of
// any type from the add panel. Likely this will happen via creating a visualization "inline",
// without navigating away from the UX.
if ($routeParams[DashboardConstants.NEW_VISUALIZATION_ID_PARAM]) {
container.addSavedObjectEmbeddable(
VISUALIZE_EMBEDDABLE_TYPE,
$routeParams[DashboardConstants.NEW_VISUALIZATION_ID_PARAM]
);
kbnUrl.removeParam(DashboardConstants.ADD_VISUALIZATION_TO_DASHBOARD_MODE_PARAM);
kbnUrl.removeParam(DashboardConstants.NEW_VISUALIZATION_ID_PARAM);
if ($routeParams[DashboardConstants.ADD_EMBEDDABLE_TYPE]) {
const type = $routeParams[DashboardConstants.ADD_EMBEDDABLE_TYPE];
const id = $routeParams[DashboardConstants.ADD_EMBEDDABLE_ID];
container.addSavedObjectEmbeddable(type, id);
kbnUrl.removeParam(DashboardConstants.ADD_EMBEDDABLE_TYPE);
kbnUrl.removeParam(DashboardConstants.ADD_EMBEDDABLE_ID);
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@

export const DashboardConstants = {
ADD_VISUALIZATION_TO_DASHBOARD_MODE_PARAM: 'addToDashboard',
NEW_VISUALIZATION_ID_PARAM: 'addVisualization',
LANDING_PAGE_PATH: '/dashboards',
CREATE_NEW_DASHBOARD_URL: '/dashboard',
ADD_EMBEDDABLE_ID: 'addEmbeddableId',
ADD_EMBEDDABLE_TYPE: 'addEmbeddableType',
};

export function createDashboardEditUrl(id: string) {
Expand Down
8 changes: 6 additions & 2 deletions src/legacy/core_plugins/kibana/public/visualize/np_ready/editor/editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ import { unhashUrl } from '../../../../../../../plugins/kibana_utils/public';

import { initVisEditorDirective } from './visualization_editor';
import { initVisualizationDirective } from './visualization';

import {
VISUALIZE_EMBEDDABLE_TYPE,
subscribeWithScope,
absoluteToParsedUrl,
KibanaParsedUrl,
Expand Down Expand Up @@ -588,7 +588,11 @@ function VisualizeAppController(
getBasePath()
);
dashboardParsedUrl.addQueryParameter(
DashboardConstants.NEW_VISUALIZATION_ID_PARAM,
DashboardConstants.ADD_EMBEDDABLE_TYPE,
VISUALIZE_EMBEDDABLE_TYPE
);
dashboardParsedUrl.addQueryParameter(
DashboardConstants.ADD_EMBEDDABLE_ID,
savedVis.id
);
kbnUrl.change(dashboardParsedUrl.appPath);
Expand Down
4 changes: 2 additions & 2 deletions src/legacy/core_plugins/kibana/public/visualize/np_ready/wizard/new_vis_modal.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ describe('NewVisModal', () => {
expect(window.location.assign).toBeCalledWith('#/visualize/create?type=vis&foo=true&bar=42');
});

it('closes if visualization with aliasUrl and addToDashboard in editorParams', () => {
it('closes and redirects properly if visualization with aliasUrl and addToDashboard in editorParams', () => {
const onClose = jest.fn();
window.location.assign = jest.fn();
const wrapper = mountWithIntl(
Expand All @@ -160,7 +160,7 @@ describe('NewVisModal', () => {
);
const visButton = wrapper.find('button[data-test-subj="visType-visWithAliasUrl"]');
visButton.simulate('click');
expect(window.location.assign).toBeCalledWith('testbasepath/aliasUrl');
expect(window.location.assign).toBeCalledWith('testbasepath/aliasUrl?addToDashboard');
expect(onClose).toHaveBeenCalled();
});
});
Expand Down
7 changes: 5 additions & 2 deletions src/legacy/core_plugins/kibana/public/visualize/np_ready/wizard/new_vis_modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,15 +143,18 @@ class NewVisModal extends React.Component<TypeSelectionProps, TypeSelectionState
this.trackUiMetric(METRIC_TYPE.CLICK, visType.name);
}

let params;
if ('aliasUrl' in visType) {
window.location.assign(this.props.addBasePath(visType.aliasUrl));
params = this.props.addBasePath(visType.aliasUrl);
if (this.props.editorParams && this.props.editorParams.includes('addToDashboard')) {
params = `${params}?addToDashboard`;
this.props.onClose();
}
window.location.assign(params);
return;
}

let params = [`type=${encodeURIComponent(visType.name)}`];
params = [`type=${encodeURIComponent(visType.name)}`];

if (searchType) {
params.push(`${searchType === 'search' ? 'savedSearchId' : 'indexPattern'}=${searchId}`);
Expand Down
4 changes: 4 additions & 0 deletions test/functional/page_objects/visualize_page.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,10 @@ export function VisualizePageProvider({ getService, getPageObjects }: FtrProvide
async () => (await globalNav.getLastBreadcrumb()) === vizName
);
}

public async clickLensWidget() {
await this.clickVisType('lens');
}
}

return new VisualizePage();
Expand Down
25 changes: 25 additions & 0 deletions x-pack/legacy/plugins/lens/public/app_plugin/app.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ describe('Lens App', () => {
docId?: string;
docStorage: SavedObjectStore;
redirectTo: (id?: string) => void;
addToDashboardMode?: boolean;
}> {
return ({
editorFrame: createMockFrame(),
Expand Down Expand Up @@ -126,6 +127,7 @@ describe('Lens App', () => {
docId?: string;
docStorage: SavedObjectStore;
redirectTo: (id?: string) => void;
addToDashboardMode?: boolean;
}>;
}

Expand Down Expand Up @@ -306,6 +308,7 @@ describe('Lens App', () => {
docId?: string;
docStorage: SavedObjectStore;
redirectTo: (id?: string) => void;
addToDashboardMode?: boolean;
}>;

beforeEach(() => {
Expand Down Expand Up @@ -344,14 +347,19 @@ describe('Lens App', () => {

async function save({
initialDocId,
addToDashboardMode,
...saveProps
}: SaveProps & {
initialDocId?: string;
addToDashboardMode?: boolean;
}) {
const args = {
...defaultArgs,
docId: initialDocId,
};
if (addToDashboardMode) {
args.addToDashboardMode = addToDashboardMode;
}
args.editorFrame = frame;
(args.docStorage.load as jest.Mock).mockResolvedValue({
id: '1234',
Expand Down Expand Up @@ -543,6 +551,23 @@ describe('Lens App', () => {

expect(getButton(instance).disableButton).toEqual(false);
});

it('saves new doc and redirects to dashboard', async () => {
const { args } = await save({
initialDocId: undefined,
addToDashboardMode: true,
newCopyOnSave: false,
newTitle: 'hello there',
});

expect(args.docStorage.save).toHaveBeenCalledWith({
expression: 'kibana 3',
id: undefined,
title: 'hello there',
});

expect(args.redirectTo).toHaveBeenCalledWith('aaa');
});
});
});

Expand Down
14 changes: 12 additions & 2 deletions x-pack/legacy/plugins/lens/public/app_plugin/app.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { SavedObjectSaveModal } from 'ui/saved_objects/components/saved_object_s
import { AppMountContext, NotificationsStart } from 'src/core/public';
import { IStorageWrapper } from 'src/plugins/kibana_utils/public';
import { npStart } from 'ui/new_platform';
import { FormattedMessage } from '@kbn/i18n/react';
import { KibanaContextProvider } from '../../../../../../src/plugins/kibana_react/public';
import { Document, SavedObjectStore } from '../persistence';
import { EditorFrameInstance } from '../types';
Expand Down Expand Up @@ -50,6 +51,7 @@ export function App({
docId,
docStorage,
redirectTo,
addToDashboardMode,
}: {
editorFrame: EditorFrameInstance;
data: DataPublicPluginStart;
Expand All @@ -58,6 +60,7 @@ export function App({
docId?: string;
docStorage: SavedObjectStore;
redirectTo: (id?: string) => void;
addToDashboardMode?: boolean;
}) {
const language =
storage.get('kibana.userQueryLanguage') || core.uiSettings.get('search:queryLanguage');
Expand Down Expand Up @@ -166,6 +169,13 @@ export function App({

const { TopNavMenu } = npStart.plugins.navigation.ui;

const confirmButton = addToDashboardMode ? (
<FormattedMessage
id="xpack.lens.app.saveAddToDashboard"
defaultMessage="Save and add to dashboard"
/>
) : null;

return (
<I18nProvider>
<KibanaContextProvider
Expand Down Expand Up @@ -320,7 +330,6 @@ export function App({
persistedDoc: newDoc,
lastKnownDoc: newDoc,
}));

if (docId !== id) {
redirectTo(id);
}
Expand All @@ -337,10 +346,11 @@ export function App({
}}
onClose={() => setState(s => ({ ...s, isSaveModalVisible: false }))}
title={lastKnownDoc.title || ''}
showCopyOnSave={true}
showCopyOnSave={!addToDashboardMode}
objectType={i18n.translate('xpack.lens.app.saveModalType', {
defaultMessage: 'Lens visualization',
})}
confirmButtonLabel={confirmButton}
/>
)}
</KibanaContextProvider>
Expand Down
70 changes: 61 additions & 9 deletions x-pack/legacy/plugins/lens/public/app_plugin/plugin.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@ import 'uiExports/visResponseHandlers';
import 'uiExports/savedObjectTypes';

import React from 'react';
import { I18nProvider, FormattedMessage } from '@kbn/i18n/react';
import { HashRouter, Switch, Route, RouteComponentProps } from 'react-router-dom';
import { FormattedMessage, I18nProvider } from '@kbn/i18n/react';
import { HashRouter, Route, RouteComponentProps, Switch } from 'react-router-dom';
import { render, unmountComponentAtNode } from 'react-dom';
import { CoreSetup, CoreStart, SavedObjectsClientContract } from 'src/core/public';
import { DataPublicPluginStart } from 'src/plugins/data/public';
import rison, { RisonObject, RisonValue } from 'rison-node';
import { isObject } from 'lodash';
import { DataStart } from '../../../../../../src/legacy/core_plugins/data/public';
import { Storage } from '../../../../../../src/plugins/kibana_utils/public';
import { editorFrameSetup, editorFrameStart, editorFrameStop } from '../editor_frame_plugin';
Expand All @@ -41,6 +43,12 @@ import {
import { NOT_INTERNATIONALIZED_PRODUCT_NAME } from '../../common';
import { KibanaLegacySetup } from '../../../../../../src/plugins/kibana_legacy/public';
import { EditorFrameStart } from '../types';
import {
getKibanaBasePathFromDashboardUrl,
addEmbeddableToDashboardUrl,
getDashboardUrlWithQueryParams,
getUrlVars,
} from './url_helper';

export interface LensPluginSetupDependencies {
kibana_legacy: KibanaLegacySetup;
Expand All @@ -51,6 +59,9 @@ export interface LensPluginStartDependencies {
dataShim: DataStart;
}

export const isRisonObject = (value: RisonValue): value is RisonObject => {
return isObject(value);
};
export class AppPlugin {
private startDependencies: {
data: DataPublicPluginStart;
Expand Down Expand Up @@ -93,9 +104,54 @@ export class AppPlugin {
http: core.http,
})
);
const updateUrlTime = (urlVars: Record<string, string>): void => {
const decoded: RisonObject = rison.decode(urlVars._g) as RisonObject;
Copy link
Contributor

Choose a reason for hiding this comment

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

This piece of code can be simplified from a typing perspective:

          const decoded = rison.decode(urlVars._g);
          if (!isRisonObject(decoded)) {
            return;
          }
          decoded.time = data.query.timefilter.timefilter.getTime();
          urlVars._g = rison.encode(decoded);

if (!decoded) {
return;
}
// @ts-ignore
decoded.time = data.query.timefilter.timefilter.getTime();
urlVars._g = rison.encode((decoded as unknown) as RisonObject);
};
const redirectTo = (
routeProps: RouteComponentProps<{ id?: string }>,
addToDashboardMode: boolean,
id?: string
) => {
if (!id) {
routeProps.history.push('/lens');
} else if (!addToDashboardMode) {
routeProps.history.push(`/lens/edit/${id}`);
} else if (addToDashboardMode && id) {
routeProps.history.push(`/lens/edit/${id}`);
const url = context.core.chrome.navLinks.get('kibana:dashboard');
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 adding more dependencies on context, which is being deprecated. Is there a better way to do this?

if (!url) {
return;
}
const lastDashboardAbsoluteUrl = url.url;
Copy link
Contributor

Choose a reason for hiding this comment

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

This also seems like a bunch of detail that belongs to the dashboard module, not to Lens. Maybe something like this:

  const url = dashboardUrl.addEmbeddable(url.url, id, 'lens');
  if (url) {
    window.history.pushState({}, '', url);
  }

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 was the original intention, but then we figured the time range is not properly set. So for the hack with setting timerange to work, this won't be so clean.

const lensUrl = `${getKibanaBasePathFromDashboardUrl(
lastDashboardAbsoluteUrl
)}/lens/edit/${id}`;
if (lastDashboardAbsoluteUrl && lensUrl) {
const urlVars = getUrlVars(lastDashboardAbsoluteUrl);
updateUrlTime(urlVars);
window.history.pushState({}, '', lensUrl);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we generally avoid using pushState directly, and instead use a core service to do this, but I'm not sure. Might be worth looking into, anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it exists in the new platform. I think the pushState is our only option for now.

const dashboardUrl = getDashboardUrlWithQueryParams(
lastDashboardAbsoluteUrl,
urlVars
);
const dashboardParsedUrl = addEmbeddableToDashboardUrl(dashboardUrl, id, 'lens');
if (dashboardParsedUrl) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If this returns null, we silently fail. Is that the right thing? It seems like a null here is a legit error that should be surfaced in some way.

window.history.pushState({}, '', dashboardParsedUrl);
}
}
}
};

const renderEditor = (routeProps: RouteComponentProps<{ id?: string }>) => {
trackUiEvent('loaded');
const addToDashboardMode =
!!routeProps.location.search && routeProps.location.search.includes('addToDashboard');
return (
<App
core={context.core}
Expand All @@ -104,13 +160,8 @@ export class AppPlugin {
storage={new Storage(localStorage)}
docId={routeProps.match.params.id}
docStorage={new SavedObjectIndexStore(savedObjectsClient)}
redirectTo={id => {
if (!id) {
routeProps.history.push('/lens');
} else {
routeProps.history.push(`/lens/edit/${id}`);
}
}}
redirectTo={id => redirectTo(routeProps, addToDashboardMode, id)}
addToDashboardMode={addToDashboardMode}
/>
);
};
Expand All @@ -119,6 +170,7 @@ export class AppPlugin {
trackUiEvent('loaded_404');
return <FormattedMessage id="xpack.lens.app404" defaultMessage="404 Not Found" />;
}

render(
<I18nProvider>
<HashRouter>
Expand Down
Loading