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

[APM] Use license management locator #158278

Merged

Conversation

MiriamAparicio
Copy link
Contributor

@MiriamAparicio MiriamAparicio commented May 23, 2023

Closes #153014

@MiriamAparicio MiriamAparicio force-pushed the 153014-license-managment branch from eea4770 to c2472bc Compare May 25, 2023 14:07
@MiriamAparicio MiriamAparicio added Team:APM All issues that need APM UI Team support release_note:skip Skip the PR/issue when compiling release notes labels May 25, 2023
@MiriamAparicio MiriamAparicio marked this pull request as ready for review May 25, 2023 14:09
@MiriamAparicio MiriamAparicio requested a review from a team as a code owner May 25, 2023 14:09
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:APM)

@MiriamAparicio MiriamAparicio marked this pull request as draft May 25, 2023 14:45
@MiriamAparicio MiriamAparicio force-pushed the 153014-license-managment branch from 3827f83 to 7ee2069 Compare May 31, 2023 10:47
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
apm 3.5MB 3.5MB -44.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
securitySolution 416 420 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
securitySolution 500 504 +4
total +6

History

  • 💔 Build #130200 failed 3827f83bb425f3f8546cd1c4f9c28d0ca75195e6
  • 💔 Build #129831 failed eea4770205709f0710f58681c453c15da6959202
  • 💔 Build #129635 failed 19ad09a9dc39f51134c5bfce5db2ede4568f43a0

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

@MiriamAparicio MiriamAparicio marked this pull request as ready for review May 31, 2023 12:51
Copy link
Contributor

@yngrdyn yngrdyn left a comment

Choose a reason for hiding this comment

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

LGTM 🌴

? licenseManagement?.locator?.useUrl({
page: 'dashboard',
})
: licensePageUrl;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

hi @sqren,
We talk about what will happen if licenseManagement.locator is undefined, but I'm actually not sure if we want to fall back to the licensePageUrl = '/app/management/stack/license_management'
what else we can do in this case? any suggestions?

Copy link
Member

@sorenlouv sorenlouv Jun 2, 2023

Choose a reason for hiding this comment

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

In the case where license management plugin is disabled we don't want to link to the license page, because it'll display a 404. In that case I think we can just not show the link.
That being said, I don't think this will ever happen. If the license management is not enabled, the user ought to have something like an enterprise license, with access to everything.

@MiriamAparicio MiriamAparicio merged commit 5d9432a into elastic:main Jun 6, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jun 6, 2023
@MiriamAparicio MiriamAparicio deleted the 153014-license-managment branch June 6, 2023 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support v8.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[APM] Check is License Management is enabled when linking to it
6 participants