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

[Reporting] Correct the docvalue_fields params in the search query Download CSV from Dashboard Panel #52833

Merged
merged 3 commits into from
Jan 2, 2020

Conversation

tsullivan
Copy link
Member

@tsullivan tsullivan commented Dec 12, 2019

Summary

Closes #50648

Release note: Fixed an issue with CSV download from a saved search panel in a dashboard, where docvalue_fields would not be in the CSV.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

await esArchiver.load('reporting/ecommerce');
await esArchiver.load('reporting/ecommerce_kibana');

const params = {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a copy of the POST body that I captured from a network inspector after running the use case through a browser.

@tsullivan tsullivan requested a review from lukasolson December 12, 2019 00:38
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

History

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

if (jobParams.post && jobParams.post.state) {
({
post: {
state: { query: payloadQuery, sort: payloadSort = [] },
state: { query: payloadQuery, sort: payloadSort = [], docvalue_fields: docValueFields },
Copy link
Member Author

Choose a reason for hiding this comment

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

@elastic/kibana-app can you help us understand why the docvalue_fields info is part of the state that we get from the panel info? It means the id of the saved search doc isn't enough to derive an actual ES query - the state data is additional information.

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, would need a bit more time to dive into the code to answer your questions, tested the CSV generation in Chrome, works as expected.

@tsullivan
Copy link
Member Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@tsullivan tsullivan merged commit 01dd08e into elastic:master Jan 2, 2020
@tsullivan tsullivan deleted the reporting/fix-csv-search branch January 2, 2020 22:30
tsullivan added a commit to tsullivan/kibana that referenced this pull request Jan 2, 2020
…wnload CSV from Dashboard Panel (elastic#52833)

* fix the bug and add a test

* fix query bug of empty array

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jan 3, 2020
* master:
  Rename `/api/security/oidc` to `/api/security/oidc/callback`. (elastic#53886)
  Updating transitive dependencies to use handlebars@4.5.3 (elastic#53899)
  [Reporting/Tests] consolidate functional test configs (elastic#52671)
  [Reporting] Correct the docvalue_fields params in the search query Download CSV from Dashboard Panel (elastic#52833)
  [Test/Newsfeed] Re-enable test and add news item to be filtered (elastic#53905)
  cleanup server-log action (elastic#53326)
  [Uptime] Delete uptime eslint rule skip (elastic#50912)
  [skip-ci] Expression Lifecycle Docs (elastic#51494)
  [Endpoint] add react router to endpoint app (elastic#53808)
  [SIEM][Detection Engine] Silence 409 errors on signal creation (elastic#53859)
  [Maps] get max_result_window and max_inner_result_window from index settings (elastic#53500)
  [ML] New Platform server shim: update analytics routes to use new platform router (elastic#53521)
  fixes typo on engine detection page (elastic#53877)
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jan 3, 2020
* master:
  Rename `/api/security/oidc` to `/api/security/oidc/callback`. (elastic#53886)
  Updating transitive dependencies to use handlebars@4.5.3 (elastic#53899)
  [Reporting/Tests] consolidate functional test configs (elastic#52671)
  [Reporting] Correct the docvalue_fields params in the search query Download CSV from Dashboard Panel (elastic#52833)
  [Test/Newsfeed] Re-enable test and add news item to be filtered (elastic#53905)
  cleanup server-log action (elastic#53326)
  [Uptime] Delete uptime eslint rule skip (elastic#50912)
  [skip-ci] Expression Lifecycle Docs (elastic#51494)
  [Endpoint] add react router to endpoint app (elastic#53808)
  [SIEM][Detection Engine] Silence 409 errors on signal creation (elastic#53859)
  [Maps] get max_result_window and max_inner_result_window from index settings (elastic#53500)
  [ML] New Platform server shim: update analytics routes to use new platform router (elastic#53521)
  fixes typo on engine detection page (elastic#53877)
  [Maps] push mapbox value extraction from VectorStyle and into DynamicStyleProperty (elastic#53806)
  Fix suggested value for time_zone in range query (elastic#53841)
  Clean up generic hooks, use react-use instead (elastic#53822)
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jan 3, 2020
…ris/kibana into alerting/created_at-and-updated_at

* 'alerting/created_at-and-updated_at' of github.com:gmmorris/kibana:
  Rename `/api/security/oidc` to `/api/security/oidc/callback`. (elastic#53886)
  Updating transitive dependencies to use handlebars@4.5.3 (elastic#53899)
  [Reporting/Tests] consolidate functional test configs (elastic#52671)
  [Reporting] Correct the docvalue_fields params in the search query Download CSV from Dashboard Panel (elastic#52833)
  [Test/Newsfeed] Re-enable test and add news item to be filtered (elastic#53905)
  cleanup server-log action (elastic#53326)
  [Uptime] Delete uptime eslint rule skip (elastic#50912)
  [skip-ci] Expression Lifecycle Docs (elastic#51494)
  [Endpoint] add react router to endpoint app (elastic#53808)
  [SIEM][Detection Engine] Silence 409 errors on signal creation (elastic#53859)
  [Maps] get max_result_window and max_inner_result_window from index settings (elastic#53500)
  [ML] New Platform server shim: update analytics routes to use new platform router (elastic#53521)
  fixes typo on engine detection page (elastic#53877)
  [Maps] push mapbox value extraction from VectorStyle and into DynamicStyleProperty (elastic#53806)
  Fix suggested value for time_zone in range query (elastic#53841)
  Clean up generic hooks, use react-use instead (elastic#53822)
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jan 3, 2020
…t-types

* alerting/created_at-and-updated_at:
  updatedAt should equal createdAt on creation
  Move index patterns: src/legacy/core_plugins/data 👉 src/plugins/data (elastic#53794)
  moved Task Manager server code under "server" directory (elastic#53777)
  Rename `/api/security/oidc` to `/api/security/oidc/callback`. (elastic#53886)
  Updating transitive dependencies to use handlebars@4.5.3 (elastic#53899)
  [Reporting/Tests] consolidate functional test configs (elastic#52671)
  [Reporting] Correct the docvalue_fields params in the search query Download CSV from Dashboard Panel (elastic#52833)
  [Test/Newsfeed] Re-enable test and add news item to be filtered (elastic#53905)
  cleanup server-log action (elastic#53326)
  [Uptime] Delete uptime eslint rule skip (elastic#50912)
  [skip-ci] Expression Lifecycle Docs (elastic#51494)
  [Endpoint] add react router to endpoint app (elastic#53808)
  [SIEM][Detection Engine] Silence 409 errors on signal creation (elastic#53859)
  [Maps] get max_result_window and max_inner_result_window from index settings (elastic#53500)
  [ML] New Platform server shim: update analytics routes to use new platform router (elastic#53521)
  fixes typo on engine detection page (elastic#53877)
  [Maps] push mapbox value extraction from VectorStyle and into DynamicStyleProperty (elastic#53806)
  Fix suggested value for time_zone in range query (elastic#53841)
  Clean up generic hooks, use react-use instead (elastic#53822)
tsullivan added a commit that referenced this pull request Jan 3, 2020
…wnload CSV from Dashboard Panel (#52833) (#53918)

* fix the bug and add a test

* fix query bug of empty array

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

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
} else {
docValueFields = [indexPatternTimeField];
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I can no longer remember why this block of code was added, but it seems to be causing a regression where the time field value shows up as a multi-value. The effect is even baked into a unit test fixture: https://github.com/elastic/kibana/blob/a8b1a6b/x-pack/test/reporting_api_integration/fixtures.ts#L179

Noticed this while working on: #56371

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.

CSV export doesn't export all fields
4 participants