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

[Dashboard First] Lens By Value #70272

Closed
wants to merge 70 commits into from
Closed
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
70 commits
Select commit Hold shift + click to select a range
297dba0
Made lens work with the attribute service copied from #68719
ThomThomson Jun 25, 2020
168825a
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Jun 25, 2020
0039a97
Editing lens by value now works.
ThomThomson Jun 26, 2020
8ec8020
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Jun 26, 2020
681625f
Allow by value lens embeddable to be made by reference via the save a…
ThomThomson Jun 26, 2020
473a052
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Jun 29, 2020
3ee8263
fixed up some jest tests
ThomThomson Jun 29, 2020
a34d585
functional and jest test fixes
ThomThomson Jun 30, 2020
b7ff957
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Jun 30, 2020
5d05ac9
fix for visualize by value
ThomThomson Jun 30, 2020
adc1298
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Jul 7, 2020
a3d0adc
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Jul 13, 2020
3ecb3d9
Restructured adding and editing to minimize deletion of embeddables a…
ThomThomson Jul 14, 2020
72a068a
eslint changes
ThomThomson Jul 14, 2020
d1ab683
cleanup continued...
ThomThomson Jul 15, 2020
9cb9a7c
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Jul 15, 2020
a0fffd0
Finished cleanup - separated embeddableId and savedObjectId in lens t…
ThomThomson Jul 17, 2020
4d64d5e
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Jul 17, 2020
9e5f7a9
Test fixes
ThomThomson Jul 17, 2020
580fbba
More test fixes, started on feature flag creation
ThomThomson Jul 17, 2020
df5fad0
Finished feature flag
ThomThomson Jul 17, 2020
c6d87c7
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Jul 20, 2020
cd61dfc
Updated jest test - because the newly created prop is now set correctly.
ThomThomson Jul 20, 2020
7f86d23
remove byValueMode from editorState in favor of using the embeddableI…
ThomThomson Jul 22, 2020
43657f9
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Jul 22, 2020
888ca49
rename edit mode for clarity. Update ternary
ThomThomson Jul 22, 2020
b9d9c5c
Type fixes
ThomThomson Jul 23, 2020
c823217
Jest fix
ThomThomson Jul 23, 2020
87a780c
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Jul 23, 2020
d3d2cfd
merge fix
ThomThomson Jul 23, 2020
b793ca3
Merge branch 'master' into feature/lensByValue
elasticmachine Jul 27, 2020
ef65220
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Jul 29, 2020
ee625ec
merge fix
ThomThomson Jul 29, 2020
4e39564
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Jul 30, 2020
ed461d3
cleaned up visualize state transfer package, set new lens flow defaul…
ThomThomson Jul 30, 2020
c9878c5
cleaned up visualize state transfer package, set new lens flow defaul…
ThomThomson Jul 30, 2020
cb72a3e
Merge branch 'feature/lensByValue' of github.com:ThomThomson/kibana i…
ThomThomson Jul 30, 2020
a5910f2
Added an interface for all by value or by reference embeddables to in…
ThomThomson Jul 31, 2020
ac5c91a
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Jul 31, 2020
068ebaf
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Aug 4, 2020
c1435e0
Removed lens by value config in favor of the dashboard-wide config fr…
ThomThomson Aug 4, 2020
ac94aec
removed unused config related code from lens
ThomThomson Aug 5, 2020
642eea8
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Aug 6, 2020
9af2c06
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Aug 12, 2020
8b43747
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Aug 17, 2020
df876af
Merge continued...
ThomThomson Aug 17, 2020
aeb25ee
Merge continued...
ThomThomson Aug 17, 2020
6fc1f00
Maps Update
ThomThomson Aug 17, 2020
9598011
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Aug 18, 2020
10d98e3
Finished merge with visualize editor
ThomThomson Aug 18, 2020
e85caeb
Merge branch 'master' into feature/lensByValue
elasticmachine Aug 20, 2020
2ef26c6
Merge branch 'feature/lensByValue' of github.com:ThomThomson/kibana i…
ThomThomson Aug 20, 2020
ff13e40
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Aug 20, 2020
3411c36
Added custom save method to attribute service to allow lens specific …
ThomThomson Aug 20, 2020
1d5b123
Small fix for lens embeddables on canvas, made cancel button show up …
ThomThomson Aug 20, 2020
f497244
Test fix
ThomThomson Aug 20, 2020
5b6d397
Removed accidental duplication of ref_or_val_embeddable. Added tests …
ThomThomson Aug 21, 2020
d345571
M o n s t e r M e r g e branch 'master' of github.com:elastic/kibana…
ThomThomson Aug 21, 2020
0dee8a7
Made attribute service unwrap references as well
ThomThomson Aug 22, 2020
12b9aa4
type and jest fix
ThomThomson Aug 22, 2020
e3dc91a
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Aug 24, 2020
29ae393
Simplified and separated redirectTo method, made dashboard merge inco…
ThomThomson Aug 24, 2020
719d67c
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Aug 24, 2020
3470c63
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Aug 25, 2020
566500c
changed feature flag config to match dashboard
ThomThomson Aug 25, 2020
8796ab0
cleaned up types after 75560
ThomThomson Aug 25, 2020
216fd7f
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Aug 25, 2020
d0e3fd8
Type cleanup continued
ThomThomson Aug 25, 2020
6f28a7f
Merge branch 'master' of github.com:elastic/kibana into feature/lensB…
ThomThomson Aug 31, 2020
cf74955
update eslint
ThomThomson Aug 31, 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
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import {
ViewMode,
ContainerOutput,
EmbeddableInput,
SavedObjectEmbeddableInput,
} from '../../../embeddable/public';
import { NavAction, SavedDashboardPanel } from '../types';

Expand Down Expand Up @@ -428,19 +429,23 @@ export class DashboardAppController {

const incomingState = embeddable
.getStateTransfer(scopedHistory())
.getIncomingEmbeddablePackage();
.getIncomingEmbeddablePackage({ keysToRemoveAfterFetch: ['id', 'type', 'input'] });
if (incomingState) {
if ('id' in incomingState) {
container.addNewEmbeddable<EmbeddableInput>(incomingState.type, {
container.addOrUpdateEmbeddable<SavedObjectEmbeddableInput>(incomingState.type, {
savedObjectId: incomingState.id,
id: incomingState.embeddableIdToReplace,
});
} else if ('input' in incomingState) {
const input = incomingState.input;
delete input.id;
const explicitInput = {
savedVis: input,
};
container.addNewEmbeddable<EmbeddableInput>(incomingState.type, explicitInput);
// TODO: Get rid of this, maybe by making the visualize embeddable also use the attributeService
const explicitInput =
incomingState.type === 'lens'
? incomingState.input
: {
savedVis: incomingState.input,
};

container.addOrUpdateEmbeddable<EmbeddableInput>(incomingState.type, explicitInput);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ import {
} from '../../../../kibana_react/public';
import { PLACEHOLDER_EMBEDDABLE } from './placeholder';
import { PanelPlacementMethod, IPanelPlacementArgs } from './panel/dashboard_panel_placement';
import { EmbeddableStateTransfer } from '../../../../embeddable/public';
import { EmbeddableStateTransfer, EmbeddableOutput } from '../../../../embeddable/public';

export interface DashboardContainerInput extends ContainerInput {
viewMode: ViewMode;
Expand Down Expand Up @@ -159,29 +159,55 @@ export class DashboardContainer extends Container<InheritedChildInput, Dashboard
[placeholderPanelState.explicitInput.id]: placeholderPanelState,
},
});
newStateComplete.then((newPanelState: Partial<PanelState>) => {
const finalPanels = { ...this.input.panels };
delete finalPanels[placeholderPanelState.explicitInput.id];
const newPanelId = newPanelState.explicitInput?.id
? newPanelState.explicitInput.id
: uuid.v4();
finalPanels[newPanelId] = {
...placeholderPanelState,
...newPanelState,
gridData: {
...placeholderPanelState.gridData,
i: newPanelId,
},
newStateComplete.then((newPanelState: Partial<PanelState>) =>
this.replacePanel(placeholderPanelState, newPanelState)
);
}

public replacePanel(
ThomThomson marked this conversation as resolved.
Show resolved Hide resolved
previousPanelState: DashboardPanelState<EmbeddableInput>,
newPanelState: Partial<PanelState>
) {
// TODO: In the current infrastructure, embeddables in a container do not react properly to
// changes. Removing the existing embeddable, and adding a new one is a temporary workaround
// until the container logic is fixed.
const finalPanels = { ...this.input.panels };
delete finalPanels[previousPanelState.explicitInput.id];
const newPanelId = newPanelState.explicitInput?.id ? newPanelState.explicitInput.id : uuid.v4();
finalPanels[newPanelId] = {
...previousPanelState,
...newPanelState,
gridData: {
...previousPanelState.gridData,
i: newPanelId,
},
explicitInput: {
...newPanelState.explicitInput,
id: newPanelId,
},
};
this.updateInput({
panels: finalPanels,
lastReloadRequestTime: new Date().getTime(),
});
}

public async addOrUpdateEmbeddable<
EEI extends EmbeddableInput = EmbeddableInput,
EEO extends EmbeddableOutput = EmbeddableOutput,
E extends IEmbeddable<EEI, EEO> = IEmbeddable<EEI, EEO>
>(type: string, explicitInput: Partial<EEI>) {
if (explicitInput.id && this.input.panels[explicitInput.id]) {
this.replacePanel(this.input.panels[explicitInput.id], {
type,
explicitInput: {
...newPanelState.explicitInput,
id: newPanelId,
...explicitInput,
id: uuid.v4(),
},
};
this.updateInput({
panels: finalPanels,
lastReloadRequestTime: new Date().getTime(),
});
});
} else {
this.addNewEmbeddable<EEI, EEO, E>(type, explicitInput);
}
}

public render(dom: HTMLElement) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,42 @@ test('is compatible when edit url is available, in edit mode and editable', asyn
).toBe(true);
});

test('redirects to app using state transfer', async () => {
test('redirects to app using state transfer with by value mode', async () => {
applicationMock.currentAppId$ = of('superCoolCurrentApp');
const action = new EditPanelAction(getFactory, applicationMock, stateTransferMock);
const embeddable = new EditableEmbeddable({ id: '123', viewMode: ViewMode.EDIT }, true);
const embeddable = new EditableEmbeddable(
{ id: '123', viewMode: ViewMode.EDIT, coolInput1: 1, coolInput2: 2 },
true
);
embeddable.getOutput = jest.fn(() => ({ editApp: 'ultraVisualize', editPath: '/123' }));
await action.execute({ embeddable });
expect(stateTransferMock.navigateToEditor).toHaveBeenCalledWith('ultraVisualize', {
path: '/123',
state: {
byValueMode: true,
originatingApp: 'superCoolCurrentApp',
valueInput: {
id: '123',
viewMode: ViewMode.EDIT,
coolInput1: 1,
coolInput2: 2,
},
},
});
});

test('redirects to app using state transfer without by value mode', async () => {
applicationMock.currentAppId$ = of('superCoolCurrentApp');
const action = new EditPanelAction(getFactory, applicationMock, stateTransferMock);
const embeddable = new EditableEmbeddable(
{ id: '123', viewMode: ViewMode.EDIT, savedObjectId: '1234' },
true
);
embeddable.getOutput = jest.fn(() => ({ editApp: 'ultraVisualize', editPath: '/123' }));
await action.execute({ embeddable });
expect(stateTransferMock.navigateToEditor).toHaveBeenCalledWith('ultraVisualize', {
path: '/123',
state: { originatingApp: 'superCoolCurrentApp' },
state: { originatingApp: 'superCoolCurrentApp', byValueMode: false, valueInput: undefined },
});
});

Expand Down
15 changes: 12 additions & 3 deletions src/plugins/embeddable/public/lib/actions/edit_panel_action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ import { take } from 'rxjs/operators';
import { ViewMode } from '../types';
import { EmbeddableFactoryNotFoundError } from '../errors';
import { EmbeddableStart } from '../../plugin';
import { IEmbeddable, EmbeddableEditorState, EmbeddableStateTransfer } from '../..';
import { IEmbeddable, EmbeddableStateTransfer, EmbeddableEditorState } from '../..';
import { SavedObjectEmbeddableInput } from '..';

export const ACTION_EDIT_PANEL = 'editPanel';

Expand Down Expand Up @@ -109,8 +110,16 @@ export class EditPanelAction implements Action<ActionContext> {
const app = embeddable ? embeddable.getOutput().editApp : undefined;
const path = embeddable ? embeddable.getOutput().editPath : undefined;
if (app && path) {
const state = this.currentAppId ? { originatingApp: this.currentAppId } : undefined;
return { app, path, state };
if (this.currentAppId) {
ThomThomson marked this conversation as resolved.
Show resolved Hide resolved
const byValueMode = !(embeddable.getInput() as SavedObjectEmbeddableInput).savedObjectId;
const state: EmbeddableEditorState = {
originatingApp: this.currentAppId,
byValueMode,
valueInput: byValueMode ? embeddable.getInput() : undefined,
};
return { app, path, state };
}
return { app, path };
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/plugins/embeddable/public/lib/containers/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,8 @@ export abstract class Container<
return {
type: factory.type,
explicitInput: {
id: embeddableId,
...explicitInput,
id: embeddableId,
} as TEmbeddableInput,
};
}
Expand Down
1 change: 1 addition & 0 deletions src/plugins/embeddable/public/lib/state_transfer/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export function isEmbeddableEditorState(state: unknown): state is EmbeddableEdit
export interface EmbeddablePackageByReferenceState {
type: string;
id: string;
embeddableIdToReplace?: string;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ export class VisualizeEmbeddableFactory
showNewVisModal({
originatingApp: await this.getCurrentAppId(),
outsideVisualizeApp: true,
createByValue: true,
Copy link
Member

Choose a reason for hiding this comment

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

why do we need to pass this one down ? (was not needed for visualize by value ?)

Copy link
Member

Choose a reason for hiding this comment

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

also, should we not go thru visualize embeddable to create a lens embeddable from dashboard ? i would prefer if we would directly work with lens embeddable there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The createByValue argument was used to differentiate between editing a by value embeddable, creating a by reference embeddable, and creating a by value embeddable. I was able to remove this prop by checking for the presence of the embeddableId in the stateTransferService instead, which should make it a little cleaner.

In terms of creating a lens embeddable via the visualize embeddable factory, it's been that way for a while due to the lens app being registered as an alias App in visualize. Changing this flow will be a larger initiative, but we're already making progress on it. @ryankeairns has a really nice design for a new 'add panel' flow.

});
return undefined;
}
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/visualizations/public/wizard/new_vis_modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ interface TypeSelectionProps {
application: ApplicationStart;
outsideVisualizeApp?: boolean;
stateTransfer?: EmbeddableStateTransfer;
createByValue?: boolean;
originatingApp?: string;
}

Expand Down Expand Up @@ -174,7 +175,7 @@ class NewVisModal extends React.Component<TypeSelectionProps, TypeSelectionState
if (this.props.stateTransfer && this.props.originatingApp) {
this.props.stateTransfer.navigateToEditor(appId, {
path: params,
state: { originatingApp: this.props.originatingApp },
state: { originatingApp: this.props.originatingApp, byValueMode: this.props.createByValue },
});
} else {
this.props.application.navigateToApp(appId, {
Expand Down
3 changes: 3 additions & 0 deletions src/plugins/visualizations/public/wizard/show_new_vis.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export interface ShowNewVisModalParams {
onClose?: () => void;
originatingApp?: string;
outsideVisualizeApp?: boolean;
createByValue?: boolean;
}

/**
Expand All @@ -49,6 +50,7 @@ export function showNewVisModal({
onClose,
originatingApp,
outsideVisualizeApp,
createByValue,
}: ShowNewVisModalParams = {}) {
const container = document.createElement('div');
let isClosed = false;
Expand All @@ -69,6 +71,7 @@ export function showNewVisModal({
isOpen={true}
onClose={handleClose}
originatingApp={originatingApp}
createByValue={createByValue}
stateTransfer={getEmbeddable().getStateTransfer()}
outsideVisualizeApp={outsideVisualizeApp}
editorParams={editorParams}
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/lens/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ export function getBasePath() {
return `#/`;
}

export function getEditPath(id: string) {
return `#/edit/${encodeURIComponent(id)}`;
export function getEditPath(id: string | undefined) {
return id ? `#/edit/${encodeURIComponent(id)}` : '#/edit/value';
}
32 changes: 26 additions & 6 deletions x-pack/plugins/lens/public/app_plugin/app.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,13 @@ describe('Lens App', () => {
storage: Storage;
docId?: string;
docStorage: SavedObjectStore;
redirectTo: (id?: string, returnToOrigin?: boolean, newlyCreated?: boolean) => void;
redirectTo: (
id?: string,
documentByValue?: Document,
returnToOrigin?: boolean,
newlyCreated?: boolean,
embeddableIdToReplace?: string
) => void;
originatingApp: string | undefined;
onAppLeave: AppMountParameters['onAppLeave'];
history: History;
Expand Down Expand Up @@ -163,7 +169,15 @@ describe('Lens App', () => {
load: jest.fn(),
save: jest.fn(),
},
redirectTo: jest.fn((id?: string, returnToOrigin?: boolean, newlyCreated?: boolean) => {}),
redirectTo: jest.fn(
(
id?: string,
documentByValue?: Document,
returnToOrigin?: boolean,
newlyCreated?: boolean,
embeddableIdToReplace?: string
) => {}
),
onAppLeave: jest.fn(),
history: createMemoryHistory(),
} as unknown) as jest.Mocked<{
Expand All @@ -174,7 +188,13 @@ describe('Lens App', () => {
storage: Storage;
docId?: string;
docStorage: SavedObjectStore;
redirectTo: (id?: string, returnToOrigin?: boolean, newlyCreated?: boolean) => void;
redirectTo: (
id?: string,
documentByValue?: Document,
returnToOrigin?: boolean,
newlyCreated?: boolean,
embeddableIdToReplace?: string
) => void;
originatingApp: string | undefined;
onAppLeave: AppMountParameters['onAppLeave'];
history: History;
Expand Down Expand Up @@ -517,7 +537,7 @@ describe('Lens App', () => {
expression: 'kibana 3',
});

expect(args.redirectTo).toHaveBeenCalledWith('aaa', undefined, true);
expect(args.redirectTo).toHaveBeenCalledWith('aaa', undefined, undefined, true, undefined);

inst.setProps({ docId: 'aaa' });

Expand All @@ -537,7 +557,7 @@ describe('Lens App', () => {
expression: 'kibana 3',
});

expect(args.redirectTo).toHaveBeenCalledWith('aaa', undefined, true);
expect(args.redirectTo).toHaveBeenCalledWith('aaa', undefined, undefined, true, undefined);

inst.setProps({ docId: 'aaa' });

Expand Down Expand Up @@ -605,7 +625,7 @@ describe('Lens App', () => {
title: 'hello there',
});

expect(args.redirectTo).toHaveBeenCalledWith('aaa', true, true);
expect(args.redirectTo).toHaveBeenCalledWith('aaa', undefined, true, true, undefined);
});

it('saves app filters and does not save pinned filters', async () => {
Expand Down
Loading