From 297dba06041ecc14dc1e3a9468b6d508e5ed0922 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Thu, 25 Jun 2020 12:39:53 -0400 Subject: [PATCH 01/40] Made lens work with the attribute service copied from #68719 --- src/plugins/embeddable/public/index.ts | 2 +- .../lib/actions/edit_panel_action.test.tsx | 2 +- .../public/lib/actions/edit_panel_action.ts | 2 +- .../embeddable_state_transfer.test.ts | 8 +- .../embeddable_state_transfer.ts | 16 +- .../public/lib/state_transfer/index.ts | 2 +- .../public/lib/state_transfer/types.ts | 24 ++- src/plugins/embeddable/public/mocks.tsx | 4 +- .../visualize_embeddable_factory.tsx | 1 + .../public/wizard/new_vis_modal.test.tsx | 2 +- .../public/wizard/new_vis_modal.tsx | 5 +- .../public/wizard/show_new_vis.tsx | 3 + x-pack/plugins/lens/common/constants.ts | 4 +- x-pack/plugins/lens/public/app_plugin/app.tsx | 71 +++++---- .../lens/public/app_plugin/mounter.tsx | 22 +-- .../embeddable/attribute_service.ts | 54 +++++++ .../embeddable/embeddable.tsx | 137 +++++++++++++----- .../embeddable/embeddable_factory.ts | 80 +++++----- 18 files changed, 291 insertions(+), 148 deletions(-) create mode 100644 x-pack/plugins/lens/public/editor_frame_service/embeddable/attribute_service.ts diff --git a/src/plugins/embeddable/public/index.ts b/src/plugins/embeddable/public/index.ts index 1d1dc79121937b..35fbfe2e0aa38f 100644 --- a/src/plugins/embeddable/public/index.ts +++ b/src/plugins/embeddable/public/index.ts @@ -69,7 +69,7 @@ export { isRangeSelectTriggerContext, isValueClickTriggerContext, EmbeddableStateTransfer, - EmbeddableOriginatingAppState, + EmbeddableEditorState, EmbeddablePackageState, EmbeddableRenderer, EmbeddableRendererProps, diff --git a/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx b/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx index 4b602efb027177..594a7ad73c3965 100644 --- a/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx +++ b/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx @@ -59,7 +59,7 @@ test('redirects to app using state transfer', async () => { const embeddable = new EditableEmbeddable({ id: '123', viewMode: ViewMode.EDIT }, true); embeddable.getOutput = jest.fn(() => ({ editApp: 'ultraVisualize', editPath: '/123' })); await action.execute({ embeddable }); - expect(stateTransferMock.navigateToWithOriginatingApp).toHaveBeenCalledWith('ultraVisualize', { + expect(stateTransferMock.navigateToEditor).toHaveBeenCalledWith('ultraVisualize', { path: '/123', state: { originatingApp: 'superCoolCurrentApp' }, }); diff --git a/src/plugins/embeddable/public/lib/actions/edit_panel_action.ts b/src/plugins/embeddable/public/lib/actions/edit_panel_action.ts index d983dc9f418535..ac046e978c9cb9 100644 --- a/src/plugins/embeddable/public/lib/actions/edit_panel_action.ts +++ b/src/plugins/embeddable/public/lib/actions/edit_panel_action.ts @@ -88,7 +88,7 @@ export class EditPanelAction implements Action { const appTarget = this.getAppTarget(context); if (appTarget) { if (this.stateTransfer && appTarget.state) { - await this.stateTransfer.navigateToWithOriginatingApp(appTarget.app, { + await this.stateTransfer.navigateToEditor(appTarget.app, { path: appTarget.path, state: appTarget.state, }); diff --git a/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.test.ts b/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.test.ts index 0d5ae6be68185b..b7dd95ccba32ca 100644 --- a/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.test.ts +++ b/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.test.ts @@ -38,7 +38,7 @@ describe('embeddable state transfer', () => { }); it('can send an outgoing originating app state', async () => { - await stateTransfer.navigateToWithOriginatingApp(destinationApp, { state: { originatingApp } }); + await stateTransfer.navigateToEditor(destinationApp, { state: { originatingApp } }); expect(application.navigateToApp).toHaveBeenCalledWith('superUltraVisualize', { state: { originatingApp: 'superUltraTestDashboard' }, }); @@ -50,7 +50,7 @@ describe('embeddable state transfer', () => { application.navigateToApp, (historyMock as unknown) as ScopedHistory ); - await stateTransfer.navigateToWithOriginatingApp(destinationApp, { + await stateTransfer.navigateToEditor(destinationApp, { state: { originatingApp }, appendToExistingState: true, }); @@ -94,7 +94,7 @@ describe('embeddable state transfer', () => { application.navigateToApp, (historyMock as unknown) as ScopedHistory ); - const fetchedState = stateTransfer.getIncomingOriginatingApp(); + const fetchedState = stateTransfer.getIncomingEditorState(); expect(fetchedState).toEqual({ originatingApp: 'extremeSportsKibana' }); }); @@ -104,7 +104,7 @@ describe('embeddable state transfer', () => { application.navigateToApp, (historyMock as unknown) as ScopedHistory ); - const fetchedState = stateTransfer.getIncomingOriginatingApp(); + const fetchedState = stateTransfer.getIncomingEditorState(); expect(fetchedState).toBeUndefined(); }); diff --git a/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.ts b/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.ts index 57b425d2df45c2..f47d40fdc70499 100644 --- a/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.ts +++ b/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.ts @@ -20,8 +20,8 @@ import { cloneDeep } from 'lodash'; import { ScopedHistory, ApplicationStart } from '../../../../../core/public'; import { - EmbeddableOriginatingAppState, - isEmbeddableOriginatingAppState, + EmbeddableEditorState, + isEmbeddableEditorState, EmbeddablePackageState, isEmbeddablePackageState, } from './types'; @@ -45,10 +45,10 @@ export class EmbeddableStateTransfer { * @param history - the scoped history to fetch from * @param options.keysToRemoveAfterFetch - an array of keys to be removed from the state after they are retrieved */ - public getIncomingOriginatingApp(options?: { + public getIncomingEditorState(options?: { keysToRemoveAfterFetch?: string[]; - }): EmbeddableOriginatingAppState | undefined { - return this.getIncomingState(isEmbeddableOriginatingAppState, { + }): EmbeddableEditorState | undefined { + return this.getIncomingState(isEmbeddableEditorState, { keysToRemoveAfterFetch: options?.keysToRemoveAfterFetch, }); } @@ -72,15 +72,15 @@ export class EmbeddableStateTransfer { * A wrapper around the {@link ApplicationStart.navigateToApp} method which navigates to the specified appId * with {@link EmbeddableOriginatingAppState | originating app state} */ - public async navigateToWithOriginatingApp( + public async navigateToEditor( appId: string, options?: { path?: string; - state: EmbeddableOriginatingAppState; + state: EmbeddableEditorState; appendToExistingState?: boolean; } ): Promise { - await this.navigateToWithState(appId, options); + await this.navigateToWithState(appId, options); } /** diff --git a/src/plugins/embeddable/public/lib/state_transfer/index.ts b/src/plugins/embeddable/public/lib/state_transfer/index.ts index e51efc5dcca26b..7daa7a0ea81d64 100644 --- a/src/plugins/embeddable/public/lib/state_transfer/index.ts +++ b/src/plugins/embeddable/public/lib/state_transfer/index.ts @@ -18,4 +18,4 @@ */ export { EmbeddableStateTransfer } from './embeddable_state_transfer'; -export { EmbeddableOriginatingAppState, EmbeddablePackageState } from './types'; +export { EmbeddableEditorState, EmbeddablePackageState } from './types'; diff --git a/src/plugins/embeddable/public/lib/state_transfer/types.ts b/src/plugins/embeddable/public/lib/state_transfer/types.ts index 8eae441d1be23c..3bdb4bfde5f56e 100644 --- a/src/plugins/embeddable/public/lib/state_transfer/types.ts +++ b/src/plugins/embeddable/public/lib/state_transfer/types.ts @@ -17,17 +17,18 @@ * under the License. */ +import { EmbeddableInput } from '..'; + /** * Represents a state package that contains the last active app id. * @public */ -export interface EmbeddableOriginatingAppState { +export interface EmbeddableEditorState { originatingApp: string; + byValueMode?: boolean; } -export function isEmbeddableOriginatingAppState( - state: unknown -): state is EmbeddableOriginatingAppState { +export function isEmbeddableEditorState(state: unknown): state is EmbeddableEditorState { return ensureFieldOfTypeExists('originatingApp', state, 'string'); } @@ -35,15 +36,24 @@ export function isEmbeddableOriginatingAppState( * Represents a state package that contains all fields necessary to create an embeddable in a container. * @public */ -export interface EmbeddablePackageState { +export interface EmbeddablePackageByReferenceState { type: string; id: string; } +export interface EmbeddablePackageByValueState { + input: EmbeddableInput; +} + +export type EmbeddablePackageState = + | EmbeddablePackageByReferenceState + | EmbeddablePackageByValueState; + export function isEmbeddablePackageState(state: unknown): state is EmbeddablePackageState { return ( - ensureFieldOfTypeExists('type', state, 'string') && - ensureFieldOfTypeExists('id', state, 'string') + (ensureFieldOfTypeExists('type', state, 'string') && + ensureFieldOfTypeExists('id', state, 'string')) || + ensureFieldOfTypeExists('input', state, 'object') ); } diff --git a/src/plugins/embeddable/public/mocks.tsx b/src/plugins/embeddable/public/mocks.tsx index 49910525c7ab18..6d94af1f228290 100644 --- a/src/plugins/embeddable/public/mocks.tsx +++ b/src/plugins/embeddable/public/mocks.tsx @@ -78,9 +78,9 @@ export const createEmbeddablePanelMock = ({ export const createEmbeddableStateTransferMock = (): Partial => { return { - getIncomingOriginatingApp: jest.fn(), + getIncomingEditorState: jest.fn(), getIncomingEmbeddablePackage: jest.fn(), - navigateToWithOriginatingApp: jest.fn(), + navigateToEditor: jest.fn(), navigateToWithEmbeddablePackage: jest.fn(), }; }; diff --git a/src/plugins/visualizations/public/embeddable/visualize_embeddable_factory.tsx b/src/plugins/visualizations/public/embeddable/visualize_embeddable_factory.tsx index eb4b66401820f1..72c794528b5460 100644 --- a/src/plugins/visualizations/public/embeddable/visualize_embeddable_factory.tsx +++ b/src/plugins/visualizations/public/embeddable/visualize_embeddable_factory.tsx @@ -130,6 +130,7 @@ export class VisualizeEmbeddableFactory showNewVisModal({ originatingApp: await this.getCurrentAppId(), outsideVisualizeApp: true, + createByValue: true, }); return undefined; } diff --git a/src/plugins/visualizations/public/wizard/new_vis_modal.test.tsx b/src/plugins/visualizations/public/wizard/new_vis_modal.test.tsx index dd89e98fb8fe53..f48febfef5b437 100644 --- a/src/plugins/visualizations/public/wizard/new_vis_modal.test.tsx +++ b/src/plugins/visualizations/public/wizard/new_vis_modal.test.tsx @@ -165,7 +165,7 @@ describe('NewVisModal', () => { ); const visButton = wrapper.find('button[data-test-subj="visType-visWithAliasUrl"]'); visButton.simulate('click'); - expect(stateTransfer.navigateToWithOriginatingApp).toBeCalledWith('otherApp', { + expect(stateTransfer.navigateToEditor).toBeCalledWith('otherApp', { path: '#/aliasUrl', state: { originatingApp: 'coolJestTestApp' }, }); diff --git a/src/plugins/visualizations/public/wizard/new_vis_modal.tsx b/src/plugins/visualizations/public/wizard/new_vis_modal.tsx index 84a5bca0ed0edd..d705e3a52e432a 100644 --- a/src/plugins/visualizations/public/wizard/new_vis_modal.tsx +++ b/src/plugins/visualizations/public/wizard/new_vis_modal.tsx @@ -43,6 +43,7 @@ interface TypeSelectionProps { application: ApplicationStart; outsideVisualizeApp?: boolean; stateTransfer?: EmbeddableStateTransfer; + createByValue?: boolean; originatingApp?: string; } @@ -172,9 +173,9 @@ class NewVisModal extends React.Component void; originatingApp?: string; outsideVisualizeApp?: boolean; + createByValue?: boolean; } /** @@ -49,6 +50,7 @@ export function showNewVisModal({ onClose, originatingApp, outsideVisualizeApp, + createByValue, }: ShowNewVisModalParams = {}) { const container = document.createElement('div'); let isClosed = false; @@ -69,6 +71,7 @@ export function showNewVisModal({ isOpen={true} onClose={handleClose} originatingApp={originatingApp} + createByValue={createByValue} stateTransfer={getEmbeddable().getStateTransfer()} outsideVisualizeApp={outsideVisualizeApp} editorParams={editorParams} diff --git a/x-pack/plugins/lens/common/constants.ts b/x-pack/plugins/lens/common/constants.ts index 16397d340d951b..9433828fb248d9 100644 --- a/x-pack/plugins/lens/common/constants.ts +++ b/x-pack/plugins/lens/common/constants.ts @@ -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'; } diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index 0ab547bed6d37c..5ebf506c708d1e 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -38,6 +38,7 @@ import { SavedQuery, UI_SETTINGS, } from '../../../../../src/plugins/data/public'; +import { EmbeddableEditorState } from '../../../../../src/plugins/embeddable/public'; interface State { isLoading: boolean; @@ -64,7 +65,7 @@ export function App({ docId, docStorage, redirectTo, - originatingApp, + embeddableEditorIncomingState, navigation, onAppLeave, history, @@ -77,7 +78,7 @@ export function App({ docId?: string; docStorage: SavedObjectStore; redirectTo: (id?: string, returnToOrigin?: boolean, newlyCreated?: boolean) => void; - originatingApp?: string | undefined; + embeddableEditorIncomingState?: EmbeddableEditorState; onAppLeave: AppMountParameters['onAppLeave']; history: History; }) { @@ -256,6 +257,7 @@ export function App({ if (!lastKnownDoc) { return; } + const [pinnedFilters, appFilters] = _.partition( lastKnownDoc.state?.filters, esFilters.isFilterPinned @@ -297,32 +299,34 @@ export function App({ ); const newlyCreated: boolean = saveProps.newCopyOnSave || !lastKnownDoc?.id; - docStorage - .save(doc) - .then(({ id }) => { - // Prevents unnecessary network request and disables save button - const newDoc = { ...doc, id }; - setState((s) => ({ - ...s, - isSaveModalVisible: false, - persistedDoc: newDoc, - lastKnownDoc: newDoc, - })); - if (docId !== id || saveProps.returnToOrigin) { - redirectTo(id, saveProps.returnToOrigin, newlyCreated); - } - }) - .catch((e) => { - // eslint-disable-next-line no-console - console.dir(e); - trackUiEvent('save_failed'); - core.notifications.toasts.addDanger( - i18n.translate('xpack.lens.app.docSavingError', { - defaultMessage: 'Error saving document', - }) - ); - setState((s) => ({ ...s, isSaveModalVisible: false })); - }); + if (!embeddableEditorIncomingState?.byValueMode) { + docStorage + .save(doc) + .then(({ id }) => { + // Prevents unnecessary network request and disables save button + const newDoc = { ...doc, id }; + setState((s) => ({ + ...s, + isSaveModalVisible: false, + persistedDoc: newDoc, + lastKnownDoc: newDoc, + })); + if (docId !== id || saveProps.returnToOrigin) { + redirectTo(id, saveProps.returnToOrigin, newlyCreated); + } + }) + .catch((e) => { + // eslint-disable-next-line no-console + console.dir(e); + trackUiEvent('save_failed'); + core.notifications.toasts.addDanger( + i18n.translate('xpack.lens.app.docSavingError', { + defaultMessage: 'Error saving document', + }) + ); + setState((s) => ({ ...s, isSaveModalVisible: false })); + }); + } }; const onError = useCallback( @@ -349,7 +353,8 @@ export function App({
{ if (isSaveable && lastKnownDoc) { setState((s) => ({ ...s, isSaveModalVisible: true })); @@ -502,7 +509,7 @@ export function App({
{lastKnownDoc && state.isSaveModalVisible && ( runSave(props)} onClose={() => setState((s) => ({ ...s, isSaveModalVisible: false }))} documentInfo={{ diff --git a/x-pack/plugins/lens/public/app_plugin/mounter.tsx b/x-pack/plugins/lens/public/app_plugin/mounter.tsx index 7a33241792a58f..41548dd0efa99a 100644 --- a/x-pack/plugins/lens/public/app_plugin/mounter.tsx +++ b/x-pack/plugins/lens/public/app_plugin/mounter.tsx @@ -37,8 +37,9 @@ export async function mountApp( ); const stateTransfer = embeddable?.getStateTransfer(params.history); - const { originatingApp } = - stateTransfer?.getIncomingOriginatingApp({ keysToRemoveAfterFetch: ['originatingApp'] }) || {}; + const embeddableEditorIncomingState = stateTransfer?.getIncomingEditorState({ + keysToRemoveAfterFetch: ['originatingApp'], + }); const instance = await createEditorFrame(); @@ -56,17 +57,20 @@ export async function mountApp( ) => { if (!id) { routeProps.history.push('/'); - } else if (!originatingApp) { + } else if (!embeddableEditorIncomingState?.originatingApp) { routeProps.history.push(`/edit/${id}`); - } else if (!!originatingApp && id && returnToOrigin) { + } else if (!!embeddableEditorIncomingState?.originatingApp && id && returnToOrigin) { routeProps.history.push(`/edit/${id}`); if (newlyCreated && stateTransfer) { - stateTransfer.navigateToWithEmbeddablePackage(originatingApp, { - state: { id, type: LENS_EMBEDDABLE_TYPE }, - }); + stateTransfer.navigateToWithEmbeddablePackage( + embeddableEditorIncomingState?.originatingApp, + { + state: { id, type: LENS_EMBEDDABLE_TYPE }, + } + ); } else { - coreStart.application.navigateToApp(originatingApp); + coreStart.application.navigateToApp(embeddableEditorIncomingState?.originatingApp); } } }; @@ -86,7 +90,7 @@ export async function mountApp( redirectTo={(id, returnToOrigin, newlyCreated) => redirectTo(routeProps, id, returnToOrigin, newlyCreated) } - originatingApp={originatingApp} + embeddableEditorIncomingState={embeddableEditorIncomingState} onAppLeave={params.onAppLeave} history={routeProps.history} /> diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/attribute_service.ts b/x-pack/plugins/lens/public/editor_frame_service/embeddable/attribute_service.ts new file mode 100644 index 00000000000000..b49e05e8be5384 --- /dev/null +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/attribute_service.ts @@ -0,0 +1,54 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { + EmbeddableInput, + SavedObjectEmbeddableInput, + isSavedObjectEmbeddableInput, + IEmbeddable, +} from '../../../../../../src/plugins/embeddable/public'; +import { SavedObjectsClientContract, SimpleSavedObject } from '../../../../../../src/core/public'; + +export class AttributeService< + SavedObjectAttributes, + ValType extends EmbeddableInput & { attributes: SavedObjectAttributes }, + RefType extends SavedObjectEmbeddableInput +> { + constructor(private type: string, private savedObjectsClient: SavedObjectsClientContract) {} + + public async unwrapAttributes(input: RefType | ValType): Promise { + if (isSavedObjectEmbeddableInput(input)) { + const savedObject: SimpleSavedObject = await this.savedObjectsClient.get< + SavedObjectAttributes + >(this.type, input.savedObjectId); + return savedObject.attributes; + } + return input.attributes; + } + + public async wrapAttributes( + newAttributes: SavedObjectAttributes, + useRefType: boolean, + embeddable?: IEmbeddable + ): Promise> { + const savedObjectId = + embeddable && isSavedObjectEmbeddableInput(embeddable.getInput()) + ? (embeddable.getInput() as SavedObjectEmbeddableInput).savedObjectId + : undefined; + + if (useRefType) { + if (savedObjectId) { + await this.savedObjectsClient.update(this.type, savedObjectId, newAttributes); + return { savedObjectId } as RefType; + } else { + const savedItem = await this.savedObjectsClient.create(this.type, newAttributes); + return { savedObjectId: savedItem.id } as RefType; + } + } else { + return { attributes: newAttributes } as ValType; + } + } +} diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx index bbd2b18907e9b4..77d952ab0e4ea2 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx @@ -13,6 +13,7 @@ import { Query, TimefilterContract, TimeRange, + IndexPattern, } from 'src/plugins/data/public'; import { Subscription } from 'rxjs'; @@ -27,21 +28,25 @@ import { EmbeddableInput, EmbeddableOutput, IContainer, + SavedObjectEmbeddableInput, } from '../../../../../../src/plugins/embeddable/public'; import { DOC_TYPE, Document } from '../../persistence'; import { ExpressionWrapper } from './expression_wrapper'; import { UiActionsStart } from '../../../../../../src/plugins/ui_actions/public'; import { isLensBrushEvent, isLensFilterEvent } from '../../types'; +import { AttributeService } from './attribute_service'; +import { IndexPatternsContract } from '../../../../../../src/plugins/data/public'; +import { getEditPath } from '../../../common'; +import { IBasePath } from '../../../../../../src/core/public'; -export interface LensEmbeddableConfiguration { - savedVis: Document; - editUrl: string; - editPath: string; - editable: boolean; - indexPatterns?: IIndexPattern[]; -} +export type LensSavedObjectAttributes = Omit; -export interface LensEmbeddableInput extends EmbeddableInput { +export type LensEmbeddableInput = LensByValueInput | LensByReferenceInput; + +export type LensByValueInput = { attributes: LensSavedObjectAttributes } & LensInheritedInput; +export type LensByReferenceInput = SavedObjectEmbeddableInput & LensInheritedInput; + +export interface LensInheritedInput extends EmbeddableInput { timeRange?: TimeRange; query?: Query; filters?: Filter[]; @@ -51,12 +56,25 @@ export interface LensEmbeddableOutput extends EmbeddableOutput { indexPatterns?: IIndexPattern[]; } +export interface LensEmbeddableDeps { + attributeService: AttributeService< + LensSavedObjectAttributes, + LensByValueInput, + LensByReferenceInput + >; + editable: boolean; + indexPatternService: IndexPatternsContract; + expressionRenderer: ReactExpressionRendererType; + timefilter: TimefilterContract; + basePath: IBasePath; + getTrigger?: UiActionsStart['getTrigger'] | undefined; +} + export class Embeddable extends AbstractEmbeddable { type = DOC_TYPE; private expressionRenderer: ReactExpressionRendererType; - private getTrigger: UiActionsStart['getTrigger'] | undefined; - private savedVis: Document; + private savedVis: Document | undefined; private domNode: HTMLElement | Element | undefined; private subscription: Subscription; private autoRefreshFetchSubscription: Subscription; @@ -69,42 +87,36 @@ export class Embeddable extends AbstractEmbeddable this.onContainerStateChanged(input)); - this.onContainerStateChanged(initialInput); + this.expressionRenderer = deps.expressionRenderer; + this.initializeSavedVis(initialInput).then(() => this.onContainerStateChanged(initialInput)); + + this.subscription = this.getInput$().subscribe((input) => { + // await this.documentChanged(input); + this.onContainerStateChanged(input); + }); - this.autoRefreshFetchSubscription = timefilter + this.autoRefreshFetchSubscription = deps.timefilter .getAutoRefreshFetch$() .subscribe(this.reload.bind(this)); } public supportedTriggers() { + if (!this.savedVis) { + return []; + } switch (this.savedVis.visualizationType) { case 'lnsXY': return [VIS_EVENT_TO_TRIGGER.filter, VIS_EVENT_TO_TRIGGER.brush]; @@ -117,6 +129,17 @@ export class Embeddable extends AbstractEmbeddable !filter.meta.disabled) @@ -133,9 +156,7 @@ export class Embeddable extends AbstractEmbeddable { - if (!this.getTrigger || this.input.disableTriggers) { + if (!this.deps.getTrigger || this.input.disableTriggers) { return; } if (isLensBrushEvent(event)) { - this.getTrigger(VIS_EVENT_TO_TRIGGER[event.name]).exec({ + this.deps.getTrigger(VIS_EVENT_TO_TRIGGER[event.name]).exec({ data: event.data, embeddable: this, }); } if (isLensFilterEvent(event)) { - this.getTrigger(VIS_EVENT_TO_TRIGGER[event.name]).exec({ + this.deps.getTrigger(VIS_EVENT_TO_TRIGGER[event.name]).exec({ data: event.data, embeddable: this, }); @@ -186,7 +210,7 @@ export class Embeddable extends AbstractEmbeddable { + try { + return await this.deps.indexPatternService.get(id); + } catch (error) { + // Unable to load index pattern, ignore error as the index patterns are only used to + // configure the filter and query bar - there is still a good chance to get the visualization + // to show. + return null; + } + } + ); + const indexPatterns = ( + await Promise.all(promises) + ).filter((indexPattern: IndexPattern | null): indexPattern is IndexPattern => + Boolean(indexPattern) + ); + // passing edit url and index patterns to the output of this embeddable for + // the container to pick them up and use them to configure filter bar and + // config dropdown correctly. + const input = this.getInput(); + const title = input.hidePanelTitles + ? '' + : input.title === undefined + ? this.savedVis.title + : input.title; + const savedObjectId = (input as LensByReferenceInput).savedObjectId; + this.updateOutput({ + ...this.getOutput(), + defaultTitle: this.savedVis.title, + title, + editPath: getEditPath(savedObjectId), + editUrl: this.deps.basePath.prepend(`app/lens${getEditPath(savedObjectId)}`), + indexPatterns, + }); + } } diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts index c23d44aa8e4b60..ef8e2f517decb5 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts @@ -13,20 +13,23 @@ import { import { i18n } from '@kbn/i18n'; import { IndexPatternsContract, - IndexPattern, TimefilterContract, } from '../../../../../../src/plugins/data/public'; import { ReactExpressionRendererType } from '../../../../../../src/plugins/expressions/public'; import { EmbeddableFactoryDefinition, - ErrorEmbeddable, - EmbeddableInput, IContainer, } from '../../../../../../src/plugins/embeddable/public'; -import { Embeddable } from './embeddable'; -import { SavedObjectIndexStore, DOC_TYPE } from '../../persistence'; -import { getEditPath } from '../../../common'; +import { + Embeddable, + LensSavedObjectAttributes, + LensByValueInput, + LensByReferenceInput, + LensEmbeddableInput, +} from './embeddable'; +import { DOC_TYPE } from '../../persistence'; import { UiActionsStart } from '../../../../../../src/plugins/ui_actions/public'; +import { AttributeService } from './attribute_service'; interface StartServices { timefilter: TimefilterContract; @@ -48,6 +51,12 @@ export class EmbeddableFactory implements EmbeddableFactoryDefinition { getIconForSavedObject: () => 'lensApp', }; + private attributeService?: AttributeService< + LensSavedObjectAttributes, + LensByValueInput, + LensByReferenceInput + >; + constructor(private getStartServices: () => Promise) {} public isEditable = async () => { @@ -67,55 +76,44 @@ export class EmbeddableFactory implements EmbeddableFactoryDefinition { createFromSavedObject = async ( savedObjectId: string, - input: Partial & { id: string }, + input: LensEmbeddableInput, parent?: IContainer ) => { + return this.create(input, parent); + }; + + async create(input: LensEmbeddableInput, parent?: IContainer) { const { - savedObjectsClient, - coreHttp, - indexPatternService, timefilter, expressionRenderer, uiActions, + coreHttp, + indexPatternService, } = await this.getStartServices(); - const store = new SavedObjectIndexStore(savedObjectsClient); - const savedVis = await store.load(savedObjectId); - - const promises = savedVis.state.datasourceMetaData.filterableIndexPatterns.map( - async ({ id }) => { - try { - return await indexPatternService.get(id); - } catch (error) { - // Unable to load index pattern, ignore error as the index patterns are only used to - // configure the filter and query bar - there is still a good chance to get the visualization - // to show. - return null; - } - } - ); - const indexPatterns = ( - await Promise.all(promises) - ).filter((indexPattern: IndexPattern | null): indexPattern is IndexPattern => - Boolean(indexPattern) - ); - return new Embeddable( - timefilter, - expressionRenderer, - uiActions?.getTrigger, { - savedVis, - editPath: getEditPath(savedObjectId), - editUrl: coreHttp.basePath.prepend(`app/lens${getEditPath(savedObjectId)}`), + attributeService: await this.getAttributeService(), + indexPatternService, + timefilter, + expressionRenderer, editable: await this.isEditable(), - indexPatterns, + basePath: coreHttp.basePath, + getTrigger: uiActions?.getTrigger, }, input, parent ); - }; + } - async create(input: EmbeddableInput) { - return new ErrorEmbeddable('Lens can only be created from a saved object', input); + private async getAttributeService() { + const savedObjectsService = (await this.getStartServices()).savedObjectsClient; + if (!this.attributeService) { + this.attributeService = new AttributeService< + LensSavedObjectAttributes, + LensByValueInput, + LensByReferenceInput + >(this.type, savedObjectsService); + } + return this.attributeService; } } From 0039a9702be3f0893991bfa41ec95f6024d466a5 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Fri, 26 Jun 2020 15:52:42 -0400 Subject: [PATCH 02/40] Editing lens by value now works. --- .../application/dashboard_app_controller.tsx | 16 +- .../embeddable/dashboard_container.tsx | 68 +++++--- .../public/lib/actions/edit_panel_action.ts | 17 +- .../public/lib/state_transfer/types.ts | 2 + x-pack/plugins/lens/public/app_plugin/app.tsx | 150 +++++++++++------- .../lens/public/app_plugin/mounter.tsx | 28 +++- .../embeddable/embeddable.tsx | 4 +- 7 files changed, 188 insertions(+), 97 deletions(-) diff --git a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx index 3c559a6cde211d..5e998469543ac9 100644 --- a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx +++ b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx @@ -60,6 +60,7 @@ import { ViewMode, SavedObjectEmbeddableInput, ContainerOutput, + EmbeddableInput, } from '../../../embeddable/public'; import { NavAction, SavedDashboardPanel } from '../types'; @@ -428,11 +429,18 @@ export class DashboardAppController { const incomingState = embeddable .getStateTransfer(scopedHistory()) - .getIncomingEmbeddablePackage(); + .getIncomingEmbeddablePackage({ keysToRemoveAfterFetch: ['id', 'type', 'input'] }); if (incomingState) { - container.addNewEmbeddable(incomingState.type, { - savedObjectId: incomingState.id, - }); + if ('id' in incomingState) { + container.addNewEmbeddable(incomingState.type, { + savedObjectId: incomingState.id, + }); + } else if ('input' in incomingState) { + container.addOrUpdateEmbeddable( + incomingState.type, + incomingState.input + ); + } } } diff --git a/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx b/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx index f1ecd0f221926b..ff74580ba256be 100644 --- a/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx +++ b/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx @@ -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; @@ -159,29 +159,55 @@ export class DashboardContainer extends Container) => { - 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) => + this.replacePanel(placeholderPanelState, newPanelState) + ); + } + + public replacePanel( + previousPanelState: DashboardPanelState, + newPanelState: Partial + ) { + // 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 = IEmbeddable + >(type: string, explicitInput: Partial) { + 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(type, explicitInput); + } } public render(dom: HTMLElement) { diff --git a/src/plugins/embeddable/public/lib/actions/edit_panel_action.ts b/src/plugins/embeddable/public/lib/actions/edit_panel_action.ts index ac046e978c9cb9..a09704fee91b15 100644 --- a/src/plugins/embeddable/public/lib/actions/edit_panel_action.ts +++ b/src/plugins/embeddable/public/lib/actions/edit_panel_action.ts @@ -24,7 +24,8 @@ import { take } from 'rxjs/operators'; import { ViewMode } from '../types'; import { EmbeddableFactoryNotFoundError } from '../errors'; import { EmbeddableStart } from '../../plugin'; -import { IEmbeddable, EmbeddableOriginatingAppState, EmbeddableStateTransfer } from '../..'; +import { IEmbeddable, EmbeddableStateTransfer } from '../..'; +import { SavedObjectEmbeddableInput, EmbeddableEditorState } from '..'; export const ACTION_EDIT_PANEL = 'editPanel'; @@ -35,7 +36,7 @@ interface ActionContext { interface NavigationContext { app: string; path: string; - state?: EmbeddableOriginatingAppState; + state?: EmbeddableEditorState; } export class EditPanelAction implements Action { @@ -109,8 +110,16 @@ export class EditPanelAction implements Action { 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) { + 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 }; } } diff --git a/src/plugins/embeddable/public/lib/state_transfer/types.ts b/src/plugins/embeddable/public/lib/state_transfer/types.ts index 3bdb4bfde5f56e..d0a8d59fa033e8 100644 --- a/src/plugins/embeddable/public/lib/state_transfer/types.ts +++ b/src/plugins/embeddable/public/lib/state_transfer/types.ts @@ -26,6 +26,7 @@ import { EmbeddableInput } from '..'; export interface EmbeddableEditorState { originatingApp: string; byValueMode?: boolean; + valueInput?: EmbeddableInput; } export function isEmbeddableEditorState(state: unknown): state is EmbeddableEditorState { @@ -42,6 +43,7 @@ export interface EmbeddablePackageByReferenceState { } export interface EmbeddablePackageByValueState { + type: string; input: EmbeddableInput; } diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index 5ebf506c708d1e..8589329436b339 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -39,6 +39,7 @@ import { UI_SETTINGS, } from '../../../../../src/plugins/data/public'; import { EmbeddableEditorState } from '../../../../../src/plugins/embeddable/public'; +import { LensByValueInput } from '../editor_frame_service/embeddable/embeddable'; interface State { isLoading: boolean; @@ -57,6 +58,25 @@ interface State { savedQuery?: SavedQuery; } +interface LensAppProps { + editorFrame: EditorFrameInstance; + data: DataPublicPluginStart; + navigation: NavigationPublicPluginStart; + core: AppMountContext['core']; + storage: IStorageWrapper; + docId?: string; + docStorage: SavedObjectStore; + redirectTo: ( + id?: string, + documentByValue?: Document, + returnToOrigin?: boolean, + newlyCreated?: boolean + ) => void; + embeddableEditorIncomingState?: EmbeddableEditorState; + onAppLeave: AppMountParameters['onAppLeave']; + history: History; +} + export function App({ editorFrame, data, @@ -69,19 +89,7 @@ export function App({ navigation, onAppLeave, history, -}: { - editorFrame: EditorFrameInstance; - data: DataPublicPluginStart; - navigation: NavigationPublicPluginStart; - core: AppMountContext['core']; - storage: IStorageWrapper; - docId?: string; - docStorage: SavedObjectStore; - redirectTo: (id?: string, returnToOrigin?: boolean, newlyCreated?: boolean) => void; - embeddableEditorIncomingState?: EmbeddableEditorState; - onAppLeave: AppMountParameters['onAppLeave']; - history: History; -}) { +}: LensAppProps) { const language = storage.get('kibana.userQueryLanguage') || core.uiSettings.get(UI_SETTINGS.SEARCH_QUERY_LANGUAGE); @@ -155,6 +163,7 @@ export function App({ // Confirm when the user has made any changes to an existing doc // or when the user has configured something without saving if ( + !embeddableEditorIncomingState?.byValueMode && core.application.capabilities.visualize.save && (state.persistedDoc?.expression ? !_.isEqual(lastKnownDoc?.expression, state.persistedDoc.expression) @@ -189,41 +198,24 @@ export function App({ }, { text: state.persistedDoc - ? state.persistedDoc.title + ? embeddableEditorIncomingState?.byValueMode + ? i18n.translate('xpack.lens.breadcrumbsByValue', { defaultMessage: 'By Value' }) + : state.persistedDoc.title : i18n.translate('xpack.lens.breadcrumbsCreate', { defaultMessage: 'Create' }), }, ]); }, [core.application, core.chrome, core.http.basePath, state.persistedDoc]); useEffect(() => { - if (docId && (!state.persistedDoc || state.persistedDoc.id !== docId)) { + if ( + !embeddableEditorIncomingState?.byValueMode && + docId && + (!state.persistedDoc || state.persistedDoc.id !== docId) + ) { setState((s) => ({ ...s, isLoading: true })); docStorage .load(docId) - .then((doc) => { - getAllIndexPatterns( - doc.state.datasourceMetaData.filterableIndexPatterns, - data.indexPatterns, - core.notifications - ) - .then((indexPatterns) => { - // Don't overwrite any pinned filters - data.query.filterManager.setAppFilters(doc.state.filters); - setState((s) => ({ - ...s, - isLoading: false, - persistedDoc: doc, - lastKnownDoc: doc, - query: doc.state.query, - indexPatternsForTopNav: indexPatterns, - })); - }) - .catch(() => { - setState((s) => ({ ...s, isLoading: false })); - - redirectTo(); - }); - }) + .then((doc) => updateDoc(doc)) .catch(() => { setState((s) => ({ ...s, isLoading: false })); @@ -235,6 +227,16 @@ export function App({ redirectTo(); }); + } else if ( + embeddableEditorIncomingState?.byValueMode && + embeddableEditorIncomingState?.valueInput + ) { + const doc = { + ...(embeddableEditorIncomingState?.valueInput as LensByValueInput).attributes, + id: embeddableEditorIncomingState?.valueInput.id, + }; + updateDoc(doc); + redirectTo(); } }, [ core.notifications, @@ -247,6 +249,31 @@ export function App({ // state.persistedDoc, ]); + const updateDoc = async (doc: Document) => { + getAllIndexPatterns( + doc.state.datasourceMetaData.filterableIndexPatterns, + data.indexPatterns, + core.notifications + ) + .then((indexPatterns) => { + // Don't overwrite any pinned filters + data.query.filterManager.setAppFilters(doc.state.filters); + setState((s) => ({ + ...s, + isLoading: false, + persistedDoc: doc, + lastKnownDoc: doc, + query: doc.state.query, + indexPatternsForTopNav: indexPatterns, + })); + }) + .catch(() => { + setState((s) => ({ ...s, isLoading: false })); + + redirectTo(); + }); + }; + const runSave = async ( saveProps: Omit & { returnToOrigin: boolean; @@ -279,27 +306,30 @@ export function App({ title: saveProps.newTitle, }; - await checkForDuplicateTitle( - { - ...doc, - copyOnSave: saveProps.newCopyOnSave, - lastSavedTitle: lastKnownDoc?.title, - getEsType: () => 'lens', - getDisplayName: () => - i18n.translate('xpack.lens.app.saveModalType', { - defaultMessage: 'Lens visualization', - }), - }, - saveProps.isTitleDuplicateConfirmed, - saveProps.onTitleDuplicate, - { - savedObjectsClient: core.savedObjects.client, - overlays: core.overlays, - } - ); - const newlyCreated: boolean = saveProps.newCopyOnSave || !lastKnownDoc?.id; - if (!embeddableEditorIncomingState?.byValueMode) { + + if (embeddableEditorIncomingState?.byValueMode) { + redirectTo(doc.id, doc, saveProps.returnToOrigin, newlyCreated); + } else { + await checkForDuplicateTitle( + { + ...doc, + copyOnSave: saveProps.newCopyOnSave, + lastSavedTitle: lastKnownDoc?.title, + getEsType: () => 'lens', + getDisplayName: () => + i18n.translate('xpack.lens.app.saveModalType', { + defaultMessage: 'Lens visualization', + }), + }, + saveProps.isTitleDuplicateConfirmed, + saveProps.onTitleDuplicate, + { + savedObjectsClient: core.savedObjects.client, + overlays: core.overlays, + } + ); + docStorage .save(doc) .then(({ id }) => { @@ -312,7 +342,7 @@ export function App({ lastKnownDoc: newDoc, })); if (docId !== id || saveProps.returnToOrigin) { - redirectTo(id, saveProps.returnToOrigin, newlyCreated); + redirectTo(id, undefined, saveProps.returnToOrigin, newlyCreated); } }) .catch((e) => { diff --git a/x-pack/plugins/lens/public/app_plugin/mounter.tsx b/x-pack/plugins/lens/public/app_plugin/mounter.tsx index 41548dd0efa99a..0da0b15043d2aa 100644 --- a/x-pack/plugins/lens/public/app_plugin/mounter.tsx +++ b/x-pack/plugins/lens/public/app_plugin/mounter.tsx @@ -5,6 +5,8 @@ */ import React from 'react'; +import uuid from 'uuid'; + import { AppMountParameters, CoreSetup } from 'kibana/public'; import { FormattedMessage, I18nProvider } from '@kbn/i18n/react'; import { HashRouter, Route, RouteComponentProps, Switch } from 'react-router-dom'; @@ -18,7 +20,7 @@ import { LensReportManager, setReportManager, trackUiEvent } from '../lens_ui_te import { App } from './app'; import { EditorFrameStart } from '../types'; import { addHelpMenuToAppChrome } from '../help_menu_util'; -import { SavedObjectIndexStore } from '../persistence'; +import { Document, SavedObjectIndexStore } from '../persistence'; import { LensPluginStartDependencies } from '../plugin'; import { LENS_EMBEDDABLE_TYPE } from '../../common'; @@ -52,23 +54,36 @@ export async function mountApp( const redirectTo = ( routeProps: RouteComponentProps<{ id?: string }>, id?: string, + documentByValue?: Document, returnToOrigin?: boolean, newlyCreated?: boolean ) => { - if (!id) { + if (!id && !embeddableEditorIncomingState?.byValueMode) { routeProps.history.push('/'); } else if (!embeddableEditorIncomingState?.originatingApp) { routeProps.history.push(`/edit/${id}`); - } else if (!!embeddableEditorIncomingState?.originatingApp && id && returnToOrigin) { + } else if (!!embeddableEditorIncomingState?.originatingApp && returnToOrigin) { routeProps.history.push(`/edit/${id}`); - if (newlyCreated && stateTransfer) { + if (newlyCreated && id) { stateTransfer.navigateToWithEmbeddablePackage( embeddableEditorIncomingState?.originatingApp, { state: { id, type: LENS_EMBEDDABLE_TYPE }, } ); + } else if (documentByValue) { + const { type, ...rest } = documentByValue; + const idToUse = id ? id : uuid.v4(); + stateTransfer.navigateToWithEmbeddablePackage( + embeddableEditorIncomingState?.originatingApp, + { + state: { + type: LENS_EMBEDDABLE_TYPE, + input: { id: idToUse, attributes: rest }, + }, + } + ); } else { coreStart.application.navigateToApp(embeddableEditorIncomingState?.originatingApp); } @@ -77,7 +92,6 @@ export async function mountApp( const renderEditor = (routeProps: RouteComponentProps<{ id?: string }>) => { trackUiEvent('loaded'); - return ( - redirectTo(routeProps, id, returnToOrigin, newlyCreated) + redirectTo={(id, documentByValue, returnToOrigin, newlyCreated) => + redirectTo(routeProps, id, documentByValue, returnToOrigin, newlyCreated) } embeddableEditorIncomingState={embeddableEditorIncomingState} onAppLeave={params.onAppLeave} diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx index 77d952ab0e4ea2..486dd8be867626 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx @@ -137,7 +137,9 @@ export class Embeddable extends AbstractEmbeddable Date: Fri, 26 Jun 2020 17:53:24 -0400 Subject: [PATCH 03/40] Allow by value lens embeddable to be made by reference via the save as button --- .../application/dashboard_app_controller.tsx | 3 ++- .../public/lib/state_transfer/types.ts | 1 + x-pack/plugins/lens/public/app_plugin/app.tsx | 27 ++++++++++++++----- .../lens/public/app_plugin/mounter.tsx | 17 ++++++++---- 4 files changed, 35 insertions(+), 13 deletions(-) diff --git a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx index 5e998469543ac9..dccbf30c0c61b2 100644 --- a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx +++ b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx @@ -432,8 +432,9 @@ export class DashboardAppController { .getIncomingEmbeddablePackage({ keysToRemoveAfterFetch: ['id', 'type', 'input'] }); if (incomingState) { if ('id' in incomingState) { - container.addNewEmbeddable(incomingState.type, { + container.addOrUpdateEmbeddable(incomingState.type, { savedObjectId: incomingState.id, + id: incomingState.embeddableIdToReplace, }); } else if ('input' in incomingState) { container.addOrUpdateEmbeddable( diff --git a/src/plugins/embeddable/public/lib/state_transfer/types.ts b/src/plugins/embeddable/public/lib/state_transfer/types.ts index a6721784302ac7..0eb4f63f2e4f5d 100644 --- a/src/plugins/embeddable/public/lib/state_transfer/types.ts +++ b/src/plugins/embeddable/public/lib/state_transfer/types.ts @@ -40,6 +40,7 @@ export function isEmbeddableEditorState(state: unknown): state is EmbeddableEdit export interface EmbeddablePackageByReferenceState { type: string; id: string; + embeddableIdToReplace?: string; } /** diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index 8589329436b339..b2f1d47561226a 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -70,7 +70,8 @@ interface LensAppProps { id?: string, documentByValue?: Document, returnToOrigin?: boolean, - newlyCreated?: boolean + newlyCreated?: boolean, + embeddableIdToReplace?: string ) => void; embeddableEditorIncomingState?: EmbeddableEditorState; onAppLeave: AppMountParameters['onAppLeave']; @@ -279,7 +280,8 @@ export function App({ returnToOrigin: boolean; onTitleDuplicate?: OnSaveProps['onTitleDuplicate']; newDescription?: string; - } + }, + saveToLibrary: boolean = false ) => { if (!lastKnownDoc) { return; @@ -306,9 +308,12 @@ export function App({ title: saveProps.newTitle, }; - const newlyCreated: boolean = saveProps.newCopyOnSave || !lastKnownDoc?.id; + const newlyCreated: boolean = + saveProps.newCopyOnSave || + !lastKnownDoc?.id || + (saveToLibrary && !!embeddableEditorIncomingState?.byValueMode); - if (embeddableEditorIncomingState?.byValueMode) { + if (embeddableEditorIncomingState?.byValueMode && !saveToLibrary) { redirectTo(doc.id, doc, saveProps.returnToOrigin, newlyCreated); } else { await checkForDuplicateTitle( @@ -330,6 +335,10 @@ export function App({ } ); + const embeddableId = doc.id; + if (saveToLibrary && embeddableEditorIncomingState?.byValueMode) { + delete doc.id; + } docStorage .save(doc) .then(({ id }) => { @@ -342,7 +351,11 @@ export function App({ lastKnownDoc: newDoc, })); if (docId !== id || saveProps.returnToOrigin) { - redirectTo(id, undefined, saveProps.returnToOrigin, newlyCreated); + const idToReplace = + saveToLibrary && embeddableEditorIncomingState?.byValueMode + ? embeddableId + : undefined; + redirectTo(id, undefined, saveProps.returnToOrigin, newlyCreated, idToReplace); } }) .catch((e) => { @@ -540,10 +553,10 @@ export function App({ {lastKnownDoc && state.isSaveModalVisible && ( runSave(props)} + onSave={(props) => runSave(props, true)} onClose={() => setState((s) => ({ ...s, isSaveModalVisible: false }))} documentInfo={{ - id: lastKnownDoc.id, + id: embeddableEditorIncomingState?.byValueMode ? undefined : lastKnownDoc.id, title: lastKnownDoc.title || '', description: lastKnownDoc.description || '', }} diff --git a/x-pack/plugins/lens/public/app_plugin/mounter.tsx b/x-pack/plugins/lens/public/app_plugin/mounter.tsx index 0da0b15043d2aa..85c8060f209e55 100644 --- a/x-pack/plugins/lens/public/app_plugin/mounter.tsx +++ b/x-pack/plugins/lens/public/app_plugin/mounter.tsx @@ -56,7 +56,8 @@ export async function mountApp( id?: string, documentByValue?: Document, returnToOrigin?: boolean, - newlyCreated?: boolean + newlyCreated?: boolean, + embeddableIdToReplace?: string ) => { if (!id && !embeddableEditorIncomingState?.byValueMode) { routeProps.history.push('/'); @@ -64,12 +65,11 @@ export async function mountApp( routeProps.history.push(`/edit/${id}`); } else if (!!embeddableEditorIncomingState?.originatingApp && returnToOrigin) { routeProps.history.push(`/edit/${id}`); - if (newlyCreated && id) { stateTransfer.navigateToWithEmbeddablePackage( embeddableEditorIncomingState?.originatingApp, { - state: { id, type: LENS_EMBEDDABLE_TYPE }, + state: { id, type: LENS_EMBEDDABLE_TYPE, embeddableIdToReplace }, } ); } else if (documentByValue) { @@ -101,8 +101,15 @@ export async function mountApp( storage={new Storage(localStorage)} docId={routeProps.match.params.id} docStorage={new SavedObjectIndexStore(savedObjectsClient)} - redirectTo={(id, documentByValue, returnToOrigin, newlyCreated) => - redirectTo(routeProps, id, documentByValue, returnToOrigin, newlyCreated) + redirectTo={(id, documentByValue, returnToOrigin, newlyCreated, embeddableIdToReplace) => + redirectTo( + routeProps, + id, + documentByValue, + returnToOrigin, + newlyCreated, + embeddableIdToReplace + ) } embeddableEditorIncomingState={embeddableEditorIncomingState} onAppLeave={params.onAppLeave} From 3ee8263fd81b19ca1526087922ab614b52a68b80 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Mon, 29 Jun 2020 18:31:54 -0400 Subject: [PATCH 04/40] fixed up some jest tests --- .../lib/actions/edit_panel_action.test.tsx | 33 +++- .../lens/public/app_plugin/app.test.tsx | 26 ++- .../embeddable/embeddable.test.tsx | 171 ++++++++++++------ .../embeddable/embeddable.tsx | 5 +- 4 files changed, 172 insertions(+), 63 deletions(-) diff --git a/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx b/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx index 594a7ad73c3965..2cc0261ff08441 100644 --- a/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx +++ b/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx @@ -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 }, }); }); diff --git a/x-pack/plugins/lens/public/app_plugin/app.test.tsx b/x-pack/plugins/lens/public/app_plugin/app.test.tsx index cd6fbf96d67500..c14943c0aa1ac4 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.test.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.test.tsx @@ -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; @@ -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<{ @@ -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; diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx index 69447b3b9a9b83..8a9d1890e8af05 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx @@ -5,12 +5,27 @@ */ import { Subject } from 'rxjs'; -import { Embeddable } from './embeddable'; +import { + Embeddable, + LensByValueInput, + LensByReferenceInput, + LensSavedObjectAttributes, + LensEmbeddableInput, +} from './embeddable'; import { ReactExpressionRendererProps } from 'src/plugins/expressions/public'; -import { Query, TimeRange, Filter, TimefilterContract } from 'src/plugins/data/public'; +import { + Query, + TimeRange, + Filter, + TimefilterContract, + IndexPatternsContract, +} from 'src/plugins/data/public'; import { Document } from '../../persistence'; import { dataPluginMock } from '../../../../../../src/plugins/data/public/mocks'; import { VIS_EVENT_TO_TRIGGER } from '../../../../../../src/plugins/visualizations/public/embeddable'; +import { AttributeService } from './attribute_service'; +import { coreMock, httpServiceMock } from '../../../../../../src/core/public/mocks'; +import { IBasePath } from '../../../../../../src/core/public'; jest.mock('../../../../../../src/plugins/inspector/public/', () => ({ isAvailable: false, @@ -32,59 +47,86 @@ const savedVis: Document = { visualizationType: '', }; +// TODO: After #68719 is merged, use a mock from the embeddable plugin for this. +const attributeServiceMockFromSavedVis = ( + document: Document +): AttributeService => { + const service = new AttributeService< + LensSavedObjectAttributes, + LensByValueInput, + LensByReferenceInput + >('lens', coreMock.createStart().savedObjects.client); + service.unwrapAttributes = jest.fn((input: LensByValueInput | LensByReferenceInput) => { + return Promise.resolve({ ...document } as LensSavedObjectAttributes); + }); + service.wrapAttributes = jest.fn(); + return service; +}; + describe('embeddable', () => { let mountpoint: HTMLDivElement; let expressionRenderer: jest.Mock; let getTrigger: jest.Mock; let trigger: { exec: jest.Mock }; + let basePath: IBasePath; + let attributeService: AttributeService< + LensSavedObjectAttributes, + LensByValueInput, + LensByReferenceInput + >; beforeEach(() => { mountpoint = document.createElement('div'); expressionRenderer = jest.fn((_props) => null); trigger = { exec: jest.fn() }; getTrigger = jest.fn(() => trigger); + attributeService = attributeServiceMockFromSavedVis(savedVis); + const http = httpServiceMock.createSetupContract({ basePath: '/test' }); + basePath = http.basePath; }); afterEach(() => { mountpoint.remove(); }); - it('should render expression with expression renderer', () => { + it('should render expression with expression renderer', async () => { const embeddable = new Embeddable( - dataPluginMock.createSetupContract().query.timefilter.timefilter, - expressionRenderer, - getTrigger, { - editPath: '', - editUrl: '', + timefilter: dataPluginMock.createSetupContract().query.timefilter.timefilter, + attributeService, + expressionRenderer, + basePath, + indexPatternService: {} as IndexPatternsContract, editable: true, - savedVis, + getTrigger, }, - { id: '123' } + {} as LensEmbeddableInput ); + await embeddable.initializeSavedVis({} as LensEmbeddableInput); embeddable.render(mountpoint); expect(expressionRenderer).toHaveBeenCalledTimes(1); expect(expressionRenderer.mock.calls[0][0]!.expression).toEqual(savedVis.expression); }); - it('should re-render if new input is pushed', () => { + it('should re-render if new input is pushed', async () => { const timeRange: TimeRange = { from: 'now-15d', to: 'now' }; const query: Query = { language: 'kquery', query: '' }; const filters: Filter[] = [{ meta: { alias: 'test', negate: false, disabled: false } }]; const embeddable = new Embeddable( - dataPluginMock.createSetupContract().query.timefilter.timefilter, - expressionRenderer, - getTrigger, { - editPath: '', - editUrl: '', + timefilter: dataPluginMock.createSetupContract().query.timefilter.timefilter, + attributeService, + expressionRenderer, + basePath, + indexPatternService: {} as IndexPatternsContract, editable: true, - savedVis, + getTrigger, }, - { id: '123' } + { id: '123' } as LensEmbeddableInput ); + await embeddable.initializeSavedVis({ id: '123' } as LensEmbeddableInput); embeddable.render(mountpoint); embeddable.updateInput({ @@ -96,45 +138,54 @@ describe('embeddable', () => { expect(expressionRenderer).toHaveBeenCalledTimes(2); }); - it('should pass context to embeddable', () => { + it('should pass context to embeddable', async () => { const timeRange: TimeRange = { from: 'now-15d', to: 'now' }; const query: Query = { language: 'kquery', query: '' }; const filters: Filter[] = [{ meta: { alias: 'test', negate: false, disabled: false } }]; const embeddable = new Embeddable( - dataPluginMock.createSetupContract().query.timefilter.timefilter, - expressionRenderer, - getTrigger, { - editPath: '', - editUrl: '', + timefilter: dataPluginMock.createSetupContract().query.timefilter.timefilter, + attributeService, + expressionRenderer, + basePath, + indexPatternService: {} as IndexPatternsContract, editable: true, - savedVis, + getTrigger, }, - { id: '123', timeRange, query, filters } + { id: '123', timeRange, query, filters } as LensEmbeddableInput ); - embeddable.render(mountpoint); - - expect(expressionRenderer.mock.calls[0][0].searchContext).toEqual({ + await embeddable.initializeSavedVis({ + id: '123', timeRange, query, filters, - }); + } as LensEmbeddableInput); + embeddable.render(mountpoint); + + expect(expressionRenderer.mock.calls[0][0].searchContext).toEqual( + expect.objectContaining({ + timeRange, + query, + filters, + }) + ); }); - it('should execute trigger on event from expression renderer', () => { + it('should execute trigger on event from expression renderer', async () => { const embeddable = new Embeddable( - dataPluginMock.createSetupContract().query.timefilter.timefilter, - expressionRenderer, - getTrigger, { - editPath: '', - editUrl: '', + timefilter: dataPluginMock.createSetupContract().query.timefilter.timefilter, + attributeService, + expressionRenderer, + basePath, + indexPatternService: {} as IndexPatternsContract, editable: true, - savedVis, + getTrigger, }, - { id: '123' } + { id: '123' } as LensEmbeddableInput ); + await embeddable.initializeSavedVis({ id: '123' } as LensEmbeddableInput); embeddable.render(mountpoint); const onEvent = expressionRenderer.mock.calls[0][0].onEvent!; @@ -148,23 +199,29 @@ describe('embeddable', () => { ); }); - it('should not re-render if only change is in disabled filter', () => { + it('should not re-render if only change is in disabled filter', async () => { const timeRange: TimeRange = { from: 'now-15d', to: 'now' }; const query: Query = { language: 'kquery', query: '' }; const filters: Filter[] = [{ meta: { alias: 'test', negate: false, disabled: true } }]; const embeddable = new Embeddable( - dataPluginMock.createSetupContract().query.timefilter.timefilter, - expressionRenderer, - getTrigger, { - editPath: '', - editUrl: '', + timefilter: dataPluginMock.createSetupContract().query.timefilter.timefilter, + attributeService, + expressionRenderer, + basePath, + indexPatternService: {} as IndexPatternsContract, editable: true, - savedVis, + getTrigger, }, - { id: '123', timeRange, query, filters } + { id: '123', timeRange, query, filters } as LensEmbeddableInput ); + await embeddable.initializeSavedVis({ + id: '123', + timeRange, + query, + filters, + } as LensEmbeddableInput); embeddable.render(mountpoint); embeddable.updateInput({ @@ -176,7 +233,7 @@ describe('embeddable', () => { expect(expressionRenderer).toHaveBeenCalledTimes(1); }); - it('should re-render on auto refresh fetch observable', () => { + it('should re-render on auto refresh fetch observable', async () => { const timeRange: TimeRange = { from: 'now-15d', to: 'now' }; const query: Query = { language: 'kquery', query: '' }; const filters: Filter[] = [{ meta: { alias: 'test', negate: false, disabled: true } }]; @@ -187,17 +244,23 @@ describe('embeddable', () => { } as unknown) as TimefilterContract; const embeddable = new Embeddable( - timefilter, - expressionRenderer, - getTrigger, { - editPath: '', - editUrl: '', + timefilter, + attributeService, + expressionRenderer, + basePath, + indexPatternService: {} as IndexPatternsContract, editable: true, - savedVis, + getTrigger, }, - { id: '123', timeRange, query, filters } + { id: '123', timeRange, query, filters } as LensEmbeddableInput ); + await embeddable.initializeSavedVis({ + id: '123', + timeRange, + query, + filters, + } as LensEmbeddableInput); embeddable.render(mountpoint); autoRefreshFetchSubject.next(); diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx index 486dd8be867626..f4810935c6e5eb 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx @@ -104,7 +104,6 @@ export class Embeddable extends AbstractEmbeddable this.onContainerStateChanged(initialInput)); this.subscription = this.getInput$().subscribe((input) => { - // await this.documentChanged(input); this.onContainerStateChanged(input); }); @@ -134,11 +133,11 @@ export class Embeddable extends AbstractEmbeddable Date: Tue, 30 Jun 2020 13:02:54 -0400 Subject: [PATCH 05/40] functional and jest test fixes --- src/plugins/embeddable/public/lib/containers/container.ts | 2 +- x-pack/plugins/lens/public/app_plugin/app.test.tsx | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/plugins/embeddable/public/lib/containers/container.ts b/src/plugins/embeddable/public/lib/containers/container.ts index 8cf3d1a15883b5..1af859386eee25 100644 --- a/src/plugins/embeddable/public/lib/containers/container.ts +++ b/src/plugins/embeddable/public/lib/containers/container.ts @@ -198,8 +198,8 @@ export abstract class Container< return { type: factory.type, explicitInput: { - id: embeddableId, ...explicitInput, + id: embeddableId, } as TEmbeddableInput, }; } diff --git a/x-pack/plugins/lens/public/app_plugin/app.test.tsx b/x-pack/plugins/lens/public/app_plugin/app.test.tsx index c14943c0aa1ac4..b90da8cbf73edb 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.test.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.test.tsx @@ -536,7 +536,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' }); @@ -556,7 +556,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' }); @@ -624,7 +624,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 () => { From 5d05ac93b788f0a6fe231a4bef70b8eb6372d6c9 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Tue, 30 Jun 2020 15:34:07 -0400 Subject: [PATCH 06/40] fix for visualize by value --- .../application/dashboard_app_controller.tsx | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx index e0cef65278e411..3e6e6607eddec6 100644 --- a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx +++ b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx @@ -60,6 +60,7 @@ import { ViewMode, ContainerOutput, EmbeddableInput, + SavedObjectEmbeddableInput, } from '../../../embeddable/public'; import { NavAction, SavedDashboardPanel } from '../types'; @@ -436,10 +437,15 @@ export class DashboardAppController { id: incomingState.embeddableIdToReplace, }); } else if ('input' in incomingState) { - container.addOrUpdateEmbeddable( - incomingState.type, - incomingState.input - ); + // 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(incomingState.type, explicitInput); } } } From 3ecb3d9e379cff329d0bd3a4fbf529b07dad5d03 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Mon, 13 Jul 2020 21:15:53 -0400 Subject: [PATCH 07/40] Restructured adding and editing to minimize deletion of embeddables and re-rendering --- .../application/dashboard_app_controller.tsx | 54 ++++++++++++------- .../public/lib/state_transfer/types.ts | 30 +++-------- .../public/wizard/new_vis_modal.tsx | 5 +- x-pack/plugins/lens/public/app_plugin/app.tsx | 54 ++++++++----------- .../lens/public/app_plugin/mounter.tsx | 34 +++++------- 5 files changed, 81 insertions(+), 96 deletions(-) diff --git a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx index a3ef907f4b6374..9385a2fc586a26 100644 --- a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx +++ b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx @@ -300,6 +300,10 @@ export class DashboardAppController { return emptyScreenProps; }; + const incomingEmbeddable = embeddable + .getStateTransfer(scopedHistory()) + .getIncomingEmbeddablePackage({ keysToRemoveAfterFetch: ['id', 'type', 'input'] }); + const getDashboardInput = (): DashboardContainerInput => { const embeddablesMap: { [key: string]: DashboardPanelState; @@ -307,6 +311,22 @@ export class DashboardAppController { dashboardStateManager.getPanels().forEach((panel: SavedDashboardPanel) => { embeddablesMap[panel.panelIndex] = convertSavedDashboardPanelToPanelState(panel); }); + + console.log('incomingEmbeddable', incomingEmbeddable); + + // If the incoming embeddable state's id already exists in the embeddables map, replace the input, retaining the existing gridData for that panel. + if (incomingEmbeddable?.id && embeddablesMap[incomingEmbeddable.id]) { + const originalPanelState = { ...embeddablesMap[incomingEmbeddable.id] }; + embeddablesMap[incomingEmbeddable.id] = { + gridData: originalPanelState.gridData, + type: incomingEmbeddable.type, + explicitInput: { + ...incomingEmbeddable.input, + id: incomingEmbeddable.id, + }, + }; + } + let expandedPanelId; if (dashboardContainer && !isErrorEmbeddable(dashboardContainer)) { expandedPanelId = dashboardContainer.getInput().expandedPanelId; @@ -427,26 +447,20 @@ export class DashboardAppController { refreshDashboardContainer(); }); - const incomingState = embeddable - .getStateTransfer(scopedHistory()) - .getIncomingEmbeddablePackage({ keysToRemoveAfterFetch: ['id', 'type', 'input'] }); - if (incomingState) { - if ('id' in incomingState) { - container.addOrUpdateEmbeddable(incomingState.type, { - savedObjectId: incomingState.id, - id: incomingState.embeddableIdToReplace, - }); - } else if ('input' in incomingState) { - // 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(incomingState.type, explicitInput); - } + // If the incomingEmbeddable does not yet exist in the panels listing, create a new panel using the container's addEmbeddable method. + if ( + incomingEmbeddable && + (!incomingEmbeddable.id || !container.getInput().panels[incomingEmbeddable.id]) + ) { + // TODO: Get rid of this, maybe by making the visualize embeddable also use the attributeService + const explicitInput = + incomingEmbeddable.type === 'lens' || + (incomingEmbeddable.input as SavedObjectEmbeddableInput).savedObjectId + ? incomingEmbeddable.input + : { + savedVis: incomingEmbeddable.input, + }; + container.addNewEmbeddable(incomingEmbeddable.type, explicitInput); } } diff --git a/src/plugins/embeddable/public/lib/state_transfer/types.ts b/src/plugins/embeddable/public/lib/state_transfer/types.ts index 0eb4f63f2e4f5d..f5e386681b425a 100644 --- a/src/plugins/embeddable/public/lib/state_transfer/types.ts +++ b/src/plugins/embeddable/public/lib/state_transfer/types.ts @@ -17,7 +17,8 @@ * under the License. */ -import { EmbeddableInput } from '..'; +import { EmbeddableInput, SavedObjectEmbeddableInput } from '..'; +import { Optional } from '@kbn/utility-types'; /** * Represents a state package that contains the last active app id. @@ -25,8 +26,7 @@ import { EmbeddableInput } from '..'; */ export interface EmbeddableEditorState { originatingApp: string; - byValueMode?: boolean; - valueInput?: EmbeddableInput; + valueInput?: EmbeddableInput | 'createByValue'; } export function isEmbeddableEditorState(state: unknown): state is EmbeddableEditorState { @@ -34,32 +34,18 @@ export function isEmbeddableEditorState(state: unknown): state is EmbeddableEdit } /** - * Represents a state package that contains all fields necessary to create an embeddable by reference in a container. + * Represents a state package that contains all fields necessary to create an embeddable by reference or by value in a container. * @public */ -export interface EmbeddablePackageByReferenceState { +export interface EmbeddablePackageState { + id?: string; type: string; - id: string; - embeddableIdToReplace?: string; + input: Optional | Optional; } -/** - * Represents a state package that contains all fields necessary to create an embeddable by value in a container. - * @public - */ -export interface EmbeddablePackageByValueState { - type: string; - input: EmbeddableInput; -} - -export type EmbeddablePackageState = - | EmbeddablePackageByReferenceState - | EmbeddablePackageByValueState; - export function isEmbeddablePackageState(state: unknown): state is EmbeddablePackageState { return ( - (ensureFieldOfTypeExists('type', state, 'string') && - ensureFieldOfTypeExists('id', state, 'string')) || + ensureFieldOfTypeExists('type', state, 'string') && ensureFieldOfTypeExists('input', state, 'object') ); } diff --git a/src/plugins/visualizations/public/wizard/new_vis_modal.tsx b/src/plugins/visualizations/public/wizard/new_vis_modal.tsx index d705e3a52e432a..305cec77981fab 100644 --- a/src/plugins/visualizations/public/wizard/new_vis_modal.tsx +++ b/src/plugins/visualizations/public/wizard/new_vis_modal.tsx @@ -175,7 +175,10 @@ class NewVisModal extends React.Component void; embeddableEditorIncomingState?: EmbeddableEditorState; onAppLeave: AppMountParameters['onAppLeave']; @@ -84,7 +83,7 @@ export function App({ data, core, storage, - docId, + savedObjectId, docStorage, redirectTo, embeddableEditorIncomingState, @@ -99,7 +98,7 @@ export function App({ const [state, setState] = useState(() => { const currentRange = data.query.timefilter.timefilter.getTime(); return { - isLoading: !!docId, + isLoading: !!savedObjectId, isSaveModalVisible: false, indexPatternsForTopNav: [], query: { query: '', language }, @@ -183,7 +182,7 @@ export function App({ // Confirm when the user has made any changes to an existing doc // or when the user has configured something without saving if ( - !embeddableEditorIncomingState?.byValueMode && + !embeddableEditorIncomingState?.valueInput && core.application.capabilities.visualize.save && (state.persistedDoc?.expression ? !_.isEqual(lastKnownDoc?.expression, state.persistedDoc.expression) @@ -218,7 +217,7 @@ export function App({ }, { text: state.persistedDoc - ? embeddableEditorIncomingState?.byValueMode + ? embeddableEditorIncomingState?.valueInput ? i18n.translate('xpack.lens.breadcrumbsByValue', { defaultMessage: 'By Value' }) : state.persistedDoc.title : i18n.translate('xpack.lens.breadcrumbsCreate', { defaultMessage: 'Create' }), @@ -228,13 +227,13 @@ export function App({ useEffect(() => { if ( - !embeddableEditorIncomingState?.byValueMode && - docId && - (!state.persistedDoc || state.persistedDoc.id !== docId) + !embeddableEditorIncomingState?.valueInput && + savedObjectId && + (!state.persistedDoc || state.persistedDoc.id !== savedObjectId) ) { setState((s) => ({ ...s, isLoading: true })); docStorage - .load(docId) + .load(savedObjectId) .then((doc) => updateDoc(doc)) .catch(() => { setState((s) => ({ ...s, isLoading: false })); @@ -248,8 +247,8 @@ export function App({ redirectTo(); }); } else if ( - embeddableEditorIncomingState?.byValueMode && - embeddableEditorIncomingState?.valueInput + !!embeddableEditorIncomingState?.valueInput && + embeddableEditorIncomingState?.valueInput !== 'createByValue' ) { const doc = { ...(embeddableEditorIncomingState?.valueInput as LensByValueInput).attributes, @@ -262,7 +261,7 @@ export function App({ core.notifications, data.indexPatterns, data.query.filterManager, - docId, + savedObjectId, // TODO: These dependencies are changing too often // docStorage, // redirectTo, @@ -327,12 +326,9 @@ export function App({ title: saveProps.newTitle, }; - const newlyCreated: boolean = - saveProps.newCopyOnSave || - !lastKnownDoc?.id || - (saveToLibrary && !!embeddableEditorIncomingState?.byValueMode); + const newlyCreated = saveProps.newCopyOnSave || !lastKnownDoc?.id || saveToLibrary; - if (embeddableEditorIncomingState?.byValueMode && !saveToLibrary) { + if (!!embeddableEditorIncomingState?.valueInput && !saveToLibrary) { redirectTo(doc.id, doc, saveProps.returnToOrigin, newlyCreated); } else { await checkForDuplicateTitle( @@ -353,9 +349,7 @@ export function App({ overlays: core.overlays, } ); - - const embeddableId = doc.id; - if (saveToLibrary && embeddableEditorIncomingState?.byValueMode) { + if (saveToLibrary && embeddableEditorIncomingState?.valueInput) { delete doc.id; } docStorage @@ -369,12 +363,8 @@ export function App({ persistedDoc: newDoc, lastKnownDoc: newDoc, })); - if (docId !== id || saveProps.returnToOrigin) { - const idToReplace = - saveToLibrary && embeddableEditorIncomingState?.byValueMode - ? embeddableId - : undefined; - redirectTo(id, undefined, saveProps.returnToOrigin, newlyCreated, idToReplace); + if (savedObjectId !== id || saveProps.returnToOrigin) { + redirectTo(id, undefined, saveProps.returnToOrigin, newlyCreated); } }) .catch((e) => { @@ -416,7 +406,7 @@ export function App({ runSave(props, true)} onClose={() => setState((s) => ({ ...s, isSaveModalVisible: false }))} documentInfo={{ - id: embeddableEditorIncomingState?.byValueMode ? undefined : lastKnownDoc.id, + id: embeddableEditorIncomingState?.valueInput ? undefined : lastKnownDoc.id, title: lastKnownDoc.title || '', description: lastKnownDoc.description || '', }} diff --git a/x-pack/plugins/lens/public/app_plugin/mounter.tsx b/x-pack/plugins/lens/public/app_plugin/mounter.tsx index 85c8060f209e55..7e6c6a6c2b5038 100644 --- a/x-pack/plugins/lens/public/app_plugin/mounter.tsx +++ b/x-pack/plugins/lens/public/app_plugin/mounter.tsx @@ -53,34 +53,33 @@ export async function mountApp( ); const redirectTo = ( routeProps: RouteComponentProps<{ id?: string }>, - id?: string, + savedObjectId?: string, documentByValue?: Document, returnToOrigin?: boolean, - newlyCreated?: boolean, - embeddableIdToReplace?: string + newlyCreated?: boolean ) => { - if (!id && !embeddableEditorIncomingState?.byValueMode) { + if (!savedObjectId && !embeddableEditorIncomingState?.valueInput) { routeProps.history.push('/'); } else if (!embeddableEditorIncomingState?.originatingApp) { - routeProps.history.push(`/edit/${id}`); + routeProps.history.push(`/edit/${savedObjectId}`); } else if (!!embeddableEditorIncomingState?.originatingApp && returnToOrigin) { - routeProps.history.push(`/edit/${id}`); - if (newlyCreated && id) { + routeProps.history.push(`/edit/${savedObjectId}`); + if (newlyCreated && savedObjectId) { stateTransfer.navigateToWithEmbeddablePackage( embeddableEditorIncomingState?.originatingApp, { - state: { id, type: LENS_EMBEDDABLE_TYPE, embeddableIdToReplace }, + state: { type: LENS_EMBEDDABLE_TYPE, input: { savedObjectId } }, } ); } else if (documentByValue) { - const { type, ...rest } = documentByValue; - const idToUse = id ? id : uuid.v4(); + const { type, id, ...rest } = documentByValue; stateTransfer.navigateToWithEmbeddablePackage( embeddableEditorIncomingState?.originatingApp, { state: { + id, type: LENS_EMBEDDABLE_TYPE, - input: { id: idToUse, attributes: rest }, + input: { attributes: rest }, }, } ); @@ -99,17 +98,10 @@ export async function mountApp( navigation={navigation} editorFrame={instance} storage={new Storage(localStorage)} - docId={routeProps.match.params.id} + savedObjectId={routeProps.match.params.id} docStorage={new SavedObjectIndexStore(savedObjectsClient)} - redirectTo={(id, documentByValue, returnToOrigin, newlyCreated, embeddableIdToReplace) => - redirectTo( - routeProps, - id, - documentByValue, - returnToOrigin, - newlyCreated, - embeddableIdToReplace - ) + redirectTo={(savedObjectId, documentByValue, returnToOrigin, newlyCreated) => + redirectTo(routeProps, savedObjectId, documentByValue, returnToOrigin, newlyCreated) } embeddableEditorIncomingState={embeddableEditorIncomingState} onAppLeave={params.onAppLeave} From 72a068a0c8869e5f6fe4bac4ba1d0030dd365022 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Tue, 14 Jul 2020 11:17:04 -0400 Subject: [PATCH 08/40] eslint changes --- .../dashboard/public/application/dashboard_app_controller.tsx | 2 -- src/plugins/embeddable/public/lib/state_transfer/types.ts | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx index 9385a2fc586a26..cd0f6b79d84caa 100644 --- a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx +++ b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx @@ -312,8 +312,6 @@ export class DashboardAppController { embeddablesMap[panel.panelIndex] = convertSavedDashboardPanelToPanelState(panel); }); - console.log('incomingEmbeddable', incomingEmbeddable); - // If the incoming embeddable state's id already exists in the embeddables map, replace the input, retaining the existing gridData for that panel. if (incomingEmbeddable?.id && embeddablesMap[incomingEmbeddable.id]) { const originalPanelState = { ...embeddablesMap[incomingEmbeddable.id] }; diff --git a/src/plugins/embeddable/public/lib/state_transfer/types.ts b/src/plugins/embeddable/public/lib/state_transfer/types.ts index f5e386681b425a..969859a5b3d8c5 100644 --- a/src/plugins/embeddable/public/lib/state_transfer/types.ts +++ b/src/plugins/embeddable/public/lib/state_transfer/types.ts @@ -17,8 +17,8 @@ * under the License. */ -import { EmbeddableInput, SavedObjectEmbeddableInput } from '..'; import { Optional } from '@kbn/utility-types'; +import { EmbeddableInput, SavedObjectEmbeddableInput } from '..'; /** * Represents a state package that contains the last active app id. From d1ab6837e739c31b374f3a90218802aac6cedfcb Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Wed, 15 Jul 2020 11:14:34 -0400 Subject: [PATCH 09/40] cleanup continued... --- .../public/lib/state_transfer/types.ts | 10 ++-- .../public/wizard/new_vis_modal.tsx | 2 +- x-pack/plugins/lens/kibana.json | 5 +- .../lens/public/app_plugin/app.test.tsx | 2 +- x-pack/plugins/lens/public/app_plugin/app.tsx | 38 ++++++------- .../lens/public/app_plugin/mounter.tsx | 19 ++----- .../editor_frame_service/editor_frame/save.ts | 2 +- .../editor_frame/state_management.test.ts | 2 +- .../editor_frame/state_management.ts | 2 +- .../embeddable/attribute_service.ts | 54 ------------------- .../embeddable/embeddable.tsx | 5 +- .../embeddable/embeddable_factory.ts | 29 ++++------ .../public/editor_frame_service/service.tsx | 3 +- .../persistence/saved_object_store.test.ts | 2 +- .../public/persistence/saved_object_store.ts | 16 +++--- 15 files changed, 58 insertions(+), 133 deletions(-) delete mode 100644 x-pack/plugins/lens/public/editor_frame_service/embeddable/attribute_service.ts diff --git a/src/plugins/embeddable/public/lib/state_transfer/types.ts b/src/plugins/embeddable/public/lib/state_transfer/types.ts index 969859a5b3d8c5..6a4cc6b53f9adf 100644 --- a/src/plugins/embeddable/public/lib/state_transfer/types.ts +++ b/src/plugins/embeddable/public/lib/state_transfer/types.ts @@ -21,12 +21,14 @@ import { Optional } from '@kbn/utility-types'; import { EmbeddableInput, SavedObjectEmbeddableInput } from '..'; /** - * Represents a state package that contains the last active app id. + * A state package that contains information an editor will need to create or edit an embeddable then redirect back. * @public */ export interface EmbeddableEditorState { originatingApp: string; - valueInput?: EmbeddableInput | 'createByValue'; + embeddableId?: string; + valueInput?: EmbeddableInput; + byValueMode?: boolean; } export function isEmbeddableEditorState(state: unknown): state is EmbeddableEditorState { @@ -34,13 +36,13 @@ export function isEmbeddableEditorState(state: unknown): state is EmbeddableEdit } /** - * Represents a state package that contains all fields necessary to create an embeddable by reference or by value in a container. + * A state package that contains all fields necessary to create or update an embeddable by reference or by value in a container. * @public */ export interface EmbeddablePackageState { - id?: string; type: string; input: Optional | Optional; + embeddableId?: string; } export function isEmbeddablePackageState(state: unknown): state is EmbeddablePackageState { diff --git a/src/plugins/visualizations/public/wizard/new_vis_modal.tsx b/src/plugins/visualizations/public/wizard/new_vis_modal.tsx index 305cec77981fab..420d283a3dba1c 100644 --- a/src/plugins/visualizations/public/wizard/new_vis_modal.tsx +++ b/src/plugins/visualizations/public/wizard/new_vis_modal.tsx @@ -177,7 +177,7 @@ class NewVisModal extends React.Component { act(() => onChange({ filterableIndexPatterns: [], - doc: { id: initialDocId, ...lastKnownDoc } as Document, + doc: { savedObjectId: initialDocId, ...lastKnownDoc } as Document, }) ); diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index 0bd1ae6617d6ea..fd47f4617df7f0 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -227,9 +227,8 @@ export function App({ useEffect(() => { if ( - !embeddableEditorIncomingState?.valueInput && savedObjectId && - (!state.persistedDoc || state.persistedDoc.id !== savedObjectId) + (!state.persistedDoc || state.persistedDoc.savedObjectId !== savedObjectId) ) { setState((s) => ({ ...s, isLoading: true })); docStorage @@ -246,13 +245,9 @@ export function App({ redirectTo(); }); - } else if ( - !!embeddableEditorIncomingState?.valueInput && - embeddableEditorIncomingState?.valueInput !== 'createByValue' - ) { - const doc = { + } else if (!!embeddableEditorIncomingState?.valueInput) { + const doc: Document = { ...(embeddableEditorIncomingState?.valueInput as LensByValueInput).attributes, - id: embeddableEditorIncomingState?.valueInput.id, }; updateDoc(doc); redirectTo(); @@ -298,8 +293,7 @@ export function App({ returnToOrigin: boolean; onTitleDuplicate?: OnSaveProps['onTitleDuplicate']; newDescription?: string; - }, - saveToLibrary: boolean = false + } ) => { if (!lastKnownDoc) { return; @@ -319,17 +313,17 @@ export function App({ } : lastKnownDoc; - const doc = { + const doc: Document = { ...lastDocWithoutPinned, description: saveProps.newDescription, - id: saveProps.newCopyOnSave ? undefined : lastKnownDoc.id, + savedObjectId: saveProps.newCopyOnSave ? undefined : lastKnownDoc.savedObjectId, title: saveProps.newTitle, }; - const newlyCreated = saveProps.newCopyOnSave || !lastKnownDoc?.id || saveToLibrary; + const newlyCreated = saveProps.newCopyOnSave || !lastKnownDoc?.savedObjectId; - if (!!embeddableEditorIncomingState?.valueInput && !saveToLibrary) { - redirectTo(doc.id, doc, saveProps.returnToOrigin, newlyCreated); + if (!!embeddableEditorIncomingState?.valueInput && !saveProps.newCopyOnSave) { + redirectTo(doc.savedObjectId, doc, saveProps.returnToOrigin, newlyCreated); } else { await checkForDuplicateTitle( { @@ -349,14 +343,11 @@ export function App({ overlays: core.overlays, } ); - if (saveToLibrary && embeddableEditorIncomingState?.valueInput) { - delete doc.id; - } docStorage .save(doc) .then(({ id }) => { // Prevents unnecessary network request and disables save button - const newDoc = { ...doc, id }; + const newDoc: Document = { ...doc, savedObjectId: id }; setState((s) => ({ ...s, isSaveModalVisible: false, @@ -405,7 +396,8 @@ export function App({
runSave(props, true)} onClose={() => setState((s) => ({ ...s, isSaveModalVisible: false }))} documentInfo={{ - id: embeddableEditorIncomingState?.valueInput ? undefined : lastKnownDoc.id, + id: embeddableEditorIncomingState?.valueInput + ? undefined + : lastKnownDoc.savedObjectId, title: lastKnownDoc.title || '', description: lastKnownDoc.description || '', }} diff --git a/x-pack/plugins/lens/public/app_plugin/mounter.tsx b/x-pack/plugins/lens/public/app_plugin/mounter.tsx index 7e6c6a6c2b5038..354d5069c6d786 100644 --- a/x-pack/plugins/lens/public/app_plugin/mounter.tsx +++ b/x-pack/plugins/lens/public/app_plugin/mounter.tsx @@ -39,9 +39,7 @@ export async function mountApp( ); const stateTransfer = embeddable?.getStateTransfer(params.history); - const embeddableEditorIncomingState = stateTransfer?.getIncomingEditorState({ - keysToRemoveAfterFetch: ['originatingApp'], - }); + const embeddableEditorIncomingState = stateTransfer?.getIncomingEditorState(); const instance = await createEditorFrame(); @@ -54,7 +52,7 @@ export async function mountApp( const redirectTo = ( routeProps: RouteComponentProps<{ id?: string }>, savedObjectId?: string, - documentByValue?: Document, + document?: Document, returnToOrigin?: boolean, newlyCreated?: boolean ) => { @@ -65,21 +63,13 @@ export async function mountApp( } else if (!!embeddableEditorIncomingState?.originatingApp && returnToOrigin) { routeProps.history.push(`/edit/${savedObjectId}`); if (newlyCreated && savedObjectId) { - stateTransfer.navigateToWithEmbeddablePackage( - embeddableEditorIncomingState?.originatingApp, - { - state: { type: LENS_EMBEDDABLE_TYPE, input: { savedObjectId } }, - } - ); - } else if (documentByValue) { - const { type, id, ...rest } = documentByValue; stateTransfer.navigateToWithEmbeddablePackage( embeddableEditorIncomingState?.originatingApp, { state: { - id, + embeddableId: embeddableEditorIncomingState.embeddableId, type: LENS_EMBEDDABLE_TYPE, - input: { attributes: rest }, + input: { savedObjectId }, }, } ); @@ -121,6 +111,7 @@ export async function mountApp( + diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/save.ts b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/save.ts index b41e93def966e0..5d53aa090d7d90 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/save.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/save.ts @@ -46,7 +46,7 @@ export function getSavedObjectFormat({ }); return { - id: state.persistedId, + savedObjectId: state.persistedId, title: state.title, description: state.description, type: 'lens', diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_management.test.ts b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_management.test.ts index 969467b5789ec0..7b96cdc27b09b4 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_management.test.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_management.test.ts @@ -379,7 +379,7 @@ describe('editor_frame state management', () => { { type: 'VISUALIZATION_LOADED', doc: { - id: 'b', + savedObjectId: 'b', expression: '', state: { datasourceMetaData: { filterableIndexPatterns: [] }, diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_management.ts b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_management.ts index 09674ebf2ade2c..fc8daaed059ddf 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_management.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_management.ts @@ -156,7 +156,7 @@ export const reducer = (state: EditorFrameState, action: Action): EditorFrameSta case 'VISUALIZATION_LOADED': return { ...state, - persistedId: action.doc.id, + persistedId: action.doc.savedObjectId, title: action.doc.title, description: action.doc.description, datasourceStates: Object.entries(action.doc.state.datasourceStates).reduce( diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/attribute_service.ts b/x-pack/plugins/lens/public/editor_frame_service/embeddable/attribute_service.ts deleted file mode 100644 index b49e05e8be5384..00000000000000 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/attribute_service.ts +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -import { - EmbeddableInput, - SavedObjectEmbeddableInput, - isSavedObjectEmbeddableInput, - IEmbeddable, -} from '../../../../../../src/plugins/embeddable/public'; -import { SavedObjectsClientContract, SimpleSavedObject } from '../../../../../../src/core/public'; - -export class AttributeService< - SavedObjectAttributes, - ValType extends EmbeddableInput & { attributes: SavedObjectAttributes }, - RefType extends SavedObjectEmbeddableInput -> { - constructor(private type: string, private savedObjectsClient: SavedObjectsClientContract) {} - - public async unwrapAttributes(input: RefType | ValType): Promise { - if (isSavedObjectEmbeddableInput(input)) { - const savedObject: SimpleSavedObject = await this.savedObjectsClient.get< - SavedObjectAttributes - >(this.type, input.savedObjectId); - return savedObject.attributes; - } - return input.attributes; - } - - public async wrapAttributes( - newAttributes: SavedObjectAttributes, - useRefType: boolean, - embeddable?: IEmbeddable - ): Promise> { - const savedObjectId = - embeddable && isSavedObjectEmbeddableInput(embeddable.getInput()) - ? (embeddable.getInput() as SavedObjectEmbeddableInput).savedObjectId - : undefined; - - if (useRefType) { - if (savedObjectId) { - await this.savedObjectsClient.update(this.type, savedObjectId, newAttributes); - return { savedObjectId } as RefType; - } else { - const savedItem = await this.savedObjectsClient.create(this.type, newAttributes); - return { savedObjectId: savedItem.id } as RefType; - } - } else { - return { attributes: newAttributes } as ValType; - } - } -} diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx index f4810935c6e5eb..13c547a8b709e4 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx @@ -29,12 +29,13 @@ import { EmbeddableOutput, IContainer, SavedObjectEmbeddableInput, + AttributeService, } from '../../../../../../src/plugins/embeddable/public'; import { DOC_TYPE, Document } from '../../persistence'; import { ExpressionWrapper } from './expression_wrapper'; import { UiActionsStart } from '../../../../../../src/plugins/ui_actions/public'; import { isLensBrushEvent, isLensFilterEvent } from '../../types'; -import { AttributeService } from './attribute_service'; + import { IndexPatternsContract } from '../../../../../../src/plugins/data/public'; import { getEditPath } from '../../../common'; import { IBasePath } from '../../../../../../src/core/public'; @@ -133,7 +134,7 @@ export class Embeddable extends AbstractEmbeddable 'lensApp', }; - private attributeService?: AttributeService< - LensSavedObjectAttributes, - LensByValueInput, - LensByReferenceInput - >; - constructor(private getStartServices: () => Promise) {} public isEditable = async () => { @@ -85,10 +81,15 @@ export class EmbeddableFactory implements EmbeddableFactoryDefinition { uiActions, coreHttp, indexPatternService, + embeddable, } = await this.getStartServices(); return new Embeddable( { - attributeService: await this.getAttributeService(), + attributeService: embeddable.getAttributeService< + LensSavedObjectAttributes, + LensByValueInput, + LensByReferenceInput + >(this.type), indexPatternService, timefilter, expressionRenderer, @@ -100,16 +101,4 @@ export class EmbeddableFactory implements EmbeddableFactoryDefinition { parent ); } - - private async getAttributeService() { - const savedObjectsService = (await this.getStartServices()).savedObjectsClient; - if (!this.attributeService) { - this.attributeService = new AttributeService< - LensSavedObjectAttributes, - LensByValueInput, - LensByReferenceInput - >(this.type, savedObjectsService); - } - return this.attributeService; - } } diff --git a/x-pack/plugins/lens/public/editor_frame_service/service.tsx b/x-pack/plugins/lens/public/editor_frame_service/service.tsx index 47339373b6d1ae..8a6196dadf1edb 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/service.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/service.tsx @@ -36,7 +36,7 @@ export interface EditorFrameSetupPlugins { export interface EditorFrameStartPlugins { data: DataPublicPluginStart; - embeddable?: EmbeddableStart; + embeddable: EmbeddableStart; expressions: ExpressionsStart; uiActions?: UiActionsStart; } @@ -76,6 +76,7 @@ export class EditorFrameService { expressionRenderer: deps.expressions.ReactExpressionRenderer, indexPatternService: deps.data.indexPatterns, uiActions: deps.uiActions, + embeddable: deps.embeddable, }; }; diff --git a/x-pack/plugins/lens/public/persistence/saved_object_store.test.ts b/x-pack/plugins/lens/public/persistence/saved_object_store.test.ts index f7caac65493892..db286477da3d03 100644 --- a/x-pack/plugins/lens/public/persistence/saved_object_store.test.ts +++ b/x-pack/plugins/lens/public/persistence/saved_object_store.test.ts @@ -81,7 +81,7 @@ describe('LensStore', () => { test('updates and returns a visualization document', async () => { const { client, store } = testStore(); const doc = await store.save({ - id: 'Gandalf', + savedObjectId: 'Gandalf', title: 'Even the very wise cannot see all ends.', visualizationType: 'line', expression: '', diff --git a/x-pack/plugins/lens/public/persistence/saved_object_store.ts b/x-pack/plugins/lens/public/persistence/saved_object_store.ts index 7632be3d820465..7f597375151ff8 100644 --- a/x-pack/plugins/lens/public/persistence/saved_object_store.ts +++ b/x-pack/plugins/lens/public/persistence/saved_object_store.ts @@ -9,7 +9,7 @@ import { SavedObjectAttributes } from 'kibana/server'; import { Query, Filter } from '../../../../../src/plugins/data/public'; export interface Document { - id?: string; + savedObjectId?: string; type?: string; visualizationType: string | null; title: string; @@ -47,7 +47,7 @@ export interface DocumentSaver { } export interface DocumentLoader { - load: (id: string) => Promise; + load: (savedObjectId: string) => Promise; } export type SavedObjectStore = DocumentLoader & DocumentSaver; @@ -60,19 +60,19 @@ export class SavedObjectIndexStore implements SavedObjectStore { } async save(vis: Document) { - const { id, type, ...rest } = vis; + const { savedObjectId, type, ...rest } = vis; // TODO: SavedObjectAttributes should support this kind of object, // remove this workaround when SavedObjectAttributes is updated. const attributes = (rest as unknown) as SavedObjectAttributes; - const result = await (id - ? this.client.update(DOC_TYPE, id, attributes) + const result = await (savedObjectId + ? this.client.update(DOC_TYPE, savedObjectId, attributes) : this.client.create(DOC_TYPE, attributes)); return { ...vis, id: result.id }; } - async load(id: string): Promise { - const { type, attributes, error } = await this.client.get(DOC_TYPE, id); + async load(savedObjectId: string): Promise { + const { type, attributes, error } = await this.client.get(DOC_TYPE, savedObjectId); if (error) { throw error; @@ -80,7 +80,7 @@ export class SavedObjectIndexStore implements SavedObjectStore { return { ...attributes, - id, + savedObjectId, type, } as Document; } From a0fffd073c5045bfc0ee74c35f721da3da407596 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Thu, 16 Jul 2020 20:21:37 -0400 Subject: [PATCH 10/40] Finished cleanup - separated embeddableId and savedObjectId in lens to avoid confusion. Added cancel button --- .../application/dashboard_app_controller.tsx | 14 ++-- .../public/lib/actions/edit_panel_action.ts | 1 + .../embeddable_state_transfer.test.ts | 25 +++++--- src/plugins/visualize/config.ts | 2 +- .../application/utils/get_top_nav_config.tsx | 2 +- x-pack/plugins/lens/common/constants.ts | 3 +- x-pack/plugins/lens/kibana.json | 5 +- .../lens/public/app_plugin/app.test.tsx | 52 ++++++++------- x-pack/plugins/lens/public/app_plugin/app.tsx | 64 +++++++++++++------ .../lens/public/app_plugin/mounter.tsx | 19 +++--- .../editor_frame/editor_frame.tsx | 1 - .../embeddable/embeddable.test.tsx | 3 +- .../embeddable/embeddable_factory.ts | 5 +- .../public/editor_frame_service/service.tsx | 2 +- x-pack/plugins/lens/public/plugin.ts | 2 +- 15 files changed, 114 insertions(+), 86 deletions(-) diff --git a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx index cd0f6b79d84caa..7f52495b4d10f3 100644 --- a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx +++ b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx @@ -300,7 +300,7 @@ export class DashboardAppController { return emptyScreenProps; }; - const incomingEmbeddable = embeddable + let incomingEmbeddable = embeddable .getStateTransfer(scopedHistory()) .getIncomingEmbeddablePackage({ keysToRemoveAfterFetch: ['id', 'type', 'input'] }); @@ -313,16 +313,17 @@ export class DashboardAppController { }); // If the incoming embeddable state's id already exists in the embeddables map, replace the input, retaining the existing gridData for that panel. - if (incomingEmbeddable?.id && embeddablesMap[incomingEmbeddable.id]) { - const originalPanelState = { ...embeddablesMap[incomingEmbeddable.id] }; - embeddablesMap[incomingEmbeddable.id] = { + if (incomingEmbeddable?.embeddableId && embeddablesMap[incomingEmbeddable.embeddableId]) { + const originalPanelState = { ...embeddablesMap[incomingEmbeddable.embeddableId] }; + embeddablesMap[incomingEmbeddable.embeddableId] = { gridData: originalPanelState.gridData, type: incomingEmbeddable.type, explicitInput: { ...incomingEmbeddable.input, - id: incomingEmbeddable.id, + id: incomingEmbeddable.embeddableId, }, }; + incomingEmbeddable = undefined; } let expandedPanelId; @@ -448,7 +449,8 @@ export class DashboardAppController { // If the incomingEmbeddable does not yet exist in the panels listing, create a new panel using the container's addEmbeddable method. if ( incomingEmbeddable && - (!incomingEmbeddable.id || !container.getInput().panels[incomingEmbeddable.id]) + (!incomingEmbeddable.embeddableId || + !container.getInput().panels[incomingEmbeddable.embeddableId]) ) { // TODO: Get rid of this, maybe by making the visualize embeddable also use the attributeService const explicitInput = diff --git a/src/plugins/embeddable/public/lib/actions/edit_panel_action.ts b/src/plugins/embeddable/public/lib/actions/edit_panel_action.ts index 40b63e7d2303a3..86c58a3c8a19ce 100644 --- a/src/plugins/embeddable/public/lib/actions/edit_panel_action.ts +++ b/src/plugins/embeddable/public/lib/actions/edit_panel_action.ts @@ -116,6 +116,7 @@ export class EditPanelAction implements Action { originatingApp: this.currentAppId, byValueMode, valueInput: byValueMode ? embeddable.getInput() : undefined, + embeddableId: embeddable.getInput().id, }; return { app, path, state }; } diff --git a/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.test.ts b/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.test.ts index 42adb9d770e8a5..c4d4b14a4fedb4 100644 --- a/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.test.ts +++ b/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.test.ts @@ -62,10 +62,10 @@ describe('embeddable state transfer', () => { it('can send an outgoing embeddable package state', async () => { await stateTransfer.navigateToWithEmbeddablePackage(destinationApp, { - state: { type: 'coolestType', id: '150' }, + state: { type: 'coolestType', input: { savedObjectId: '150' } }, }); expect(application.navigateToApp).toHaveBeenCalledWith('superUltraVisualize', { - state: { type: 'coolestType', id: '150' }, + state: { type: 'coolestType', input: { savedObjectId: '150' } }, }); }); @@ -73,12 +73,16 @@ describe('embeddable state transfer', () => { const historyMock = mockHistoryState({ kibanaIsNowForSports: 'extremeSportsKibana' }); stateTransfer = new EmbeddableStateTransfer(application.navigateToApp, historyMock); await stateTransfer.navigateToWithEmbeddablePackage(destinationApp, { - state: { type: 'coolestType', id: '150' }, + state: { type: 'coolestType', input: { savedObjectId: '150' } }, appendToExistingState: true, }); expect(application.navigateToApp).toHaveBeenCalledWith('superUltraVisualize', { path: undefined, - state: { kibanaIsNowForSports: 'extremeSportsKibana', type: 'coolestType', id: '150' }, + state: { + kibanaIsNowForSports: 'extremeSportsKibana', + type: 'coolestType', + input: { savedObjectId: '150' }, + }, }); }); @@ -97,10 +101,13 @@ describe('embeddable state transfer', () => { }); it('can fetch an incoming embeddable package state', async () => { - const historyMock = mockHistoryState({ type: 'skisEmbeddable', id: '123' }); + const historyMock = mockHistoryState({ + type: 'skisEmbeddable', + input: { savedObjectId: '123' }, + }); stateTransfer = new EmbeddableStateTransfer(application.navigateToApp, historyMock); const fetchedState = stateTransfer.getIncomingEmbeddablePackage(); - expect(fetchedState).toEqual({ type: 'skisEmbeddable', id: '123' }); + expect(fetchedState).toEqual({ type: 'skisEmbeddable', input: { savedObjectId: '123' } }); }); it('returns undefined when embeddable package is not in the right shape', async () => { @@ -113,12 +120,12 @@ describe('embeddable state transfer', () => { it('removes all keys in the keysToRemoveAfterFetch array', async () => { const historyMock = mockHistoryState({ type: 'skisEmbeddable', - id: '123', + input: { savedObjectId: '123' }, test1: 'test1', test2: 'test2', }); stateTransfer = new EmbeddableStateTransfer(application.navigateToApp, historyMock); - stateTransfer.getIncomingEmbeddablePackage({ keysToRemoveAfterFetch: ['type', 'id'] }); + stateTransfer.getIncomingEmbeddablePackage({ keysToRemoveAfterFetch: ['type', 'input'] }); expect(historyMock.replace).toHaveBeenCalledWith( expect.objectContaining({ state: { test1: 'test1', test2: 'test2' } }) ); @@ -127,7 +134,7 @@ describe('embeddable state transfer', () => { it('leaves state as is when no keysToRemove are supplied', async () => { const historyMock = mockHistoryState({ type: 'skisEmbeddable', - id: '123', + input: { savedObjectId: '123' }, test1: 'test1', test2: 'test2', }); diff --git a/src/plugins/visualize/config.ts b/src/plugins/visualize/config.ts index ee79a37717f266..6f01c8d1d5e8b4 100644 --- a/src/plugins/visualize/config.ts +++ b/src/plugins/visualize/config.ts @@ -20,7 +20,7 @@ import { schema, TypeOf } from '@kbn/config-schema'; export const configSchema = schema.object({ - showNewVisualizeFlow: schema.boolean({ defaultValue: false }), + showNewVisualizeFlow: schema.boolean({ defaultValue: true }), }); export type ConfigSchema = TypeOf; diff --git a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx index 96f64c6478fa97..0d7944f6812da4 100644 --- a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx +++ b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx @@ -106,7 +106,7 @@ export const getTopNavConfig = ( if (newlyCreated && embeddable) { embeddable.getStateTransfer().navigateToWithEmbeddablePackage(originatingApp, { - state: { id, type: VISUALIZE_EMBEDDABLE_TYPE }, + state: { type: VISUALIZE_EMBEDDABLE_TYPE, input: { savedObjectId: id } }, }); } else { application.navigateToApp(originatingApp); diff --git a/x-pack/plugins/lens/common/constants.ts b/x-pack/plugins/lens/common/constants.ts index 9433828fb248d9..bafdee74fa676a 100644 --- a/x-pack/plugins/lens/common/constants.ts +++ b/x-pack/plugins/lens/common/constants.ts @@ -8,11 +8,12 @@ export const PLUGIN_ID = 'lens'; export const LENS_EMBEDDABLE_TYPE = 'lens'; export const NOT_INTERNATIONALIZED_PRODUCT_NAME = 'Lens Visualizations'; export const BASE_API_URL = '/api/lens'; +export const LENS_EDIT_BY_VALUE = 'edit_by_value'; export function getBasePath() { return `#/`; } export function getEditPath(id: string | undefined) { - return id ? `#/edit/${encodeURIComponent(id)}` : '#/edit/value'; + return id ? `#/edit/${encodeURIComponent(id)}` : `#/${LENS_EDIT_BY_VALUE}`; } diff --git a/x-pack/plugins/lens/kibana.json b/x-pack/plugins/lens/kibana.json index 089fa3195a09c8..d7cd8d32293579 100644 --- a/x-pack/plugins/lens/kibana.json +++ b/x-pack/plugins/lens/kibana.json @@ -11,10 +11,9 @@ "kibanaLegacy", "visualizations", "dashboard", - "charts", - "embeddable" + "charts" ], - "optionalPlugins": ["usageCollection", "taskManager", "uiActions"], + "optionalPlugins": ["usageCollection", "taskManager", "uiActions", "embeddable"], "configPath": ["xpack", "lens"], "extraPublicDirs": ["common/constants"], "requiredBundles": ["savedObjects", "kibanaUtils", "kibanaReact", "embeddable"] diff --git a/x-pack/plugins/lens/public/app_plugin/app.test.tsx b/x-pack/plugins/lens/public/app_plugin/app.test.tsx index 2b38778efe1ee5..9b4bf514163cf3 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.test.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.test.tsx @@ -122,7 +122,7 @@ describe('Lens App', () => { navigation: typeof navigationStartMock; core: typeof core; storage: Storage; - docId?: string; + savedObjectId?: string; docStorage: SavedObjectStore; redirectTo: ( id?: string, @@ -171,11 +171,10 @@ describe('Lens App', () => { }, redirectTo: jest.fn( ( - id?: string, + savedObjectId?: string, documentByValue?: Document, returnToOrigin?: boolean, - newlyCreated?: boolean, - embeddableIdToReplace?: string + newlyCreated?: boolean ) => {} ), onAppLeave: jest.fn(), @@ -186,14 +185,13 @@ describe('Lens App', () => { data: typeof dataStartMock; core: typeof core; storage: Storage; - docId?: string; + savedObjectId?: string; docStorage: SavedObjectStore; redirectTo: ( id?: string, documentByValue?: Document, returnToOrigin?: boolean, - newlyCreated?: boolean, - embeddableIdToReplace?: string + newlyCreated?: boolean ) => void; originatingApp: string | undefined; onAppLeave: AppMountParameters['onAppLeave']; @@ -279,7 +277,7 @@ describe('Lens App', () => { }, }); await act(async () => { - instance.setProps({ docId: '1234' }); + instance.setProps({ savedObjectId: '1234' }); }); expect(defaultArgs.core.chrome.setBreadcrumbs).toHaveBeenCalledWith([ @@ -313,7 +311,7 @@ describe('Lens App', () => { instance = mount(); await act(async () => { - instance.setProps({ docId: '1234' }); + instance.setProps({ savedObjectId: '1234' }); }); expect(args.docStorage.load).toHaveBeenCalledWith('1234'); @@ -351,17 +349,17 @@ describe('Lens App', () => { instance = mount(); await act(async () => { - instance.setProps({ docId: '1234' }); + instance.setProps({ savedObjectId: '1234' }); }); await act(async () => { - instance.setProps({ docId: '1234' }); + instance.setProps({ savedObjectId: '1234' }); }); expect(args.docStorage.load).toHaveBeenCalledTimes(1); await act(async () => { - instance.setProps({ docId: '9876' }); + instance.setProps({ savedObjectId: '9876' }); }); expect(args.docStorage.load).toHaveBeenCalledTimes(2); @@ -375,7 +373,7 @@ describe('Lens App', () => { instance = mount(); await act(async () => { - instance.setProps({ docId: '1234' }); + instance.setProps({ savedObjectId: '1234' }); }); expect(args.docStorage.load).toHaveBeenCalledWith('1234'); @@ -432,7 +430,7 @@ describe('Lens App', () => { }) { const args = { ...defaultArgs, - docId: initialDocId, + savedObjectId: initialDocId, }; args.editorFrame = frame; (args.docStorage.load as jest.Mock).mockResolvedValue({ @@ -444,8 +442,8 @@ describe('Lens App', () => { filters: [], }, }); - (args.docStorage.save as jest.Mock).mockImplementation(async ({ id }) => ({ - id: id || 'aaa', + (args.docStorage.save as jest.Mock).mockImplementation(async ({ savedObjectId }) => ({ + id: savedObjectId || 'aaa', expression: 'kibana 2', })); @@ -537,9 +535,9 @@ describe('Lens App', () => { expression: 'kibana 3', }); - expect(args.redirectTo).toHaveBeenCalledWith('aaa', undefined, undefined, true, undefined); + expect(args.redirectTo).toHaveBeenCalledWith('aaa', undefined, undefined, false); - inst.setProps({ docId: 'aaa' }); + inst.setProps({ savedObjectId: 'aaa' }); expect(args.docStorage.load).not.toHaveBeenCalled(); }); @@ -557,9 +555,9 @@ describe('Lens App', () => { expression: 'kibana 3', }); - expect(args.redirectTo).toHaveBeenCalledWith('aaa', undefined, undefined, true, undefined); + expect(args.redirectTo).toHaveBeenCalledWith('aaa', undefined, undefined, true); - inst.setProps({ docId: 'aaa' }); + inst.setProps({ savedObjectId: 'aaa' }); expect(args.docStorage.load).toHaveBeenCalledTimes(1); }); @@ -572,14 +570,14 @@ describe('Lens App', () => { }); expect(args.docStorage.save).toHaveBeenCalledWith({ - id: '1234', + savedObjectId: '1234', title: 'hello there', expression: 'kibana 3', }); expect(args.redirectTo).not.toHaveBeenCalled(); - inst.setProps({ docId: '1234' }); + inst.setProps({ savedObjectId: '1234' }); expect(args.docStorage.load).toHaveBeenCalledTimes(1); }); @@ -625,7 +623,7 @@ describe('Lens App', () => { title: 'hello there', }); - expect(args.redirectTo).toHaveBeenCalledWith('aaa', undefined, true, true, undefined); + expect(args.redirectTo).toHaveBeenCalledWith('aaa', undefined, true, false); }); it('saves app filters and does not save pinned filters', async () => { @@ -653,7 +651,7 @@ describe('Lens App', () => { }); expect(args.docStorage.save).toHaveBeenCalledWith({ - id: '1234', + savedObjectId: '1234', title: 'hello there2', expression: 'kibana 3', state: { @@ -1108,7 +1106,7 @@ describe('Lens App', () => { defaultArgs.editorFrame = frame; instance = mount(); await act(async () => { - instance.setProps({ docId: '1234' }); + instance.setProps({ savedObjectId: '1234' }); }); const onChange = frame.mount.mock.calls[0][1].onChange; @@ -1132,7 +1130,7 @@ describe('Lens App', () => { defaultArgs.editorFrame = frame; instance = mount(); await act(async () => { - instance.setProps({ docId: '1234' }); + instance.setProps({ savedObjectId: '1234' }); }); const onChange = frame.mount.mock.calls[0][1].onChange; @@ -1156,7 +1154,7 @@ describe('Lens App', () => { defaultArgs.editorFrame = frame; instance = mount(); await act(async () => { - instance.setProps({ docId: '1234' }); + instance.setProps({ savedObjectId: '1234' }); }); const onChange = frame.mount.mock.calls[0][1].onChange; diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index fd47f4617df7f0..66532b28f2677b 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -44,6 +44,7 @@ import { LensByValueInput } from '../editor_frame_service/embeddable/embeddable' interface State { indicateNoData: boolean; isLoading: boolean; + byValueMode: boolean; isSaveModalVisible: boolean; indexPatternsForTopNav: IndexPatternInstance[]; persistedDoc?: Document; @@ -95,11 +96,19 @@ export function App({ storage.get('kibana.userQueryLanguage') || core.uiSettings.get(UI_SETTINGS.SEARCH_QUERY_LANGUAGE); + const editFromDashMode = + !!embeddableEditorIncomingState?.originatingApp && + (!!savedObjectId || !!embeddableEditorIncomingState?.valueInput); + const [state, setState] = useState(() => { const currentRange = data.query.timefilter.timefilter.getTime(); return { - isLoading: !!savedObjectId, + isLoading: !!savedObjectId || !!embeddableEditorIncomingState?.valueInput, isSaveModalVisible: false, + byValueMode: + !!embeddableEditorIncomingState?.originatingApp && + (!!embeddableEditorIncomingState?.valueInput || + !!embeddableEditorIncomingState?.byValueMode), indexPatternsForTopNav: [], query: { query: '', language }, dateRange: { @@ -182,7 +191,6 @@ export function App({ // Confirm when the user has made any changes to an existing doc // or when the user has configured something without saving if ( - !embeddableEditorIncomingState?.valueInput && core.application.capabilities.visualize.save && (state.persistedDoc?.expression ? !_.isEqual(lastKnownDoc?.expression, state.persistedDoc.expression) @@ -217,7 +225,7 @@ export function App({ }, { text: state.persistedDoc - ? embeddableEditorIncomingState?.valueInput + ? state.byValueMode ? i18n.translate('xpack.lens.breadcrumbsByValue', { defaultMessage: 'By Value' }) : state.persistedDoc.title : i18n.translate('xpack.lens.breadcrumbsCreate', { defaultMessage: 'Create' }), @@ -245,7 +253,7 @@ export function App({ redirectTo(); }); - } else if (!!embeddableEditorIncomingState?.valueInput) { + } else if (state.byValueMode && !!embeddableEditorIncomingState?.valueInput) { const doc: Document = { ...(embeddableEditorIncomingState?.valueInput as LensByValueInput).attributes, }; @@ -293,7 +301,8 @@ export function App({ returnToOrigin: boolean; onTitleDuplicate?: OnSaveProps['onTitleDuplicate']; newDescription?: string; - } + }, + saveToLibrary: boolean = false ) => { if (!lastKnownDoc) { return; @@ -320,10 +329,9 @@ export function App({ title: saveProps.newTitle, }; - const newlyCreated = saveProps.newCopyOnSave || !lastKnownDoc?.savedObjectId; - - if (!!embeddableEditorIncomingState?.valueInput && !saveProps.newCopyOnSave) { - redirectTo(doc.savedObjectId, doc, saveProps.returnToOrigin, newlyCreated); + if (state.byValueMode && !saveToLibrary) { + await setState((s: State) => ({ ...s, persistedDoc: doc })); + redirectTo(doc.savedObjectId, doc, saveProps.returnToOrigin, saveProps.newCopyOnSave); } else { await checkForDuplicateTitle( { @@ -348,14 +356,21 @@ export function App({ .then(({ id }) => { // Prevents unnecessary network request and disables save button const newDoc: Document = { ...doc, savedObjectId: id }; + const addedToLibrary = state.byValueMode && saveToLibrary; setState((s) => ({ ...s, + byValueMode: false, isSaveModalVisible: false, persistedDoc: newDoc, lastKnownDoc: newDoc, })); if (savedObjectId !== id || saveProps.returnToOrigin) { - redirectTo(id, undefined, saveProps.returnToOrigin, newlyCreated); + redirectTo( + id, + undefined, + saveProps.returnToOrigin, + saveProps.newCopyOnSave || addedToLibrary + ); } }) .catch((e) => { @@ -396,9 +411,7 @@ export function App({
{ if (isSaveable && lastKnownDoc) { setState((s) => ({ ...s, isSaveModalVisible: true })); @@ -441,6 +452,19 @@ export function App({ testId: 'lnsApp_saveButton', disableButton: !isSaveable, }, + ...(editFromDashMode || state.byValueMode + ? [ + { + label: i18n.translate('xpack.lens.app.cancel', { + defaultMessage: 'cancel', + }), + run: () => { + redirectTo(undefined, undefined, true, false); + }, + testId: 'lnsApp_saveAndReturnButton', + }, + ] + : []), ]} data-test-subj="lnsApp_topNav" screenTitle={'lens'} @@ -559,9 +583,7 @@ export function App({ onSave={(props) => runSave(props, true)} onClose={() => setState((s) => ({ ...s, isSaveModalVisible: false }))} documentInfo={{ - id: embeddableEditorIncomingState?.valueInput - ? undefined - : lastKnownDoc.savedObjectId, + id: state.byValueMode ? undefined : lastKnownDoc.savedObjectId, title: lastKnownDoc.title || '', description: lastKnownDoc.description || '', }} diff --git a/x-pack/plugins/lens/public/app_plugin/mounter.tsx b/x-pack/plugins/lens/public/app_plugin/mounter.tsx index 354d5069c6d786..a60d83c3442fb0 100644 --- a/x-pack/plugins/lens/public/app_plugin/mounter.tsx +++ b/x-pack/plugins/lens/public/app_plugin/mounter.tsx @@ -5,8 +5,6 @@ */ import React from 'react'; -import uuid from 'uuid'; - import { AppMountParameters, CoreSetup } from 'kibana/public'; import { FormattedMessage, I18nProvider } from '@kbn/i18n/react'; import { HashRouter, Route, RouteComponentProps, Switch } from 'react-router-dom'; @@ -22,7 +20,7 @@ import { EditorFrameStart } from '../types'; import { addHelpMenuToAppChrome } from '../help_menu_util'; import { Document, SavedObjectIndexStore } from '../persistence'; import { LensPluginStartDependencies } from '../plugin'; -import { LENS_EMBEDDABLE_TYPE } from '../../common'; +import { LENS_EMBEDDABLE_TYPE, LENS_EDIT_BY_VALUE } from '../../common'; export async function mountApp( core: CoreSetup, @@ -56,20 +54,23 @@ export async function mountApp( returnToOrigin?: boolean, newlyCreated?: boolean ) => { - if (!savedObjectId && !embeddableEditorIncomingState?.valueInput) { + if (!savedObjectId && !newlyCreated && !returnToOrigin) { routeProps.history.push('/'); - } else if (!embeddableEditorIncomingState?.originatingApp) { + } else if (savedObjectId && !embeddableEditorIncomingState?.originatingApp && !returnToOrigin) { routeProps.history.push(`/edit/${savedObjectId}`); } else if (!!embeddableEditorIncomingState?.originatingApp && returnToOrigin) { - routeProps.history.push(`/edit/${savedObjectId}`); - if (newlyCreated && savedObjectId) { + if (savedObjectId) { + routeProps.history.push(`/edit/${savedObjectId}`); + } + if (newlyCreated && stateTransfer) { + const input = savedObjectId ? { savedObjectId } : { attributes: document }; stateTransfer.navigateToWithEmbeddablePackage( embeddableEditorIncomingState?.originatingApp, { state: { embeddableId: embeddableEditorIncomingState.embeddableId, type: LENS_EMBEDDABLE_TYPE, - input: { savedObjectId }, + input, }, } ); @@ -111,7 +112,7 @@ export async function mountApp( - + diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.tsx index bcceb1222ce036..600adbdeb83ecd 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.tsx @@ -97,7 +97,6 @@ export function EditorFrame(props: EditorFrameProps) { .forEach((id) => { const datasourceState = state.datasourceStates[id].state; const datasource = props.datasourceMap[id]; - const layers = datasource.getLayers(datasourceState); layers.forEach((layer) => { datasourceLayers[layer] = props.datasourceMap[id].getPublicAPI({ diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx index 8a9d1890e8af05..a8dc8e3cf4a65f 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx @@ -23,9 +23,9 @@ import { import { Document } from '../../persistence'; import { dataPluginMock } from '../../../../../../src/plugins/data/public/mocks'; import { VIS_EVENT_TO_TRIGGER } from '../../../../../../src/plugins/visualizations/public/embeddable'; -import { AttributeService } from './attribute_service'; import { coreMock, httpServiceMock } from '../../../../../../src/core/public/mocks'; import { IBasePath } from '../../../../../../src/core/public'; +import { AttributeService } from '../../../../../../src/plugins/embeddable/public'; jest.mock('../../../../../../src/plugins/inspector/public/', () => ({ isAvailable: false, @@ -47,7 +47,6 @@ const savedVis: Document = { visualizationType: '', }; -// TODO: After #68719 is merged, use a mock from the embeddable plugin for this. const attributeServiceMockFromSavedVis = ( document: Document ): AttributeService => { diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts index 38ca59234107c8..073bfe3d57bc23 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts @@ -15,7 +15,6 @@ import { ReactExpressionRendererType } from '../../../../../../src/plugins/expre import { EmbeddableFactoryDefinition, IContainer, - AttributeService, EmbeddableStart, } from '../../../../../../src/plugins/embeddable/public'; import { @@ -35,7 +34,7 @@ interface StartServices { savedObjectsClient: SavedObjectsClientContract; expressionRenderer: ReactExpressionRendererType; indexPatternService: IndexPatternsContract; - embeddable: EmbeddableStart; + embeddable?: EmbeddableStart; uiActions?: UiActionsStart; } @@ -85,7 +84,7 @@ export class EmbeddableFactory implements EmbeddableFactoryDefinition { } = await this.getStartServices(); return new Embeddable( { - attributeService: embeddable.getAttributeService< + attributeService: embeddable!.getAttributeService< LensSavedObjectAttributes, LensByValueInput, LensByReferenceInput diff --git a/x-pack/plugins/lens/public/editor_frame_service/service.tsx b/x-pack/plugins/lens/public/editor_frame_service/service.tsx index 8a6196dadf1edb..fd3946c4fbe2ad 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/service.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/service.tsx @@ -36,7 +36,7 @@ export interface EditorFrameSetupPlugins { export interface EditorFrameStartPlugins { data: DataPublicPluginStart; - embeddable: EmbeddableStart; + embeddable?: EmbeddableStart; expressions: ExpressionsStart; uiActions?: UiActionsStart; } diff --git a/x-pack/plugins/lens/public/plugin.ts b/x-pack/plugins/lens/public/plugin.ts index a767f2a17fb690..b888c0cf2836a0 100644 --- a/x-pack/plugins/lens/public/plugin.ts +++ b/x-pack/plugins/lens/public/plugin.ts @@ -48,7 +48,7 @@ export interface LensPluginStartDependencies { expressions: ExpressionsStart; navigation: NavigationPublicPluginStart; uiActions: UiActionsStart; - embeddable: EmbeddableStart; + embeddable?: EmbeddableStart; } export class LensPlugin { From 9e5f7a947a981be8c31f9e4dbd7b90d599df5b3d Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Fri, 17 Jul 2020 13:28:19 -0400 Subject: [PATCH 11/40] Test fixes --- .../public/lib/actions/edit_panel_action.test.tsx | 8 +++++++- .../embeddable_state_transfer.test.ts | 2 +- src/plugins/visualize/config.ts | 2 +- x-pack/plugins/lens/public/app_plugin/app.tsx | 14 ++++++++++---- .../public/persistence/saved_object_store.test.ts | 4 ++-- .../lens/public/persistence/saved_object_store.ts | 4 ++-- 6 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx b/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx index 2cc0261ff08441..4a484efa6bcb73 100644 --- a/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx +++ b/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx @@ -67,6 +67,7 @@ test('redirects to app using state transfer with by value mode', async () => { state: { byValueMode: true, originatingApp: 'superCoolCurrentApp', + embeddableId: '123', valueInput: { id: '123', viewMode: ViewMode.EDIT, @@ -88,7 +89,12 @@ test('redirects to app using state transfer without by value mode', async () => await action.execute({ embeddable }); expect(stateTransferMock.navigateToEditor).toHaveBeenCalledWith('ultraVisualize', { path: '/123', - state: { originatingApp: 'superCoolCurrentApp', byValueMode: false, valueInput: undefined }, + state: { + originatingApp: 'superCoolCurrentApp', + byValueMode: false, + embeddableId: '123', + valueInput: undefined, + }, }); }); diff --git a/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.test.ts b/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.test.ts index c4d4b14a4fedb4..98dfef5cbca27b 100644 --- a/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.test.ts +++ b/src/plugins/embeddable/public/lib/state_transfer/embeddable_state_transfer.test.ts @@ -142,7 +142,7 @@ describe('embeddable state transfer', () => { stateTransfer.getIncomingEmbeddablePackage(); expect(historyMock.location.state).toEqual({ type: 'skisEmbeddable', - id: '123', + input: { savedObjectId: '123' }, test1: 'test1', test2: 'test2', }); diff --git a/src/plugins/visualize/config.ts b/src/plugins/visualize/config.ts index 6f01c8d1d5e8b4..ee79a37717f266 100644 --- a/src/plugins/visualize/config.ts +++ b/src/plugins/visualize/config.ts @@ -20,7 +20,7 @@ import { schema, TypeOf } from '@kbn/config-schema'; export const configSchema = schema.object({ - showNewVisualizeFlow: schema.boolean({ defaultValue: true }), + showNewVisualizeFlow: schema.boolean({ defaultValue: false }), }); export type ConfigSchema = TypeOf; diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index 66532b28f2677b..240ed385d012b9 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -322,6 +322,12 @@ export function App({ } : lastKnownDoc; + if (saveProps.newCopyOnSave) { + if (embeddableEditorIncomingState?.embeddableId) { + embeddableEditorIncomingState.embeddableId = undefined; + } + } + const doc: Document = { ...lastDocWithoutPinned, description: saveProps.newDescription, @@ -353,9 +359,9 @@ export function App({ ); docStorage .save(doc) - .then(({ id }) => { + .then(({ savedObjectId: newSavedObjectId }) => { // Prevents unnecessary network request and disables save button - const newDoc: Document = { ...doc, savedObjectId: id }; + const newDoc: Document = { ...doc, savedObjectId: newSavedObjectId }; const addedToLibrary = state.byValueMode && saveToLibrary; setState((s) => ({ ...s, @@ -364,9 +370,9 @@ export function App({ persistedDoc: newDoc, lastKnownDoc: newDoc, })); - if (savedObjectId !== id || saveProps.returnToOrigin) { + if (savedObjectId !== newSavedObjectId || saveProps.returnToOrigin) { redirectTo( - id, + newSavedObjectId, undefined, saveProps.returnToOrigin, saveProps.newCopyOnSave || addedToLibrary diff --git a/x-pack/plugins/lens/public/persistence/saved_object_store.test.ts b/x-pack/plugins/lens/public/persistence/saved_object_store.test.ts index db286477da3d03..5446e07b41b05e 100644 --- a/x-pack/plugins/lens/public/persistence/saved_object_store.test.ts +++ b/x-pack/plugins/lens/public/persistence/saved_object_store.test.ts @@ -42,7 +42,7 @@ describe('LensStore', () => { }); expect(doc).toEqual({ - id: 'FOO', + savedObjectId: 'FOO', title: 'Hello', description: 'My doc', visualizationType: 'bar', @@ -95,7 +95,7 @@ describe('LensStore', () => { }); expect(doc).toEqual({ - id: 'Gandalf', + savedObjectId: 'Gandalf', title: 'Even the very wise cannot see all ends.', visualizationType: 'line', expression: '', diff --git a/x-pack/plugins/lens/public/persistence/saved_object_store.ts b/x-pack/plugins/lens/public/persistence/saved_object_store.ts index 7f597375151ff8..5425b63f6c51c0 100644 --- a/x-pack/plugins/lens/public/persistence/saved_object_store.ts +++ b/x-pack/plugins/lens/public/persistence/saved_object_store.ts @@ -43,7 +43,7 @@ interface SavedObjectClient { } export interface DocumentSaver { - save: (vis: Document) => Promise<{ id: string }>; + save: (vis: Document) => Promise<{ savedObjectId: string }>; } export interface DocumentLoader { @@ -68,7 +68,7 @@ export class SavedObjectIndexStore implements SavedObjectStore { ? this.client.update(DOC_TYPE, savedObjectId, attributes) : this.client.create(DOC_TYPE, attributes)); - return { ...vis, id: result.id }; + return { ...vis, savedObjectId: result.id }; } async load(savedObjectId: string): Promise { From 580fbba7ee49d72cebaf6a10da41353c5f874cb6 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Fri, 17 Jul 2020 15:42:00 -0400 Subject: [PATCH 12/40] More test fixes, started on feature flag creation --- x-pack/plugins/lens/config.ts | 1 + .../plugins/lens/public/app_plugin/app.test.tsx | 6 +++++- x-pack/plugins/lens/public/app_plugin/app.tsx | 3 +++ x-pack/plugins/lens/public/app_plugin/mounter.tsx | 6 ++++-- x-pack/plugins/lens/public/index.ts | 5 ++++- x-pack/plugins/lens/public/plugin.ts | 15 ++++++++++++--- 6 files changed, 29 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/lens/config.ts b/x-pack/plugins/lens/config.ts index 84cf02a7ea541b..10e44f6cbd7ab7 100644 --- a/x-pack/plugins/lens/config.ts +++ b/x-pack/plugins/lens/config.ts @@ -8,6 +8,7 @@ import { schema, TypeOf } from '@kbn/config-schema'; export const configSchema = schema.object({ enabled: schema.boolean({ defaultValue: true }), + showNewLensFlow: schema.boolean({ defaultValue: true }), }); export type ConfigSchema = TypeOf; diff --git a/x-pack/plugins/lens/public/app_plugin/app.test.tsx b/x-pack/plugins/lens/public/app_plugin/app.test.tsx index 9b4bf514163cf3..48ccb19487430a 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.test.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.test.tsx @@ -32,6 +32,7 @@ const dataStartMock = dataPluginMock.createStartContract(); import { navigationPluginMock } from '../../../../../src/plugins/navigation/public/mocks'; import { TopNavMenuData } from '../../../../../src/plugins/navigation/public'; import { coreMock } from 'src/core/public/mocks'; +import { FeatureFlagConfig } from '../plugin'; jest.mock('../persistence'); jest.mock('src/core/public'); @@ -134,6 +135,7 @@ describe('Lens App', () => { originatingApp: string | undefined; onAppLeave: AppMountParameters['onAppLeave']; history: History; + featureFlagConfig: FeatureFlagConfig; }> { return ({ navigation: navigationStartMock, @@ -179,6 +181,7 @@ describe('Lens App', () => { ), onAppLeave: jest.fn(), history: createMemoryHistory(), + featureFlagConfig: { showNewLensFlow: true }, } as unknown) as jest.Mocked<{ navigation: typeof navigationStartMock; editorFrame: EditorFrameInstance; @@ -196,6 +199,7 @@ describe('Lens App', () => { originatingApp: string | undefined; onAppLeave: AppMountParameters['onAppLeave']; history: History; + featureFlagConfig: FeatureFlagConfig; }>; } @@ -443,7 +447,7 @@ describe('Lens App', () => { }, }); (args.docStorage.save as jest.Mock).mockImplementation(async ({ savedObjectId }) => ({ - id: savedObjectId || 'aaa', + savedObjectId: savedObjectId || 'aaa', expression: 'kibana 2', })); diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index 240ed385d012b9..608e0cc2edc85b 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -40,6 +40,7 @@ import { } from '../../../../../src/plugins/data/public'; import { EmbeddableEditorState } from '../../../../../src/plugins/embeddable/public'; import { LensByValueInput } from '../editor_frame_service/embeddable/embeddable'; +import { FeatureFlagConfig } from '../plugin'; interface State { indicateNoData: boolean; @@ -77,6 +78,7 @@ interface LensAppProps { embeddableEditorIncomingState?: EmbeddableEditorState; onAppLeave: AppMountParameters['onAppLeave']; history: History; + featureFlagConfig: FeatureFlagConfig; } export function App({ @@ -91,6 +93,7 @@ export function App({ navigation, onAppLeave, history, + featureFlagConfig, }: LensAppProps) { const language = storage.get('kibana.userQueryLanguage') || diff --git a/x-pack/plugins/lens/public/app_plugin/mounter.tsx b/x-pack/plugins/lens/public/app_plugin/mounter.tsx index a60d83c3442fb0..f5e9167055fd09 100644 --- a/x-pack/plugins/lens/public/app_plugin/mounter.tsx +++ b/x-pack/plugins/lens/public/app_plugin/mounter.tsx @@ -19,13 +19,14 @@ import { App } from './app'; import { EditorFrameStart } from '../types'; import { addHelpMenuToAppChrome } from '../help_menu_util'; import { Document, SavedObjectIndexStore } from '../persistence'; -import { LensPluginStartDependencies } from '../plugin'; +import { LensPluginStartDependencies, FeatureFlagConfig } from '../plugin'; import { LENS_EMBEDDABLE_TYPE, LENS_EDIT_BY_VALUE } from '../../common'; export async function mountApp( core: CoreSetup, params: AppMountParameters, - createEditorFrame: EditorFrameStart['createInstance'] + createEditorFrame: EditorFrameStart['createInstance'], + featureFlagConfig: FeatureFlagConfig ) { const [coreStart, startDependencies] = await core.getStartServices(); const { data: dataStart, navigation, embeddable } = startDependencies; @@ -91,6 +92,7 @@ export async function mountApp( storage={new Storage(localStorage)} savedObjectId={routeProps.match.params.id} docStorage={new SavedObjectIndexStore(savedObjectsClient)} + featureFlagConfig={featureFlagConfig} redirectTo={(savedObjectId, documentByValue, returnToOrigin, newlyCreated) => redirectTo(routeProps, savedObjectId, documentByValue, returnToOrigin, newlyCreated) } diff --git a/x-pack/plugins/lens/public/index.ts b/x-pack/plugins/lens/public/index.ts index e49f648906af0d..ef85c0cb97a717 100644 --- a/x-pack/plugins/lens/public/index.ts +++ b/x-pack/plugins/lens/public/index.ts @@ -4,8 +4,11 @@ * you may not use this file except in compliance with the Elastic License. */ +import { PluginInitializerContext } from 'kibana/public'; import { LensPlugin } from './plugin'; export * from './types'; -export const plugin = () => new LensPlugin(); +export const plugin = (context: PluginInitializerContext) => { + return new LensPlugin(context); +}; diff --git a/x-pack/plugins/lens/public/plugin.ts b/x-pack/plugins/lens/public/plugin.ts index b888c0cf2836a0..4e5891fe80cbd5 100644 --- a/x-pack/plugins/lens/public/plugin.ts +++ b/x-pack/plugins/lens/public/plugin.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { AppMountParameters, CoreSetup, CoreStart } from 'kibana/public'; +import { AppMountParameters, CoreSetup, CoreStart, PluginInitializerContext } from 'kibana/public'; import { DataPublicPluginSetup, DataPublicPluginStart } from 'src/plugins/data/public'; import { EmbeddableSetup, EmbeddableStart } from 'src/plugins/embeddable/public'; import { ExpressionsSetup, ExpressionsStart } from 'src/plugins/expressions/public'; @@ -51,6 +51,10 @@ export interface LensPluginStartDependencies { embeddable?: EmbeddableStart; } +export interface FeatureFlagConfig { + showNewLensFlow: boolean; +} + export class LensPlugin { private datatableVisualization: DatatableVisualization; private editorFrameService: EditorFrameService; @@ -60,7 +64,7 @@ export class LensPlugin { private metricVisualization: MetricVisualization; private pieVisualization: PieVisualization; - constructor() { + constructor(private initializerContext: PluginInitializerContext) { this.datatableVisualization = new DatatableVisualization(); this.editorFrameService = new EditorFrameService(); this.indexpatternDatasource = new IndexPatternDatasource(); @@ -112,7 +116,12 @@ export class LensPlugin { navLinkStatus: AppNavLinkStatus.hidden, mount: async (params: AppMountParameters) => { const { mountApp } = await import('./app_plugin/mounter'); - return mountApp(core, params, this.createEditorFrame!); + return mountApp( + core, + params, + this.createEditorFrame!, + this.initializerContext.config.get() + ); }, }); From df5fad07e28ccb9dd5c1c5895def5fad2fca237f Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Fri, 17 Jul 2020 18:00:02 -0400 Subject: [PATCH 13/40] Finished feature flag --- x-pack/plugins/lens/config.ts | 2 +- x-pack/plugins/lens/public/app_plugin/app.tsx | 17 ++++++++--------- x-pack/plugins/lens/server/index.ts | 3 +++ 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/lens/config.ts b/x-pack/plugins/lens/config.ts index 10e44f6cbd7ab7..567b70d35b24cf 100644 --- a/x-pack/plugins/lens/config.ts +++ b/x-pack/plugins/lens/config.ts @@ -8,7 +8,7 @@ import { schema, TypeOf } from '@kbn/config-schema'; export const configSchema = schema.object({ enabled: schema.boolean({ defaultValue: true }), - showNewLensFlow: schema.boolean({ defaultValue: true }), + showNewLensFlow: schema.boolean({ defaultValue: false }), }); export type ConfigSchema = TypeOf; diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index 608e0cc2edc85b..ce2e8899f205e3 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -109,6 +109,7 @@ export function App({ isLoading: !!savedObjectId || !!embeddableEditorIncomingState?.valueInput, isSaveModalVisible: false, byValueMode: + featureFlagConfig.showNewLensFlow && !!embeddableEditorIncomingState?.originatingApp && (!!embeddableEditorIncomingState?.valueInput || !!embeddableEditorIncomingState?.byValueMode), @@ -325,7 +326,7 @@ export function App({ } : lastKnownDoc; - if (saveProps.newCopyOnSave) { + if (saveProps.newCopyOnSave && !state.byValueMode) { if (embeddableEditorIncomingState?.embeddableId) { embeddableEditorIncomingState.embeddableId = undefined; } @@ -338,9 +339,13 @@ export function App({ title: saveProps.newTitle, }; + const addedToLibrary = state.byValueMode && saveToLibrary; + const newlyCreated = + saveProps.newCopyOnSave || addedToLibrary || (!savedObjectId && !state.byValueMode); + if (state.byValueMode && !saveToLibrary) { await setState((s: State) => ({ ...s, persistedDoc: doc })); - redirectTo(doc.savedObjectId, doc, saveProps.returnToOrigin, saveProps.newCopyOnSave); + redirectTo(doc.savedObjectId, doc, saveProps.returnToOrigin, newlyCreated); } else { await checkForDuplicateTitle( { @@ -365,7 +370,6 @@ export function App({ .then(({ savedObjectId: newSavedObjectId }) => { // Prevents unnecessary network request and disables save button const newDoc: Document = { ...doc, savedObjectId: newSavedObjectId }; - const addedToLibrary = state.byValueMode && saveToLibrary; setState((s) => ({ ...s, byValueMode: false, @@ -374,12 +378,7 @@ export function App({ lastKnownDoc: newDoc, })); if (savedObjectId !== newSavedObjectId || saveProps.returnToOrigin) { - redirectTo( - newSavedObjectId, - undefined, - saveProps.returnToOrigin, - saveProps.newCopyOnSave || addedToLibrary - ); + redirectTo(newSavedObjectId, undefined, saveProps.returnToOrigin, newlyCreated); } }) .catch((e) => { diff --git a/x-pack/plugins/lens/server/index.ts b/x-pack/plugins/lens/server/index.ts index 8aeeeab4539b6a..4d72ad9363689d 100644 --- a/x-pack/plugins/lens/server/index.ts +++ b/x-pack/plugins/lens/server/index.ts @@ -13,6 +13,9 @@ import { configSchema, ConfigSchema } from '../config'; export const config: PluginConfigDescriptor = { schema: configSchema, + exposeToBrowser: { + showNewLensFlow: true, + }, }; export const plugin = (initializerContext: PluginInitializerContext) => From cd61dfc7985e9d16c53d465f1aa093034891823d Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Mon, 20 Jul 2020 11:01:49 -0400 Subject: [PATCH 14/40] Updated jest test - because the newly created prop is now set correctly. --- x-pack/plugins/lens/public/app_plugin/app.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/lens/public/app_plugin/app.test.tsx b/x-pack/plugins/lens/public/app_plugin/app.test.tsx index 48ccb19487430a..95b35f6259ae23 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.test.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.test.tsx @@ -539,7 +539,7 @@ describe('Lens App', () => { expression: 'kibana 3', }); - expect(args.redirectTo).toHaveBeenCalledWith('aaa', undefined, undefined, false); + expect(args.redirectTo).toHaveBeenCalledWith('aaa', undefined, undefined, true); inst.setProps({ savedObjectId: 'aaa' }); @@ -627,7 +627,7 @@ describe('Lens App', () => { title: 'hello there', }); - expect(args.redirectTo).toHaveBeenCalledWith('aaa', undefined, true, false); + expect(args.redirectTo).toHaveBeenCalledWith('aaa', undefined, true, true); }); it('saves app filters and does not save pinned filters', async () => { From 7f86d230a52a9e94f5e51d9939e37d6b980a3428 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Wed, 22 Jul 2020 17:26:40 -0400 Subject: [PATCH 15/40] remove byValueMode from editorState in favor of using the embeddableId to determine if newly created or editing --- src/plugins/embeddable/public/lib/state_transfer/types.ts | 1 - .../public/embeddable/visualize_embeddable_factory.tsx | 1 - src/plugins/visualizations/public/wizard/new_vis_modal.tsx | 2 -- src/plugins/visualizations/public/wizard/show_new_vis.tsx | 2 -- x-pack/plugins/lens/public/app_plugin/app.tsx | 2 +- 5 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/plugins/embeddable/public/lib/state_transfer/types.ts b/src/plugins/embeddable/public/lib/state_transfer/types.ts index 6a4cc6b53f9adf..d8b4f4801bba3c 100644 --- a/src/plugins/embeddable/public/lib/state_transfer/types.ts +++ b/src/plugins/embeddable/public/lib/state_transfer/types.ts @@ -28,7 +28,6 @@ export interface EmbeddableEditorState { originatingApp: string; embeddableId?: string; valueInput?: EmbeddableInput; - byValueMode?: boolean; } export function isEmbeddableEditorState(state: unknown): state is EmbeddableEditorState { diff --git a/src/plugins/visualizations/public/embeddable/visualize_embeddable_factory.tsx b/src/plugins/visualizations/public/embeddable/visualize_embeddable_factory.tsx index e4bd2f7c55c12a..b81ff5c1661831 100644 --- a/src/plugins/visualizations/public/embeddable/visualize_embeddable_factory.tsx +++ b/src/plugins/visualizations/public/embeddable/visualize_embeddable_factory.tsx @@ -136,7 +136,6 @@ export class VisualizeEmbeddableFactory showNewVisModal({ originatingApp: await this.getCurrentAppId(), outsideVisualizeApp: true, - createByValue: true, }); return undefined; } diff --git a/src/plugins/visualizations/public/wizard/new_vis_modal.tsx b/src/plugins/visualizations/public/wizard/new_vis_modal.tsx index 420d283a3dba1c..a9bf6bd171f155 100644 --- a/src/plugins/visualizations/public/wizard/new_vis_modal.tsx +++ b/src/plugins/visualizations/public/wizard/new_vis_modal.tsx @@ -43,7 +43,6 @@ interface TypeSelectionProps { application: ApplicationStart; outsideVisualizeApp?: boolean; stateTransfer?: EmbeddableStateTransfer; - createByValue?: boolean; originatingApp?: string; } @@ -177,7 +176,6 @@ class NewVisModal extends React.Component Date: Wed, 22 Jul 2020 18:16:15 -0400 Subject: [PATCH 16/40] rename edit mode for clarity. Update ternary --- x-pack/plugins/lens/public/app_plugin/app.tsx | 12 ++++++------ .../editor_frame_service/embeddable/embeddable.tsx | 6 +----- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index 096ee103d65ea0..f2df094f2016dc 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -99,7 +99,7 @@ export function App({ storage.get('kibana.userQueryLanguage') || core.uiSettings.get(UI_SETTINGS.SEARCH_QUERY_LANGUAGE); - const editFromDashMode = + const editFromContainerMode = !!embeddableEditorIncomingState?.originatingApp && (!!savedObjectId || !!embeddableEditorIncomingState?.valueInput); @@ -419,7 +419,7 @@ export function App({
{ if (isSaveable && lastKnownDoc) { setState((s) => ({ ...s, isSaveModalVisible: true })); @@ -460,7 +460,7 @@ export function App({ testId: 'lnsApp_saveButton', disableButton: !isSaveable, }, - ...(editFromDashMode || state.byValueMode + ...(editFromContainerMode || state.byValueMode ? [ { label: i18n.translate('xpack.lens.app.cancel', { diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx index 6779956b1d4ce0..3c2c8e48c5c204 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx @@ -251,11 +251,7 @@ export class Embeddable extends AbstractEmbeddable Date: Thu, 23 Jul 2020 12:16:23 -0400 Subject: [PATCH 17/40] Type fixes --- src/plugins/embeddable/public/lib/actions/edit_panel_action.ts | 1 - x-pack/plugins/lens/config.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/plugins/embeddable/public/lib/actions/edit_panel_action.ts b/src/plugins/embeddable/public/lib/actions/edit_panel_action.ts index 86c58a3c8a19ce..7cf5eb0b5e835a 100644 --- a/src/plugins/embeddable/public/lib/actions/edit_panel_action.ts +++ b/src/plugins/embeddable/public/lib/actions/edit_panel_action.ts @@ -114,7 +114,6 @@ export class EditPanelAction implements Action { const byValueMode = !(embeddable.getInput() as SavedObjectEmbeddableInput).savedObjectId; const state: EmbeddableEditorState = { originatingApp: this.currentAppId, - byValueMode, valueInput: byValueMode ? embeddable.getInput() : undefined, embeddableId: embeddable.getInput().id, }; diff --git a/x-pack/plugins/lens/config.ts b/x-pack/plugins/lens/config.ts index 567b70d35b24cf..10e44f6cbd7ab7 100644 --- a/x-pack/plugins/lens/config.ts +++ b/x-pack/plugins/lens/config.ts @@ -8,7 +8,7 @@ import { schema, TypeOf } from '@kbn/config-schema'; export const configSchema = schema.object({ enabled: schema.boolean({ defaultValue: true }), - showNewLensFlow: schema.boolean({ defaultValue: false }), + showNewLensFlow: schema.boolean({ defaultValue: true }), }); export type ConfigSchema = TypeOf; From c8232171e49d4a14c3a4c89802d6c0d095d5723e Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Thu, 23 Jul 2020 14:41:28 -0400 Subject: [PATCH 18/40] Jest fix --- .../embeddable/public/lib/actions/edit_panel_action.test.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx b/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx index 4a484efa6bcb73..7ba8e086f28fd0 100644 --- a/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx +++ b/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx @@ -65,7 +65,6 @@ test('redirects to app using state transfer with by value mode', async () => { expect(stateTransferMock.navigateToEditor).toHaveBeenCalledWith('ultraVisualize', { path: '/123', state: { - byValueMode: true, originatingApp: 'superCoolCurrentApp', embeddableId: '123', valueInput: { @@ -91,7 +90,6 @@ test('redirects to app using state transfer without by value mode', async () => path: '/123', state: { originatingApp: 'superCoolCurrentApp', - byValueMode: false, embeddableId: '123', valueInput: undefined, }, From d3d2cfdd5a7040f10c5ddabbb77bc3603ffb1cbd Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Thu, 23 Jul 2020 17:23:51 -0400 Subject: [PATCH 19/40] merge fix --- x-pack/plugins/lens/public/app_plugin/app.tsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index d65206dee87fb5..88fb703d3b6fd5 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -110,14 +110,13 @@ export function App({ !!embeddableEditorIncomingState?.originatingApp && (!!embeddableEditorIncomingState?.valueInput || !embeddableEditorIncomingState?.embeddableId), - originatingApp: embeddableEditorIncomingState?.originatingApp, indexPatternsForTopNav: [], query: { query: '', language }, dateRange: { fromDate: currentRange.from, toDate: currentRange.to, }, - originatingApp, + originatingApp: embeddableEditorIncomingState?.originatingApp, filters: [], indicateNoData: false, }; @@ -344,7 +343,6 @@ export function App({ const addedToLibrary = state.byValueMode && saveToLibrary; const newlyCreated = saveProps.newCopyOnSave || addedToLibrary || (!savedObjectId && !state.byValueMode); - if (state.byValueMode && !saveToLibrary) { await setState((s: State) => ({ ...s, persistedDoc: doc })); redirectTo(doc.savedObjectId, doc, saveProps.returnToOrigin, newlyCreated); @@ -372,6 +370,7 @@ export function App({ .then(({ savedObjectId: newSavedObjectId }) => { // Prevents unnecessary network request and disables save button const newDoc: Document = { ...doc, savedObjectId: newSavedObjectId }; + const currentOriginatingApp = state.originatingApp; setState((s) => ({ ...s, byValueMode: false, @@ -594,7 +593,7 @@ export function App({ {lastKnownDoc && state.isSaveModalVisible && ( runSave(props)} + onSave={(props) => runSave(props, true)} onClose={() => setState((s) => ({ ...s, isSaveModalVisible: false }))} documentInfo={{ id: state.byValueMode ? undefined : lastKnownDoc.savedObjectId, From ee625eca5b1c6e486bea27530af18f05020b52f9 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Wed, 29 Jul 2020 13:26:01 -0400 Subject: [PATCH 20/40] merge fix --- .../public/application/dashboard_app_controller.tsx | 6 +----- x-pack/plugins/lens/public/app_plugin/app.tsx | 4 ---- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx index 461da37b0446ab..ce482eadebd0e1 100644 --- a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx +++ b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx @@ -168,7 +168,7 @@ export class DashboardAppController { chrome.docTitle.change(dash.title); } - const incomingEmbeddable = embeddable + let incomingEmbeddable = embeddable .getStateTransfer(scopedHistory()) .getIncomingEmbeddablePackage(); @@ -321,10 +321,6 @@ export class DashboardAppController { return emptyScreenProps; }; - let incomingEmbeddable = embeddable - .getStateTransfer(scopedHistory()) - .getIncomingEmbeddablePackage({ keysToRemoveAfterFetch: ['id', 'type', 'input'] }); - const getDashboardInput = (): DashboardContainerInput => { const embeddablesMap: { [key: string]: DashboardPanelState; diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index 2d60f991e21ab0..043881ecd8da74 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -95,10 +95,6 @@ export function App({ history, featureFlagConfig, }: LensAppProps) { - const language = - storage.get('kibana.userQueryLanguage') || - core.uiSettings.get(UI_SETTINGS.SEARCH_QUERY_LANGUAGE); - const [state, setState] = useState(() => { const currentRange = data.query.timefilter.timefilter.getTime(); return { From ed461d310fb49afb55f6d2d03e39e75ecbeb7777 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Thu, 30 Jul 2020 10:58:51 -0400 Subject: [PATCH 21/40] cleaned up visualize state transfer package, set new lens flow default to false --- .../public/application/dashboard_app_controller.tsx | 13 ++++--------- .../public/application/utils/get_top_nav_config.tsx | 4 ++-- x-pack/plugins/lens/config.ts | 2 +- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx index ce482eadebd0e1..03b9656052d500 100644 --- a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx +++ b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx @@ -475,15 +475,10 @@ export class DashboardAppController { (!incomingEmbeddable.embeddableId || !container.getInput().panels[incomingEmbeddable.embeddableId]) ) { - // TODO: Get rid of this, maybe by making the visualize embeddable also use the attributeService - const explicitInput = - incomingEmbeddable.type === 'lens' || - (incomingEmbeddable.input as SavedObjectEmbeddableInput).savedObjectId - ? incomingEmbeddable.input - : { - savedVis: incomingEmbeddable.input, - }; - container.addNewEmbeddable(incomingEmbeddable.type, explicitInput); + container.addNewEmbeddable( + incomingEmbeddable.type, + incomingEmbeddable.input + ); } } diff --git a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx index 0abbc8cbbddb63..4ddbe1e4303855 100644 --- a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx +++ b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx @@ -244,12 +244,12 @@ export const getTopNavConfig = ( if (!originatingApp) { return; } - const input = { + const visByValue = { ...vis.serialize(), id: uuid.v4(), }; embeddable.getStateTransfer().navigateToWithEmbeddablePackage(originatingApp, { - state: { input, type: VISUALIZE_EMBEDDABLE_TYPE }, + state: { input: { savedVis: visByValue }, type: VISUALIZE_EMBEDDABLE_TYPE }, }); }; diff --git a/x-pack/plugins/lens/config.ts b/x-pack/plugins/lens/config.ts index 10e44f6cbd7ab7..567b70d35b24cf 100644 --- a/x-pack/plugins/lens/config.ts +++ b/x-pack/plugins/lens/config.ts @@ -8,7 +8,7 @@ import { schema, TypeOf } from '@kbn/config-schema'; export const configSchema = schema.object({ enabled: schema.boolean({ defaultValue: true }), - showNewLensFlow: schema.boolean({ defaultValue: true }), + showNewLensFlow: schema.boolean({ defaultValue: false }), }); export type ConfigSchema = TypeOf; From c9878c525735c567c89034d579694ab7963603f9 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Thu, 30 Jul 2020 10:58:51 -0400 Subject: [PATCH 22/40] cleaned up visualize state transfer package, set new lens flow default to false --- .../application/dashboard_app_controller.tsx | 14 ++++---------- .../application/utils/get_top_nav_config.tsx | 4 ++-- x-pack/plugins/lens/config.ts | 2 +- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx index ce482eadebd0e1..ce2b1618622787 100644 --- a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx +++ b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx @@ -57,7 +57,6 @@ import { ViewMode, ContainerOutput, EmbeddableInput, - SavedObjectEmbeddableInput, } from '../../../embeddable/public'; import { NavAction, SavedDashboardPanel } from '../types'; @@ -475,15 +474,10 @@ export class DashboardAppController { (!incomingEmbeddable.embeddableId || !container.getInput().panels[incomingEmbeddable.embeddableId]) ) { - // TODO: Get rid of this, maybe by making the visualize embeddable also use the attributeService - const explicitInput = - incomingEmbeddable.type === 'lens' || - (incomingEmbeddable.input as SavedObjectEmbeddableInput).savedObjectId - ? incomingEmbeddable.input - : { - savedVis: incomingEmbeddable.input, - }; - container.addNewEmbeddable(incomingEmbeddable.type, explicitInput); + container.addNewEmbeddable( + incomingEmbeddable.type, + incomingEmbeddable.input + ); } } diff --git a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx index 0abbc8cbbddb63..4ddbe1e4303855 100644 --- a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx +++ b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx @@ -244,12 +244,12 @@ export const getTopNavConfig = ( if (!originatingApp) { return; } - const input = { + const visByValue = { ...vis.serialize(), id: uuid.v4(), }; embeddable.getStateTransfer().navigateToWithEmbeddablePackage(originatingApp, { - state: { input, type: VISUALIZE_EMBEDDABLE_TYPE }, + state: { input: { savedVis: visByValue }, type: VISUALIZE_EMBEDDABLE_TYPE }, }); }; diff --git a/x-pack/plugins/lens/config.ts b/x-pack/plugins/lens/config.ts index 10e44f6cbd7ab7..567b70d35b24cf 100644 --- a/x-pack/plugins/lens/config.ts +++ b/x-pack/plugins/lens/config.ts @@ -8,7 +8,7 @@ import { schema, TypeOf } from '@kbn/config-schema'; export const configSchema = schema.object({ enabled: schema.boolean({ defaultValue: true }), - showNewLensFlow: schema.boolean({ defaultValue: true }), + showNewLensFlow: schema.boolean({ defaultValue: false }), }); export type ConfigSchema = TypeOf; From a5910f2563c4e2cd9d397027a70bbc739ad7ca46 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Fri, 31 Jul 2020 16:17:51 -0400 Subject: [PATCH 23/40] Added an interface for all by value or by reference embeddables to inherit from. This will be used for the add to library and unlink from library actions. Cached the attribute service --- src/plugins/embeddable/public/index.ts | 2 + .../public/lib/embeddables/index.ts | 2 +- .../attribute_service.ts | 6 +- .../ref_or_val_embeddable/index.ts | 21 +++++++ .../ref_or_val_embeddable/types.ts | 39 +++++++++++++ src/plugins/embeddable/public/plugin.tsx | 2 +- .../embeddable/embeddable.tsx | 58 +++++++++++++++---- .../embeddable/embeddable_factory.ts | 20 +++++-- 8 files changed, 128 insertions(+), 22 deletions(-) rename src/plugins/embeddable/public/lib/embeddables/{ => ref_or_val_embeddable}/attribute_service.ts (93%) create mode 100644 src/plugins/embeddable/public/lib/embeddables/ref_or_val_embeddable/index.ts create mode 100644 src/plugins/embeddable/public/lib/embeddables/ref_or_val_embeddable/types.ts diff --git a/src/plugins/embeddable/public/index.ts b/src/plugins/embeddable/public/index.ts index fafbdda148de87..c416ef61e13579 100644 --- a/src/plugins/embeddable/public/index.ts +++ b/src/plugins/embeddable/public/index.ts @@ -75,6 +75,8 @@ export { EmbeddablePackageState, EmbeddableRenderer, EmbeddableRendererProps, + ReferenceOrValueEmbeddable, + isReferenceOrValueEmbeddable, } from './lib'; export function plugin(initializerContext: PluginInitializerContext) { diff --git a/src/plugins/embeddable/public/lib/embeddables/index.ts b/src/plugins/embeddable/public/lib/embeddables/index.ts index 06cb6e322acf39..beefd73ebeb3b9 100644 --- a/src/plugins/embeddable/public/lib/embeddables/index.ts +++ b/src/plugins/embeddable/public/lib/embeddables/index.ts @@ -25,5 +25,5 @@ export { ErrorEmbeddable, isErrorEmbeddable } from './error_embeddable'; export { withEmbeddableSubscription } from './with_subscription'; export { EmbeddableRoot } from './embeddable_root'; export * from './saved_object_embeddable'; -export { AttributeService } from './attribute_service'; +export * from './ref_or_val_embeddable'; export { EmbeddableRenderer, EmbeddableRendererProps } from './embeddable_renderer'; diff --git a/src/plugins/embeddable/public/lib/embeddables/attribute_service.ts b/src/plugins/embeddable/public/lib/embeddables/ref_or_val_embeddable/attribute_service.ts similarity index 93% rename from src/plugins/embeddable/public/lib/embeddables/attribute_service.ts rename to src/plugins/embeddable/public/lib/embeddables/ref_or_val_embeddable/attribute_service.ts index a33f592350d9a8..91a1d1a627aabe 100644 --- a/src/plugins/embeddable/public/lib/embeddables/attribute_service.ts +++ b/src/plugins/embeddable/public/lib/embeddables/ref_or_val_embeddable/attribute_service.ts @@ -17,14 +17,14 @@ * under the License. */ -import { SavedObjectsClientContract } from '../../../../../core/public'; +import { SavedObjectsClientContract } from '../../../../../../core/public'; import { SavedObjectEmbeddableInput, isSavedObjectEmbeddableInput, EmbeddableInput, IEmbeddable, -} from '.'; -import { SimpleSavedObject } from '../../../../../core/public'; +} from '../'; +import { SimpleSavedObject } from '../../../../../../core/public'; export class AttributeService< SavedObjectAttributes, diff --git a/src/plugins/embeddable/public/lib/embeddables/ref_or_val_embeddable/index.ts b/src/plugins/embeddable/public/lib/embeddables/ref_or_val_embeddable/index.ts new file mode 100644 index 00000000000000..8ebb8664d6d0ea --- /dev/null +++ b/src/plugins/embeddable/public/lib/embeddables/ref_or_val_embeddable/index.ts @@ -0,0 +1,21 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +export { ReferenceOrValueEmbeddable, isReferenceOrValueEmbeddable } from './types'; +export { AttributeService } from './attribute_service'; diff --git a/src/plugins/embeddable/public/lib/embeddables/ref_or_val_embeddable/types.ts b/src/plugins/embeddable/public/lib/embeddables/ref_or_val_embeddable/types.ts new file mode 100644 index 00000000000000..7518e04759b14a --- /dev/null +++ b/src/plugins/embeddable/public/lib/embeddables/ref_or_val_embeddable/types.ts @@ -0,0 +1,39 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { EmbeddableInput, SavedObjectEmbeddableInput } from '..'; + +export interface ReferenceOrValueEmbeddable< + ValTypeInput extends EmbeddableInput = EmbeddableInput, + RefTypeInput extends SavedObjectEmbeddableInput = SavedObjectEmbeddableInput +> { + inputIsRefType: (input: ValTypeInput | RefTypeInput) => input is RefTypeInput; + getInputAsValueType: () => Promise; + getInputAsRefType: () => Promise; +} + +export function isReferenceOrValueEmbeddable( + incoming: unknown +): incoming is ReferenceOrValueEmbeddable { + return ( + !!(incoming as ReferenceOrValueEmbeddable).inputIsRefType && + !!(incoming as ReferenceOrValueEmbeddable).getInputAsValueType && + !!(incoming as ReferenceOrValueEmbeddable).getInputAsRefType + ); +} diff --git a/src/plugins/embeddable/public/plugin.tsx b/src/plugins/embeddable/public/plugin.tsx index 508c82c4247eda..20352e5c73342f 100644 --- a/src/plugins/embeddable/public/plugin.tsx +++ b/src/plugins/embeddable/public/plugin.tsx @@ -47,9 +47,9 @@ import { ChartActionContext, isRangeSelectTriggerContext, isValueClickTriggerContext, + AttributeService, } from './lib'; import { EmbeddableFactoryDefinition } from './lib/embeddables/embeddable_factory_definition'; -import { AttributeService } from './lib/embeddables/attribute_service'; import { EmbeddableStateTransfer } from './lib/state_transfer'; export interface EmbeddableSetupDependencies { diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx index 3c2c8e48c5c204..c188b4945a861f 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx @@ -30,6 +30,7 @@ import { IContainer, SavedObjectEmbeddableInput, AttributeService, + ReferenceOrValueEmbeddable, } from '../../../../../../src/plugins/embeddable/public'; import { DOC_TYPE, Document } from '../../persistence'; import { ExpressionWrapper } from './expression_wrapper'; @@ -71,7 +72,8 @@ export interface LensEmbeddableDeps { getTrigger?: UiActionsStart['getTrigger'] | undefined; } -export class Embeddable extends AbstractEmbeddable { +export class Embeddable extends AbstractEmbeddable + implements ReferenceOrValueEmbeddable { type = DOC_TYPE; private expressionRenderer: ReactExpressionRendererType; @@ -201,17 +203,6 @@ export class Embeddable extends AbstractEmbeddable { + return Boolean((input as LensByReferenceInput).savedObjectId); + }; + + public getInputAsRefType = async (): Promise => { + if (this.inputIsRefType(this.input)) { + return this.input; + } + const wrappedInput = (await this.deps.attributeService.wrapAttributes( + this.input.attributes, + true + )) as Pick; + return { + id: this.input.id, + ...wrappedInput, + }; + }; + + public getInputAsValueType = async (): Promise => { + if (!this.inputIsRefType(this.input)) { + return this.input; + } + const attributes = await this.deps.attributeService.unwrapAttributes(this.input); + return { + ...this.input, + savedObjectId: undefined, + attributes, + }; + }; + + destroy() { + super.destroy(); + if (this.domNode) { + unmountComponentAtNode(this.domNode); + } + if (this.subscription) { + this.subscription.unsubscribe(); + } + this.autoRefreshFetchSubscription.unsubscribe(); + } } diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts index 073bfe3d57bc23..99413f4fc938c8 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts @@ -16,6 +16,7 @@ import { EmbeddableFactoryDefinition, IContainer, EmbeddableStart, + AttributeService, } from '../../../../../../src/plugins/embeddable/public'; import { Embeddable, @@ -48,6 +49,12 @@ export class EmbeddableFactory implements EmbeddableFactoryDefinition { getIconForSavedObject: () => 'lensApp', }; + attributeService?: AttributeService< + LensSavedObjectAttributes, + LensByValueInput, + LensByReferenceInput + >; + constructor(private getStartServices: () => Promise) {} public isEditable = async () => { @@ -82,13 +89,16 @@ export class EmbeddableFactory implements EmbeddableFactoryDefinition { indexPatternService, embeddable, } = await this.getStartServices(); + if (!this.attributeService) { + this.attributeService = embeddable!.getAttributeService< + LensSavedObjectAttributes, + LensByValueInput, + LensByReferenceInput + >(this.type); + } return new Embeddable( { - attributeService: embeddable!.getAttributeService< - LensSavedObjectAttributes, - LensByValueInput, - LensByReferenceInput - >(this.type), + attributeService: this.attributeService!, indexPatternService, timefilter, expressionRenderer, From c1435e03e321a38a3861f0d9e4c5a63d8ebc9e7b Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Tue, 4 Aug 2020 16:29:39 -0400 Subject: [PATCH 24/40] Removed lens by value config in favor of the dashboard-wide config from #73870 --- src/plugins/dashboard/public/index.ts | 2 +- src/plugins/dashboard/public/plugin.tsx | 2 +- x-pack/plugins/lens/config.ts | 1 - .../lens/public/app_plugin/app.test.tsx | 6 ++--- x-pack/plugins/lens/public/app_plugin/app.tsx | 6 ++--- .../lens/public/app_plugin/mounter.tsx | 6 +++-- x-pack/plugins/lens/public/plugin.ts | 23 ++++++++----------- x-pack/plugins/lens/server/index.ts | 3 --- 8 files changed, 22 insertions(+), 27 deletions(-) diff --git a/src/plugins/dashboard/public/index.ts b/src/plugins/dashboard/public/index.ts index dcfde67cd9f131..098fe26742dd98 100644 --- a/src/plugins/dashboard/public/index.ts +++ b/src/plugins/dashboard/public/index.ts @@ -31,7 +31,7 @@ export { } from './application'; export { DashboardConstants, createDashboardEditUrl } from './dashboard_constants'; -export { DashboardStart, DashboardUrlGenerator } from './plugin'; +export { DashboardStart, DashboardUrlGenerator, DashboardFeatureFlagConfig } from './plugin'; export { DASHBOARD_APP_URL_GENERATOR, createDashboardUrlGenerator, diff --git a/src/plugins/dashboard/public/plugin.tsx b/src/plugins/dashboard/public/plugin.tsx index f1319665d258b4..163ef36d30af83 100644 --- a/src/plugins/dashboard/public/plugin.tsx +++ b/src/plugins/dashboard/public/plugin.tsx @@ -94,7 +94,7 @@ declare module '../../share/public' { export type DashboardUrlGenerator = UrlGeneratorContract; -interface DashboardFeatureFlagConfig { +export interface DashboardFeatureFlagConfig { allowByValueEmbeddables: boolean; } diff --git a/x-pack/plugins/lens/config.ts b/x-pack/plugins/lens/config.ts index 567b70d35b24cf..84cf02a7ea541b 100644 --- a/x-pack/plugins/lens/config.ts +++ b/x-pack/plugins/lens/config.ts @@ -8,7 +8,6 @@ import { schema, TypeOf } from '@kbn/config-schema'; export const configSchema = schema.object({ enabled: schema.boolean({ defaultValue: true }), - showNewLensFlow: schema.boolean({ defaultValue: false }), }); export type ConfigSchema = TypeOf; diff --git a/x-pack/plugins/lens/public/app_plugin/app.test.tsx b/x-pack/plugins/lens/public/app_plugin/app.test.tsx index dc66235383aead..98659803f90d45 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.test.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.test.tsx @@ -32,7 +32,7 @@ const dataStartMock = dataPluginMock.createStartContract(); import { navigationPluginMock } from '../../../../../src/plugins/navigation/public/mocks'; import { TopNavMenuData } from '../../../../../src/plugins/navigation/public'; import { coreMock } from 'src/core/public/mocks'; -import { FeatureFlagConfig } from '../plugin'; +import { DashboardFeatureFlagConfig } from 'src/plugins/dashboard/public'; jest.mock('../persistence'); jest.mock('src/core/public'); @@ -143,7 +143,7 @@ describe('Lens App', () => { originatingApp: string | undefined; onAppLeave: AppMountParameters['onAppLeave']; history: History; - featureFlagConfig: FeatureFlagConfig; + featureFlagConfig: DashboardFeatureFlagConfig; }> { return ({ navigation: navigationStartMock, @@ -208,7 +208,7 @@ describe('Lens App', () => { originatingApp: string | undefined; onAppLeave: AppMountParameters['onAppLeave']; history: History; - featureFlagConfig: FeatureFlagConfig; + featureFlagConfig: DashboardFeatureFlagConfig; }>; } diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index d001375fc6f221..4331cebc1dda80 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -11,6 +11,7 @@ import { i18n } from '@kbn/i18n'; import { NavigationPublicPluginStart } from 'src/plugins/navigation/public'; import { AppMountContext, AppMountParameters, NotificationsStart } from 'kibana/public'; import { History } from 'history'; +import { DashboardFeatureFlagConfig } from 'src/plugins/dashboard/public'; import { Query, DataPublicPluginStart, @@ -39,7 +40,6 @@ import { } from '../../../../../src/plugins/data/public'; import { EmbeddableEditorState } from '../../../../../src/plugins/embeddable/public'; import { LensByValueInput } from '../editor_frame_service/embeddable/embeddable'; -import { FeatureFlagConfig } from '../plugin'; interface State { indicateNoData: boolean; @@ -78,7 +78,7 @@ interface LensAppProps { embeddableEditorIncomingState?: EmbeddableEditorState; onAppLeave: AppMountParameters['onAppLeave']; history: History; - featureFlagConfig: FeatureFlagConfig; + featureFlagConfig: DashboardFeatureFlagConfig; } export function App({ @@ -101,7 +101,7 @@ export function App({ isLoading: !!savedObjectId || !!embeddableEditorIncomingState?.valueInput, isSaveModalVisible: false, byValueMode: - featureFlagConfig.showNewLensFlow && + featureFlagConfig.allowByValueEmbeddables && !!embeddableEditorIncomingState?.originatingApp && (!!embeddableEditorIncomingState?.valueInput || !embeddableEditorIncomingState?.embeddableId), diff --git a/x-pack/plugins/lens/public/app_plugin/mounter.tsx b/x-pack/plugins/lens/public/app_plugin/mounter.tsx index f5e9167055fd09..1fecaf5da7586a 100644 --- a/x-pack/plugins/lens/public/app_plugin/mounter.tsx +++ b/x-pack/plugins/lens/public/app_plugin/mounter.tsx @@ -11,6 +11,7 @@ import { HashRouter, Route, RouteComponentProps, Switch } from 'react-router-dom import { render, unmountComponentAtNode } from 'react-dom'; import { i18n } from '@kbn/i18n'; +import { DashboardFeatureFlagConfig } from 'src/plugins/dashboard/public'; import { Storage } from '../../../../../src/plugins/kibana_utils/public'; import { LensReportManager, setReportManager, trackUiEvent } from '../lens_ui_telemetry'; @@ -19,14 +20,14 @@ import { App } from './app'; import { EditorFrameStart } from '../types'; import { addHelpMenuToAppChrome } from '../help_menu_util'; import { Document, SavedObjectIndexStore } from '../persistence'; -import { LensPluginStartDependencies, FeatureFlagConfig } from '../plugin'; +import { LensPluginStartDependencies } from '../plugin'; import { LENS_EMBEDDABLE_TYPE, LENS_EDIT_BY_VALUE } from '../../common'; export async function mountApp( core: CoreSetup, params: AppMountParameters, createEditorFrame: EditorFrameStart['createInstance'], - featureFlagConfig: FeatureFlagConfig + getByValueFeatureFlag: () => Promise ) { const [coreStart, startDependencies] = await core.getStartServices(); const { data: dataStart, navigation, embeddable } = startDependencies; @@ -81,6 +82,7 @@ export async function mountApp( } }; + const featureFlagConfig = await getByValueFeatureFlag(); const renderEditor = (routeProps: RouteComponentProps<{ id?: string }>) => { trackUiEvent('loaded'); return ( diff --git a/x-pack/plugins/lens/public/plugin.ts b/x-pack/plugins/lens/public/plugin.ts index 4e5891fe80cbd5..3f2d2c0c9ec5c3 100644 --- a/x-pack/plugins/lens/public/plugin.ts +++ b/x-pack/plugins/lens/public/plugin.ts @@ -4,9 +4,10 @@ * you may not use this file except in compliance with the Elastic License. */ -import { AppMountParameters, CoreSetup, CoreStart, PluginInitializerContext } from 'kibana/public'; +import { AppMountParameters, CoreSetup, CoreStart } from 'kibana/public'; import { DataPublicPluginSetup, DataPublicPluginStart } from 'src/plugins/data/public'; import { EmbeddableSetup, EmbeddableStart } from 'src/plugins/embeddable/public'; +import { DashboardStart } from 'src/plugins/dashboard/public'; import { ExpressionsSetup, ExpressionsStart } from 'src/plugins/expressions/public'; import { VisualizationsSetup } from 'src/plugins/visualizations/public'; import { NavigationPublicPluginStart } from 'src/plugins/navigation/public'; @@ -48,13 +49,9 @@ export interface LensPluginStartDependencies { expressions: ExpressionsStart; navigation: NavigationPublicPluginStart; uiActions: UiActionsStart; + dashboard: DashboardStart; embeddable?: EmbeddableStart; } - -export interface FeatureFlagConfig { - showNewLensFlow: boolean; -} - export class LensPlugin { private datatableVisualization: DatatableVisualization; private editorFrameService: EditorFrameService; @@ -64,7 +61,7 @@ export class LensPlugin { private metricVisualization: MetricVisualization; private pieVisualization: PieVisualization; - constructor(private initializerContext: PluginInitializerContext) { + constructor() { this.datatableVisualization = new DatatableVisualization(); this.editorFrameService = new EditorFrameService(); this.indexpatternDatasource = new IndexPatternDatasource(); @@ -110,18 +107,18 @@ export class LensPlugin { visualizations.registerAlias(getLensAliasConfig()); + const getByValueFeatureFlag = async () => { + const [, deps] = await core.getStartServices(); + return deps.dashboard.dashboardFeatureFlagConfig; + }; + core.application.register({ id: 'lens', title: NOT_INTERNATIONALIZED_PRODUCT_NAME, navLinkStatus: AppNavLinkStatus.hidden, mount: async (params: AppMountParameters) => { const { mountApp } = await import('./app_plugin/mounter'); - return mountApp( - core, - params, - this.createEditorFrame!, - this.initializerContext.config.get() - ); + return mountApp(core, params, this.createEditorFrame!, getByValueFeatureFlag); }, }); diff --git a/x-pack/plugins/lens/server/index.ts b/x-pack/plugins/lens/server/index.ts index 4d72ad9363689d..8aeeeab4539b6a 100644 --- a/x-pack/plugins/lens/server/index.ts +++ b/x-pack/plugins/lens/server/index.ts @@ -13,9 +13,6 @@ import { configSchema, ConfigSchema } from '../config'; export const config: PluginConfigDescriptor = { schema: configSchema, - exposeToBrowser: { - showNewLensFlow: true, - }, }; export const plugin = (initializerContext: PluginInitializerContext) => From ac94aececed77f61f3308c099649a0cc665bd724 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Wed, 5 Aug 2020 12:31:25 -0400 Subject: [PATCH 25/40] removed unused config related code from lens --- x-pack/plugins/lens/public/index.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/x-pack/plugins/lens/public/index.ts b/x-pack/plugins/lens/public/index.ts index ef85c0cb97a717..e49f648906af0d 100644 --- a/x-pack/plugins/lens/public/index.ts +++ b/x-pack/plugins/lens/public/index.ts @@ -4,11 +4,8 @@ * you may not use this file except in compliance with the Elastic License. */ -import { PluginInitializerContext } from 'kibana/public'; import { LensPlugin } from './plugin'; export * from './types'; -export const plugin = (context: PluginInitializerContext) => { - return new LensPlugin(context); -}; +export const plugin = () => new LensPlugin(); From df876afdecf8fcb4fc3f61fdeb9ac1f0454c4995 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Mon, 17 Aug 2020 17:50:50 -0400 Subject: [PATCH 26/40] Merge continued... --- src/plugins/dashboard/config.ts | 2 +- .../attribute_service/attribute_service.tsx | 10 +++++++ src/plugins/embeddable/public/index.ts | 2 -- src/plugins/embeddable/public/plugin.tsx | 2 -- .../embeddable/embeddable.tsx | 28 ++++--------------- .../embeddable/embeddable_factory.ts | 9 +++--- .../public/editor_frame_service/service.tsx | 4 ++- .../public/persistence/saved_object_store.ts | 4 +-- 8 files changed, 26 insertions(+), 35 deletions(-) diff --git a/src/plugins/dashboard/config.ts b/src/plugins/dashboard/config.ts index ff968a51679e03..da3f8a61306b89 100644 --- a/src/plugins/dashboard/config.ts +++ b/src/plugins/dashboard/config.ts @@ -20,7 +20,7 @@ import { schema, TypeOf } from '@kbn/config-schema'; export const configSchema = schema.object({ - allowByValueEmbeddables: schema.boolean({ defaultValue: false }), + allowByValueEmbeddables: schema.boolean({ defaultValue: true }), }); export type ConfigSchema = TypeOf; diff --git a/src/plugins/dashboard/public/attribute_service/attribute_service.tsx b/src/plugins/dashboard/public/attribute_service/attribute_service.tsx index c2f529fe399f3f..8f5e64f69e4f6f 100644 --- a/src/plugins/dashboard/public/attribute_service/attribute_service.tsx +++ b/src/plugins/dashboard/public/attribute_service/attribute_service.tsx @@ -24,6 +24,7 @@ import { SavedObjectEmbeddableInput, isSavedObjectEmbeddableInput, IEmbeddable, + Container, } from '../embeddable_plugin'; import { SavedObjectsClientContract, @@ -105,6 +106,15 @@ export class AttributeService< return isSavedObjectEmbeddableInput(input); }; + public getExplicitInputFromEmbeddable(embeddable: IEmbeddable): ValType | RefType { + return embeddable.getRoot() && + (embeddable.getRoot() as Container).getInput().panels[embeddable.id].explicitInput + ? ((embeddable.getRoot() as Container).getInput().panels[embeddable.id].explicitInput as + | ValType + | RefType) + : (embeddable.getInput() as ValType | RefType); + } + getInputAsValueType = async (input: ValType | RefType): Promise => { if (!this.inputIsRefType(input)) { return input; diff --git a/src/plugins/embeddable/public/index.ts b/src/plugins/embeddable/public/index.ts index 9e022d048df8ab..57253c1f741abc 100644 --- a/src/plugins/embeddable/public/index.ts +++ b/src/plugins/embeddable/public/index.ts @@ -76,8 +76,6 @@ export { EmbeddablePackageState, EmbeddableRenderer, EmbeddableRendererProps, - ReferenceOrValueEmbeddable, - isReferenceOrValueEmbeddable, } from './lib'; export function plugin(initializerContext: PluginInitializerContext) { diff --git a/src/plugins/embeddable/public/plugin.tsx b/src/plugins/embeddable/public/plugin.tsx index 6f0cd9451c9090..3cbd49279564f3 100644 --- a/src/plugins/embeddable/public/plugin.tsx +++ b/src/plugins/embeddable/public/plugin.tsx @@ -37,8 +37,6 @@ import { defaultEmbeddableFactoryProvider, IEmbeddable, EmbeddablePanel, - SavedObjectEmbeddableInput, - AttributeService, } from './lib'; import { EmbeddableFactoryDefinition } from './lib/embeddables/embeddable_factory_definition'; import { EmbeddableStateTransfer } from './lib/state_transfer'; diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx index c188b4945a861f..af56e52be333a9 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx @@ -29,7 +29,6 @@ import { EmbeddableOutput, IContainer, SavedObjectEmbeddableInput, - AttributeService, ReferenceOrValueEmbeddable, } from '../../../../../../src/plugins/embeddable/public'; import { DOC_TYPE, Document } from '../../persistence'; @@ -40,6 +39,7 @@ import { isLensBrushEvent, isLensFilterEvent } from '../../types'; import { IndexPatternsContract } from '../../../../../../src/plugins/data/public'; import { getEditPath } from '../../../common'; import { IBasePath } from '../../../../../../src/core/public'; +import { AttributeService } from '../../../../../../src/plugins/dashboard/public'; export type LensSavedObjectAttributes = Omit; @@ -257,33 +257,17 @@ export class Embeddable extends AbstractEmbeddable { - return Boolean((input as LensByReferenceInput).savedObjectId); + return this.deps.attributeService.inputIsRefType(input); }; public getInputAsRefType = async (): Promise => { - if (this.inputIsRefType(this.input)) { - return this.input; - } - const wrappedInput = (await this.deps.attributeService.wrapAttributes( - this.input.attributes, - true - )) as Pick; - return { - id: this.input.id, - ...wrappedInput, - }; + const input = this.deps.attributeService.getExplicitInputFromEmbeddable(this); + return this.deps.attributeService.getInputAsRefType(input); }; public getInputAsValueType = async (): Promise => { - if (!this.inputIsRefType(this.input)) { - return this.input; - } - const attributes = await this.deps.attributeService.unwrapAttributes(this.input); - return { - ...this.input, - savedObjectId: undefined, - attributes, - }; + const input = this.deps.attributeService.getExplicitInputFromEmbeddable(this); + return this.deps.attributeService.getInputAsValueType(input); }; destroy() { diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts index 99413f4fc938c8..cd89019c001bc3 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts @@ -15,8 +15,6 @@ import { ReactExpressionRendererType } from '../../../../../../src/plugins/expre import { EmbeddableFactoryDefinition, IContainer, - EmbeddableStart, - AttributeService, } from '../../../../../../src/plugins/embeddable/public'; import { Embeddable, @@ -27,6 +25,7 @@ import { } from './embeddable'; import { DOC_TYPE } from '../../persistence'; import { UiActionsStart } from '../../../../../../src/plugins/ui_actions/public'; +import { AttributeService, DashboardStart } from '../../../../../../src/plugins/dashboard/public'; interface StartServices { timefilter: TimefilterContract; @@ -35,7 +34,7 @@ interface StartServices { savedObjectsClient: SavedObjectsClientContract; expressionRenderer: ReactExpressionRendererType; indexPatternService: IndexPatternsContract; - embeddable?: EmbeddableStart; + dashboard?: DashboardStart; uiActions?: UiActionsStart; } @@ -87,10 +86,10 @@ export class EmbeddableFactory implements EmbeddableFactoryDefinition { uiActions, coreHttp, indexPatternService, - embeddable, + dashboard, } = await this.getStartServices(); if (!this.attributeService) { - this.attributeService = embeddable!.getAttributeService< + this.attributeService = dashboard!.getAttributeService< LensSavedObjectAttributes, LensByValueInput, LensByReferenceInput diff --git a/x-pack/plugins/lens/public/editor_frame_service/service.tsx b/x-pack/plugins/lens/public/editor_frame_service/service.tsx index fd3946c4fbe2ad..a5a739ec997a15 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/service.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/service.tsx @@ -27,6 +27,7 @@ import { formatColumn } from './format_column'; import { EmbeddableFactory } from './embeddable/embeddable_factory'; import { getActiveDatasourceIdFromDoc } from './editor_frame/state_management'; import { UiActionsStart } from '../../../../../src/plugins/ui_actions/public'; +import { DashboardStart } from '../../../../../src/plugins/dashboard/public'; export interface EditorFrameSetupPlugins { data: DataPublicPluginSetup; @@ -37,6 +38,7 @@ export interface EditorFrameSetupPlugins { export interface EditorFrameStartPlugins { data: DataPublicPluginStart; embeddable?: EmbeddableStart; + dashboard?: DashboardStart; expressions: ExpressionsStart; uiActions?: UiActionsStart; } @@ -76,7 +78,7 @@ export class EditorFrameService { expressionRenderer: deps.expressions.ReactExpressionRenderer, indexPatternService: deps.data.indexPatterns, uiActions: deps.uiActions, - embeddable: deps.embeddable, + dashboard: deps.dashboard, }; }; diff --git a/x-pack/plugins/lens/public/persistence/saved_object_store.ts b/x-pack/plugins/lens/public/persistence/saved_object_store.ts index 6e8a2b4bd0a1c3..7172420e3a17f0 100644 --- a/x-pack/plugins/lens/public/persistence/saved_object_store.ts +++ b/x-pack/plugins/lens/public/persistence/saved_object_store.ts @@ -70,8 +70,8 @@ export class SavedObjectIndexStore implements SavedObjectStore { }); return ( await this.client.bulkUpdate([ - { type: DOC_TYPE, savedObjectId, attributes: resetAttributes }, - { type: DOC_TYPE, savedObjectId, attributes }, + { type: DOC_TYPE, id: savedObjectId, attributes: resetAttributes }, + { type: DOC_TYPE, id: savedObjectId, attributes }, ]) ).savedObjects[1]; } From aeb25ee50fb28343303601617ca9e8bf410c2cc2 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Mon, 17 Aug 2020 18:23:06 -0400 Subject: [PATCH 27/40] Merge continued... --- .../editor_frame_service/embeddable/embeddable.test.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx index a8dc8e3cf4a65f..6a4aaabb7cedfe 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx @@ -25,7 +25,7 @@ import { dataPluginMock } from '../../../../../../src/plugins/data/public/mocks' import { VIS_EVENT_TO_TRIGGER } from '../../../../../../src/plugins/visualizations/public/embeddable'; import { coreMock, httpServiceMock } from '../../../../../../src/core/public/mocks'; import { IBasePath } from '../../../../../../src/core/public'; -import { AttributeService } from '../../../../../../src/plugins/embeddable/public'; +import { AttributeService } from '../../../../../../src/plugins/dashboard/public'; jest.mock('../../../../../../src/plugins/inspector/public/', () => ({ isAvailable: false, @@ -50,11 +50,12 @@ const savedVis: Document = { const attributeServiceMockFromSavedVis = ( document: Document ): AttributeService => { + const core = coreMock.createStart(); const service = new AttributeService< LensSavedObjectAttributes, LensByValueInput, LensByReferenceInput - >('lens', coreMock.createStart().savedObjects.client); + >('lens', core.savedObjects.client, core.i18n.Context, core.notifications.toasts); service.unwrapAttributes = jest.fn((input: LensByValueInput | LensByReferenceInput) => { return Promise.resolve({ ...document } as LensSavedObjectAttributes); }); From 6fc1f00b50c13f78b8589e9bf5f3bba8dafd5ba5 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Mon, 17 Aug 2020 19:56:41 -0400 Subject: [PATCH 28/40] Maps Update --- .../routing/routes/maps_app/top_nav_config.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/maps/public/routing/routes/maps_app/top_nav_config.tsx b/x-pack/plugins/maps/public/routing/routes/maps_app/top_nav_config.tsx index 35d8490f1a8860..2c6f3a6d017f92 100644 --- a/x-pack/plugins/maps/public/routing/routes/maps_app/top_nav_config.tsx +++ b/x-pack/plugins/maps/public/routing/routes/maps_app/top_nav_config.tsx @@ -68,17 +68,17 @@ export function getTopNavConfig({ savedMap.description = newDescription; savedMap.copyOnSave = newCopyOnSave; - let id; + let savedObjectId; try { savedMap.syncWithStore(); - id = await savedMap.save({ + savedObjectId = await savedMap.save({ confirmOverwrite: false, isTitleDuplicateConfirmed, onTitleDuplicate, }); // id not returned when save fails because of duplicate title check. // return and let user confirm duplicate title. - if (!id) { + if (!savedObjectId) { return {}; } } catch (err) { @@ -106,7 +106,7 @@ export function getTopNavConfig({ getCoreChrome().docTitle.change(savedMap.title); setBreadcrumbs(); - goToSpecifiedPath(`/map/${id}${window.location.hash}`); + goToSpecifiedPath(`/map/${savedObjectId}${window.location.hash}`); const newlyCreated = newCopyOnSave || isNewMap; if (newlyCreated && !returnToOrigin) { @@ -114,14 +114,14 @@ export function getTopNavConfig({ } else if (!!originatingApp && returnToOrigin) { if (newlyCreated && stateTransfer) { stateTransfer.navigateToWithEmbeddablePackage(originatingApp, { - state: { id, type: MAP_SAVED_OBJECT_TYPE }, + state: { input: { savedObjectId }, type: MAP_SAVED_OBJECT_TYPE }, }); } else { getNavigateToApp()(originatingApp); } } - return { id }; + return { id: savedObjectId }; } if (hasSaveAndReturnConfig) { From 10d98e3a878cd7e9357b71ebbc92e9a5784eb09e Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Tue, 18 Aug 2020 13:42:25 -0400 Subject: [PATCH 29/40] Finished merge with visualize editor --- src/plugins/dashboard/config.ts | 2 +- .../public/application/utils/get_top_nav_config.tsx | 9 ++------- x-pack/plugins/lens/public/app_plugin/app.tsx | 2 +- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/plugins/dashboard/config.ts b/src/plugins/dashboard/config.ts index da3f8a61306b89..ff968a51679e03 100644 --- a/src/plugins/dashboard/config.ts +++ b/src/plugins/dashboard/config.ts @@ -20,7 +20,7 @@ import { schema, TypeOf } from '@kbn/config-schema'; export const configSchema = schema.object({ - allowByValueEmbeddables: schema.boolean({ defaultValue: true }), + allowByValueEmbeddables: schema.boolean({ defaultValue: false }), }); export type ConfigSchema = TypeOf; diff --git a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx index 241937ee94eb90..b091940f73c1ef 100644 --- a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx +++ b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx @@ -21,7 +21,6 @@ import React from 'react'; import { i18n } from '@kbn/i18n'; import { TopNavMenuData } from 'src/plugins/navigation/public'; -import uuid from 'uuid'; import { VISUALIZE_EMBEDDABLE_TYPE } from '../../../../visualizations/public'; import { showSaveModal, @@ -165,15 +164,11 @@ export const getTopNavConfig = ( } const state = { input: { - ...vis.serialize(), - id: embeddableId ? embeddableId : uuid.v4(), + savedVis: vis.serialize(), }, + embeddableId, type: VISUALIZE_EMBEDDABLE_TYPE, - embeddableId: '', }; - if (embeddableId) { - state.embeddableId = embeddableId; - } embeddable.getStateTransfer().navigateToWithEmbeddablePackage(originatingApp, { state }); }; diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index 9d9ca51e713c51..701104693c0e65 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -277,8 +277,8 @@ export function App({ data.indexPatterns, data.query.filterManager, savedObjectId, - state.byValueMode, // TODO: These dependencies are changing too often + // state.byValueMode, // docStorage, // redirectTo, // state.persistedDoc, From 3411c36ac4c414c91dd2ce3848a5d72e65fdd9cd Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Thu, 20 Aug 2020 16:19:28 -0400 Subject: [PATCH 30/40] Added custom save method to attribute service to allow lens specific saving. --- .../public/book/book_embeddable.tsx | 2 +- .../attribute_service/attribute_service.tsx | 25 +++++++++++-------- src/plugins/dashboard/public/plugin.tsx | 8 +++--- .../lens/public/app_plugin/mounter.tsx | 2 +- .../embeddable/embeddable.test.tsx | 9 ++++++- .../embeddable/embeddable.tsx | 2 +- .../embeddable/embeddable_factory.ts | 16 +++++++++--- .../public/persistence/saved_object_store.ts | 4 +-- 8 files changed, 46 insertions(+), 22 deletions(-) diff --git a/examples/embeddable_examples/public/book/book_embeddable.tsx b/examples/embeddable_examples/public/book/book_embeddable.tsx index 33876ab24414ee..566428a10debc3 100644 --- a/examples/embeddable_examples/public/book/book_embeddable.tsx +++ b/examples/embeddable_examples/public/book/book_embeddable.tsx @@ -109,7 +109,7 @@ export class BookEmbeddable extends Embeddable => { const input = this.attributeService.getExplicitInputFromEmbeddable(this); - return this.attributeService.getInputAsRefType(input, { showSaveModal: true }); + return this.attributeService.getInputAsRefType(input, this, { showSaveModal: true }); }; public render(node: HTMLElement) { diff --git a/src/plugins/dashboard/public/attribute_service/attribute_service.tsx b/src/plugins/dashboard/public/attribute_service/attribute_service.tsx index 88e850e5a3ecfb..9e11744e944f61 100644 --- a/src/plugins/dashboard/public/attribute_service/attribute_service.tsx +++ b/src/plugins/dashboard/public/attribute_service/attribute_service.tsx @@ -25,6 +25,9 @@ import { isSavedObjectEmbeddableInput, IEmbeddable, Container, + EmbeddableStart, + EmbeddableFactory, + EmbeddableFactoryNotFoundError, } from '../embeddable_plugin'; import { SavedObjectsClientContract, @@ -40,12 +43,6 @@ import { SaveResult, checkForDuplicateTitle, } from '../../../saved_objects/public'; -import { - EmbeddableStart, - EmbeddableFactory, - EmbeddableFactoryNotFoundError, - Container, -} from '../../../embeddable/public'; /** * The attribute service is a shared, generic service that embeddables can use to provide the functionality @@ -66,7 +63,11 @@ export class AttributeService< private overlays: OverlayStart, private i18nContext: I18nStart['Context'], private toasts: NotificationsStart['toasts'], - getEmbeddableFactory: EmbeddableStart['getEmbeddableFactory'] + getEmbeddableFactory: EmbeddableStart['getEmbeddableFactory'], + private customSaveMethod?: ( + attributes: SavedObjectAttributes, + savedObjectId?: string + ) => Promise<{ id: string }> ) { const factory = getEmbeddableFactory(this.type); if (!factory) { @@ -99,10 +100,13 @@ export class AttributeService< } else { try { if (savedObjectId) { - await this.savedObjectsClient.update(this.type, savedObjectId, newAttributes); + if (this.customSaveMethod) await this.customSaveMethod(newAttributes); + else await this.savedObjectsClient.update(this.type, savedObjectId, newAttributes); return { savedObjectId } as RefType; } else { - const savedItem = await this.savedObjectsClient.create(this.type, newAttributes); + const savedItem = this.customSaveMethod + ? await this.customSaveMethod(newAttributes, savedObjectId) + : await this.savedObjectsClient.create(this.type, newAttributes); return { savedObjectId: savedItem.id } as RefType; } } catch (error) { @@ -147,6 +151,7 @@ export class AttributeService< getInputAsRefType = async ( input: ValType | RefType, + embeddable: IEmbeddable, saveOptions?: { showSaveModal: boolean } | { title: string } ): Promise => { if (this.inputIsRefType(input)) { @@ -186,7 +191,7 @@ export class AttributeService< reject()} - title={input.attributes.title} + title={embeddable.getTitle() || input.attributes.title} showCopyOnSave={false} objectType={this.type} showDescription={false} diff --git a/src/plugins/dashboard/public/plugin.tsx b/src/plugins/dashboard/public/plugin.tsx index 672a9f4aae8738..f74d69e01c5f2d 100644 --- a/src/plugins/dashboard/public/plugin.tsx +++ b/src/plugins/dashboard/public/plugin.tsx @@ -151,7 +151,8 @@ export interface DashboardStart { V extends EmbeddableInput & { attributes: A }, R extends SavedObjectEmbeddableInput >( - type: string + type: string, + customSaveMethod?: (attributes: A, savedObjectId?: string) => Promise<{ id: string }> ) => AttributeService; } @@ -458,14 +459,15 @@ export class DashboardPlugin DashboardContainerByValueRenderer: createDashboardContainerByValueRenderer({ factory: dashboardContainerFactory, }), - getAttributeService: (type: string) => + getAttributeService: (type, customSaveMethod) => new AttributeService( type, core.savedObjects.client, core.overlays, core.i18n.Context, core.notifications.toasts, - embeddable.getEmbeddableFactory + embeddable.getEmbeddableFactory, + customSaveMethod ), }; } diff --git a/x-pack/plugins/lens/public/app_plugin/mounter.tsx b/x-pack/plugins/lens/public/app_plugin/mounter.tsx index d54367b2572250..7bd0dcef1a5577 100644 --- a/x-pack/plugins/lens/public/app_plugin/mounter.tsx +++ b/x-pack/plugins/lens/public/app_plugin/mounter.tsx @@ -99,7 +99,7 @@ export async function mountApp( redirectTo(routeProps, savedObjectId, documentByValue, returnToOrigin, newlyCreated) } embeddableEditorIncomingState={embeddableEditorIncomingState} - getAppNameFromId={stateTransfer.getAppNameFromId} + getAppNameFromId={stateTransfer?.getAppNameFromId} onAppLeave={params.onAppLeave} history={routeProps.history} /> diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx index 6a4aaabb7cedfe..a995f672b9a0b7 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx @@ -55,7 +55,14 @@ const attributeServiceMockFromSavedVis = ( LensSavedObjectAttributes, LensByValueInput, LensByReferenceInput - >('lens', core.savedObjects.client, core.i18n.Context, core.notifications.toasts); + >( + 'lens', + core.savedObjects.client, + core.overlays, + core.i18n.Context, + core.notifications.toasts, + jest.fn() + ); service.unwrapAttributes = jest.fn((input: LensByValueInput | LensByReferenceInput) => { return Promise.resolve({ ...document } as LensSavedObjectAttributes); }); diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx index af56e52be333a9..21665bf9054a54 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx @@ -262,7 +262,7 @@ export class Embeddable extends AbstractEmbeddable => { const input = this.deps.attributeService.getExplicitInputFromEmbeddable(this); - return this.deps.attributeService.getInputAsRefType(input); + return this.deps.attributeService.getInputAsRefType(input, this, { showSaveModal: true }); }; public getInputAsValueType = async (): Promise => { diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts index cd89019c001bc3..54cd73b8660604 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts @@ -23,7 +23,7 @@ import { LensByReferenceInput, LensEmbeddableInput, } from './embeddable'; -import { DOC_TYPE } from '../../persistence'; +import { DOC_TYPE, SavedObjectIndexStore } from '../../persistence'; import { UiActionsStart } from '../../../../../../src/plugins/ui_actions/public'; import { AttributeService, DashboardStart } from '../../../../../../src/plugins/dashboard/public'; @@ -48,7 +48,7 @@ export class EmbeddableFactory implements EmbeddableFactoryDefinition { getIconForSavedObject: () => 'lensApp', }; - attributeService?: AttributeService< + private attributeService?: AttributeService< LensSavedObjectAttributes, LensByValueInput, LensByReferenceInput @@ -85,16 +85,26 @@ export class EmbeddableFactory implements EmbeddableFactoryDefinition { expressionRenderer, uiActions, coreHttp, + savedObjectsClient, indexPatternService, dashboard, } = await this.getStartServices(); if (!this.attributeService) { + const savedObjectStore = new SavedObjectIndexStore(savedObjectsClient); this.attributeService = dashboard!.getAttributeService< LensSavedObjectAttributes, LensByValueInput, LensByReferenceInput - >(this.type); + >(this.type, async (attributes: LensSavedObjectAttributes, savedObjectId?: string) => { + const savedDoc = await savedObjectStore.save({ + ...attributes, + savedObjectId, + type: this.type, + }); + return { id: savedDoc.savedObjectId }; + }); } + return new Embeddable( { attributeService: this.attributeService!, diff --git a/x-pack/plugins/lens/public/persistence/saved_object_store.ts b/x-pack/plugins/lens/public/persistence/saved_object_store.ts index 7172420e3a17f0..037dd48912585c 100644 --- a/x-pack/plugins/lens/public/persistence/saved_object_store.ts +++ b/x-pack/plugins/lens/public/persistence/saved_object_store.ts @@ -44,7 +44,7 @@ export class SavedObjectIndexStore implements SavedObjectStore { this.client = client; } - async save(vis: Document) { + save = async (vis: Document) => { const { savedObjectId, type, ...rest } = vis; // TODO: SavedObjectAttributes should support this kind of object, // remove this workaround when SavedObjectAttributes is updated. @@ -55,7 +55,7 @@ export class SavedObjectIndexStore implements SavedObjectStore { : this.client.create(DOC_TYPE, attributes)); return { ...vis, savedObjectId: result.id }; - } + }; // As Lens is using an object to store its attributes, using the update API // will merge the new attribute object with the old one, not overwriting deleted From 1d5b123d8bdbb61b23bd6c24b62415b3a67e61e7 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Thu, 20 Aug 2020 17:11:23 -0400 Subject: [PATCH 31/40] Small fix for lens embeddables on canvas, made cancel button show up regardless of by value mode. --- x-pack/plugins/lens/public/app_plugin/app.tsx | 2 +- .../editor_frame_service/embeddable/embeddable_factory.ts | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index 23275aa8d19dbf..80072ea3cf0702 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -476,7 +476,7 @@ export function App({ testId: 'lnsApp_saveButton', disableButton: !isSaveable, }, - ...(editFromContainerMode || state.byValueMode + ...(state.originatingApp ? [ { label: i18n.translate('xpack.lens.app.cancel', { diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts index 54cd73b8660604..e630b536f9bfe5 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts @@ -76,6 +76,9 @@ export class EmbeddableFactory implements EmbeddableFactoryDefinition { input: LensEmbeddableInput, parent?: IContainer ) => { + if (!input.savedObjectId) { + input.savedObjectId = savedObjectId; + } return this.create(input, parent); }; From f497244a0029284111978cb92066d669480b6692 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Thu, 20 Aug 2020 17:28:01 -0400 Subject: [PATCH 32/40] Test fix --- .../attribute_service/attribute_service.tsx | 16 +++++++++------- .../embeddable/embeddable.test.tsx | 9 +-------- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/plugins/dashboard/public/attribute_service/attribute_service.tsx b/src/plugins/dashboard/public/attribute_service/attribute_service.tsx index 9e11744e944f61..c957a86ec62aa8 100644 --- a/src/plugins/dashboard/public/attribute_service/attribute_service.tsx +++ b/src/plugins/dashboard/public/attribute_service/attribute_service.tsx @@ -55,7 +55,7 @@ export class AttributeService< ValType extends EmbeddableInput & { attributes: SavedObjectAttributes }, RefType extends SavedObjectEmbeddableInput > { - private embeddableFactory: EmbeddableFactory; + private embeddableFactory?: EmbeddableFactory; constructor( private type: string, @@ -63,17 +63,19 @@ export class AttributeService< private overlays: OverlayStart, private i18nContext: I18nStart['Context'], private toasts: NotificationsStart['toasts'], - getEmbeddableFactory: EmbeddableStart['getEmbeddableFactory'], + getEmbeddableFactory?: EmbeddableStart['getEmbeddableFactory'], private customSaveMethod?: ( attributes: SavedObjectAttributes, savedObjectId?: string ) => Promise<{ id: string }> ) { - const factory = getEmbeddableFactory(this.type); - if (!factory) { - throw new EmbeddableFactoryNotFoundError(this.type); + if (getEmbeddableFactory) { + const factory = getEmbeddableFactory(this.type); + if (!factory) { + throw new EmbeddableFactoryNotFoundError(this.type); + } + this.embeddableFactory = factory; } - this.embeddableFactory = factory; } public async unwrapAttributes(input: RefType | ValType): Promise { @@ -165,7 +167,7 @@ export class AttributeService< copyOnSave: false, lastSavedTitle: '', getEsType: () => this.type, - getDisplayName: this.embeddableFactory.getDisplayName, + getDisplayName: this.embeddableFactory?.getDisplayName || (() => this.type), }, props.isTitleDuplicateConfirmed, props.onTitleDuplicate, diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx index a995f672b9a0b7..980777aa311da6 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx @@ -55,14 +55,7 @@ const attributeServiceMockFromSavedVis = ( LensSavedObjectAttributes, LensByValueInput, LensByReferenceInput - >( - 'lens', - core.savedObjects.client, - core.overlays, - core.i18n.Context, - core.notifications.toasts, - jest.fn() - ); + >('lens', core.savedObjects.client, core.overlays, core.i18n.Context, core.notifications.toasts); service.unwrapAttributes = jest.fn((input: LensByValueInput | LensByReferenceInput) => { return Promise.resolve({ ...document } as LensSavedObjectAttributes); }); From 5b6d397f537e380fbac6c65340dc71248ec1c5a7 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Fri, 21 Aug 2020 12:37:02 -0400 Subject: [PATCH 33/40] Removed accidental duplication of ref_or_val_embeddable. Added tests for lens menu items --- .../ref_or_val_embeddable/index.ts | 20 ---- .../ref_or_val_embeddable/types.ts | 39 ------ .../lens/public/app_plugin/app.test.tsx | 112 +++++++++++------- x-pack/plugins/lens/public/app_plugin/app.tsx | 9 +- 4 files changed, 75 insertions(+), 105 deletions(-) delete mode 100644 src/plugins/embeddable/public/lib/embeddables/ref_or_val_embeddable/index.ts delete mode 100644 src/plugins/embeddable/public/lib/embeddables/ref_or_val_embeddable/types.ts diff --git a/src/plugins/embeddable/public/lib/embeddables/ref_or_val_embeddable/index.ts b/src/plugins/embeddable/public/lib/embeddables/ref_or_val_embeddable/index.ts deleted file mode 100644 index e9b8521a35ba5b..00000000000000 --- a/src/plugins/embeddable/public/lib/embeddables/ref_or_val_embeddable/index.ts +++ /dev/null @@ -1,20 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -export { ReferenceOrValueEmbeddable, isReferenceOrValueEmbeddable } from './types'; diff --git a/src/plugins/embeddable/public/lib/embeddables/ref_or_val_embeddable/types.ts b/src/plugins/embeddable/public/lib/embeddables/ref_or_val_embeddable/types.ts deleted file mode 100644 index 7518e04759b14a..00000000000000 --- a/src/plugins/embeddable/public/lib/embeddables/ref_or_val_embeddable/types.ts +++ /dev/null @@ -1,39 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { EmbeddableInput, SavedObjectEmbeddableInput } from '..'; - -export interface ReferenceOrValueEmbeddable< - ValTypeInput extends EmbeddableInput = EmbeddableInput, - RefTypeInput extends SavedObjectEmbeddableInput = SavedObjectEmbeddableInput -> { - inputIsRefType: (input: ValTypeInput | RefTypeInput) => input is RefTypeInput; - getInputAsValueType: () => Promise; - getInputAsRefType: () => Promise; -} - -export function isReferenceOrValueEmbeddable( - incoming: unknown -): incoming is ReferenceOrValueEmbeddable { - return ( - !!(incoming as ReferenceOrValueEmbeddable).inputIsRefType && - !!(incoming as ReferenceOrValueEmbeddable).getInputAsValueType && - !!(incoming as ReferenceOrValueEmbeddable).getInputAsRefType - ); -} diff --git a/x-pack/plugins/lens/public/app_plugin/app.test.tsx b/x-pack/plugins/lens/public/app_plugin/app.test.tsx index 98659803f90d45..f0ec14554127b0 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.test.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.test.tsx @@ -8,7 +8,7 @@ import React from 'react'; import { Observable } from 'rxjs'; import { ReactWrapper } from 'enzyme'; import { act } from 'react-dom/test-utils'; -import { App } from './app'; +import { App, LensAppProps } from './app'; import { EditorFrameInstance } from '../types'; import { AppMountParameters } from 'kibana/public'; import { Storage } from '../../../../../src/plugins/kibana_utils/public'; @@ -33,6 +33,7 @@ import { navigationPluginMock } from '../../../../../src/plugins/navigation/publ import { TopNavMenuData } from '../../../../../src/plugins/navigation/public'; import { coreMock } from 'src/core/public/mocks'; import { DashboardFeatureFlagConfig } from 'src/plugins/dashboard/public'; +import { EmbeddableEditorState } from '../../../../../src/plugins/embeddable/public'; jest.mock('../persistence'); jest.mock('src/core/public'); @@ -125,26 +126,13 @@ describe('Lens App', () => { let core: ReturnType; let instance: ReactWrapper; - function makeDefaultArgs(): jest.Mocked<{ - editorFrame: EditorFrameInstance; - data: typeof dataStartMock; - navigation: typeof navigationStartMock; - core: typeof core; - storage: Storage; - savedObjectId?: string; - docStorage: SavedObjectStore; - redirectTo: ( - id?: string, - documentByValue?: Document, - returnToOrigin?: boolean, - newlyCreated?: boolean, - embeddableIdToReplace?: string - ) => void; - originatingApp: string | undefined; - onAppLeave: AppMountParameters['onAppLeave']; - history: History; - featureFlagConfig: DashboardFeatureFlagConfig; - }> { + const navMenuItems = { + expectedSaveButton: { emphasize: true, testId: 'lnsApp_saveButton' }, + expectedSaveAsButton: { emphasize: false, testId: 'lnsApp_saveButton' }, + expectedSaveAndReturnButton: { emphasize: true, testId: 'lnsApp_saveAndReturnButton' }, + }; + + function makeDefaultArgs(): jest.Mocked { return ({ navigation: navigationStartMock, editorFrame: createMockFrame(), @@ -191,25 +179,7 @@ describe('Lens App', () => { onAppLeave: jest.fn(), history: createMemoryHistory(), featureFlagConfig: { showNewLensFlow: true }, - } as unknown) as jest.Mocked<{ - navigation: typeof navigationStartMock; - editorFrame: EditorFrameInstance; - data: typeof dataStartMock; - core: typeof core; - storage: Storage; - savedObjectId?: string; - docStorage: SavedObjectStore; - redirectTo: ( - id?: string, - documentByValue?: Document, - returnToOrigin?: boolean, - newlyCreated?: boolean - ) => void; - originatingApp: string | undefined; - onAppLeave: AppMountParameters['onAppLeave']; - history: History; - featureFlagConfig: DashboardFeatureFlagConfig; - }>; + } as unknown) as jest.Mocked; } beforeEach(() => { @@ -292,6 +262,68 @@ describe('Lens App', () => { ); }); + it('Renders save button in by reference mode without originatingApp', async () => { + const args = makeDefaultArgs(); + args.editorFrame = frame; + + instance = mount(); + + const topNavMenuConfig = instance.find(TopNavMenu).prop('config'); + expect(topNavMenuConfig).toContainEqual( + expect.objectContaining(navMenuItems.expectedSaveButton) + ); + expect(topNavMenuConfig).not.toContainEqual( + expect.objectContaining(navMenuItems.expectedSaveAsButton) + ); + expect(topNavMenuConfig).not.toContainEqual( + expect.objectContaining(navMenuItems.expectedSaveAndReturnButton) + ); + }); + + it('Renders Save and Return and Save As in by reference mode when originatingApp is provided', async () => { + const args = makeDefaultArgs(); + args.editorFrame = frame; + args.embeddableEditorIncomingState = { originatingApp: 'ultraDashboard' }; + args.savedObjectId = 'wowASavedObject'; + (args.docStorage.load as jest.Mock).mockResolvedValue({ id: '1234' }); + + instance = mount(); + + const topNavMenuConfig = instance.find(TopNavMenu).prop('config'); + expect(topNavMenuConfig).toContainEqual( + expect.objectContaining(navMenuItems.expectedSaveAndReturnButton) + ); + expect(topNavMenuConfig).toContainEqual( + expect.objectContaining(navMenuItems.expectedSaveAsButton) + ); + expect(topNavMenuConfig).not.toContainEqual( + expect.objectContaining(navMenuItems.expectedSaveButton) + ); + }); + + it('Renders Save and Return and Save As in create by value mode', async () => { + const args = makeDefaultArgs(); + args.editorFrame = frame; + args.featureFlagConfig = { allowByValueEmbeddables: true }; + args.embeddableEditorIncomingState = { + originatingApp: 'ultraDashboard', + valueInput: { whatchaGonnaDo: 'with all these values', id: 'allTheseValuesInYourValueInput' }, + }; + + instance = mount(); + + const topNavMenuConfig = instance.find(TopNavMenu).prop('config'); + expect(topNavMenuConfig).toContainEqual( + expect.objectContaining(navMenuItems.expectedSaveAndReturnButton) + ); + expect(topNavMenuConfig).toContainEqual( + expect.objectContaining(navMenuItems.expectedSaveAsButton) + ); + expect(topNavMenuConfig).not.toContainEqual( + expect.objectContaining(navMenuItems.expectedSaveButton) + ); + }); + it('sets breadcrumbs when the document title changes', async () => { const defaultArgs = makeDefaultArgs(); instance = mount(); diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index 80072ea3cf0702..aba598808f010c 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -62,7 +62,7 @@ interface State { savedQuery?: SavedQuery; } -interface LensAppProps { +export interface LensAppProps { editorFrame: EditorFrameInstance; data: DataPublicPluginStart; navigation: NavigationPublicPluginStart; @@ -270,7 +270,6 @@ export function App({ ...(embeddableEditorIncomingState?.valueInput as LensByValueInput).attributes, }; updateDoc(doc); - redirectTo(); } }, // eslint-disable-next-line react-hooks/exhaustive-deps @@ -387,9 +386,7 @@ export function App({ byValueMode: false, isSaveModalVisible: false, originatingApp: - saveProps.newCopyOnSave && !saveProps.returnToOrigin - ? undefined - : currentOriginatingApp, + newlyCreated && !saveProps.returnToOrigin ? undefined : currentOriginatingApp, persistedDoc: newDoc, lastKnownDoc: newDoc, })); @@ -485,7 +482,7 @@ export function App({ run: () => { redirectTo(undefined, undefined, true, false); }, - testId: 'lnsApp_saveAndReturnButton', + testId: 'lnsApp_cancelButton', }, ] : []), From 0dee8a79cd1a405b88c371c4a54e1e95f25513ea Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Fri, 21 Aug 2020 21:33:54 -0400 Subject: [PATCH 34/40] Made attribute service unwrap references as well --- src/plugins/dashboard/config.ts | 2 +- .../attribute_service/attribute_service.tsx | 9 +++-- x-pack/plugins/lens/public/app_plugin/app.tsx | 34 +++++++++++++------ .../embeddable/embeddable.tsx | 8 +++-- 4 files changed, 36 insertions(+), 17 deletions(-) diff --git a/src/plugins/dashboard/config.ts b/src/plugins/dashboard/config.ts index ff968a51679e03..da3f8a61306b89 100644 --- a/src/plugins/dashboard/config.ts +++ b/src/plugins/dashboard/config.ts @@ -20,7 +20,7 @@ import { schema, TypeOf } from '@kbn/config-schema'; export const configSchema = schema.object({ - allowByValueEmbeddables: schema.boolean({ defaultValue: false }), + allowByValueEmbeddables: schema.boolean({ defaultValue: true }), }); export type ConfigSchema = TypeOf; diff --git a/src/plugins/dashboard/public/attribute_service/attribute_service.tsx b/src/plugins/dashboard/public/attribute_service/attribute_service.tsx index c957a86ec62aa8..e046fd0cfa6fb8 100644 --- a/src/plugins/dashboard/public/attribute_service/attribute_service.tsx +++ b/src/plugins/dashboard/public/attribute_service/attribute_service.tsx @@ -35,6 +35,7 @@ import { I18nStart, NotificationsStart, OverlayStart, + SavedObject, } from '../../../../core/public'; import { SavedObjectSaveModal, @@ -51,8 +52,10 @@ import { * into an embeddable input shape that contains that saved object's attributes by value. */ export class AttributeService< - SavedObjectAttributes extends { title: string }, - ValType extends EmbeddableInput & { attributes: SavedObjectAttributes }, + SavedObjectAttributes extends { title: string; references?: SavedObject['references'] }, + ValType extends EmbeddableInput & { + attributes: SavedObjectAttributes; + }, RefType extends SavedObjectEmbeddableInput > { private embeddableFactory?: EmbeddableFactory; @@ -83,7 +86,7 @@ export class AttributeService< const savedObject: SimpleSavedObject = await this.savedObjectsClient.get< SavedObjectAttributes >(this.type, input.savedObjectId); - return savedObject.attributes; + return { ...savedObject.attributes, references: savedObject.references }; } return input.attributes; } diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index a06b65b629e235..e13821b1a70507 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -258,20 +258,26 @@ export function App({ }, ] : []), - { - href: core.http.basePath.prepend(`/app/visualize#/`), - onClick: (e) => { - core.application.navigateToApp('visualize', { path: '/' }); - e.preventDefault(); - }, - text: i18n.translate('xpack.lens.breadcrumbsTitle', { - defaultMessage: 'Visualize', - }), - }, + ...(!state.byValueMode + ? [ + { + href: core.http.basePath.prepend(`/app/visualize#/`), + onClick: (e) => { + core.application.navigateToApp('visualize', { path: '/' }); + e.preventDefault(); + }, + text: i18n.translate('xpack.lens.breadcrumbsTitle', { + defaultMessage: 'Visualize', + }), + }, + ] + : []), { text: state.persistedDoc ? state.byValueMode - ? i18n.translate('xpack.lens.breadcrumbsByValue', { defaultMessage: 'By Value' }) + ? i18n.translate('xpack.lens.breadcrumbsByValue', { + defaultMessage: 'Edit Visualization', + }) : state.persistedDoc.title : i18n.translate('xpack.lens.breadcrumbsCreate', { defaultMessage: 'Create' }), }, @@ -368,6 +374,12 @@ export function App({ return; } + if (saveProps.newCopyOnSave && !state.byValueMode) { + if (embeddableEditorIncomingState?.embeddableId) { + embeddableEditorIncomingState.embeddableId = undefined; + } + } + const doc = { ...getLastKnownDocWithoutPinnedFilters()!, description: saveProps.newDescription, diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx index 8975954839c173..3fcac71ea3beac 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx @@ -40,12 +40,16 @@ import { isLensBrushEvent, isLensFilterEvent } from '../../types'; import { IndexPatternsContract } from '../../../../../../src/plugins/data/public'; import { getEditPath } from '../../../common'; -import { IBasePath } from '../../../../../../src/core/public'; +import { IBasePath, SavedObject } from '../../../../../../src/core/public'; import { AttributeService } from '../../../../../../src/plugins/dashboard/public'; export type LensSavedObjectAttributes = Omit; -export type LensByValueInput = { attributes: LensSavedObjectAttributes } & LensInheritedInput; +export type LensByValueInput = { + attributes: LensSavedObjectAttributes; + references: SavedObject['references']; +} & LensInheritedInput; + export type LensByReferenceInput = SavedObjectEmbeddableInput & LensInheritedInput; export type LensEmbeddableInput = LensByValueInput | LensByReferenceInput; From 12b9aa44a4b702cf9da65b0cdbd0b14b3417c6ad Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Fri, 21 Aug 2020 22:54:08 -0400 Subject: [PATCH 35/40] type and jest fix --- .../lens/public/app_plugin/app.test.tsx | 86 +++++++++++-------- x-pack/plugins/lens/public/app_plugin/app.tsx | 3 +- 2 files changed, 51 insertions(+), 38 deletions(-) diff --git a/x-pack/plugins/lens/public/app_plugin/app.test.tsx b/x-pack/plugins/lens/public/app_plugin/app.test.tsx index a2695ab0402764..66c01f09122c31 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.test.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.test.tsx @@ -261,16 +261,18 @@ describe('Lens App', () => { instance = mount(); - const topNavMenuConfig = instance.find(TopNavMenu).prop('config'); - expect(topNavMenuConfig).toContainEqual( - expect.objectContaining(navMenuItems.expectedSaveButton) - ); - expect(topNavMenuConfig).not.toContainEqual( - expect.objectContaining(navMenuItems.expectedSaveAsButton) - ); - expect(topNavMenuConfig).not.toContainEqual( - expect.objectContaining(navMenuItems.expectedSaveAndReturnButton) - ); + await act(async () => { + const topNavMenuConfig = instance.find(TopNavMenu).prop('config'); + expect(topNavMenuConfig).toContainEqual( + expect.objectContaining(navMenuItems.expectedSaveButton) + ); + expect(topNavMenuConfig).not.toContainEqual( + expect.objectContaining(navMenuItems.expectedSaveAsButton) + ); + expect(topNavMenuConfig).not.toContainEqual( + expect.objectContaining(navMenuItems.expectedSaveAndReturnButton) + ); + }); }); it('Renders Save and Return and Save As in by reference mode when originatingApp is provided', async () => { @@ -282,16 +284,18 @@ describe('Lens App', () => { instance = mount(); - const topNavMenuConfig = instance.find(TopNavMenu).prop('config'); - expect(topNavMenuConfig).toContainEqual( - expect.objectContaining(navMenuItems.expectedSaveAndReturnButton) - ); - expect(topNavMenuConfig).toContainEqual( - expect.objectContaining(navMenuItems.expectedSaveAsButton) - ); - expect(topNavMenuConfig).not.toContainEqual( - expect.objectContaining(navMenuItems.expectedSaveButton) - ); + await act(async () => { + const topNavMenuConfig = instance.find(TopNavMenu).prop('config'); + expect(topNavMenuConfig).toContainEqual( + expect.objectContaining(navMenuItems.expectedSaveAndReturnButton) + ); + expect(topNavMenuConfig).toContainEqual( + expect.objectContaining(navMenuItems.expectedSaveAsButton) + ); + expect(topNavMenuConfig).not.toContainEqual( + expect.objectContaining(navMenuItems.expectedSaveButton) + ); + }); }); it('Renders Save and Return and Save As in create by value mode', async () => { @@ -300,21 +304,29 @@ describe('Lens App', () => { args.featureFlagConfig = { allowByValueEmbeddables: true }; args.embeddableEditorIncomingState = { originatingApp: 'ultraDashboard', - valueInput: { whatchaGonnaDo: 'with all these values', id: 'allTheseValuesInYourValueInput' }, + valueInput: { + id: 'whatchaGonnaDoWith', + attributes: { + allTheseReferences: 'all these references in your value Input', + references: [], + }, + }, }; instance = mount(); - const topNavMenuConfig = instance.find(TopNavMenu).prop('config'); - expect(topNavMenuConfig).toContainEqual( - expect.objectContaining(navMenuItems.expectedSaveAndReturnButton) - ); - expect(topNavMenuConfig).toContainEqual( - expect.objectContaining(navMenuItems.expectedSaveAsButton) - ); - expect(topNavMenuConfig).not.toContainEqual( - expect.objectContaining(navMenuItems.expectedSaveButton) - ); + await act(async () => { + const topNavMenuConfig = instance.find(TopNavMenu).prop('config'); + expect(topNavMenuConfig).toContainEqual( + expect.objectContaining(navMenuItems.expectedSaveAndReturnButton) + ); + expect(topNavMenuConfig).toContainEqual( + expect.objectContaining(navMenuItems.expectedSaveAsButton) + ); + expect(topNavMenuConfig).not.toContainEqual( + expect.objectContaining(navMenuItems.expectedSaveButton) + ); + }); }); it('sets breadcrumbs when the document title changes', async () => { @@ -336,7 +348,7 @@ describe('Lens App', () => { references: [], }); await act(async () => { - instance.setProps({ docId: '1234' }); + instance.setProps({ savedObjectId: '1234' }); }); expect(defaultArgs.core.chrome.setBreadcrumbs).toHaveBeenCalledWith([ @@ -360,11 +372,11 @@ describe('Lens App', () => { (defaultArgs.docStorage.load as jest.Mock).mockResolvedValue({ id: '1234', title: 'Daaaaaaadaumching!', - expression: 'valid expression', state: { query: 'fake query', - datasourceMetaData: { filterableIndexPatterns: [{ id: '1', title: 'saved' }] }, + filters: [], }, + references: [{}, {}], }); await act(async () => { instance.setProps({ savedObjectId: '1234' }); @@ -624,7 +636,7 @@ describe('Lens App', () => { expect(args.docStorage.save).toHaveBeenCalledWith( expect.objectContaining({ - id: undefined, + savedObjectId: undefined, title: 'hello there', }) ); @@ -645,7 +657,7 @@ describe('Lens App', () => { expect(args.docStorage.save).toHaveBeenCalledWith( expect.objectContaining({ - id: undefined, + savedObjectId: undefined, title: 'hello there', }) ); @@ -716,7 +728,7 @@ describe('Lens App', () => { expect(args.docStorage.save).toHaveBeenCalledWith( expect.objectContaining({ - id: undefined, + savedObjectId: undefined, title: 'hello there', }) ); diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index e13821b1a70507..016d2f61d7ea7f 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -12,6 +12,7 @@ import { NavigationPublicPluginStart } from 'src/plugins/navigation/public'; import { AppMountContext, AppMountParameters, NotificationsStart } from 'kibana/public'; import { History } from 'history'; import { DashboardFeatureFlagConfig } from 'src/plugins/dashboard/public'; +import { EuiBreadcrumb } from '@elastic/eui'; import { Query, DataPublicPluginStart, @@ -269,7 +270,7 @@ export function App({ text: i18n.translate('xpack.lens.breadcrumbsTitle', { defaultMessage: 'Visualize', }), - }, + } as EuiBreadcrumb, ] : []), { From 29ae393719e93f1a12b1e13d5bcaa4234f7ea7c2 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Mon, 24 Aug 2020 16:44:07 -0400 Subject: [PATCH 36/40] Simplified and separated redirectTo method, made dashboard merge incoming input to preserve custom panel titles --- src/plugins/dashboard/config.ts | 2 +- .../application/dashboard_app_controller.tsx | 1 + .../attribute_service/attribute_service.tsx | 23 ++++--- src/plugins/dashboard/public/plugin.tsx | 7 ++- .../lens/public/app_plugin/app.test.tsx | 20 +++--- x-pack/plugins/lens/public/app_plugin/app.tsx | 39 +++++++----- .../lens/public/app_plugin/mounter.tsx | 61 +++++++++---------- .../embeddable/embeddable_factory.ts | 22 ++++--- 8 files changed, 96 insertions(+), 79 deletions(-) diff --git a/src/plugins/dashboard/config.ts b/src/plugins/dashboard/config.ts index da3f8a61306b89..ff968a51679e03 100644 --- a/src/plugins/dashboard/config.ts +++ b/src/plugins/dashboard/config.ts @@ -20,7 +20,7 @@ import { schema, TypeOf } from '@kbn/config-schema'; export const configSchema = schema.object({ - allowByValueEmbeddables: schema.boolean({ defaultValue: true }), + allowByValueEmbeddables: schema.boolean({ defaultValue: false }), }); export type ConfigSchema = TypeOf; diff --git a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx index ce2b1618622787..a1c83c375441f9 100644 --- a/src/plugins/dashboard/public/application/dashboard_app_controller.tsx +++ b/src/plugins/dashboard/public/application/dashboard_app_controller.tsx @@ -335,6 +335,7 @@ export class DashboardAppController { gridData: originalPanelState.gridData, type: incomingEmbeddable.type, explicitInput: { + ...originalPanelState.explicitInput, ...incomingEmbeddable.input, id: incomingEmbeddable.embeddableId, }, diff --git a/src/plugins/dashboard/public/attribute_service/attribute_service.tsx b/src/plugins/dashboard/public/attribute_service/attribute_service.tsx index e046fd0cfa6fb8..0dfd28a2ec13de 100644 --- a/src/plugins/dashboard/public/attribute_service/attribute_service.tsx +++ b/src/plugins/dashboard/public/attribute_service/attribute_service.tsx @@ -35,7 +35,6 @@ import { I18nStart, NotificationsStart, OverlayStart, - SavedObject, } from '../../../../core/public'; import { SavedObjectSaveModal, @@ -51,8 +50,13 @@ import { * can also be used as a higher level wrapper to transform an embeddable input shape that references a saved object * into an embeddable input shape that contains that saved object's attributes by value. */ +export interface AttributeServiceOptions { + customSaveMethod?: (attributes: A, savedObjectId?: string) => Promise<{ id: string }>; + customUnwrapMethod?: (savedObject: SimpleSavedObject) => A; +} + export class AttributeService< - SavedObjectAttributes extends { title: string; references?: SavedObject['references'] }, + SavedObjectAttributes extends { title: string }, ValType extends EmbeddableInput & { attributes: SavedObjectAttributes; }, @@ -67,10 +71,7 @@ export class AttributeService< private i18nContext: I18nStart['Context'], private toasts: NotificationsStart['toasts'], getEmbeddableFactory?: EmbeddableStart['getEmbeddableFactory'], - private customSaveMethod?: ( - attributes: SavedObjectAttributes, - savedObjectId?: string - ) => Promise<{ id: string }> + private options?: AttributeServiceOptions ) { if (getEmbeddableFactory) { const factory = getEmbeddableFactory(this.type); @@ -86,7 +87,9 @@ export class AttributeService< const savedObject: SimpleSavedObject = await this.savedObjectsClient.get< SavedObjectAttributes >(this.type, input.savedObjectId); - return { ...savedObject.attributes, references: savedObject.references }; + return this.options?.customUnwrapMethod + ? this.options?.customUnwrapMethod(savedObject) + : { ...savedObject.attributes }; } return input.attributes; } @@ -105,12 +108,12 @@ export class AttributeService< } else { try { if (savedObjectId) { - if (this.customSaveMethod) await this.customSaveMethod(newAttributes); + if (this.options?.customSaveMethod) await this.options.customSaveMethod(newAttributes); else await this.savedObjectsClient.update(this.type, savedObjectId, newAttributes); return { savedObjectId } as RefType; } else { - const savedItem = this.customSaveMethod - ? await this.customSaveMethod(newAttributes, savedObjectId) + const savedItem = this.options?.customSaveMethod + ? await this.options.customSaveMethod(newAttributes, savedObjectId) : await this.savedObjectsClient.create(this.type, newAttributes); return { savedObjectId: savedItem.id } as RefType; } diff --git a/src/plugins/dashboard/public/plugin.tsx b/src/plugins/dashboard/public/plugin.tsx index f74d69e01c5f2d..4547d30d5ce2e2 100644 --- a/src/plugins/dashboard/public/plugin.tsx +++ b/src/plugins/dashboard/public/plugin.tsx @@ -100,6 +100,7 @@ import { ACTION_ADD_TO_LIBRARY, AddToLibraryActionContext, } from './application/actions/add_to_library_action'; +import { AttributeServiceOptions } from './attribute_service/attribute_service'; declare module '../../share/public' { export interface UrlGeneratorStateMapping { @@ -152,7 +153,7 @@ export interface DashboardStart { R extends SavedObjectEmbeddableInput >( type: string, - customSaveMethod?: (attributes: A, savedObjectId?: string) => Promise<{ id: string }> + options?: AttributeServiceOptions ) => AttributeService; } @@ -459,7 +460,7 @@ export class DashboardPlugin DashboardContainerByValueRenderer: createDashboardContainerByValueRenderer({ factory: dashboardContainerFactory, }), - getAttributeService: (type, customSaveMethod) => + getAttributeService: (type, options) => new AttributeService( type, core.savedObjects.client, @@ -467,7 +468,7 @@ export class DashboardPlugin core.i18n.Context, core.notifications.toasts, embeddable.getEmbeddableFactory, - customSaveMethod + options ), }; } diff --git a/x-pack/plugins/lens/public/app_plugin/app.test.tsx b/x-pack/plugins/lens/public/app_plugin/app.test.tsx index 123e4faede078b..d0ce6970e17314 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.test.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.test.tsx @@ -27,6 +27,8 @@ import { import { navigationPluginMock } from '../../../../../src/plugins/navigation/public/mocks'; import { TopNavMenuData } from '../../../../../src/plugins/navigation/public'; import { coreMock } from 'src/core/public/mocks'; +import { Optional } from '@kbn/utility-types'; +import { LensEmbeddableInput } from '../editor_frame_service/embeddable/embeddable'; jest.mock('../editor_frame_service/editor_frame/expression_helpers'); jest.mock('src/core/public'); @@ -161,14 +163,8 @@ describe('Lens App', () => { load: jest.fn(), save: jest.fn(), }, - redirectTo: jest.fn( - ( - savedObjectId?: string, - documentByValue?: Document, - returnToOrigin?: boolean, - newlyCreated?: boolean - ) => {} - ), + redirectTo: jest.fn((savedObjectId?: string) => {}), + redirectToOrigin: jest.fn((input?: Optional) => {}), onAppLeave: jest.fn(), history: createMemoryHistory(), featureFlagConfig: { showNewLensFlow: true }, @@ -357,7 +353,7 @@ describe('Lens App', () => { ]); }); - it.skip('sets originatingApp breadcrumb when the document title changes', async () => { + it('sets originatingApp breadcrumb when the document title changes', async () => { const defaultArgs = makeDefaultArgs(); defaultArgs.embeddableEditorIncomingState = { originatingApp: 'ultraCoolDashboard' }; defaultArgs.getAppNameFromId = () => 'The Coolest Container Ever Made'; @@ -641,7 +637,7 @@ describe('Lens App', () => { }) ); - expect(args.redirectTo).toHaveBeenCalledWith('aaa', undefined, undefined, true); + expect(args.redirectTo).toHaveBeenCalledWith('aaa'); inst.setProps({ savedObjectId: 'aaa' }); @@ -662,7 +658,7 @@ describe('Lens App', () => { }) ); - expect(args.redirectTo).toHaveBeenCalledWith('aaa', undefined, undefined, true); + expect(args.redirectTo).toHaveBeenCalledWith('aaa'); inst.setProps({ savedObjectId: 'aaa' }); @@ -733,7 +729,7 @@ describe('Lens App', () => { }) ); - expect(args.redirectTo).toHaveBeenCalledWith('aaa', undefined, true, true); + expect(args.redirectToOrigin).toHaveBeenCalledWith({ savedObjectId: 'aaa' }); }); it('saves app filters and does not save pinned filters', async () => { diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index 016d2f61d7ea7f..19af8bc530e50b 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -13,6 +13,7 @@ import { AppMountContext, AppMountParameters, NotificationsStart } from 'kibana/ import { History } from 'history'; import { DashboardFeatureFlagConfig } from 'src/plugins/dashboard/public'; import { EuiBreadcrumb } from '@elastic/eui'; +import { Optional } from '@kbn/utility-types'; import { Query, DataPublicPluginStart, @@ -41,9 +42,12 @@ import { SavedQuery, } from '../../../../../src/plugins/data/public'; import { EmbeddableEditorState } from '../../../../../src/plugins/embeddable/public'; -import { LensByValueInput } from '../editor_frame_service/embeddable/embeddable'; +import { + LensByValueInput, + LensEmbeddableInput, +} from '../editor_frame_service/embeddable/embeddable'; -interface State { +interface LensAppState { indicateNoData: boolean; isLoading: boolean; byValueMode: boolean; @@ -52,6 +56,7 @@ interface State { originatingApp?: string; persistedDoc?: Document; lastKnownDoc?: Document; + embeddableId?: string; // Properties needed to interface with TopNav dateRange: { @@ -72,12 +77,8 @@ export interface LensAppProps { storage: IStorageWrapper; savedObjectId?: string; docStorage: SavedObjectStore; - redirectTo: ( - savedObjectId?: string, - documentByValue?: Document, - returnToOrigin?: boolean, - newlyCreated?: boolean - ) => void; + redirectTo: (savedObjectId?: string) => void; + redirectToOrigin?: (input?: Optional) => void; embeddableEditorIncomingState?: EmbeddableEditorState; onAppLeave: AppMountParameters['onAppLeave']; history: History; @@ -93,6 +94,7 @@ export function App({ savedObjectId, docStorage, redirectTo, + redirectToOrigin, embeddableEditorIncomingState, navigation, onAppLeave, @@ -100,7 +102,7 @@ export function App({ featureFlagConfig, getAppNameFromId, }: LensAppProps) { - const [state, setState] = useState(() => { + const [state, setState] = useState(() => { const currentRange = data.query.timefilter.timefilter.getTime(); return { isLoading: !!savedObjectId || !!embeddableEditorIncomingState?.valueInput, @@ -389,11 +391,14 @@ export function App({ }; const addedToLibrary = state.byValueMode && saveToLibrary; + const newlyCreated = saveProps.newCopyOnSave || addedToLibrary || (!savedObjectId && !state.byValueMode); - if (state.byValueMode && !saveToLibrary) { - await setState((s: State) => ({ ...s, persistedDoc: doc })); - redirectTo(doc.savedObjectId, doc, saveProps.returnToOrigin, newlyCreated); + + if (state.byValueMode && !saveToLibrary && redirectToOrigin) { + await setState((s: LensAppState) => ({ ...s, persistedDoc: doc })); + const { savedObjectId: id, type, ...attributes } = doc; + redirectToOrigin({ attributes }); } else { await checkForDuplicateTitle( { @@ -428,8 +433,10 @@ export function App({ persistedDoc: newDoc, lastKnownDoc: newDoc, })); - if (savedObjectId !== newSavedObjectId || saveProps.returnToOrigin) { - redirectTo(newSavedObjectId, undefined, saveProps.returnToOrigin, newlyCreated); + if (saveProps.returnToOrigin && redirectToOrigin) { + redirectToOrigin(newlyCreated ? { savedObjectId: newSavedObjectId } : undefined); + } else if (savedObjectId !== newSavedObjectId) { + redirectTo(newSavedObjectId); } }) .catch((e) => { @@ -518,7 +525,9 @@ export function App({ defaultMessage: 'cancel', }), run: () => { - redirectTo(undefined, undefined, true, false); + if (redirectToOrigin) { + redirectToOrigin(); + } }, testId: 'lnsApp_cancelButton', }, diff --git a/x-pack/plugins/lens/public/app_plugin/mounter.tsx b/x-pack/plugins/lens/public/app_plugin/mounter.tsx index 7bd0dcef1a5577..65d3e51198fc90 100644 --- a/x-pack/plugins/lens/public/app_plugin/mounter.tsx +++ b/x-pack/plugins/lens/public/app_plugin/mounter.tsx @@ -12,6 +12,7 @@ import { render, unmountComponentAtNode } from 'react-dom'; import { i18n } from '@kbn/i18n'; import { DashboardFeatureFlagConfig } from 'src/plugins/dashboard/public'; +import { Optional } from '@kbn/utility-types'; import { Storage } from '../../../../../src/plugins/kibana_utils/public'; import { LensReportManager, setReportManager, trackUiEvent } from '../lens_ui_telemetry'; @@ -19,9 +20,10 @@ import { LensReportManager, setReportManager, trackUiEvent } from '../lens_ui_te import { App } from './app'; import { EditorFrameStart } from '../types'; import { addHelpMenuToAppChrome } from '../help_menu_util'; -import { Document, SavedObjectIndexStore } from '../persistence'; +import { SavedObjectIndexStore } from '../persistence'; import { LensPluginStartDependencies } from '../plugin'; import { LENS_EMBEDDABLE_TYPE, LENS_EDIT_BY_VALUE } from '../../common'; +import { LensEmbeddableInput } from '../editor_frame_service/embeddable/embeddable'; export async function mountApp( core: CoreSetup, @@ -49,36 +51,32 @@ export async function mountApp( http: core.http, }) ); - const redirectTo = ( - routeProps: RouteComponentProps<{ id?: string }>, - savedObjectId?: string, - document?: Document, - returnToOrigin?: boolean, - newlyCreated?: boolean - ) => { - if (!savedObjectId && !newlyCreated && !returnToOrigin) { + + const redirectTo = (routeProps: RouteComponentProps<{ id?: string }>, savedObjectId?: string) => { + if (!savedObjectId) { routeProps.history.push('/'); - } else if (savedObjectId && !embeddableEditorIncomingState?.originatingApp && !returnToOrigin) { + } else { routeProps.history.push(`/edit/${savedObjectId}`); - } else if (!!embeddableEditorIncomingState?.originatingApp && returnToOrigin) { - if (savedObjectId) { - routeProps.history.push(`/edit/${savedObjectId}`); - } - if (newlyCreated && stateTransfer) { - const input = savedObjectId ? { savedObjectId } : { attributes: document }; - stateTransfer.navigateToWithEmbeddablePackage( - embeddableEditorIncomingState?.originatingApp, - { - state: { - embeddableId: embeddableEditorIncomingState.embeddableId, - type: LENS_EMBEDDABLE_TYPE, - input, - }, - } - ); - } else { - coreStart.application.navigateToApp(embeddableEditorIncomingState?.originatingApp); - } + } + }; + + const redirectToOrigin = ( + routeProps: RouteComponentProps<{ id?: string }>, + input?: Optional + ) => { + if (!embeddableEditorIncomingState?.originatingApp) { + throw new Error('redirectToOrigin called without an originating app'); + } + if (stateTransfer && input) { + stateTransfer.navigateToWithEmbeddablePackage(embeddableEditorIncomingState?.originatingApp, { + state: { + embeddableId: embeddableEditorIncomingState.embeddableId, + type: LENS_EMBEDDABLE_TYPE, + input, + }, + }); + } else { + coreStart.application.navigateToApp(embeddableEditorIncomingState?.originatingApp); } }; @@ -95,8 +93,9 @@ export async function mountApp( savedObjectId={routeProps.match.params.id} docStorage={new SavedObjectIndexStore(savedObjectsClient)} featureFlagConfig={featureFlagConfig} - redirectTo={(savedObjectId, documentByValue, returnToOrigin, newlyCreated) => - redirectTo(routeProps, savedObjectId, documentByValue, returnToOrigin, newlyCreated) + redirectTo={(savedObjectId?: string) => redirectTo(routeProps, savedObjectId)} + redirectToOrigin={(input?: Optional) => + redirectToOrigin(routeProps, input) } embeddableEditorIncomingState={embeddableEditorIncomingState} getAppNameFromId={stateTransfer?.getAppNameFromId} diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts index 5d0606fd4ab72a..897cf02ece3b08 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts @@ -102,13 +102,21 @@ export class EmbeddableFactory implements EmbeddableFactoryDefinition { LensSavedObjectAttributes, LensByValueInput, LensByReferenceInput - >(this.type, async (attributes: LensSavedObjectAttributes, savedObjectId?: string) => { - const savedDoc = await savedObjectStore.save({ - ...attributes, - savedObjectId, - type: this.type, - }); - return { id: savedDoc.savedObjectId }; + >(this.type, { + customSaveMethod: async (attributes: LensSavedObjectAttributes, savedObjectId?: string) => { + const savedDoc = await savedObjectStore.save({ + ...attributes, + savedObjectId, + type: this.type, + }); + return { id: savedDoc.savedObjectId }; + }, + customUnwrapMethod: (savedObject) => { + return { + ...savedObject.attributes, + references: savedObject.references, + }; + }, }); } From 566500c5ee7869f5f441f1b5072de591e41efa07 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Tue, 25 Aug 2020 11:07:34 -0400 Subject: [PATCH 37/40] changed feature flag config to match dashboard --- x-pack/plugins/lens/public/app_plugin/app.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/app_plugin/app.test.tsx b/x-pack/plugins/lens/public/app_plugin/app.test.tsx index a72c9180010fc9..eb6012393978d4 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.test.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.test.tsx @@ -167,7 +167,7 @@ describe('Lens App', () => { redirectToOrigin: jest.fn((input?: Optional) => {}), onAppLeave: jest.fn(), history: createMemoryHistory(), - featureFlagConfig: { showNewLensFlow: true }, + featureFlagConfig: { allowByValueEmbeddables: false }, } as unknown) as jest.Mocked; } From 8796ab0c224b77734ce59cdada16c34245ec4520 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Tue, 25 Aug 2020 12:07:15 -0400 Subject: [PATCH 38/40] cleaned up types after 75560 --- .../public/lib/actions/edit_panel_action.test.tsx | 9 +++++++-- .../public/application/utils/get_top_nav_config.tsx | 4 ++-- x-pack/plugins/lens/public/app_plugin/app.test.tsx | 13 +++++++++---- x-pack/plugins/lens/public/app_plugin/app.tsx | 9 +++++++-- .../embeddable/embeddable_factory.ts | 4 ++-- 5 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx b/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx index 7ba8e086f28fd0..9c20eab61cc213 100644 --- a/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx +++ b/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx @@ -57,7 +57,12 @@ 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, coolInput1: 1, coolInput2: 2 }, + ({ + id: '123', + viewMode: ViewMode.EDIT, + coolInput1: 1, + coolInput2: 2, + } as unknown) as EmbeddableInput, true ); embeddable.getOutput = jest.fn(() => ({ editApp: 'ultraVisualize', editPath: '/123' })); @@ -81,7 +86,7 @@ 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' }, + { id: '123', viewMode: ViewMode.EDIT, savedObjectId: '1234' } as SavedObjectEmbeddableInput, true ); embeddable.getOutput = jest.fn(() => ({ editApp: 'ultraVisualize', editPath: '/123' })); diff --git a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx index 01e7357aa8cf07..721210b964cbda 100644 --- a/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx +++ b/src/plugins/visualize/public/application/utils/get_top_nav_config.tsx @@ -21,7 +21,7 @@ import React from 'react'; import { i18n } from '@kbn/i18n'; import { TopNavMenuData } from 'src/plugins/navigation/public'; -import { VISUALIZE_EMBEDDABLE_TYPE } from '../../../../visualizations/public'; +import { VISUALIZE_EMBEDDABLE_TYPE, VisualizeInput } from '../../../../visualizations/public'; import { showSaveModal, SavedObjectSaveModalOrigin, @@ -167,7 +167,7 @@ export const getTopNavConfig = ( const state = { input: { savedVis: vis.serialize(), - }, + } as VisualizeInput, embeddableId, type: VISUALIZE_EMBEDDABLE_TYPE, }; diff --git a/x-pack/plugins/lens/public/app_plugin/app.test.tsx b/x-pack/plugins/lens/public/app_plugin/app.test.tsx index eb6012393978d4..7042053850bc42 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.test.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.test.tsx @@ -28,7 +28,11 @@ import { navigationPluginMock } from '../../../../../src/plugins/navigation/publ import { TopNavMenuData } from '../../../../../src/plugins/navigation/public'; import { coreMock } from 'src/core/public/mocks'; import { Optional } from '@kbn/utility-types'; -import { LensEmbeddableInput } from '../editor_frame_service/embeddable/embeddable'; +import { + LensEmbeddableInput, + LensByValueInput, +} from '../editor_frame_service/embeddable/embeddable'; +import { SavedObjectReference } from '../../../../../src/core/types'; jest.mock('../editor_frame_service/editor_frame/expression_helpers'); jest.mock('src/core/public'); @@ -303,10 +307,11 @@ describe('Lens App', () => { valueInput: { id: 'whatchaGonnaDoWith', attributes: { - allTheseReferences: 'all these references in your value Input', - references: [], + title: + 'whatcha gonna do with all these references? All these references in your value Input', + references: [] as SavedObjectReference[], }, - }, + } as LensByValueInput, }; instance = mount(); diff --git a/x-pack/plugins/lens/public/app_plugin/app.tsx b/x-pack/plugins/lens/public/app_plugin/app.tsx index 19af8bc530e50b..97ef0805b0e399 100644 --- a/x-pack/plugins/lens/public/app_plugin/app.tsx +++ b/x-pack/plugins/lens/public/app_plugin/app.tsx @@ -45,6 +45,7 @@ import { EmbeddableEditorState } from '../../../../../src/plugins/embeddable/pub import { LensByValueInput, LensEmbeddableInput, + LensByReferenceInput, } from '../editor_frame_service/embeddable/embeddable'; interface LensAppState { @@ -398,7 +399,7 @@ export function App({ if (state.byValueMode && !saveToLibrary && redirectToOrigin) { await setState((s: LensAppState) => ({ ...s, persistedDoc: doc })); const { savedObjectId: id, type, ...attributes } = doc; - redirectToOrigin({ attributes }); + redirectToOrigin({ attributes } as LensByValueInput); } else { await checkForDuplicateTitle( { @@ -434,7 +435,11 @@ export function App({ lastKnownDoc: newDoc, })); if (saveProps.returnToOrigin && redirectToOrigin) { - redirectToOrigin(newlyCreated ? { savedObjectId: newSavedObjectId } : undefined); + redirectToOrigin( + newlyCreated + ? ({ savedObjectId: newSavedObjectId } as LensByReferenceInput) + : undefined + ); } else if (savedObjectId !== newSavedObjectId) { redirectTo(newSavedObjectId); } diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts index 897cf02ece3b08..0fce1702eb2d9a 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable_factory.ts @@ -79,8 +79,8 @@ export class EmbeddableFactory implements EmbeddableFactoryDefinition { input: LensEmbeddableInput, parent?: IContainer ) => { - if (!input.savedObjectId) { - input.savedObjectId = savedObjectId; + if (!(input as LensByReferenceInput).savedObjectId) { + (input as LensByReferenceInput).savedObjectId = savedObjectId; } return this.create(input, parent); }; From d0e3fd8ae32efd9b4f47fae96add6682b0fbb0c2 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Tue, 25 Aug 2020 12:34:50 -0400 Subject: [PATCH 39/40] Type cleanup continued --- .../embeddable/public/lib/actions/edit_panel_action.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx b/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx index 9c20eab61cc213..ba24913c6d1c02 100644 --- a/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx +++ b/src/plugins/embeddable/public/lib/actions/edit_panel_action.test.tsx @@ -18,7 +18,7 @@ */ import { EditPanelAction } from './edit_panel_action'; -import { Embeddable, EmbeddableInput } from '../embeddables'; +import { Embeddable, EmbeddableInput, SavedObjectEmbeddableInput } from '../embeddables'; import { ViewMode } from '../types'; import { ContactCardEmbeddable } from '../test_samples'; import { embeddablePluginMock } from '../../mocks'; From cf74955a52e108713516cf7f53e99fdb452de7bc Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Mon, 31 Aug 2020 15:34:54 -0400 Subject: [PATCH 40/40] update eslint --- examples/embeddable_examples/public/book/book_embeddable.tsx | 3 ++- .../lens/public/editor_frame_service/embeddable/embeddable.tsx | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/examples/embeddable_examples/public/book/book_embeddable.tsx b/examples/embeddable_examples/public/book/book_embeddable.tsx index 566428a10debc3..1316edf07517fd 100644 --- a/examples/embeddable_examples/public/book/book_embeddable.tsx +++ b/examples/embeddable_examples/public/book/book_embeddable.tsx @@ -60,7 +60,8 @@ function getHasMatch(search?: string, savedAttributes?: BookSavedObjectAttribute ); } -export class BookEmbeddable extends Embeddable +export class BookEmbeddable + extends Embeddable implements ReferenceOrValueEmbeddable { public readonly type = BOOK_EMBEDDABLE; private subscription: Subscription; diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx index 3fcac71ea3beac..e1be8b69a61abe 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx @@ -79,7 +79,8 @@ export interface LensEmbeddableDeps { getTrigger?: UiActionsStart['getTrigger'] | undefined; } -export class Embeddable extends AbstractEmbeddable +export class Embeddable + extends AbstractEmbeddable implements ReferenceOrValueEmbeddable { type = DOC_TYPE;