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

Refresh index pattern list before redirecting #63329

Merged
merged 14 commits into from
Apr 22, 2020

Conversation

flash1293
Copy link
Contributor

@flash1293 flash1293 commented Apr 13, 2020

Summary

Fixes #63173
Fixes #63180
Fixes #62269

Before redirecting to the management page because no index pattern is available, refresh the cached index pattern list. This prevents a confusing (and unnecessary) redirect to the management page when accessing Discover/Visualize/Dashboard in a bunch of situations because the index pattern cache can become stale e.g. when importing a saved object.

Initially I was refreshing the cache every time the ensureDefaultIndexPattern helper was called, but this slows down a lot of page navigations (e.g. switching between visualizations), so I moved it to the mount callbacks of the individual apps.

@flash1293 flash1293 added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.8.0 v7.7.1 labels Apr 13, 2020
@flash1293
Copy link
Contributor Author

@elasticmachine merge upstream

@flash1293
Copy link
Contributor Author

@elasticmachine merge upstream

@flash1293 flash1293 marked this pull request as ready for review April 15, 2020 11:19
@flash1293 flash1293 requested a review from a team April 15, 2020 11:19
@@ -215,6 +215,8 @@ export function DashboardPageProvider({ getService, getPageObjects }: FtrProvide

public async clickNewDashboard() {
await listingTable.clickNewButton('createDashboardPromptButton');
// make sure the dashboard page is shown
await this.waitForRenderComplete();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When switching to a dashboard, we introduce an additional delay with this change because another network roundtrip is necessary. This stabilizes the test so it doesn't fail.

@flash1293 flash1293 added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Apr 16, 2020
@elasticmachine
Copy link
Contributor

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

@flash1293 flash1293 requested a review from a team as a code owner April 17, 2020 09:38
@flash1293
Copy link
Contributor Author

@kertal Moved the fix for index pattern deletion over here as well.

@@ -348,6 +348,7 @@ uiModules

Promise.resolve($scope.indexPattern.destroy())
.then(function() {
npStart.plugins.data.indexPatterns.clearCache();
$location.url('/management/kibana/index_patterns');
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be better if we clear the index pattern cache upon arrival to management?
Otherwise, if we get to that route from a different path, we'll have different behaviors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm just realizing we don't even need this clear cache to fix the bug because it already gets cleared when switching over to discover.

The index pattern listing in management is not affected by this because it's using the raw saved object client.

I will just revert this line.

@flash1293
Copy link
Contributor Author

@elasticmachine merge upstream

@kertal
Copy link
Member

kertal commented Apr 21, 2020

ACK, will review today

@kertal kertal self-requested a review April 21, 2020 06:41
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

Copy link
Contributor

@lizozom lizozom left a comment

Choose a reason for hiding this comment

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

LGTM

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, fixes also: #62269, tested locally in Chrome80, deleted index patterns no longer show up in the index pattern selection, so the cache is deleted, and all problems dependent to this invalid cache is solved

@flash1293 flash1293 merged commit 2f794e6 into elastic:master Apr 22, 2020
flash1293 added a commit to flash1293/kibana that referenced this pull request Apr 22, 2020
gmmorris added a commit to gmmorris/kibana that referenced this pull request Apr 22, 2020
* master: (29 commits)
  [Dashboard] Deangularize navbar, attempt nr. 2 (elastic#61611)
  refactor action filter creation utils (elastic#62969)
  Refresh index pattern list before redirecting (elastic#63329)
  [APM]fixing custom link unit tests (elastic#64045)
  [Ingest] EPM & Fleet are enabled when Ingest is enabled (elastic#64103)
  [Alerting] Fixed bug with no possibility to edit the index name after adding (elastic#64033)
  [Maps] Map settings: min and max zoom (elastic#63714)
  [kbn-storybook] Use raw loader for text files (elastic#64108)
  [EPM] /packages/{package} endpoint to support upgrades (elastic#63629)
  [SIEM] New Platform Saved Objects Registration (elastic#64029)
  [Endpoint] Hook to handle events needing navigation via Router (elastic#63863)
  Fixed small issue in clone functionality (elastic#64085)
  [Endpoint]EMT-146: use ingest agent for status info (elastic#63921)
  [SIEM] Server NP Followup (elastic#64010)
  Register uiSettings on New Platform (elastic#64015)
  [Reporting] Integration polling config with client code (elastic#63754)
  [Docs]7.7 SIEM doc updates (elastic#63951)
  [SIEM] [Cases] Tags suggestions (elastic#63878)
  Include datasource UUID in agent config yaml, adjust overflow height of yaml view (elastic#64027)
  [DOCS] Add file size setting for Data Visualizer (elastic#64006)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Apr 22, 2020
…ana into task-manager/cancel-logging

* 'task-manager/cancel-logging' of github.com:gmmorris/kibana: (28 commits)
  [Dashboard] Deangularize navbar, attempt nr. 2 (elastic#61611)
  refactor action filter creation utils (elastic#62969)
  Refresh index pattern list before redirecting (elastic#63329)
  [APM]fixing custom link unit tests (elastic#64045)
  [Ingest] EPM & Fleet are enabled when Ingest is enabled (elastic#64103)
  [Alerting] Fixed bug with no possibility to edit the index name after adding (elastic#64033)
  [Maps] Map settings: min and max zoom (elastic#63714)
  [kbn-storybook] Use raw loader for text files (elastic#64108)
  [EPM] /packages/{package} endpoint to support upgrades (elastic#63629)
  [SIEM] New Platform Saved Objects Registration (elastic#64029)
  [Endpoint] Hook to handle events needing navigation via Router (elastic#63863)
  Fixed small issue in clone functionality (elastic#64085)
  [Endpoint]EMT-146: use ingest agent for status info (elastic#63921)
  [SIEM] Server NP Followup (elastic#64010)
  Register uiSettings on New Platform (elastic#64015)
  [Reporting] Integration polling config with client code (elastic#63754)
  [Docs]7.7 SIEM doc updates (elastic#63951)
  [SIEM] [Cases] Tags suggestions (elastic#63878)
  Include datasource UUID in agent config yaml, adjust overflow height of yaml view (elastic#64027)
  [DOCS] Add file size setting for Data Visualizer (elastic#64006)
  ...
@flash1293 flash1293 added v7.9.0 and removed v7.7.1 labels May 8, 2020
@kertal kertal mentioned this pull request Jun 16, 2020
7 tasks
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 Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.8.0 v7.9.0 v8.0.0
Projects
None yet
5 participants