Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic uiActions & license support #68507

Merged
merged 22 commits into from
Jun 26, 2020
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
10d1323
uiActions & license support
Dosant Jun 8, 2020
03792f0
fixes
Dosant Jun 8, 2020
2f879a4
Merge branch 'master' of github.com:elastic/kibana into dev/uiactions…
Dosant Jun 11, 2020
9636632
review
Dosant Jun 11, 2020
8852123
improve sorting, improving showing error in a list
Dosant Jun 11, 2020
794a9d7
a bit of space of error icon
Dosant Jun 11, 2020
be2449d
Merge branch 'master' into dev/uiactions-license
elasticmachine Jun 15, 2020
0787389
Merge branch 'master' of github.com:elastic/kibana into dev/uiactions…
Dosant Jun 17, 2020
a9efdc0
Merge branch 'dev/uiactions-license' of github.com:Dosant/kibana into…
Dosant Jun 17, 2020
41ff5c3
refactor using `registerActionHook` approach
Dosant Jun 17, 2020
5eea2b8
fix ts
Dosant Jun 17, 2020
f77c4cf
Merge branch 'master' of github.com:elastic/kibana into dev/uiactions…
Dosant Jun 17, 2020
fa41858
@streamich review
Dosant Jun 17, 2020
a58692c
Merge branch 'master' of github.com:elastic/kibana into dev/uiactions…
Dosant Jun 23, 2020
e655e68
Merge branch 'master' of github.com:elastic/kibana into dev/uiactions…
Dosant Jun 24, 2020
fa1dcf2
fix i18n
Dosant Jun 24, 2020
2278f55
fix i18n
Dosant Jun 24, 2020
9c4a462
Merge branch 'master' into dev/uiactions-license
elasticmachine Jun 25, 2020
a9ee244
Merge branch 'master' of github.com:elastic/kibana into dev/uiactions…
Dosant Jun 26, 2020
8144666
Merge branch 'dev/uiactions-license' of github.com:Dosant/kibana into…
Dosant Jun 26, 2020
491fee6
simplify uiActions license implementation after Stacey review
Dosant Jun 26, 2020
7f02c0f
clean up
Dosant Jun 26, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions src/plugins/ui_actions/public/actions/action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ export interface Action<Context extends {} = {}, T = ActionType>
*/
readonly type: T;

/**
* Extension point for extra information on Action
*/
readonly enhancements?: unknown;

/**
* Optional EUI icon type that can be displayed along with the title.
*/
Expand Down Expand Up @@ -85,10 +90,29 @@ export interface ActionDefinition<Context extends object = object>
*/
readonly type?: ActionType;

/**
* Extension point for extra information on Action
*/
readonly enhancements?: unknown;

/**
* Executes the action.
*/
execute(context: Context): Promise<void>;
}

export type ActionContext<A> = A extends ActionDefinition<infer Context> ? Context : never;

/**
* Extension point allows to hook into {@link Action} methods
*/
export interface ActionHook {
/**
* onIsCompatible hook allows to extend action.isCompatible with additional compatibility checks
* It doesn't override original isCompatible checks, so it isn't possible to _force_ compatibility with this hook.
*
* @param action
* @param context
*/
onIsCompatible?: (action: Action, context: unknown) => boolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is context needed? I don't see it used and since it's unknown, probably not that helpful anyway.

Should it return Promise<boolean> to match isCompatible fns?

}
11 changes: 10 additions & 1 deletion src/plugins/ui_actions/public/actions/action_internal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,16 @@ const defaultActionDef: ActionDefinition = {

describe('ActionInternal', () => {
test('can instantiate from action definition', () => {
const action = new ActionInternal(defaultActionDef);
const action = new ActionInternal(defaultActionDef, () => []);
expect(action.id).toBe('test-action');
});

test('can hook into action methods', async () => {
const action = new ActionInternal(defaultActionDef, () => [
{
onIsCompatible: () => false,
},
]);
expect(await action.isCompatible({})).toBe(false);
});
});
12 changes: 10 additions & 2 deletions src/plugins/ui_actions/public/actions/action_internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,21 @@

// @ts-ignore
import React from 'react';
import { Action, ActionContext as Context, ActionDefinition } from './action';
import { Action, ActionContext as Context, ActionDefinition, ActionHook } from './action';
import { Presentable } from '../util/presentable';
import { uiToReactComponent } from '../../../kibana_react/public';
import { ActionType } from '../types';

/**
* @internal
*/
export class ActionInternal<A extends ActionDefinition = ActionDefinition>
implements Action<Context<A>>, Presentable<Context<A>> {
constructor(public readonly definition: A) {}
constructor(public readonly definition: A, private readonly getActionHooks: () => ActionHook[]) {}

public readonly id: string = this.definition.id;
public readonly type: ActionType = this.definition.type || '';
public readonly enhancements: unknown = this.definition.enhancements;
public readonly order: number = this.definition.order || 0;
public readonly MenuItem? = this.definition.MenuItem;
public readonly ReactMenuItem? = this.MenuItem ? uiToReactComponent(this.MenuItem) : undefined;
Expand All @@ -54,6 +58,10 @@ export class ActionInternal<A extends ActionDefinition = ActionDefinition>
}

public async isCompatible(context: Context<A>): Promise<boolean> {
for (const { onIsCompatible } of this.getActionHooks()) {
if (onIsCompatible && !onIsCompatible(this, context)) return false;
}

if (!this.definition.isCompatible) return true;
return await this.definition.isCompatible(context);
}
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/ui_actions/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,4 @@ export {
applyFilterTrigger,
} from './triggers';
export { TriggerContextMapping, TriggerId, ActionContextMapping, ActionType } from './types';
export { ActionByType } from './actions';
export { ActionByType, ActionInternal } from './actions';
1 change: 1 addition & 0 deletions src/plugins/ui_actions/public/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const createSetupContract = (): Setup => {
registerAction: jest.fn(),
registerTrigger: jest.fn(),
unregisterAction: jest.fn(),
registerActionHook: jest.fn(),
};
return setupContract;
};
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/ui_actions/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,10 @@ export type UiActionsSetup = Pick<
| 'registerAction'
| 'registerTrigger'
| 'unregisterAction'
| 'registerActionHook'
>;

export type UiActionsStart = PublicMethodsOf<UiActionsService>;
export type UiActionsStart = Omit<PublicMethodsOf<UiActionsService>, 'registerActionHook'>;
streamich marked this conversation as resolved.
Show resolved Hide resolved

export class UiActionsPlugin implements Plugin<UiActionsSetup, UiActionsStart> {
private readonly service = new UiActionsService();
Expand Down
54 changes: 53 additions & 1 deletion src/plugins/ui_actions/public/service/ui_actions_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import { UiActionsService } from './ui_actions_service';
import { Action, ActionInternal, createAction } from '../actions';
import { createHelloWorldAction } from '../tests/test_samples';
import { ActionRegistry, TriggerRegistry, TriggerId, ActionType } from '../types';
import { TriggerRegistry, TriggerId, ActionType, ActionRegistry } from '../types';
import { Trigger } from '../triggers';

// Casting to ActionType or TriggerId is a hack - in a real situation use
Expand Down Expand Up @@ -491,4 +491,56 @@ describe('UiActionsService', () => {
);
});
});

describe('action hooks', () => {
describe('onIsCompatible', () => {
test('can make action incompatible', async () => {
const service = new UiActionsService();
service.registerActionHook({
onIsCompatible: () => false,
});
service.registerActionHook({
onIsCompatible: () => true,
});
const actionDef: Action = {
id: 'action1',
order: 1,
type: 'type1' as ActionType,
execute: async () => {},
getDisplayName: () => 'test',
getIconType: () => '',
isCompatible: async () => true,
};
service.registerAction(actionDef);
const action = service.getAction(actionDef.id);

const isCompatible = await action.isCompatible({});
expect(isCompatible).toBe(false);
});

test("can't force compatibility of incompatible action", async () => {
const service = new UiActionsService();
service.registerActionHook({
onIsCompatible: () => true,
});
service.registerActionHook({
onIsCompatible: () => true,
});
const actionDef: Action = {
id: 'action1',
order: 1,
type: 'type1' as ActionType,
execute: async () => {},
getDisplayName: () => 'test',
getIconType: () => '',
isCompatible: async () => false,
};
service.registerAction(actionDef);
const action = service.getAction(actionDef.id);

const isCompatible = await action.isCompatible({});
expect(isCompatible).toBe(false);
});
});
});
});
15 changes: 12 additions & 3 deletions src/plugins/ui_actions/public/service/ui_actions_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ import {
TriggerToActionsRegistry,
TriggerId,
TriggerContextMapping,
ActionHooksRegistry,
} from '../types';
import { ActionInternal, Action, ActionDefinition, ActionContext } from '../actions';
import { ActionInternal, Action, ActionDefinition, ActionContext, ActionHook } from '../actions';
import { Trigger, TriggerContext } from '../triggers/trigger';
import { TriggerInternal } from '../triggers/trigger_internal';
import { TriggerContract } from '../triggers/trigger_contract';
Expand All @@ -37,21 +38,26 @@ export interface UiActionsServiceParams {
* A 1-to-N mapping from `Trigger` to zero or more `Action`.
*/
readonly triggerToActions?: TriggerToActionsRegistry;

readonly actionHooks?: ActionHooksRegistry;
}

export class UiActionsService {
protected readonly triggers: TriggerRegistry;
protected readonly actions: ActionRegistry;
protected readonly triggerToActions: TriggerToActionsRegistry;
protected readonly actionHooksRegistry: ActionHooksRegistry;

constructor({
triggers = new Map(),
actions = new Map(),
triggerToActions = new Map(),
actionHooks = [],
}: UiActionsServiceParams = {}) {
this.triggers = triggers;
this.actions = actions;
this.triggerToActions = triggerToActions;
this.actionHooksRegistry = actionHooks;
}

public readonly registerTrigger = (trigger: Trigger) => {
Expand Down Expand Up @@ -82,7 +88,7 @@ export class UiActionsService {
throw new Error(`Action [action.id = ${definition.id}] already registered.`);
}

const action = new ActionInternal(definition);
const action = new ActionInternal(definition, () => this.actionHooksRegistry);

this.actions.set(action.id, action);

Expand Down Expand Up @@ -146,6 +152,10 @@ export class UiActionsService {
this.attachAction(triggerId, action.id);
};

public readonly registerActionHook = (hook: ActionHook) => {
this.actionHooksRegistry.push(hook);
};

public readonly getAction = <T extends ActionDefinition>(
id: string
): Action<ActionContext<T>> => {
Expand Down Expand Up @@ -220,7 +230,6 @@ export class UiActionsService {
for (const [key, value] of this.actions.entries()) actions.set(key, value);
for (const [key, value] of this.triggerToActions.entries())
triggerToActions.set(key, [...value]);

return new UiActionsService({ triggers, actions, triggerToActions });
};
}
2 changes: 2 additions & 0 deletions src/plugins/ui_actions/public/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,12 @@ import { Filter } from '../../data/public';
import { SELECT_RANGE_TRIGGER, VALUE_CLICK_TRIGGER, APPLY_FILTER_TRIGGER } from './triggers';
import { IEmbeddable } from '../../embeddable/public';
import { RangeSelectTriggerContext, ValueClickTriggerContext } from '../../embeddable/public';
import { ActionHook } from './actions';

export type TriggerRegistry = Map<TriggerId, TriggerInternal<any>>;
export type ActionRegistry = Map<string, ActionInternal>;
export type TriggerToActionsRegistry = Map<TriggerId, string[]>;
export type ActionHooksRegistry = ActionHook[];

const DEFAULT_TRIGGER = '';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ export class DashboardToUrlDrilldown implements Drilldown<Config, ActionContext>

public readonly order = 8;

readonly minimalLicense = 'gold'; // example of minimal license support
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only thing required for consumer 🙌


public readonly getDisplayName = () => 'Go to URL (example)';

public readonly euiIcon = 'link';
Expand Down
14 changes: 13 additions & 1 deletion x-pack/plugins/licensing/public/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { BehaviorSubject } from 'rxjs';
import { LicensingPluginSetup } from './types';
import { LicensingPluginSetup, LicensingPluginStart } from './types';
import { licenseMock } from '../common/licensing.mock';

const createSetupMock = () => {
Expand All @@ -18,7 +18,19 @@ const createSetupMock = () => {
return mock;
};

const createStartMock = () => {
const license = licenseMock.createLicense();
const mock: jest.Mocked<LicensingPluginStart> = {
license$: new BehaviorSubject(license),
refresh: jest.fn(),
};
mock.refresh.mockResolvedValue(license);

return mock;
};

export const licensingMock = {
createSetup: createSetupMock,
createStart: createStartMock,
...licenseMock,
};
3 changes: 2 additions & 1 deletion x-pack/plugins/ui_actions_enhanced/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
"configPath": ["xpack", "ui_actions_enhanced"],
"requiredPlugins": [
"embeddable",
"uiActions"
"uiActions",
"licensing"
],
"server": false,
"ui": true
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* 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 {
Action,
ActionType,
UiActionsActionDefinition as ActionDefinition,
} from '../../../../../src/plugins/ui_actions/public';
import { LicenseType } from '../../../licensing/public';

export interface ActionEnhanced<Context extends {} = {}, T = ActionType>
extends Action<Context, T> {
readonly enhancements: {
/**
* Minimal license
* if missing, then no license limitations
*/
minimalLicense?: LicenseType;
};
}

export interface ActionEnhancedDefinition<Context extends object = object>
extends ActionDefinition<Context> {
readonly enhancements: {
/**
* Minimal license
* if missing, then no license limitations
*/
minimalLicense?: LicenseType;
};
}

export type ActionEnhancedContext<A> = A extends ActionEnhancedDefinition<infer Context>
? Context
: never;

export function isEnhancedAction<Context extends {} = {}, T = ActionType>(
action: Action<Context, T>
): action is ActionEnhanced<Context, T> {
return !!action.enhancements;
}
12 changes: 12 additions & 0 deletions x-pack/plugins/ui_actions_enhanced/public/actions/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* 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.
*/

export {
ActionEnhanced,
ActionEnhancedDefinition,
ActionEnhancedContext,
isEnhancedAction,
} from './action_enhanced';
Loading