Simplified and separated redirectTo method, made dashboard merge inco…

…ming input to preserve custom panel titles
elastic · Aug 24, 2020 · 29ae393 · 29ae393
1 parent e3dc91a
commit 29ae393
Show file tree

Hide file tree

Showing 8 changed files with 96 additions and 79 deletions.
diff --git 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<typeof configSchema>;
diff --git 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
@@ -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<A extends { title: string }> {
+  customSaveMethod?: (attributes: A, savedObjectId?: string) => Promise<{ id: string }>;
+  customUnwrapMethod?: (savedObject: SimpleSavedObject<A>) => 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<SavedObjectAttributes>
   ) {
     if (getEmbeddableFactory) {
       const factory = getEmbeddableFactory(this.type);
@@ -86,7 +87,9 @@ export class AttributeService<
       const savedObject: SimpleSavedObject<SavedObjectAttributes> = 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
@@ -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<A>
   ) => AttributeService<A, V, R>;
 }
 
@@ -459,15 +460,15 @@ export class DashboardPlugin
       DashboardContainerByValueRenderer: createDashboardContainerByValueRenderer({
         factory: dashboardContainerFactory,
       }),
-      getAttributeService: (type, customSaveMethod) =>
+      getAttributeService: (type, options) =>
         new AttributeService(
           type,
           core.savedObjects.client,
           core.overlays,
           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
@@ -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<LensEmbeddableInput, 'id'>) => {}),
       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
@@ -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<LensEmbeddableInput, 'id'>) => void;
   embeddableEditorIncomingState?: EmbeddableEditorState;
   onAppLeave: AppMountParameters['onAppLeave'];
   history: History;
@@ -93,14 +94,15 @@ export function App({
   savedObjectId,
   docStorage,
   redirectTo,
+  redirectToOrigin,
   embeddableEditorIncomingState,
   navigation,
   onAppLeave,
   history,
   featureFlagConfig,
   getAppNameFromId,
 }: LensAppProps) {
-  const [state, setState] = useState<State>(() => {
+  const [state, setState] = useState<LensAppState>(() => {
     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
@@ -12,16 +12,18 @@ 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';
 
 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<LensPluginStartDependencies, void>,
@@ -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<LensEmbeddableInput, 'id'>
+  ) => {
+    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<LensEmbeddableInput, 'id'>) =>
+          redirectToOrigin(routeProps, input)
         }
         embeddableEditorIncomingState={embeddableEditorIncomingState}
         getAppNameFromId={stateTransfer?.getAppNameFromId}