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 bug with setup mode appearing on pages it shouldn't #80343

Merged

Conversation

chrisronline
Copy link
Contributor

Resolves #79477

#73127 introduced a slight change in how we handle determining if setup mode UIs should appear.

There are two parts of detecting setup mode - there is a global component but also a custom renderer that is meant to determine if setup mode should appear or not (on the particular page).

The change in #73127 changed a couple of spots in code to only look at the global component but it really needs to look at both.

To test, navigate to Stack Monitoring and then click on a page that does not support setup mode (such as Elasticsearch -> Indices) and make sure no setup mode UIs appear.

@chrisronline chrisronline added review Team:Monitoring Stack Monitoring team v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v7.11.0 labels Oct 13, 2020
@chrisronline chrisronline requested a review from a team October 13, 2020 13:51
@chrisronline chrisronline self-assigned this Oct 13, 2020
@elasticmachine
Copy link
Contributor

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

@chrisronline
Copy link
Contributor Author

@igoristic Hold off on this review. I think we can fix this a better way.

@chrisronline
Copy link
Contributor Author

@igoristic Nevermind. I think we need to make this better but it'll be easier once we rid ourselves of angular routing.

@chrisronline
Copy link
Contributor Author

@elasticmachine merge upstream

@chrisronline
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

@kbn/optimizer bundle module count

id before after diff
monitoring 632 633 +1

async chunks size

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

History

To update your PR or re-run it, just comment with:
@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.

@chrisronline Love the use of React context here! ❤️

Works as expected, and seems to also fix the Cannot read property 'elasticsearch' of null triggered in setup_mode.js due to null/undefined data reported earlier

@chrisronline chrisronline merged commit b019b57 into elastic:master Oct 20, 2020
@chrisronline chrisronline deleted the monitoring/check_local_setupmode_data branch October 20, 2020 02:05
chrisronline added a commit to chrisronline/kibana that referenced this pull request Oct 20, 2020
…lastic#80343)

* Ensure we check for local setup mode data first

* Use a context to ensure deeply nested components have access

* Fix snapshots

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

* Ensure we check for local setup mode data first

* Use a context to ensure deeply nested components have access

* Fix snapshots

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

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@chrisronline
Copy link
Contributor Author

chrisronline commented Oct 20, 2020

Backport:

7.x: 84592a5
7.10: ac7b6c0

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)
  ...
chrisronline added a commit that referenced this pull request Oct 20, 2020
…80343) (#81100)

* Ensure we check for local setup mode data first

* Use a context to ensure deeply nested components have access

* Fix snapshots

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

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes review Team:Monitoring Stack Monitoring team v7.10.0 v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Monitoring] Do not allow setup mode on pages that do not support it
4 participants