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.

Sign up for GitHub

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

Jump to bottom

[ML] Fixes handling of unrecognised URLs #133157

Merged
merged 8 commits into from
Jun 1, 2022
Merged

[ML] Fixes handling of unrecognised URLs #133157

merged 8 commits into from
Jun 1, 2022

Conversation

darnautov
Copy link
Contributor

@darnautov darnautov commented May 30, 2022 •
edited
Loading

Summary

Fixes #132675

If the ML app doesn't recognize a route, redirects the user to the Overview page and displays a warning banner (similar to the Dashboard and Discover apps) for 15 seconds.

May-31-2022 15-06-35

Checklist

@darnautov darnautov added release_note:fix :ml Team:ML Team label for ML (also use :ml) v8.3.0 labels May 30, 2022
@darnautov darnautov requested a review from a team as a code owner May 30, 2022 15:29
@darnautov darnautov self-assigned this May 30, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@darnautov
Copy link
Contributor Author

@elasticmachine merge upstream

jgowdyelastic
jgowdyelastic approved these changes Jun 1, 2022
View reviewed changes
Copy link
Member

@jgowdyelastic jgowdyelastic left a comment

Choose a reason for hiding this comment

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

Tested, LGTM

@darnautov darnautov requested a review from qn895 June 1, 2022 09:52
@jgowdyelastic jgowdyelastic self-requested a review June 1, 2022 10:45
@jgowdyelastic
Copy link
Member

jgowdyelastic commented Jun 1, 2022 •
edited
Loading

The redirect warning appears when running with a basic license.
I don't think it should appear here.

image

Also, and I know this isn't caused by this PR, the Anomaly Detection menu item should be greyed out like the others.

@darnautov
Copy link
Contributor Author

The redirect warning appears when running with a basic license. I don't think it should appear here.

image

Also, and I know this isn't caused by this PR, the Anomaly Detection menu item should be greyed out like the others.

it's been fixed in 2bad074, did you pull the latest changes?

@darnautov
Copy link
Contributor Author

@jgowdyelastic the Anomaly Detection section for the basic licence has been fixed in 5de9fc5

szabosteve
szabosteve approved these changes Jun 1, 2022
View reviewed changes
Copy link
Contributor

@szabosteve szabosteve left a comment

Choose a reason for hiding this comment

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

One nit, otherwise UI text LGTM!

x-pack/plugins/ml/public/application/routing/use_active_route.tsx Outdated Show resolved Hide resolved
@darnautov darnautov enabled auto-merge (squash) June 1, 2022 12:12
@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
ml 3.3MB 3.3MB +846.0B

History

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

cc @darnautov

@darnautov darnautov merged commit 6d290df into elastic:main Jun 1, 2022
@darnautov darnautov deleted the ml-132675-not-found-page branch June 1, 2022 14:01
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jun 3, 2022
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 133157 locally

@darnautov darnautov added auto-backport Deprecated - use backport:version if exact versions are needed v8.4.0 and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Jun 3, 2022
kibanamachine pushed a commit that referenced this pull request Jun 3, 2022
@darnautov
[ML] Fixes handling of unrecognised URLs (#133157)
093652e 
* not found page

* replace not found page with a warning banner

* add check for non-empty pathname

* functional test

* disabled anomaly detection section

* update message

(cherry picked from commit 6d290df)
@kibanamachine kibanamachine mentioned this pull request Jun 3, 2022
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.3

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Jun 3, 2022
@kibanamachine @darnautov
[ML] Fixes handling of unrecognised URLs (#133157) (#133531)
f37b558 
* not found page

* replace not found page with a warning banner

* add check for non-empty pathname

* functional test

* disabled anomaly detection section

* update message

(cherry picked from commit 6d290df)

Co-authored-by: Dima Arnautov <[email protected]>
@rudolf rudolf mentioned this pull request Aug 22, 2022
Closed
@darnautov darnautov mentioned this pull request Aug 23, 2022
Closed
@jgowdyelastic jgowdyelastic mentioned this pull request Oct 14, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@szabosteve szabosteve szabosteve approved these changes

@jgowdyelastic jgowdyelastic jgowdyelastic approved these changes

@alvarezmelissa87 alvarezmelissa87 Awaiting requested review from alvarezmelissa87

@qn895 qn895 Awaiting requested review from qn895

Assignees

@darnautov darnautov

Labels
auto-backport Deprecated - use backport:version if exact versions are needed :ml release_note:fix Team:ML Team label for ML (also use :ml) v8.3.0 v8.4.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[ML] Missing 404 handling in Machine Learning plugin.
6 participants
@darnautov @elasticmachine @jgowdyelastic @kibana-ci @kibanamachine @szabosteve