[Tines connector] SubActions framework changes (#142475)

* validators and template render

* frontend use_sub_action changes

* rollback custom validations in favor of 142376

* types fix

* rollback test

* extract render template function type

* fix type

* use abort controller in useSubAction hook

* renderTemplate in register test

* [CI] Auto-commit changed files from 'node scripts/build_plugin_list_docs'

* improve test

* register test improved

* test the renderedTemrenderedTemplate return value

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

x-pack/plugins/actions/server/sub_action_framework/README.md

@@ -351,6 +351,7 @@ plugins.actions.registerSubActionConnectorType({
   minimumLicenseRequired: 'platinum' as const,
   schema: { config: TestConfigSchema, secrets: TestSecretsSchema },
   Service: TestSubActionConnector,
+  renderParameterTemplates: renderTestTemplate
 });
 ```
 

x-pack/plugins/actions/server/sub_action_framework/register.test.ts

@@ -18,6 +18,9 @@ import {
 import { register } from './register';
 
 describe('Registration', () => {
+  const renderedVariables = { body: '' };
+  const mockRenderParameterTemplates = jest.fn().mockReturnValue(renderedVariables);
+
   const connector = {
     id: '.test',
     name: 'Test',
@@ -28,6 +31,7 @@ describe('Registration', () => {
       secrets: TestSecretsSchema,
     },
     Service: TestSubActionConnector,
+    renderParameterTemplates: mockRenderParameterTemplates,
   };
 
   const actionTypeRegistry = actionTypeRegistryMock.create();
@@ -35,7 +39,6 @@ describe('Registration', () => {
   const logger = loggingSystemMock.createLogger();
 
   beforeEach(() => {
-    jest.resetAllMocks();
     jest.clearAllMocks();
   });
 
@@ -54,7 +57,27 @@ describe('Registration', () => {
       minimumLicenseRequired: connector.minimumLicenseRequired,
       supportedFeatureIds: connector.supportedFeatureIds,
       validate: expect.anything(),
-      executor: expect.anything(),
+      executor: expect.any(Function),
+      renderParameterTemplates: expect.any(Function),
     });
   });
+
+  it('registers the renderParameterTemplates correctly', async () => {
+    register<TestConfig, TestSecrets>({
+      actionTypeRegistry,
+      connector,
+      configurationUtilities: mockedActionsConfig,
+      logger,
+    });
+
+    const params = {};
+    const variables = {};
+    const actionId = 'action-id';
+
+    const { renderParameterTemplates } = actionTypeRegistry.register.mock.calls[0][0];
+    const rendered = renderParameterTemplates?.(params, variables, actionId);
+
+    expect(mockRenderParameterTemplates).toHaveBeenCalledWith(params, variables, actionId);
+    expect(rendered).toBe(renderedVariables);
+  });
 });

x-pack/plugins/actions/server/sub_action_framework/register.ts

@@ -54,5 +54,6 @@ export const register = <Config extends ActionTypeConfig, Secrets extends Action
     supportedFeatureIds: connector.supportedFeatureIds,
     validate: validators,
     executor,
+    renderParameterTemplates: connector.renderParameterTemplates,
   });
 };

x-pack/plugins/actions/server/sub_action_framework/sub_action_connector.ts

@@ -8,16 +8,9 @@
 import { isPlainObject, isEmpty } from 'lodash';
 import { Type } from '@kbn/config-schema';
 import { Logger } from '@kbn/logging';
-import axios, {
-  AxiosInstance,
-  AxiosRequestConfig,
-  AxiosResponse,
-  Method,
-  AxiosError,
-  AxiosRequestHeaders,
-} from 'axios';
+import axios, { AxiosInstance, AxiosResponse, AxiosError, AxiosRequestHeaders } from 'axios';
 import { ActionsConfigurationUtilities } from '../actions_config';
-import { SubAction } from './types';
+import { SubAction, SubActionRequestParams } from './types';
 import { ServiceParams } from './types';
 import * as i18n from './translations';
 import { request } from '../lib/axios_utils';
@@ -123,11 +116,7 @@ export abstract class SubActionConnector<Config, Secrets> {
     responseSchema,
     headers,
     ...config
-  }: {
-    url: string;
-    responseSchema: Type<R>;
-    method?: Method;
-  } & AxiosRequestConfig): Promise<AxiosResponse<R>> {
+  }: SubActionRequestParams<R>): Promise<AxiosResponse<R>> {
     try {
       this.assertURL(url);
       this.ensureUriAllowed(url);

x-pack/plugins/actions/server/sub_action_framework/types.ts

@@ -6,12 +6,18 @@
  */
 
 import type { Type } from '@kbn/config-schema';
-import { Logger } from '@kbn/logging';
+import type { Logger } from '@kbn/logging';
 import type { LicenseType } from '@kbn/licensing-plugin/common/types';
 
-import { ActionsConfigurationUtilities } from '../actions_config';
-import { ActionTypeParams, Services, ValidatorType as ValidationSchema } from '../types';
-import { SubActionConnector } from './sub_action_connector';
+import type { Method, AxiosRequestConfig } from 'axios';
+import type { ActionsConfigurationUtilities } from '../actions_config';
+import type {
+  ActionTypeParams,
+  RenderParameterTemplates,
+  Services,
+  ValidatorType as ValidationSchema,
+} from '../types';
+import type { SubActionConnector } from './sub_action_connector';
 
 export interface ServiceParams<Config, Secrets> {
   /**
@@ -26,6 +32,12 @@ export interface ServiceParams<Config, Secrets> {
   services: Services;
 }
 
+export type SubActionRequestParams<R> = {
+  url: string;
+  responseSchema: Type<R>;
+  method?: Method;
+} & AxiosRequestConfig;
+
 export type IService<Config, Secrets> = new (
   params: ServiceParams<Config, Secrets>
 ) => SubActionConnector<Config, Secrets>;
@@ -68,6 +80,7 @@ export interface SubActionConnectorType<Config, Secrets> {
   };
   validators?: Array<ConfigValidator<Config> | SecretsValidator<Secrets>>;
   Service: IService<Config, Secrets>;
+  renderParameterTemplates?: RenderParameterTemplates<ExecutorParams>;
 }
 
 export interface ExecutorParams extends ActionTypeParams {

x-pack/plugins/actions/server/types.ts

@@ -108,6 +108,12 @@ export interface ActionValidationService {
   isUriAllowed(uri: string): boolean;
 }
 
+export type RenderParameterTemplates<Params extends ActionTypeParams> = (
+  params: Params,
+  variables: Record<string, unknown>,
+  actionId?: string
+) => Params;
+
 export interface ActionType<
   Config extends ActionTypeConfig = ActionTypeConfig,
   Secrets extends ActionTypeSecrets = ActionTypeSecrets,
@@ -126,11 +132,7 @@ export interface ActionType<
     connector?: (config: Config, secrets: Secrets) => string | null;
   };
 
-  renderParameterTemplates?(
-    params: Params,
-    variables: Record<string, unknown>,
-    actionId?: string
-  ): Params;
+  renderParameterTemplates?: RenderParameterTemplates<Params>;
 
   executor: ExecutorType<Config, Secrets, Params, ExecutorResultData>;
 }

x-pack/plugins/stack_connectors/server/connector_types/stack/es_index/index.ts

@@ -148,7 +148,7 @@ async function executor(
 function renderParameterTemplates(
   params: ActionParamsType,
   variables: Record<string, unknown>,
-  actionId: string
+  actionId?: string
 ): ActionParamsType {
   const { documents, indexOverride } = renderMustacheObject<ActionParamsType>(params, variables);
 

x-pack/plugins/triggers_actions_ui/public/application/hooks/use_sub_action.test.tsx

@@ -5,7 +5,7 @@
  * 2.0.
  */
 
-import { renderHook } from '@testing-library/react-hooks';
+import { act, renderHook } from '@testing-library/react-hooks';
 import { useKibana } from '../../common/lib/kibana';
 import { useSubAction } from './use_sub_action';
 
@@ -19,10 +19,14 @@ describe('useSubAction', () => {
     subAction: 'test',
     subActionParams: {},
   };
+  useKibanaMock().services.http.post = jest
+    .fn()
+    .mockImplementation(() => Promise.resolve({ status: 'ok', data: {} }));
+  let abortSpy = jest.spyOn(window, 'AbortController');
 
   beforeEach(() => {
     jest.clearAllMocks();
-    useKibanaMock().services.http.post = jest.fn().mockResolvedValue({ status: 'ok', data: {} });
+    abortSpy.mockRestore();
   });
 
   it('init', async () => {
@@ -30,7 +34,6 @@ describe('useSubAction', () => {
     await waitForNextUpdate();
 
     expect(result.current).toEqual({
-      isError: false,
       isLoading: false,
       response: {},
       error: null,
@@ -43,30 +46,136 @@ describe('useSubAction', () => {
 
     expect(useKibanaMock().services.http.post).toHaveBeenCalledWith(
       '/api/actions/connector/test-id/_execute',
-      { body: '{"params":{"subAction":"test","subActionParams":{}}}' }
+      {
+        body: '{"params":{"subAction":"test","subActionParams":{}}}',
+        signal: new AbortController().signal,
+      }
     );
   });
 
+  it('executes sub action if subAction parameter changes', async () => {
+    const { rerender, waitForNextUpdate } = renderHook(useSubAction, { initialProps: params });
+    await waitForNextUpdate();
+
+    expect(useKibanaMock().services.http.post).toHaveBeenCalledTimes(1);
+
+    await act(async () => {
+      rerender({ ...params, subAction: 'test-2' });
+      await waitForNextUpdate();
+    });
+
+    expect(useKibanaMock().services.http.post).toHaveBeenCalledTimes(2);
+  });
+
+  it('executes sub action if connectorId parameter changes', async () => {
+    const { rerender, waitForNextUpdate } = renderHook(useSubAction, { initialProps: params });
+    await waitForNextUpdate();
+
+    expect(useKibanaMock().services.http.post).toHaveBeenCalledTimes(1);
+
+    await act(async () => {
+      rerender({ ...params, connectorId: 'test-id-2' });
+      await waitForNextUpdate();
+    });
+
+    expect(useKibanaMock().services.http.post).toHaveBeenCalledTimes(2);
+  });
+
+  it('returns memoized response if subActionParams changes but values are equal', async () => {
+    const { result, rerender, waitForNextUpdate } = renderHook(useSubAction, {
+      initialProps: { ...params, subActionParams: { foo: 'bar' } },
+    });
+    await waitForNextUpdate();
+
+    expect(useKibanaMock().services.http.post).toHaveBeenCalledTimes(1);
+    const previous = result.current;
+
+    await act(async () => {
+      rerender({ ...params, subActionParams: { foo: 'bar' } });
+      await waitForNextUpdate();
+    });
+
+    expect(result.current.response).toBe(previous.response);
+    expect(useKibanaMock().services.http.post).toHaveBeenCalledTimes(1);
+  });
+
+  it('executes sub action if subActionParams changes and values are not equal', async () => {
+    const { result, rerender, waitForNextUpdate } = renderHook(useSubAction, {
+      initialProps: { ...params, subActionParams: { foo: 'bar' } },
+    });
+    await waitForNextUpdate();
+
+    expect(useKibanaMock().services.http.post).toHaveBeenCalledTimes(1);
+    const previous = result.current;
+
+    await act(async () => {
+      rerender({ ...params, subActionParams: { foo: 'baz' } });
+      await waitForNextUpdate();
+    });
+
+    expect(result.current.response).not.toBe(previous.response);
+    expect(useKibanaMock().services.http.post).toHaveBeenCalledTimes(2);
+  });
+
   it('returns an error correctly', async () => {
-    useKibanaMock().services.http.post = jest.fn().mockRejectedValue(new Error('error executing'));
+    const error = new Error('error executing');
+    useKibanaMock().services.http.post = jest.fn().mockRejectedValueOnce(error);
 
     const { result, waitForNextUpdate } = renderHook(() => useSubAction(params));
     await waitForNextUpdate();
 
     expect(result.current).toEqual({
-      isError: true,
       isLoading: false,
       response: undefined,
-      error: expect.anything(),
+      error,
     });
   });
 
-  it('does not execute if params are null', async () => {
-    const { result } = renderHook(() => useSubAction(null));
+  it('should not set error if aborted', async () => {
+    const firstAbortCtrl = new AbortController();
+    firstAbortCtrl.abort();
+    abortSpy = jest.spyOn(window, 'AbortController').mockReturnValueOnce(firstAbortCtrl);
+
+    const error = new Error('error executing');
+    useKibanaMock().services.http.post = jest.fn().mockRejectedValueOnce(error);
+
+    const { result } = renderHook(() => useSubAction(params));
+
+    expect(result.current.error).toBe(null);
+  });
+
+  it('should abort on unmount', async () => {
+    const firstAbortCtrl = new AbortController();
+    abortSpy = jest.spyOn(window, 'AbortController').mockReturnValueOnce(firstAbortCtrl);
+
+    const { unmount } = renderHook(useSubAction, { initialProps: params });
+
+    unmount();
+
+    expect(firstAbortCtrl.signal.aborted).toEqual(true);
+  });
+
+  it('should abort on parameter change', async () => {
+    const firstAbortCtrl = new AbortController();
+    abortSpy = jest.spyOn(window, 'AbortController').mockImplementation(() => {
+      abortSpy.mockRestore();
+      return firstAbortCtrl;
+    });
+
+    const { rerender } = renderHook(useSubAction, { initialProps: params });
+
+    await act(async () => {
+      rerender({ ...params, connectorId: 'test-id-2' });
+    });
+
+    expect(firstAbortCtrl.signal.aborted).toEqual(true);
+  });
+
+  it('does not execute if disabled', async () => {
+    const { result } = renderHook(() => useSubAction({ ...params, disabled: true }));
 
     expect(useKibanaMock().services.http.post).not.toHaveBeenCalled();
     expect(result.current).toEqual({
-      isError: false,
       isLoading: false,
       response: undefined,
       error: null,

x-pack/plugins/triggers_actions_ui/public/application/hooks/use_sub_action.tsx

@@ -5,140 +5,135 @@
  * 2.0.
  */
 
-import { useCallback, useEffect, useReducer, useRef } from 'react';
-import { ActionTypeExecutorResult } from '@kbn/actions-plugin/common';
+import deepEqual from 'fast-deep-equal';
+import { Reducer, useEffect, useReducer, useRef } from 'react';
 import { useKibana } from '../../common/lib/kibana';
 import { executeAction } from '../lib/action_connector_api';
 
-interface UseSubActionParams {
-  connectorId: string;
-  subAction: string;
-  subActionParams: Record<string, unknown>;
+interface UseSubActionParams<P> {
+  connectorId?: string;
+  subAction?: string;
+  subActionParams?: P;
+  disabled?: boolean;
 }
 
-interface SubActionsState<T> {
+interface SubActionsState<R> {
   isLoading: boolean;
-  isError: boolean;
-  response: unknown | undefined;
+  response: R | undefined;
   error: Error | null;
 }
 
-enum SubActionsActionsList {
-  INIT,
-  LOADING,
+const enum SubActionsActionsList {
+  START,
   SUCCESS,
   ERROR,
 }
 
-type Action<T> =
-  | { type: SubActionsActionsList.INIT }
-  | { type: SubActionsActionsList.LOADING }
-  | { type: SubActionsActionsList.SUCCESS; payload: T | undefined }
+type Action<R> =
+  | { type: SubActionsActionsList.START }
+  | { type: SubActionsActionsList.SUCCESS; payload: R | undefined }
   | { type: SubActionsActionsList.ERROR; payload: Error | null };
 
-const dataFetchReducer = <T,>(state: SubActionsState<T>, action: Action<T>): SubActionsState<T> => {
+const dataFetchReducer = <R,>(state: SubActionsState<R>, action: Action<R>): SubActionsState<R> => {
   switch (action.type) {
-    case SubActionsActionsList.INIT:
-      return {
-        ...state,
-        isLoading: false,
-        isError: false,
-      };
-
-    case SubActionsActionsList.LOADING:
+    case SubActionsActionsList.START:
       return {
         ...state,
         isLoading: true,
-        isError: false,
+        error: null,
       };
-
     case SubActionsActionsList.SUCCESS:
       return {
         ...state,
         response: action.payload,
         isLoading: false,
-        isError: false,
+        error: null,
       };
-
     case SubActionsActionsList.ERROR:
       return {
         ...state,
         error: action.payload,
         isLoading: false,
-        isError: true,
       };
-
     default:
       return state;
   }
 };
 
-export const useSubAction = <T,>(params: UseSubActionParams | null) => {
+const useMemoParams = <P,>(subActionsParams: P): P => {
+  const paramsRef = useRef<P>(subActionsParams);
+  if (!deepEqual(paramsRef.current, subActionsParams)) {
+    paramsRef.current = subActionsParams;
+  }
+  return paramsRef.current;
+};
+
+export const useSubAction = <P, R>({
+  connectorId,
+  subAction,
+  subActionParams,
+  disabled = false,
+}: UseSubActionParams<P>) => {
   const { http } = useKibana().services;
-  const [state, dispatch] = useReducer(dataFetchReducer, {
-    isError: false,
+  const [{ isLoading, response, error }, dispatch] = useReducer<
+    Reducer<SubActionsState<R>, Action<R>>
+  >(dataFetchReducer, {
     isLoading: false,
     response: undefined,
     error: null,
   });
+  const memoParams = useMemoParams(subActionParams);
 
-  const abortCtrl = useRef(new AbortController());
-  const isMounted = useRef(false);
-
-  const executeSubAction = useCallback(async () => {
-    if (params == null) {
+  useEffect(() => {
+    if (disabled || !connectorId || !subAction) {
       return;
     }
 
-    const { connectorId, subAction, subActionParams } = params;
-    dispatch({ type: SubActionsActionsList.INIT });
+    const abortCtrl = new AbortController();
+    let isMounted = true;
 
-    try {
-      abortCtrl.current.abort();
-      abortCtrl.current = new AbortController();
-      dispatch({ type: SubActionsActionsList.LOADING });
+    const executeSubAction = async () => {
+      try {
+        dispatch({ type: SubActionsActionsList.START });
 
-      const res = (await executeAction({
-        id: connectorId,
-        http,
-        params: {
-          subAction,
-          subActionParams,
-        },
-      })) as ActionTypeExecutorResult<T>;
+        const res = await executeAction<R>({
+          id: connectorId,
+          params: {
+            subAction,
+            subActionParams: memoParams,
+          },
+          http,
+          signal: abortCtrl.signal,
+        });
 
-      if (isMounted.current) {
-        if (res.status && res.status === 'error') {
+        if (isMounted) {
+          if (res.status && res.status === 'ok') {
+            dispatch({ type: SubActionsActionsList.SUCCESS, payload: res.data });
+          } else {
+            dispatch({
+              type: SubActionsActionsList.ERROR,
+              payload: new Error(`${res.message}: ${res.serviceMessage}`),
+            });
+          }
+        }
+        return res.data;
+      } catch (e) {
+        if (isMounted && !abortCtrl.signal.aborted) {
           dispatch({
             type: SubActionsActionsList.ERROR,
-            payload: new Error(`${res.message}: ${res.serviceMessage}`),
+            payload: e,
           });
         }
-
-        dispatch({ type: SubActionsActionsList.SUCCESS, payload: res.data });
       }
-
-      return res.data;
-    } catch (e) {
-      if (isMounted.current) {
-        dispatch({
-          type: SubActionsActionsList.ERROR,
-          payload: e,
-        });
-      }
-    }
-  }, [http, params]);
-
-  useEffect(() => {
-    isMounted.current = true;
-    executeSubAction();
-    return () => {
-      isMounted.current = false;
-      abortCtrl.current.abort();
     };
-  }, [executeSubAction]);
 
-  return {
-    ...state,
-  };
+    executeSubAction();
+
+    return () => {
+      isMounted = false;
+      abortCtrl.abort();
+    };
+  }, [memoParams, disabled, connectorId, subAction, http]);
+
+  return { isLoading, response, error };
 };

x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api/execute.test.ts

@@ -19,13 +19,14 @@ describe('executeAction', () => {
       stringParams: 'someString',
       numericParams: 123,
     };
+    const signal = new AbortController().signal;
 
     http.post.mockResolvedValueOnce({
       connector_id: id,
       status: 'ok',
     });
 
-    const result = await executeAction({ id, http, params });
+    const result = await executeAction({ id, http, params, signal });
     expect(result).toEqual({
       actionId: id,
       status: 'ok',
@@ -35,6 +36,7 @@ describe('executeAction', () => {
         "/api/actions/connector/12%2F3/_execute",
         Object {
           "body": "{\\"params\\":{\\"stringParams\\":\\"someString\\",\\"numericParams\\":123}}",
+          "signal": AbortSignal {},
         },
       ]
     `);

x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api/execute.ts

@@ -6,33 +6,36 @@
  */
 
 import { HttpSetup } from '@kbn/core/public';
-import { ActionTypeExecutorResult, RewriteRequestCase } from '@kbn/actions-plugin/common';
+import { ActionTypeExecutorResult, AsApiContract } from '@kbn/actions-plugin/common';
 import { BASE_ACTION_API_PATH } from '../../constants';
 
-const rewriteBodyRes: RewriteRequestCase<ActionTypeExecutorResult<unknown>> = ({
+const rewriteBodyRes = <R>({
   connector_id: actionId,
   service_message: serviceMessage,
   ...res
-}) => ({
+}: AsApiContract<ActionTypeExecutorResult<R>>): ActionTypeExecutorResult<R> => ({
   ...res,
   actionId,
   serviceMessage,
 });
 
-export async function executeAction({
+export async function executeAction<R>({
   id,
   params,
   http,
+  signal,
 }: {
   id: string;
   http: HttpSetup;
   params: Record<string, unknown>;
-}): Promise<ActionTypeExecutorResult<unknown>> {
-  const res = await http.post<Parameters<typeof rewriteBodyRes>[0]>(
+  signal?: AbortSignal;
+}): Promise<ActionTypeExecutorResult<R>> {
+  const res = await http.post<AsApiContract<ActionTypeExecutorResult<R>>>(
     `${BASE_ACTION_API_PATH}/connector/${encodeURIComponent(id)}/_execute`,
     {
       body: JSON.stringify({ params }),
+      signal,
     }
   );
-  return rewriteBodyRes(res);
+  return rewriteBodyRes<R>(res);
 }