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

fixing issue with dashboard csv download #42964

Merged

Conversation

bmcconaghy
Copy link
Contributor

Fixes #42963

Pretty sure this will work with multisorted saved searches too. Not sure why this broke as it seems to have worked fine on 7.3 (see #42945).

@bmcconaghy bmcconaghy added (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead v8.0.0 Team:Stack Services v7.4.0 labels Aug 8, 2019
@bmcconaghy bmcconaghy requested a review from joelgriffith August 8, 2019 17:33
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-stack-services

@elasticmachine
Copy link
Contributor

💔 Build Failed

@bmcconaghy bmcconaghy closed this Aug 8, 2019
@bmcconaghy
Copy link
Contributor Author

this behavior is due to a bug in saved searches, ignore this PR

@bmcconaghy bmcconaghy deleted the fix_issue_with_dashboard_csv_download branch August 8, 2019 19:38
@bmcconaghy bmcconaghy restored the fix_issue_with_dashboard_csv_download branch August 9, 2019 11:24
@bmcconaghy bmcconaghy reopened this Aug 9, 2019
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Contributor

@joelgriffith joelgriffith 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!

@bmcconaghy
Copy link
Contributor Author

retest

@bmcconaghy bmcconaghy added the release_note:skip Skip the PR/issue when compiling release notes label Aug 9, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@bmcconaghy
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@bmcconaghy bmcconaghy merged commit 5e76040 into elastic:master Aug 9, 2019
@bmcconaghy bmcconaghy deleted the fix_issue_with_dashboard_csv_download branch August 9, 2019 19:20
jloleysens added a commit to jloleysens/kibana that referenced this pull request Aug 12, 2019
…p-metrics-selectall

* 'master' of github.com:elastic/kibana: (22 commits)
  [Code]: downgrade the log level of error message from subprocess (elastic#42925)
  [Code] Cancel clone/update job in the middle if disk space over the watermark (elastic#42890)
  Add Kibana App specific URL to the help menu (elastic#34739) (elastic#42580)
  [Maps] refactor createShapeFilterWithMeta to support more than just polygons (elastic#43042)
  Skip flaky es_ui_shared/request tests.
  Pass uiSettings to all data plugin services (elastic#42159)
  [SIEM] Upgrades react-redux and utilize React.memo for performance gains  (elastic#43029)
  [skip-ci][Maps] add maki icon sheet to docs (elastic#43063)
  Adding "style-src 'unsafe-inline' 'self'" to default CSP rules (elastic#41305)
  Update dependency commander to v3 (elastic#43041)
  Update dependency @percy/agent to ^0.10.0 (elastic#40517)
  [Maps] only show top hits checkbox if index has date fields (elastic#43056)
  run chained_controls on Firefox to catch regression (elastic#43044)
  fixing issue with dashboard csv download (elastic#42964)
  Expose task manager as plugin instead of server argument (elastic#42966)
  Expose createRouter from HttpService, prepare handlers for context introduction (elastic#42686)
  [Code] disk watermark supports percentage and absolute modes (elastic#42987)
  [apps/dashboard] skip part of filtering tests on FF (elastic#43047)
  [ML] Kibana management jobs list (elastic#42570)
  [ML] Fix check for watcher being enabled (elastic#43025)
  ...
@tsullivan
Copy link
Member

I'm afraid there's still a bug that exists with saved searches migrated from an earlier version of Kibana: #43230

Unless my saved search object was supposed to go through a migration to have the sort fields upgraded to a new data model.

@bmcconaghy
Copy link
Contributor Author

@tsullivan, when did you test this? @Bargs just merged this: #43038 so please confirm that you are seeing the bug with his code.

@tsullivan
Copy link
Member

@bmcconaghy confirmed my issue is fixed. We're good! Thanks.

@Bargs
Copy link
Contributor

Bargs commented Aug 14, 2019

@bmcconaghy could you backport this to 7.x? I think #43234 will fail until it is backported.

@bmcconaghy
Copy link
Contributor Author

@Bargs sorry about that, totally forgot to backport. Got a PR up to do so now and will merge when it goes green.

bmcconaghy added a commit that referenced this pull request Aug 14, 2019
* fixing issue with dashboard csv download

* fixing test fixture to match new sort storage format for saved searches

* fixing sort in additional fixtures
@Bargs
Copy link
Contributor

Bargs commented Aug 14, 2019

Thanks @bmcconaghy !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
(Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead release_note:skip Skip the PR/issue when compiling release notes v7.4.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when using download as CSV from dashboard
5 participants