Decouple actions from embeddables: step 1 (#44503) (#44891)

* Decouple actions from embeddables: step 1

* prefer as any instead of is-ignore

* Remove unneccessary test, no more triggerContext to be null.

* Fix bug and fix the test that should have caught it.  Be more strict about checking isCompatible.

src/legacy/core_plugins/dashboard_embeddable_container/public/np_ready/public/lib/actions/expand_panel_action.tsx

@@ -21,7 +21,6 @@ import { i18n } from '@kbn/i18n';
 import {
   Action,
   IEmbeddable,
-  ActionContext,
   IncompatibleActionError,
 } from '../../../../../../embeddable_api/public/np_ready/public';
 import { DASHBOARD_CONTAINER_TYPE, DashboardContainer } from '../embeddable';
@@ -40,7 +39,11 @@ function isExpanded(embeddable: IEmbeddable) {
   return embeddable.id === embeddable.parent.getInput().expandedPanelId;
 }
 
-export class ExpandPanelAction extends Action {
+interface ActionContext {
+  embeddable: IEmbeddable;
+}
+
+export class ExpandPanelAction extends Action<ActionContext> {
   public readonly type = EXPAND_PANEL_ACTION;
 
   constructor() {
@@ -80,7 +83,7 @@ export class ExpandPanelAction extends Action {
     return Boolean(embeddable.parent && isDashboard(embeddable.parent));
   }
 
-  public execute({ embeddable }: ActionContext) {
+  public async execute({ embeddable }: ActionContext) {
     if (!embeddable.parent || !isDashboard(embeddable.parent)) {
       throw new IncompatibleActionError();
     }

src/legacy/core_plugins/embeddable_api/public/np_ready/public/api/execute_trigger_actions.ts

@@ -18,9 +18,9 @@
  */
 
 import { EmbeddableApiPure } from './types';
-import { Action, ActionContext, buildContextMenuForActions, openContextMenu } from '../lib';
+import { Action, buildContextMenuForActions, openContextMenu } from '../lib';
 
-const executeSingleAction = async (action: Action, actionContext: ActionContext) => {
+const executeSingleAction = async <A extends {} = {}>(action: Action<A>, actionContext: A) => {
   const href = action.getHref(actionContext);
 
   // TODO: Do we need a `getHref()` special case?

src/legacy/core_plugins/embeddable_api/public/np_ready/public/api/types.ts

@@ -24,7 +24,6 @@ import {
   EmbeddableFactory,
   ExecuteTriggerActions,
   GetEmbeddableFactories,
-  TriggerContext,
 } from '../lib';
 
 export interface EmbeddableApi {
@@ -35,7 +34,7 @@ export interface EmbeddableApi {
   getEmbeddableFactories: GetEmbeddableFactories;
   getTrigger: (id: string) => Trigger;
   getTriggerActions: (id: string) => Action[];
-  getTriggerCompatibleActions: (triggerId: string, context: TriggerContext) => Promise<Action[]>;
+  getTriggerCompatibleActions: <C>(triggerId: string, context: C) => Promise<Array<Action<C>>>;
   registerAction: (action: Action) => void;
   // TODO: Make `registerEmbeddableFactory` receive only `factory` argument.
   registerEmbeddableFactory: (id: string, factory: EmbeddableFactory) => void;

src/legacy/core_plugins/embeddable_api/public/np_ready/public/index.ts

@@ -26,7 +26,6 @@ export {
   APPLY_FILTER_TRIGGER,
   PANEL_BADGE_TRIGGER,
   Action,
-  ActionContext,
   Adapters,
   AddPanelAction,
   ApplyFilterAction,
@@ -59,7 +58,6 @@ export {
   PropertySpec,
   SavedObjectMetaData,
   Trigger,
-  TriggerContext,
   ViewMode,
   isErrorEmbeddable,
   openAddPanelFlyout,

src/legacy/core_plugins/embeddable_api/public/np_ready/public/lib/actions/action.ts

@@ -17,20 +17,7 @@
  * under the License.
  */
 
-import { IEmbeddable } from '../embeddables';
-
-export interface ActionContext<
-  TEmbeddable extends IEmbeddable = IEmbeddable,
-  TTriggerContext extends {} = {}
-> {
-  embeddable: TEmbeddable;
-  triggerContext?: TTriggerContext;
-}
-
-export abstract class Action<
-  TEmbeddable extends IEmbeddable = IEmbeddable,
-  TTriggerContext extends {} = {}
-> {
+export abstract class Action<ActionContext extends {} = {}> {
   /**
    * Determined the order when there is more than one action matched to a trigger.
    * Higher numbers are displayed first.
@@ -43,35 +30,33 @@ export abstract class Action<
   /**
    * Optional EUI icon type that can be displayed along with the title.
    */
-  public getIconType(context: ActionContext<TEmbeddable, TTriggerContext>): string | undefined {
+  public getIconType(context: ActionContext): string | undefined {
     return undefined;
   }
 
   /**
    * Returns a title to be displayed to the user.
    * @param context
    */
-  public abstract getDisplayName(context: ActionContext<TEmbeddable, TTriggerContext>): string;
+  public abstract getDisplayName(context: ActionContext): string;
 
   /**
    * Returns a promise that resolves to true if this action is compatible given the context,
    * otherwise resolves to false.
    */
-  public async isCompatible(
-    context: ActionContext<TEmbeddable, TTriggerContext>
-  ): Promise<boolean> {
+  public async isCompatible(context: ActionContext): Promise<boolean> {
     return true;
   }
 
   /**
    * If this returns something truthy, this is used in addition to the `execute` method when clicked.
    */
-  public getHref(context: ActionContext<TEmbeddable, TTriggerContext>): string | undefined {
+  public getHref(context: ActionContext): string | undefined {
     return undefined;
   }
 
   /**
    * Executes the action.
    */
-  public abstract execute(context: ActionContext<TEmbeddable, TTriggerContext>): void;
+  public abstract async execute(context: ActionContext): Promise<void>;
 }

src/legacy/core_plugins/embeddable_api/public/np_ready/public/lib/actions/apply_filter_action.test.ts

@@ -48,9 +48,7 @@ describe('isCompatible()', () => {
           }),
         }),
       } as any,
-      triggerContext: {
-        filters: [],
-      },
+      filters: [],
     });
     expect(result).toBe(true);
   });
@@ -65,9 +63,7 @@ describe('isCompatible()', () => {
           }),
         }),
       } as any,
-      triggerContext: {
-        filters: [],
-      },
+      filters: [],
     });
     expect(result).toBe(false);
   });
@@ -83,25 +79,8 @@ describe('isCompatible()', () => {
           }),
         }),
       } as any,
-      triggerContext: {
-        // filters: [],
-      } as any,
-    });
+    } as any);
     expect(result1).toBe(false);
-
-    const result2 = await action.isCompatible({
-      embeddable: {
-        getRoot: () => ({
-          getInput: () => ({
-            filters: [],
-          }),
-        }),
-      } as any,
-      // triggerContext: {
-      // filters: [],
-      // } as any
-    });
-    expect(result2).toBe(false);
   });
 });
 
@@ -125,42 +104,41 @@ describe('execute()', () => {
       const error = expectError(() =>
         action.execute({
           embeddable: getEmbeddable(),
-          triggerContext: {},
         } as any)
       );
       expect(error).toBeInstanceOf(Error);
     });
 
-    test('updates filter input on success', () => {
+    test('updates filter input on success', async done => {
       const action = new ApplyFilterAction();
       const [embeddable, root] = getEmbeddable();
 
-      action.execute({
+      await action.execute({
         embeddable,
-        triggerContext: {
-          filters: ['FILTER' as any],
-        },
+        filters: ['FILTER' as any],
       });
 
       expect(root.updateInput).toHaveBeenCalledTimes(1);
       expect(root.updateInput.mock.calls[0][0]).toMatchObject({
         filters: ['FILTER'],
       });
+
+      done();
     });
 
-    test('checks if action isCompatible', () => {
+    test('checks if action isCompatible', async done => {
       const action = new ApplyFilterAction();
       const spy = jest.spyOn(action, 'isCompatible');
       const [embeddable] = getEmbeddable();
 
-      action.execute({
+      await action.execute({
         embeddable,
-        triggerContext: {
-          filters: ['FILTER' as any],
-        },
+        filters: ['FILTER' as any],
       });
 
       expect(spy).toHaveBeenCalledTimes(1);
+
+      done();
     });
   });
 });

src/legacy/core_plugins/embeddable_api/public/np_ready/public/lib/actions/apply_filter_action.ts

@@ -20,14 +20,18 @@
 import { i18n } from '@kbn/i18n';
 import { Filter } from '@kbn/es-query';
 import { IEmbeddable, EmbeddableInput } from '../embeddables';
-import { Action, ActionContext } from './action';
+import { Action } from './action';
 import { IncompatibleActionError } from '../errors';
 
 export const APPLY_FILTER_ACTION = 'APPLY_FILTER_ACTION';
 
 type RootEmbeddable = IEmbeddable<EmbeddableInput & { filters: Filter[] }>;
+interface ActionContext {
+  embeddable: IEmbeddable;
+  filters: Filter[];
+}
 
-export class ApplyFilterAction extends Action<IEmbeddable, { filters: Filter[] }> {
+export class ApplyFilterAction extends Action<ActionContext> {
   public readonly type = APPLY_FILTER_ACTION;
 
   constructor() {
@@ -40,30 +44,27 @@ export class ApplyFilterAction extends Action<IEmbeddable, { filters: Filter[] }
     });
   }
 
-  public async isCompatible(context: ActionContext<IEmbeddable, { filters: Filter[] }>) {
+  public async isCompatible(context: ActionContext) {
+    if (context.embeddable === undefined) {
+      return false;
+    }
     const root = context.embeddable.getRoot() as RootEmbeddable;
-    return Boolean(
-      root.getInput().filters !== undefined &&
-        context.triggerContext &&
-        context.triggerContext.filters !== undefined
-    );
+    return Boolean(root.getInput().filters !== undefined && context.filters !== undefined);
   }
 
-  public execute({
-    embeddable,
-    triggerContext,
-  }: ActionContext<IEmbeddable, { filters: Filter[] }>) {
-    if (!triggerContext) {
-      throw new Error('Applying a filter requires a filter as context');
+  public async execute({ embeddable, filters }: ActionContext) {
+    if (!filters || !embeddable) {
+      throw new Error('Applying a filter requires a filter and embeddable as context');
     }
-    const root = embeddable.getRoot() as RootEmbeddable;
 
-    if (!this.isCompatible({ triggerContext, embeddable })) {
+    if (!(await this.isCompatible({ embeddable, filters }))) {
       throw new IncompatibleActionError();
     }
 
+    const root = embeddable.getRoot() as RootEmbeddable;
+
     root.updateInput({
-      filters: triggerContext.filters,
+      filters,
     });
   }
 }

src/legacy/core_plugins/embeddable_api/public/np_ready/public/lib/actions/edit_panel_action.ts

@@ -18,13 +18,18 @@
  */
 
 import { i18n } from '@kbn/i18n';
-import { Action, ActionContext } from './action';
+import { Action } from './action';
 import { GetEmbeddableFactory, ViewMode } from '../types';
 import { EmbeddableFactoryNotFoundError } from '../errors';
+import { IEmbeddable } from '../embeddables';
 
 export const EDIT_PANEL_ACTION_ID = 'editPanel';
 
-export class EditPanelAction extends Action {
+interface ActionContext {
+  embeddable: IEmbeddable;
+}
+
+export class EditPanelAction extends Action<ActionContext> {
   public readonly type = EDIT_PANEL_ACTION_ID;
   constructor(private readonly getEmbeddableFactory: GetEmbeddableFactory) {
     super(EDIT_PANEL_ACTION_ID);
@@ -56,8 +61,8 @@ export class EditPanelAction extends Action {
     return Boolean(canEditEmbeddable && inDashboardEditMode);
   }
 
-  public execute() {
-    return undefined;
+  public async execute() {
+    return;
   }
 
   public getHref({ embeddable }: ActionContext): string {

src/legacy/core_plugins/embeddable_api/public/np_ready/public/lib/actions/index.ts

@@ -17,6 +17,6 @@
  * under the License.
  */
 
-export { Action, ActionContext } from './action';
+export { Action } from './action';
 export * from './apply_filter_action';
 export * from './edit_panel_action';