Skip to content

Fix broken CSV export from data table. - #34131

Merged
lukeelmers merged 2 commits into
elastic:masterfrom
lukeelmers:fix/data-table-export
Apr 1, 2019
Merged

lukeelmers merged 2 commits into
elastic:masterfrom
lukeelmers:fix/data-table-export

Conversation

@lukeelmers

Copy link
Copy Markdown
Contributor

Fixes #33581

In 7.0 and later, the clicking to download a raw or formatted CSV from the data table vis failed. I think this may have been introduced in #28746, though not 100% sure.

This updates agg_table to handle tabified data, and also makes sure that formatter.convert is called for each value when exporting formatted CSV.

We have tests that would have caught this, but they were using mocked data in the old format so they were still passing & giving a false positive. I've updated those as well & added a few new tests.

@lukeelmers lukeelmers added Feature:Data Table Data table visualization feature regression v7.0.0 Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v8.0.0 v7.2.0 labels Mar 28, 2019
@lukeelmers
lukeelmers requested review from ppisljar and timroes March 28, 2019 23:36
@elasticmachine

Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-app

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Weird thing I noticed when writing tests: the way we had defined the default formatter, the same object was being passed to every field that used the default format, which meant mutating convert on one field would actually change all the default fields.

I wasn't sure how likely it would be for this to cause problems in the real world, but went ahead and changed it anyway just to be safe.

@elasticmachine

This comment has been minimized.

@lukeelmers

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@lukeelmers lukeelmers added the release_note:skip Skip the PR/issue when compiling release notes label Mar 29, 2019
@lukeelmers
lukeelmers force-pushed the fix/data-table-export branch from 2a0c802 to f81b5e8 Compare March 29, 2019 15:13
@lukeelmers lukeelmers added release_note:fix and removed release_note:skip Skip the PR/issue when compiling release notes labels Mar 29, 2019
@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@ppisljar ppisljar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, tested on chrome linux

@lukeelmers
lukeelmers merged commit 37f9994 into elastic:master Apr 1, 2019
@lukeelmers
lukeelmers deleted the fix/data-table-export branch April 1, 2019 17:01
lukeelmers added a commit to lukeelmers/kibana that referenced this pull request Apr 1, 2019
lukeelmers added a commit to lukeelmers/kibana that referenced this pull request Apr 1, 2019
@lukeelmers

Copy link
Copy Markdown
Contributor Author

patrykkopycinski pushed a commit to patrykkopycinski/kibana that referenced this pull request May 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Data Table Data table visualization feature regression release_note:fix Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.0.0 v7.2.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants