[Agg based pie][Lens]Navigate to lens Agg based pie #140879

VladLasitsa · 2022-09-16T12:39:49Z

Summary

Completes part of #138236.

As part of phasing out TSVB and Visualize all Legacy agg based visulizations should support "open in lens" functionality.
In that PR converter for Aggregation based Pie was added.

…hub.com:Kunzetsov/kibana into navigate-to-lens-context-converting-imporvement

…ntext-converting-imporvement

…hub.com:Kunzetsov/kibana into navigate-to-lens-context-converting-imporvement # Conflicts: # src/plugins/vis_types/timeseries/public/convert_to_lens/types.ts

…d difference

…t for 'format'

…hub.com:Kunzetsov/kibana into navigate-to-lens-context-converting-imporvement

elasticmachine · 2022-09-26T12:43:54Z

Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors)

stratoula

This looks good. Some comments from my side:

I would like to see a functional test that tests the transition from the agg pie to the lens pie.
This text here is not aligned with our guidelines. I feel that the aggregation should not start with a capital letter.

- If I display the legend in agg pie, the legend is not visible in lens

I choose to hide the labels but when I navigate to Lens I can see them

Same for hide values

stratoula · 2022-09-27T08:15:27Z

..._expressions/expression_partition_vis/public/expression_renderers/partition_vis_renderer.tsx

-        ]);
+          canNavigateToLens ? `render_${visualizationType}_${visType}_convertable` : undefined,
+        ].filter(Boolean) as string[];
+        plugins.usageCollection?.reportUiCounter(containerType, METRIC_TYPE.COUNT, events);


What is this going to measure if the events are undefined? Why don't we allow the reportCounter only if events are defined?

@stratoula, as I see from the code, events will always contain render_${visualizationType}_${visType}. WDYT?

...s/chart_expressions/expression_partition_vis/common/expression_functions/pie_vis_function.ts

mbondyra

Why aren't we allowed to convert three-slices level pie?

stratoula · 2022-09-27T09:25:53Z

I agree with Marta and I think we should block the transition when split chart is selected

# Conflicts: # src/plugins/visualizations/common/convert_to_lens/types/configurations.ts

Kuznietsov · 2022-09-28T10:20:15Z

@stratoula, @mbondyra, while I'm adding functional tests, could you, please, review this PR again? I've fixed all the problems you mentioned. Thanks)

mbondyra

Rechecked everything and works as expected 👌🏼 Approved!

stratoula

Also checked it again, all comments have been addressed! Thanx Yaroslav!

kibana-ci · 2022-09-28T18:49:08Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 2df9949

Failed CI Steps

Test Failures

[job] [logs] FTR Configs #15 / machine learning - data frame analytics results view content and total feature importance binary classification job "before all" hook for "should display the total feature importance in the results view"
[job] [logs] FTR Configs #49 / telemetry Telemetry service detects that telemetry cannot be sent in screenshot mode

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`visTypePie`	13	21	+8

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
`visualizations`	663	695	+32

Async chunks

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

id	before	after	diff
`lens`	1.3MB	1.3MB	+450.0B
`visTypePie`	8.7KB	8.7KB	+1.0B
`visualizations`	239.2KB	239.4KB	+226.0B
total			+677.0B

Page load bundle

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

id	before	after	diff
`expressionPartitionVis`	23.6KB	23.8KB	+195.0B
`visTypePie`	7.5KB	12.2KB	+4.7KB
`visualizations`	51.2KB	52.1KB	+866.0B
total			+5.7KB

Unknown metric groups

API count

id	before	after	diff
`visualizations`	693	725	+32

History

💔 Build #76490 failed 6574932
💚 Build #76398 succeeded 4270ef5
💔 Build #76385 failed 0b59a97
💔 Build #76357 failed fed6ccc
💚 Build #76325 succeeded ffb45b4

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

cc @VladLasitsa

Kuznietsov and others added 30 commits August 24, 2022 15:24

Merge branch 'navigate-to-lens-context-converting-imporvement' of git…

f804cab

…hub.com:Kunzetsov/kibana into navigate-to-lens-context-converting-imporvement

Merge remote-tracking branch 'upstream/main' into navigate-to-lens-co…

8cbb4ad

…ntext-converting-imporvement

Fix types

2c0c001

Replaced usage of timeseries.

3e2d943

Merge branch 'navigate-to-lens-context-converting-imporvement' of git…

0bc07fd

…hub.com:Kunzetsov/kibana into navigate-to-lens-context-converting-imporvement # Conflicts: # src/plugins/vis_types/timeseries/public/convert_to_lens/types.ts

Added window everywhere.

12669a3

Add std_deviation and value_count, fix some issues

9e9593c

Fix interval for date_histogram and fix getting filter for metrics an…

7fae45e

…d difference

Fix tests for 'getDatasourceSuggestionsForVisualizeCharts' except tes…

afe393d

…t for 'format'

Refactored code.

421c39b

Refactoring 'createNewLayerWithMetricAggregationFromVizEditor'

ca82caf

Removed not used code.

0e847fa

Merge branch 'navigate-to-lens-context-converting-imporvement' of git…

3f36734

…hub.com:Kunzetsov/kibana into navigate-to-lens-context-converting-imporvement

Removed unused import.

8ddaed9

Fixed types.

f49d11a

Removed tests with type troubles.

f472b42

Some code refactoring + formatter.

3ffa2c6

Removed not used comment.

1380bb8

Added formatter validation.

51ef491

Added tests for getFormat.

0105536

Fix getFormat method

26bf0a4

createColumn test added.

8576a9b

Merge branch 'navigate-to-lens-context-converting-imporvement' of git…

309a861

…hub.com:Kunzetsov/kibana into navigate-to-lens-context-converting-imporvement

Added tests for excludeMetaFromColumn and isColumnWithMeta.

dcbf498

Added tests for counter_rate.

ab7ed74

Added tests for cumulative_sum.

3233c51

Updated cumulative_sum tests.

3bce624

Added date_histogram tests.

a7a6e8f

Added filter ratio tests.

74ed9d6

Filters test added.

2e1cafa

Kuznietsov added the v8.6.0 label Sep 26, 2022

Kuznietsov requested a review from alexwizp September 26, 2022 09:34

Made some changes for uniformity.

d48fab2

Kuznietsov marked this pull request as ready for review September 26, 2022 12:43

Kuznietsov requested a review from a team as a code owner September 26, 2022 12:43

stratoula reviewed Sep 27, 2022

View reviewed changes

mbondyra reviewed Sep 27, 2022

View reviewed changes

Kuznietsov added 9 commits September 28, 2022 10:12

Merge branch 'main' into navigate-to-lens-agg-based-pie-vis

9c75962

Fixed label of go back to.

ffb45b4

Fixed legend display if uiState was changed.

7a88548

Fixed configuration.

874fe6c

Removed as string[].

624840a

Fixed code according to the nit.

fed6ccc

Allowed 3 segments and restricted converting of split_row/split_column.

2446489

Fixed tests.

73f0e12

Merge branch 'main' into navigate-to-lens-agg-based-pie-vis

0b59a97

# Conflicts: # src/plugins/visualizations/common/convert_to_lens/types/configurations.ts

Fixed tests.

4270ef5

mbondyra approved these changes Sep 28, 2022

View reviewed changes

stratoula approved these changes Sep 28, 2022

View reviewed changes

Kuznietsov added 3 commits September 28, 2022 15:56

Refactored functional tests.

0ceaa43

Added functional tests for pie.

6574932

Merge branch 'main' into navigate-to-lens-agg-based-pie-vis

2df9949

Kuznietsov merged commit ea046ac into elastic:main Sep 29, 2022

Kuznietsov removed the WIP Work in progress label Sep 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Agg based pie][Lens]Navigate to lens Agg based pie #140879

[Agg based pie][Lens]Navigate to lens Agg based pie #140879

VladLasitsa commented Sep 16, 2022 •

edited by Kuznietsov

Loading

elasticmachine commented Sep 26, 2022

stratoula left a comment

stratoula Sep 27, 2022

Kuznietsov Sep 28, 2022

mbondyra left a comment •

edited

Loading

stratoula commented Sep 27, 2022

Kuznietsov commented Sep 28, 2022

mbondyra left a comment

stratoula left a comment

kibana-ci commented Sep 28, 2022

API count

[Agg based pie][Lens]Navigate to lens Agg based pie #140879

[Agg based pie][Lens]Navigate to lens Agg based pie #140879

Conversation

VladLasitsa commented Sep 16, 2022 • edited by Kuznietsov Loading

Summary

elasticmachine commented Sep 26, 2022

stratoula left a comment

Choose a reason for hiding this comment

stratoula Sep 27, 2022

Choose a reason for hiding this comment

Kuznietsov Sep 28, 2022

Choose a reason for hiding this comment

mbondyra left a comment • edited Loading

Choose a reason for hiding this comment

stratoula commented Sep 27, 2022

Kuznietsov commented Sep 28, 2022

mbondyra left a comment

Choose a reason for hiding this comment

stratoula left a comment

Choose a reason for hiding this comment

kibana-ci commented Sep 28, 2022

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

History

VladLasitsa commented Sep 16, 2022 •

edited by Kuznietsov

Loading

mbondyra left a comment •

edited

Loading