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

[Monitoring] Fix cluster listing page in how it handles global state #78979

Merged
merged 7 commits into from
Oct 19, 2020

Conversation

chrisronline
Copy link
Contributor

Resolves #78824

On the cluster listing page, we attempt to link to each cluster in way to ensure we actually load that cluster on the cluster overview page, instead of showing whichever cluster is returned first. This is however not working properly, for multiple reasons, but mainly because we are improperly setting global state for the cluster_uuid while on the listing page.

This PR ensure we do not do that, and fixes a hole in the getSafeForExternalLink code to properly use any provided query string parameters.

To test, start two separate clusters and connect Kibana to one of them through a remote cluster connection. Ensure you see them both in the cluster listing page. Then, click on one and ensure that one is loaded on the cluster overview page. Then go back to the listing page, and click the other. Make sure you can properly switch between the two.

@elasticmachine
Copy link
Contributor

Pinging @elastic/stack-monitoring (Team:Monitoring)

@chrisronline
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@Zacqary Zacqary left a comment

Choose a reason for hiding this comment

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

Tested, can confirm this is working. LGTM

@chrisronline
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Member

@simianhacker simianhacker left a comment

Choose a reason for hiding this comment

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

LGTM (on behalf of @Zacqary)

Copy link
Member

@jasonrhodes jasonrhodes left a comment

Choose a reason for hiding this comment

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

Approving on behalf of @Zacqary :)

@chrisronline
Copy link
Contributor Author

Woah, easy folks! Not all at once!

@Zacqary
Copy link
Contributor

Zacqary commented Oct 19, 2020

I'd like to also approve this on behalf of myself

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

async chunks size

id before after diff
monitoring 1.1MB 1.1MB +1.4KB

History

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

@chrisronline chrisronline merged commit 3f97872 into elastic:master Oct 19, 2020
chrisronline added a commit to chrisronline/kibana that referenced this pull request Oct 19, 2020
…lastic#78979)

* Properly unset global state on the listing page

* Fix how we handle global state in getSafeForExternalLink

* Fix breadcrumbs for clusters link

* Fix tests

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
chrisronline added a commit to chrisronline/kibana that referenced this pull request Oct 19, 2020
…lastic#78979)

* Properly unset global state on the listing page

* Fix how we handle global state in getSafeForExternalLink

* Fix breadcrumbs for clusters link

* Fix tests

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
chrisronline added a commit that referenced this pull request Oct 19, 2020
…78979) (#81049)

* Properly unset global state on the listing page

* Fix how we handle global state in getSafeForExternalLink

* Fix breadcrumbs for clusters link

* Fix tests

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
chrisronline added a commit that referenced this pull request Oct 19, 2020
…78979) (#81048)

* Properly unset global state on the listing page

* Fix how we handle global state in getSafeForExternalLink

* Fix breadcrumbs for clusters link

* Fix tests

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@chrisronline chrisronline deleted the monitoring/fix_safe_link branch October 20, 2020 01:00
@chrisronline
Copy link
Contributor Author

Backport:

7.x: e1b720a
7.10: a0d84e6

jloleysens added a commit to jloleysens/kibana that referenced this pull request Oct 20, 2020
…lout-for-warm-and-cold-tier

* 'master' of github.com:elastic/kibana: (126 commits)
  Add cumulative sum expression function (elastic#80129)
  [APM] Fix link to trace (elastic#80993)
  Provide url rewritten in onPreRouting interceptor (elastic#80810)
  limit renovate to npm packages
  Fix bug in logs UI link (elastic#80943)
  [Monitoring] Fix bug with setup mode appearing on pages it shouldn't (elastic#80343)
  [Security Solution][Detection Engine] Fixes false positives caused by empty records in threat list
  docs test (elastic#81080)
  Fixed alerts ui test timeout issue, related to the multiple server calls for delete all alerts, by reducing the number of alerts to the two and increasing retry timeout. (elastic#81067)
  [APM] Fix service map highlighted edge on node select (elastic#80791)
  Fix typo in toast, slight copy adjustment. (elastic#80843)
  [Security Solution] reduce optimizer limits (elastic#80997)
  [maps] 7.10 documentation updates (elastic#79917)
  [Workplace Search] Fix Group Prioritization route and clean up design (elastic#80903)
  [Enterprise Search] Added reusable HiddenText component to Credentials (elastic#80033)
  Upgrade EUI to v29.5.0 (elastic#80753)
  [Maps] Fix layer-flash when changing style (elastic#80948)
  [Security Solution] [Detections] Disable edit button when user does not have actions privileges w/ rule + actions (elastic#80220)
  [Enterprise Search] Handle loading state on Credentials page (elastic#80035)
  [Monitoring] Fix cluster listing page in how it handles global state (elastic#78979)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Monitoring] Cluster listing page with CCS breaks after previously visiting CCS cluster
6 participants