From 29ae393719e93f1a12b1e13d5bcaa4234f7ea7c2 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Mon, 24 Aug 2020 16:44:07 -0400 Subject: [PATCH] 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 da3f8a61306b8..ff968a51679e0 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 ce2b161862278..a1c83c375441f 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 e046fd0cfa6fb..0dfd28a2ec13d 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 f74d69e01c5f2..4547d30d5ce2e 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 123e4faede078..d0ce6970e1731 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 016d2f61d7ea7..19af8bc530e50 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 7bd0dcef1a557..65d3e51198fc9 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 5d0606fd4ab72..897cf02ece3b0 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, + }; + }, }); }