Dynamic uiActions & license support

[Dosant]

Summary

Needed for #55324

This pr adds convenient license support to dynamic uiActions in x-pack.
Works for actions created with action factories & drilldowns.

All is needed from consumer is to specify optional

minimalLicense?: LicenseType; on ActionFactoryDefinition or DrilldownDefinition

Implements this nice state for action_wizard and drilldows list ⬇️

1. In case there is an available action factory, but not sufficient license, we still show that action factory, but in disabled state. On hover there is a tooltip. Also a link to subscriptions page in the header (like in alerting)
   

2. In case when there is existing Drilldowns with higher license, then current license, we show this warning in a drilldown list:

To try

yarn start --run-examples. Example url drilldown is available from gold license.

Checklist

Delete any items that are not applicable to this PR.

• Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Documentation was added for features that require explanation or tutorials
• Unit or functional tests were updated or added to match the most common scenarios

For maintainers

• This was checked for breaking API changes and was labeled appropriately

[Dosant]

only thing required for consumer 🙌

[elasticmachine]

Pinging @elastic/kibana-app-arch (Team:AppArch)

[pgayvallet]

LGTM for licensing plugin change once the createStartMock return type is fixed.

[mdefazio]

We have previously avoided the lock to signify options behind a license. We should however still keep the tooltip (I don't think we need the title 'License' though). In alerting, there is a link to the right of the 'Actions' header that goes to the Licensing page on Elastic.co

As for the management screen, let's bring the disabled indicator closer to the title (and show the message Insufficient license level in a tooltip).

[mdefazio]

This looks good! Only tiny nit is to increase the spacing between the name and error icon (Try $euiSizeM)

[Dosant]

@elasticmachine merge upstream

[streamich]

I'm thinking maybe we should simplify and instead of creating a way of "enhancing" ActionInternal leave it internal as-is, but allow users to "hook" into specific functionality of actions. Something like this:

And ui_actions_enhanced would use it as follows:

plugins.uiActions.registerActionHook({
  isCompatible: (action) => isCompatibleLicense(action),
});

I think it should simplify the architecture and then we don't need to export internal ActionInternal class. Do you see problems with this approach?

[streamich]

Chatted with @Dosant, he said he could explore the "hook" approach if @ppisljar, @stacey-gammon you are OK with it.

Idea being that in the "hook" approach we get:

1. Less and simpler code.
2. Actions are still registered as before when they are registered, instead definitions being registered and actions created in start life-cycle.
3. We don't inroduce setup and start life-cycles into, UiActionsService. As before, UiActionsService does not need to know about those life-cycles, actions are simply registered whenever they are registered.
4. We don't need to export ActionInternal class from ui_actions.

It could looks something like this:

interface ActionDefinition {
  // Add extra information to action definition from X-Pack
  enhancements: unknown;
}

// Register action "hooks" in X-Pack.
plugins.ui_actions.registerActionHook({
  isCompatible: (action, context) => isLicenseValid(action.enhancements),
});

[ppisljar]

code LGTM

[Dosant]

@elasticmachine merge upstream

[stacey-gammon]

From the perspective of a consumer of the ui_actions services, it's not clear to me how to use this functionality, and I think I can easily get into trouble with it.

I first tried to do something like:

    deps.uiActions.registerActionHook({
      onIsCompatible: (action: Action) => {
        return action.isCompatible({}) && ...? // 
      },
    });

Can't do that because onIsCompatible is sync, while isCompatible is async. Even if it was async, and I did the above, I'd hit a circular loop. It'd also pass the typescript check, but because of contra-variance, it'd throw a runtime exception on actions that require more than an empty object to be passed in as context.

Maybe I'm forgetting some context, but is there a reason we can't just do something like this, and rely on our current public APIs to support this:

const createHelloWorldAction = (getStartServices: () => Promise<licensing>) => {
  return createAction({
    type: ACTION_HELLO_WORLD,
    isCompatible: () => return (await getStartServices()).licensing.license$.getValue().hasAtLeast('gold');
    getDisplayName: () => 'Hello World!',
    execute: async () => {},
});

const helloWorldAction = createHelloWorldAction(async () => ({
   licensing: (await core.getStartServices())[0].licensing,
}));

uiActions.registerAction(helloWorldAction);

It's less refactored, but also introduces no extra public APIs.

[stacey-gammon]

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?

[stacey-gammon]

Also... don't let me block this PR, especially since I'm going on vacation for a couple weeks. I just wanted to bring up some thoughts about maybe simplifying/minimizing public APIs. Also just trying to focus on our public APIs from a public consumer standpoint.

We often add public APIs for very specific use cases, and try to make them general/flexible enough for future use cases, but don't spend enough time thinking about examples/tests for the general cases. What would an example plugin look like to showcase how another plugin can use enhancements and action hooks?

Are we introducing overly flexible APIs that will cause us issues later on the line and maintenance headaches?

Do we want to support giving the ability for one plugin to turn off another plugins actions?

Do we have an answer for how to avoid clashes if multiple plugins add custom state/functionality onto the enhancements object?

If there is an easier way to solve this, without introducing new APIs, might be worth considering, to the support of our dev principles:

• Don't share code prematurely
• Don't abstract code prenamturely

Just some thoughts, but ultimately, I leave it to the team to make the final decision!

[Dosant]

@stacey-gammon, thanks for this review! I made a step back and reverted all changes to static uiActions level. I agree that we don't have right now a use case for that functionality.

I left only minimalLicense change on actionFactory and drilldown level.

---

Initially I was inspired looking at this #63170, What I did in this pr wasn't solving that particular issue, but I thought of similar use cases, where new static action registered in x-pack could benefit of simple minimalRequiredLicense option. And I agree it is not needed now for URL drilldown.

---

@streamich, @ppisljar, Since Stacey enjoying vacation now, could you please re-review this simpler version? 🙏

[streamich]

LGTM

[stacey-gammon]

So much simpler, really like the changes. 🎉

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 7f02c0f

Build metrics

✅ unchanged

History

• 💚 Build #56661 succeeded 491fee6
• 💚 Build #56260 succeeded 9c4a462
• 💚 Build #55995 succeeded 2278f55
• 💔 Build #55577 failed a58692c
• 💚 Build #54746 succeeded fa41858

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream