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

[NP] Get rid of usage redirectWhenMissing service #59777

Merged
merged 18 commits into from
Mar 17, 2020

Conversation

sulemanof
Copy link
Contributor

@sulemanof sulemanof commented Mar 10, 2020

Summary

A part of #58999.

This cutover redirectWhenMissing to kibana_utils.
Affects shimmed plugins: dashboard, visualize, discover.
Removes the reference in x-pack/legacy/plugins/monitoring since unused.

This contains couple of fixes:

  • fix all redirects which check error instance of SavedObjectNotFound, look at this comment for details;
  • fix redirection when trying to load a visualization with an invalid type, added a proper warning message, redirect to the listing page;
  • fix broken redirection in scope of the same plugin, e.x. opening visualization with non-exists id ->
  1. open any saved visualiztion
  2. change the id in the url and press enter (or copy the url with wrong id and open in a new browser tab)
  3. a toast appears, but redirection is not happening

redirect

Checklist

Delete any items that are not applicable to this PR.

For maintainers

…n_missing

# Conflicts:
#	src/legacy/core_plugins/kibana/public/discover/np_ready/angular/discover.js
#	src/legacy/core_plugins/kibana/public/visualize/np_ready/legacy_app.js
@sulemanof sulemanof requested review from flash1293 and kertal March 11, 2020 10:01
@sulemanof sulemanof marked this pull request as ready for review March 11, 2020 10:39
@sulemanof sulemanof requested a review from a team March 11, 2020 10:39
@sulemanof sulemanof requested a review from a team as a code owner March 11, 2020 10:39
@sulemanof sulemanof requested a review from a team March 11, 2020 10:39
@sulemanof sulemanof added Feature:NP Migration release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.7.0 v8.0.0 labels Mar 11, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@sulemanof
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@igoristic igoristic left a comment

Choose a reason for hiding this comment

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

Looks good from Stack Monitoring 👍 Thanks for doing this, was already going to do it eventually: #59673

@sulemanof
Copy link
Contributor Author

@elasticmachine merge upstream

…n_missing

# Conflicts:
#	src/legacy/core_plugins/kibana/public/discover/kibana_services.ts
#	src/legacy/core_plugins/kibana/public/visualize/np_ready/editor/editor.js
Copy link
Member

@kertal kertal 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, tested locally in Chrome, works, the management redirection issue is no regression, I'll create an issue for that. Many thanks for taking care of ancient 🐛

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@kertal kertal dismissed flash1293’s stale review March 17, 2020 09:38

Changes was addressed , reviewer in his well deserved holidays

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

@sulemanof sulemanof merged commit eddbdc8 into elastic:master Mar 17, 2020
@sulemanof sulemanof deleted the shim/redirect_when_missing branch March 17, 2020 11:02
sulemanof added a commit to sulemanof/kibana that referenced this pull request Mar 17, 2020
* Move redirect_when_missing to kibana utils

* Replace redirectWhenMissing in dashboard

* Replace redirectWhenMissing in discover

* Remove redirect in monitoring

* Remove extra import

* Move invalid vistype check into editor.js

* Mock the history folder

* Fix redirect when missing index or saved object

* Move history to discover services

* Use redirect to listing page

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 17, 2020
* master: (30 commits)
  [TSVB] fix text color when using custom background color (elastic#60261)
  Fix import to timefilter from in TSVB (elastic#60296)
  [NP] Get rid of usage redirectWhenMissing service (elastic#59777)
  [SIEM] Fix Timeline footer styling (elastic#59587)
  [ML] Fixes to error handling for analytics jobs and file data viz (elastic#60249)
  Give better stack traces for Unhandled Promise Rejection warnings (elastic#60235)
  resolves elastic#58905 (elastic#60120)
  Added variables button for text fields in Pagerduty component. (elastic#60189)
  adds test that action vars are rendered for alert action parms (elastic#60310)
  Closes 59786 by removing the update toast (elastic#60172)
  [EPM] Packages list tabs (elastic#60167)
  Added message variables button for Webhook body form field (elastic#60174)
  Revert "adds new test (elastic#60064)"
  [Maps] move MapSavedObject type out of telemetry (elastic#60127)
  [Reporting] Fix error handling for job handler in route (elastic#60161)
  [Endpoint] TEST: verify alerts page header says 'Alerts' (elastic#60206)
  EMT-248: implement ack resource to accept event payload to acknowledge agent actions (elastic#60218)
  Migrate dual validated range (elastic#59689)
  Embeddable triggers (elastic#58440)
  [Endpoint] Sample data generator CLI script (elastic#59952)
  ...
sulemanof added a commit that referenced this pull request Mar 17, 2020
* Move redirect_when_missing to kibana utils

* Replace redirectWhenMissing in dashboard

* Replace redirectWhenMissing in discover

* Remove redirect in monitoring

* Remove extra import

* Move invalid vistype check into editor.js

* Mock the history folder

* Fix redirect when missing index or saved object

* Move history to discover services

* Use redirect to listing page

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

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 17, 2020
* master: (51 commits)
  do not update cell background if is label cell (elastic#60308)
  FTR configurable test users (elastic#52431)
  [Reporting] Wholesale moves client to newest-platform (elastic#58945)
  [Ingest] Support `show_user` package registry flag (elastic#60338)
  [SIEM] Adds 'Closes one signal when more than one opened signals are selected' test again (elastic#60380)
  [SIEM][Detections Engine] - Add rule markdown field to rule create, detail, and edit flows (elastic#60108)
  [Fleet] Add config revision to fleet agents (elastic#60292)
  Allow kbn-config-schema to ignore unknown keys (elastic#59560)
  [ML] Functional tests - disable df analytics clone tests
  skip flaky suite (elastic#58643) (elastic#58991)
  [FTR] Add support for --include and --exclude files via tags (elastic#60123)
  [SIEM] Fix link on overview page (elastic#60348)
  skip flaky test (elastic#60369)
  [Endpoint] Adds take action dropdown and tests to alert details flyout (elastic#59242)
  [Lens] Simplify state management from visualization (elastic#58279)
  Changing default type to start and allowing it to be configured by the event category (elastic#60323)
  [ML] Adds the class_assignment_objective to classification (elastic#60358)
  [TSVB] fix text color when using custom background color (elastic#60261)
  Fix import to timefilter from in TSVB (elastic#60296)
  [NP] Get rid of usage redirectWhenMissing service (elastic#59777)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:NP Migration release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants