[ML] Migrate to React BrowserRouter and Kibana provided History. by walterra · Pull Request #71941 · elastic/kibana

walterra · 2020-07-15T19:55:15Z

Summary

Part of #72013

Migrate to React BrowserRouter and Kibana provided History including a fallback to redirect legacy hash based URLs.
Migrate breadcrumbs away from hash based URLs.
Make sure relative custom urls still work after migration.

Checklist

Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2020-07-16T08:11:16Z

Pinging @elastic/ml-ui (:ml)

jgowdyelastic · 2020-07-16T08:42:08Z

Clicking the root Machine Learning breadcrumb at the top of the page causes the app to crash.

it looks like it's navigating away from ML and clearing our dependency cache.

jgowdyelastic · 2020-07-16T08:50:39Z

The first link click once the ML app has loaded throws this error in the console.

e.g. once the overview page has loaded, navigate to Anomaly Detection

walterra · 2020-07-16T15:25:27Z

@jgowdyelastic Did the following fixes:

The legacy hash url check is now using useEffect() to avoid the "pure function" warning (2c02364).
Breadcrumbs have been rewritten to no longer use hash based URLs (635e32e) and to not trigger full page refreshes (85fec20).
Fixed navigation on the job type selection page (1d9c8b4).

jgowdyelastic · 2020-07-17T13:23:39Z

x-pack/plugins/ml/public/application/jobs/new_job/pages/job_type/page.tsx

  };

+  const navigateToPath = (path: string | undefined) =>
+    navigateToUrl(getUrlForApp('ml', { path: `${path}${getUrlParams()}` }));


seeing as we use this function combination a lot, could this be wrapped in a provider function that just takes the path and supplied by useMlContext ?
it should also use PLUGIN_ID from kibana/x-pack/plugins/ml/common/constants/app.ts

As discussed, decided to do it in this PR, we now have useNavigateToPath() based on useMlContext(), available via contexts/kibana. Update in fad85ec.

jgowdyelastic

LGTM

qn895 · 2020-07-29T17:59:23Z

@qn895 Looks like the navigation issue with Index Management wasn't introduced in this PR, see issue #72854 and PR #72863. For the other navigation problem I created a separate issue since it wasn't introduced by this PR: #73261.

Thanks for creating those issues 👍

Changes LGTM

walterra · 2020-07-30T04:57:06Z

@elasticmachine merge upstream

peteharverson · 2020-07-30T10:51:58Z

x-pack/plugins/ml/public/application/util/custom_url_utils.ts

-    window.open(fullUrl, '_blank');
+    // If `url` is a relative path, we need to prefix the base path.
+    if (url.charAt(0) !== '/') {
+      url = `${basePath}${isKibanaUrl(urlConfig) ? '/app/' : '/'}${url}`;


Looks like we need to add the security app URLs to this check. Currently this is trying to open up e.g.

1:46 http://localhost:5601/pgy/security/hosts/ml-hosts?_g=()&query=(query:%27process.name%20:%20%22filebeat%22%27,language:kuery)&timerange=(global:(linkTo:!(timeline),timerange:(from:%272019-08-20T23:00:00.000Z%27,kind:absolute,to:%272019-08-21T22:59:59.999Z%27)),timeline:(linkTo:!(global),timerange:(from:%272019-08-20T23:00:00.000Z%27,kind:absolute,to:%272019-08-21T22:59:59.999Z%27)))

which is missing the app part before security.

Pushed a fix in 1e64ab3.

…l-migrate-router

peteharverson

Tested latest edits and LGTM

walterra · 2020-07-30T14:10:19Z

@elasticmachine merge upstream

walterra · 2020-07-30T15:11:59Z

@elasticmachine merge upstream

walterra · 2020-07-30T16:30:52Z

@elasticmachine merge upstream

kibanamachine · 2020-07-30T18:12:46Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: f2c47e3

Build metrics

@kbn/optimizer bundle module count

id	value	diff	baseline
ml	1225	+1	1224

page load bundle size

id	value	diff	baseline
ml	4.3MB	+9.8KB	4.3MB

History

💔 Build #65549 failed 12afcba
💔 Build #65533 failed dbce65d
💛 Build #65499 was flaky b7248f5
💚 Build #65455 succeeded 8ddbc80
💔 Build #65320 failed ed7d8d7

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

…stic#71941) - Migrate to React BrowserRouter and Kibana provided History including a fallback to redirect legacy hash based URLs. - Migrate breadcrumbs away from hash based URLs. - Make sure relative custom urls still work after migration.

* master: (54 commits) [ML] Migrate to React BrowserRouter and Kibana provided History. (elastic#71941) [Discover] Improve saveSearch functional test handling (elastic#73626) [Metrics UI] Fix all threshold alert conditions disappearing due to alert prefill (elastic#73708) [Metrics UI] Fix alert previews of ungrouped alerts (elastic#73735) [SIEM] Fixes "include building block button" to operate (elastic#73900) [Metrics UI] Fix alert management to open without refresh (elastic#73739) [Security Solution][Lists] - Tests cleanup and remove unnecessary import (elastic#73865) [Ingest Management] main branch uses epr-snapshot. Others production (elastic#73555) [Canvas][tech-debt] Fix SVG not shrinking vertically properly (elastic#73867) [Maps] upgrade turf (elastic#73816) [Security Solution][Telemetry] Concurrent telemetry requests (elastic#73558) [Security Solution][Exceptions] - Update how nested entries are displayed in exceptions viewer (elastic#73745) [Security Solution][Exceptions] Adds autocomplete workaround for .text fields (elastic#73761) [Metrics UI] Fix previewing of No Data results (elastic#73753) Closes elastic#72914 by hiding anomaly detection settings links when the ml plugin is disabled. (elastic#73638) [Ingest Manager] Fix config selection in enrollment flyout from config list page (elastic#73833) [DOCS] Fixes typo in Alerting actions (elastic#73756) [APM] fixes linking errors to ML and Discover (elastic#73758) Handle promise rejections when building artifacts (elastic#73831) [Security Solution][Detections] Change from sha1 to sha256 (elastic#73741) ...

) (#73919) - Migrate to React BrowserRouter and Kibana provided History including a fallback to redirect legacy hash based URLs. - Migrate breadcrumbs away from hash based URLs. - Make sure relative custom urls still work after migration.

* master: (38 commits) [Discover] Context unskip date nanos functional tests (elastic#73781) [ML] Migrate to React BrowserRouter and Kibana provided History. (elastic#71941) [Discover] Improve saveSearch functional test handling (elastic#73626) [Metrics UI] Fix all threshold alert conditions disappearing due to alert prefill (elastic#73708) [Metrics UI] Fix alert previews of ungrouped alerts (elastic#73735) [SIEM] Fixes "include building block button" to operate (elastic#73900) [Metrics UI] Fix alert management to open without refresh (elastic#73739) [Security Solution][Lists] - Tests cleanup and remove unnecessary import (elastic#73865) [Ingest Management] main branch uses epr-snapshot. Others production (elastic#73555) [Canvas][tech-debt] Fix SVG not shrinking vertically properly (elastic#73867) [Maps] upgrade turf (elastic#73816) [Security Solution][Telemetry] Concurrent telemetry requests (elastic#73558) [Security Solution][Exceptions] - Update how nested entries are displayed in exceptions viewer (elastic#73745) [Security Solution][Exceptions] Adds autocomplete workaround for .text fields (elastic#73761) [Metrics UI] Fix previewing of No Data results (elastic#73753) Closes elastic#72914 by hiding anomaly detection settings links when the ml plugin is disabled. (elastic#73638) [Ingest Manager] Fix config selection in enrollment flyout from config list page (elastic#73833) [DOCS] Fixes typo in Alerting actions (elastic#73756) [APM] fixes linking errors to ML and Discover (elastic#73758) Handle promise rejections when building artifacts (elastic#73831) ...

[ML] Migrate to React BrowserRouter and Kibana provided History.

3301ff5

walterra added :ml Feature:Anomaly Detection ML anomaly detection refactoring v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:Data Frame Analytics ML data frame analytics features v7.10.0 labels Jul 15, 2020

walterra self-assigned this Jul 15, 2020

walterra added 3 commits July 16, 2020 09:17

Merge branch 'master' into ml-migrate-router

2cb3f93

[ML] Fixes redirects and adds comments for router related components.

b8e6abf

[ML] Pass history on as part of PageDependencies.

686e6d8

walterra mentioned this pull request Jul 16, 2020

[ML] Switch from HashRouter to platform-provided BrowserRouter #72013

Closed

9 tasks

walterra marked this pull request as ready for review July 16, 2020 08:11

walterra requested a review from a team as a code owner July 16, 2020 08:11

walterra requested review from jgowdyelastic, peteharverson and qn895 July 16, 2020 08:12

walterra added 4 commits July 16, 2020 10:59

[ML] Use useEffect for legacy redirect.

2c02364

[ML] Migrate breadcrumbs away from hash based urls.

635e32e

[ML] Fixes breadcrumb full refreshes.

85fec20

[ML] Fixes job creation card navigation.

1d9c8b4

walterra added 3 commits July 17, 2020 09:01

[ML] Fix translations.

38994c1

Merge branch 'master' into ml-migrate-router

4859521

[ML] Fix job creation button.

9580a79

jgowdyelastic reviewed Jul 17, 2020

View reviewed changes

jgowdyelastic approved these changes Jul 29, 2020

View reviewed changes

Merge branch 'master' into ml-migrate-router

5cfa29a

qn895 approved these changes Jul 29, 2020

View reviewed changes

walterra added 2 commits July 29, 2020 20:19

Merge branch 'master' into ml-migrate-router

7c43179

[ML] Tweak button code.

ed7d8d7

elasticmachine and others added 2 commits July 29, 2020 22:57

Merge branch 'master' into ml-migrate-router

8ddbc80

Merge branch 'master' into ml-migrate-router

ab504a5

peteharverson reviewed Jul 30, 2020

View reviewed changes

walterra added 4 commits July 30, 2020 13:14

[ML] Add security to isKibanaUrl check.

1e64ab3

Merge branch 'ml-migrate-router' of github.com:walterra/kibana into m…

d329b40

…l-migrate-router

[ML] Fix security link.

a4e5a2b

[Ml] Add legacy SIEM link to isKibanaUrl().

b7248f5

peteharverson approved these changes Jul 30, 2020

View reviewed changes

Merge branch 'master' into ml-migrate-router

dbce65d

Merge branch 'master' into ml-migrate-router

12afcba

Merge branch 'master' into ml-migrate-router

f2c47e3

walterra merged commit 145f2ee into elastic:master Jul 31, 2020

walterra deleted the ml-migrate-router branch July 31, 2020 08:25

walterra mentioned this pull request Jul 31, 2020

[7.x] [ML] Migrate to React BrowserRouter and Kibana provided History. (#71941) #73919

Merged

walterra mentioned this pull request Aug 4, 2020

[ML] Fix analytics list on management page. #74254

Merged

2 tasks

Conversation

walterra commented Jul 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

For maintainers

Uh oh!

elasticmachine commented Jul 16, 2020

Uh oh!

jgowdyelastic commented Jul 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jgowdyelastic commented Jul 16, 2020

Uh oh!

walterra commented Jul 16, 2020

Uh oh!

jgowdyelastic Jul 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

walterra Jul 17, 2020

Choose a reason for hiding this comment

Uh oh!

jgowdyelastic left a comment

Choose a reason for hiding this comment

Uh oh!

qn895 commented Jul 29, 2020

Uh oh!

walterra commented Jul 30, 2020

Uh oh!

peteharverson Jul 30, 2020

Choose a reason for hiding this comment

Uh oh!

walterra Jul 30, 2020

Choose a reason for hiding this comment

Uh oh!

peteharverson left a comment

Choose a reason for hiding this comment

Uh oh!

walterra commented Jul 30, 2020

Uh oh!

walterra commented Jul 30, 2020

Uh oh!

walterra commented Jul 30, 2020

Uh oh!

kibanamachine commented Jul 30, 2020

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

page load bundle size

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

walterra commented Jul 15, 2020 •

edited

Loading

jgowdyelastic commented Jul 16, 2020 •

edited

Loading

jgowdyelastic Jul 17, 2020 •

edited

Loading