[Lens][Datatable] Fix share export and inspect data #193780

nickofthyme · 2024-09-23T18:29:54Z

Summary

This PR fixes #187551 where the table provided in the inspector and the share export, did not match what was visible in the table.

Changes

Viewing the datatable from Inspect action

Shows the transposed data table in addition to the untransposed datatable.
The default selected table is set as the transposed and the user can select to view the untransposed datatable.
Filters out the ghost columns created by the formula metrics as described in [Lens] Inspector and CSV download does not filter partial columns from formula #121063.

Zight.Recording.2024-09-23.at.02.06.10.PM.mp4

Exporting table to csv via the Share action

Now exports only the transposed data table.
Fixes column ordering of transposed data. Transposed metrics are zipped together by like terms (i.e. ['css › X', 'css › Y', 'deb › X', 'deb › Y']).
Now sorts rows based on active column sorting parameters (only for export not inspect). Always places null values at the bottom to match eui behavior.

The Lens table above would output a csv that shows as shown below (in Numbers).

Or in raw form as.

"Top 3 values of agent.keyword","(empty) › Count of records","gz › Count of records","css › Count of records","zip › Count of records","deb › Count of records"
"Mozilla/5.0 (X11; Linux x86_64; rv:6.0a1) Gecko/20110421 Firefox/6.0a1",268,131,122,81,70
"Mozilla/5.0 (X11; Linux i686) AppleWebKit/534.24 (KHTML, like Gecko) Chrome/11.0.696.50 Safari/534.24",235,109,98,81,68
"Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; SV1; .NET CLR 1.1.4322)",210,93,82,64,52

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This was checked for cross-browser compatibility

- Add transposed table to list of tables - Fix table select dropdown styles

- export transpose datatable from share - support sorting transposed columns - sort transposed columns by args column order (zippered) - removed hidden columns from share output - sort rows when column sorting is present - push empty value to bottom

- Set transposed table as default selection in inspector - Export only transposed table on share - Remove unused prop-types

nickofthyme

Comments to reviewers

src/plugins/data/public/utils/table_inspector_view/components/data_table_selector.tsx

src/plugins/data/common/exports/export_csv.tsx

x-pack/plugins/lens/common/expressions/datatable/transpose_helpers.ts

x-pack/plugins/lens/public/app_plugin/csv_download_provider/csv_download_provider.tsx

x-pack/plugins/lens/public/visualizations/datatable/visualization.tsx

dej611

Initial code review

src/plugins/data/common/exports/export_csv.tsx

- used sorted column order from state - use column order from transposed table via activeData - remove transpose logic from csv export logic - use prepareLogTable to strip out ghost columns ids from formulas

…rted to the csv export utility - move column & row sorting into vis definition - use `@kbn/sort-predicates` to sort rows - remove sorted columns logic - remove column sorting logic

x-pack/plugins/lens/public/app_plugin/lens_top_nav.tsx

src/plugins/data/common/exports/export_csv.tsx

packages/kbn-transpose-helpers/index.ts

- remove circular dep issue with transpose-helpers pkg - split transpose logic to share only id util functions - move transpose table logic back to plugin - rename pkg to utils instead of helpers - fixup failing tests from transpose sorting changes

x-pack/plugins/lens/common/expressions/datatable/transpose_helpers.test.ts

…ocs'

…form in `workpace_panel`

…e-exports

elasticmachine · 2024-10-22T03:48:12Z

Pinging @elastic/kibana-visualizations (Team:Visualizations)

nreese

kibana-presentation changes LGTM
code review only

dej611

Tested, approving for not blocking this.

Few observations who could be addressed in a follow up:

While now there's the option to select between tables in the Inspector > Data page, there's no clear distinctions nor explanation on why there are multiple tables. Better namings here would help (out of scope of this PR)
The download now leads to multiple downloads with generic names Inspector 1 and Inspector 2 which has the same issue as before: why 2 files? Which one is the transposed one? (Out of scope of this PR)
I've noticed some performance hit when switching between Request and Data page in the inspector when there's a Split by metric. (Out of scope of this PR)

nickofthyme · 2024-10-24T01:50:09Z

Thanks @dej611 as for...

While now there's the option to select between tables in the Inspector > Data page, there's no clear distinctions nor explanation on why there are multiple tables. Better namings here would help.

Yeah I agree, I thought about changing this to allow tables to have titles and descriptions but it's been unchanged for 4 years, so I just left as is.

The download now leads to multiple downloads with generic names Inspector 1 and Inspector 2 which has the same issue as before: why 2 files? Which one is the transposed one?

Yeah this is the default behavior. The names could be better as mentioned above. I changed this on the share to export just the final table but not in the inspector downloads. I think adding options to the TablesAdapter to configure this would be great. Opened #197555 for both issues.

I've noticed some performance hit when switching between Request and Data page in the inspector when there's a Split by metric. (Out of scope of this PR)

I can take a look at this to see if there's duplicate code execution somewhere.

elasticmachine · 2024-10-24T03:10:19Z

💚 Build Succeeded

Buildkite Build
Commit: 4592f10

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`expressionHeatmap`	178	179	+1
`lens`	1466	1463	-3
total			-2

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/transpose-utils`	-	7	+7
`expressions`	1765	1769	+4
`lens`	589	590	+1
total			+12

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`data`	48.0KB	48.1KB	+98.0B
`expressionHeatmap`	26.7KB	26.8KB	+62.0B
`lens`	1.5MB	1.5MB	+3.2KB
total			+3.4KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`lens`	62	63	+1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`data`	419.9KB	419.9KB	-68.0B
`expressions`	99.8KB	100.3KB	+434.0B
`lens`	51.4KB	50.6KB	-859.0B
total			-493.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/transpose-utils`	-	9	+9
`expressions`	2235	2241	+6
`lens`	691	692	+1
total			+16

History

kibanamachine · 2024-10-24T16:51:58Z

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11503862348

The exported table data table provided in the inspector and the share export now match what was visible in the UI. (cherry picked from commit a854ff8)

kibanamachine · 2024-10-24T16:57:22Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

…197696) # Backport This will backport the following commits from `main` to `8.x`: - [[Lens][Datatable] Fix share export and inspect data (#193780)](#193780)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Nick Partridge <[email protected]>

nickofthyme added 4 commits September 19, 2024 14:50

chore: strengthen context types with default adapter type

caf0d04

fix: table inspect flyout export data

64aaadc

- Add transposed table to list of tables - Fix table select dropdown styles

improve inspect and export flow

5158787

- Set transposed table as default selection in inspector - Export only transposed table on share - Remove unused prop-types

nickofthyme added Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Lens labels Sep 23, 2024

fix: filter out ghost columns in inspect created from formula metrics

de71e9f

nickofthyme commented Sep 23, 2024

View reviewed changes

dej611 reviewed Sep 24, 2024

View reviewed changes

src/plugins/data/common/exports/export_csv.tsx Outdated Show resolved Hide resolved

src/plugins/data/common/exports/export_csv.tsx Outdated Show resolved Hide resolved

src/plugins/data/common/exports/export_csv.tsx Outdated Show resolved Hide resolved

nickofthyme added 6 commits September 29, 2024 13:05

Merge branch 'main' into fix-table-exports

5bde6b9

Merge branch 'main' into fix-table-exports

351812f

Move transpose helpers into shared kbn package

1317666

cleanup columns sorting logic

dfc2221

- used sorted column order from state - use column order from transposed table via activeData - remove transpose logic from csv export logic - use prepareLogTable to strip out ghost columns ids from formulas

Merge branch 'main' into fix-table-exports

fd5974d

cleanup csv sorting logic, now we directly pass all tables to be expo…

44baf77

…rted to the csv export utility - move column & row sorting into vis definition - use `@kbn/sort-predicates` to sort rows - remove sorted columns logic - remove column sorting logic

nickofthyme commented Oct 14, 2024

View reviewed changes

x-pack/plugins/lens/public/app_plugin/lens_top_nav.tsx Show resolved Hide resolved

src/plugins/data/common/exports/export_csv.tsx Show resolved Hide resolved

nickofthyme requested review from dej611 and markov00 October 14, 2024 22:55

kibanamachine added 2 commits October 14, 2024 23:05

[CI] Auto-commit changed files from 'node scripts/lint_packages --fix'

11f8cb2

[CI] Auto-commit changed files from 'node scripts/notice'

15eaf73

nickofthyme commented Oct 16, 2024

View reviewed changes

packages/kbn-transpose-helpers/index.ts Outdated Show resolved Hide resolved

nickofthyme added 3 commits October 20, 2024 18:09

fix quick check issues

d8abdf6

- remove circular dep issue with transpose-helpers pkg - split transpose logic to share only id util functions - move transpose table logic back to plugin - rename pkg to utils instead of helpers - fixup failing tests from transpose sorting changes

update jest tests with new changes

609dc1c

Merge branch 'main' into fix-table-exports

50ffcdf

nickofthyme commented Oct 20, 2024

View reviewed changes

x-pack/plugins/lens/common/expressions/datatable/transpose_helpers.test.ts Show resolved Hide resolved

kibanamachine and others added 5 commits October 20, 2024 23:36

[CI] Auto-commit changed files from 'node scripts/build_plugin_list_d…

1e9f9b5

…ocs'

fix jest type errors

25a6eb0

fix activeData error caused by conditional onActiveDataChange trans…

5c3ccd8

…form in `workpace_panel`

Merge remote-tracking branch 'origin/fix-table-exports' into fix-tabl…

313c934

…e-exports

Merge branch 'main' into fix-table-exports

b454f2a

nickofthyme marked this pull request as ready for review October 22, 2024 03:48

nickofthyme requested review from a team as code owners October 22, 2024 03:48

nickofthyme added release_note:fix backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Oct 22, 2024

nreese approved these changes Oct 22, 2024

View reviewed changes

botelastic bot added the Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) label Oct 22, 2024

nickofthyme added 2 commits October 22, 2024 18:18

fix jest tests and fn tests

ac0ff49

Merge branch 'main' into fix-table-exports

0d580bd

dej611 approved these changes Oct 23, 2024

View reviewed changes

nickofthyme added 2 commits October 23, 2024 20:12

Merge branch 'main' into fix-table-exports

a37edcb

fix type error in unit test

4592f10

nickofthyme merged commit a854ff8 into elastic:main Oct 24, 2024
38 checks passed

nickofthyme deleted the fix-table-exports branch October 24, 2024 16:51

kibanamachine added the v9.0.0 label Oct 24, 2024

kibanamachine mentioned this pull request Oct 24, 2024

[8.x] [Lens][Datatable] Fix share export and inspect data (#193780) #197696

Merged

kibanamachine added the v8.17.0 label Oct 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Lens][Datatable] Fix share export and inspect data #193780

[Lens][Datatable] Fix share export and inspect data #193780

nickofthyme commented Sep 23, 2024 •

edited by kibanamachine

Loading

nickofthyme left a comment

dej611 left a comment

elasticmachine commented Oct 22, 2024

nreese left a comment

dej611 left a comment

nickofthyme commented Oct 24, 2024

elasticmachine commented Oct 24, 2024

API count

kibanamachine commented Oct 24, 2024

kibanamachine commented Oct 24, 2024

[Lens][Datatable] Fix share export and inspect data #193780

[Lens][Datatable] Fix share export and inspect data #193780

Conversation

nickofthyme commented Sep 23, 2024 • edited by kibanamachine Loading

Summary

Changes

Viewing the datatable from Inspect action

Exporting table to csv via the Share action

Checklist

nickofthyme left a comment

Choose a reason for hiding this comment

dej611 left a comment

Choose a reason for hiding this comment

elasticmachine commented Oct 22, 2024

nreese left a comment

Choose a reason for hiding this comment

dej611 left a comment

Choose a reason for hiding this comment

nickofthyme commented Oct 24, 2024

elasticmachine commented Oct 24, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Public APIs missing exports

Page load bundle

API count

History

kibanamachine commented Oct 24, 2024

kibanamachine commented Oct 24, 2024

💚 All backports created successfully

Questions ?

nickofthyme commented Sep 23, 2024 •

edited by kibanamachine

Loading