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

[uiActions] notify action usage #76294

Merged
merged 5 commits into from
Sep 2, 2020
Merged

Conversation

Dosant
Copy link
Contributor

@Dosant Dosant commented Aug 31, 2020

Summary

closes #72019

Notify feature usage, when dynamic actions with specified license requirements is executed:

  1. notifyFeature is called inside action.execute. For example, in case of drill-downs, nofifyFeature is called when end user drilling down into that chart.
  2. notifyUsage is not called when dynamic action (or drill down) is created. (I think this is fine, most likely user who creates
    a drill down will go and test it end-to-end from a chart anyways and that is when notifyUsage is going to be called.
  3. There is an edge case for actions which implement getHref: When drilldown popup is opened and then user used an action with right click + open in a new tab, then action.execute() and notifyUsage is not called. I think this is fine, as it shouldn't me a common use case comparing to regular click.
  4. There is a known issue when registering same featureName more then once: Licensing: allow multiple registration of the same feature #76272
  5. I also renamed licence -> license, it was inconsistent: https://www.grammarly.com/blog/licence-license/

Checklist

Delete any items that are not applicable to this PR.

@Dosant Dosant requested a review from pgayvallet August 31, 2020 15:31
@Dosant
Copy link
Contributor Author

Dosant commented Sep 1, 2020

@elasticmachine merge upstream

@Dosant Dosant added Feature:Drilldowns Embeddable panel Drilldowns Feature:License Feature:UIActions UI actions. These are client side only, not related to the server side actions.. release_note:skip Skip the PR/issue when compiling release notes Team:AppArch v7.10.0 v8.0.0 labels Sep 1, 2020
@Dosant Dosant marked this pull request as ready for review September 1, 2020 09:00
@Dosant Dosant requested review from a team as code owners September 1, 2020 09:00
@elasticmachine
Copy link
Contributor

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

@Dosant Dosant requested a review from streamich September 1, 2020 09:00
Copy link
Contributor

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

code LGTM

@Dosant
Copy link
Contributor Author

Dosant commented Sep 2, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

page load bundle size

id value diff baseline
uiActionsEnhanced 163.5KB +2.5KB 161.0KB

History

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

@Dosant Dosant merged commit 25c1762 into elastic:master Sep 2, 2020
Dosant added a commit to Dosant/kibana that referenced this pull request Sep 2, 2020
Notify feature usage when dynamic actions with specified license requirements are executed

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Dosant added a commit that referenced this pull request Sep 2, 2020
Notify feature usage when dynamic actions with specified license requirements are executed

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Drilldowns Embeddable panel Drilldowns Feature:License Feature:UIActions UI actions. These are client side only, not related to the server side actions.. release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[uiActions] notify feature usage
5 participants