-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[ML] Migrate machine learning URLs to BrowserRouter format for APM, Security, and Infra #78209
Conversation
…security solution
…place-hash # Conflicts: # x-pack/plugins/apm/public/components/shared/Links/MachineLearningLinks/MLJobLink.test.tsx # x-pack/plugins/apm/public/components/shared/Links/MachineLearningLinks/MLLink.test.tsx # x-pack/plugins/apm/public/components/shared/Links/MachineLearningLinks/MLLink.tsx # x-pack/plugins/apm/public/components/shared/Links/MachineLearningLinks/useTimeSeriesExplorerHref.test.ts # x-pack/plugins/apm/public/components/shared/Links/MachineLearningLinks/useTimeSeriesExplorerHref.ts # x-pack/plugins/ml/public/plugin.ts
x-pack/plugins/apm/public/components/shared/Links/MachineLearningLinks/MLLink.tsx
Show resolved
Hide resolved
…place-hash # Conflicts: # x-pack/plugins/ml/public/plugin.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -113,6 +114,12 @@ export const createStartServicesMock = (): StartServices => { | |||
}, | |||
security, | |||
storage, | |||
ml: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not critical, but: it would be nice if ml provided a mock for this contract!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Ryland! Updated here 1c390c1
💚 Build SucceededMetrics [docs]@kbn/optimizer bundle module count
async chunks size
page load bundle size
History
To update your PR or re-run it, just comment with: |
* master: (36 commits) [I18n] fix eui tokens (elastic#78951) Changed the color of the confirm button in trusted app deletion dialog. (elastic#78768) Make the actual Vislib import async (elastic#78949) Fix ML conditionals links Cypress tests (elastic#78568) [Drilldowns][Docs] Communicate the visualization types that support drilldowns (elastic#78761) [UX] Improve page-load axis (elastic#78392) [SECURITY SOLUTIONS] Map embeddable working with index patterns selection (elastic#78610) Data plugin README (elastic#78750) [TSVB] Request validation error: [panels.0.series.0.metrics.0.percentiles.1.value] (elastic#79009) fixing api test (elastic#78964) [Task names in TaskManager] Rename "telemetry" to "usage" (elastic#78129) [Loggers] Rename "telemetry" to "usage" (elastic#78130) [Usage Collection] [schema] `ui_metric` (elastic#78827) [Actions][Jira] Set parent issue for Sub-task issue type (elastic#78772) [Discover] Unskip doc link functional test (elastic#78600) [ML] Functional tests - stabilize calendar edit tests (elastic#78950) [UX] Improve page responsive (elastic#78759) [QA][Code Coverage] Team Assignment Docs Update (elastic#78890) [ML] Migrate machine learning URLs to BrowserRouter format for APM, Security, and Infra (elastic#78209) [ts] enable "resolveJsonModule" and disable existing failures (elastic#78855) ...
Friendly reminder: Looks like this PR hasn’t been backported yet. |
…ecurity, and Infra (elastic#78209) * [ML] Adds ability to pass multiple jobIds to job management url * [ML][APM] Update links to jobs management page for MLLink and LegacyJobsCallout * [ML][APM] Update useTimeSeriesExplorerHref * [ML][APM] Update tests * [ML][APM] Move test from useTimeSeriesExplorerHref to MLJobLink.test.tsx * [ML][Infra] Update ML links in infra to non-hash paths * [ML] Move MlUrlGenerator registration outside of licensing block for security solution * [ML][Security] Update ml links in security * [ML][APM] Update test snapshots * [ML][APM] Update snapshots * [ML][Security solution] Update tests * [ML] Update MLLink to include globalState * [ML] Update useTimeSeriesExplorerHref * [ML] Update apm and security_solution to use useMlHref hook * [ML] Update APM to use useUrlParams hook, update security solution hook * [ML] Update tests, fix duplicate imports * [ML] Update imports, remove ml exports to shared cause it's not needed [ML] Add import * [ML] Update snapshot * [ML] Fix warnings for jobs_table.test.tsx Co-authored-by: Elastic Machine <[email protected]> # Conflicts: # x-pack/plugins/apm/public/utils/testHelpers.tsx
…PM, Security, and Infra (#78209) (#79487) Co-authored-by: Elastic Machine <[email protected]>
Tested in Safari, Chrome, and Firefox: |
Summary
This PR replaces machine learning URLs from the previous hash-based paths to the new paths as part of the migration to BrowserRouter #72013. Updates were made to APM, Infra, and Security solutions plugins.
Changes include:
mlUrlGenerator
on ML plugin’s contract. Then consumers would declare that they need ml as a dependency and would access mlUrlGenerator from deps.mlUrlGenerator passed in start or setup hooks.basePath/app/ml
ifml.urlGenerator
is not available to generate the urls for some reasons.Checklist
Delete any items that are not applicable to this PR.