[VisBuilder-Next] Pie Chart Integration for VisBuilder #7752

ananzh · 2024-08-19T15:59:18Z

Description

This PR integrates pie charts into VisBuilder using Vega rendering.

Issues Resolved

#7607

Screenshot

pie integration

2024-08-20_09-22-11.mp4

split chart after split series

2024-08-20_09-54-03.mp4

Changelog

feat: [VisBuilder-Next] Pie Chart Integration for VisBuilder

Check List

All tests pass
- yarn test:jest
- yarn test:jest_integration
New functionality includes testing.
New functionality has been documented.
Update CHANGELOG.md
Commits are signed per the DCO using --signoff

github-actions · 2024-08-19T15:59:50Z

❌ Empty Changelog Section

The Changelog section in your PR description is empty. Please add a valid changelog entry or entries. If you did add a changelog entry, check to make sure that it was not accidentally included inside the comment block in the Changelog section.

codecov · 2024-08-19T16:16:06Z

Codecov Report

Attention: Patch coverage is 72.54902% with 28 lines in your changes missing coverage. Please review.

Project coverage is 60.95%. Comparing base (2ffb11f) to head (59d1126).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...izations/vislib/pie/components/pie_vis_options.tsx	0.00%	8 Missing ⚠️
...uilder/public/visualizations/vega/utils/helpers.ts	74.07%	3 Missing and 4 partials ⚠️
...er/public/visualizations/vega/vega_spec_factory.ts	25.00%	6 Missing ⚠️
...r/public/application/components/data_tab/index.tsx	0.00%	5 Missing ⚠️
...plugins/vis_builder/public/visualizations/index.ts	50.00%	0 Missing and 1 partial ⚠️
...ic/visualizations/vega/vega_spec_slices_builder.ts	92.30%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7752      +/-   ##
==========================================
+ Coverage   60.93%   60.95%   +0.01%     
==========================================
  Files        3771     3776       +5     
  Lines       89545    89638      +93     
  Branches    14017    14045      +28     
==========================================
+ Hits        54566    54637      +71     
- Misses      31569    31588      +19     
- Partials     3410     3413       +3

Flag	Coverage Δ
Linux_1	`29.08% <72.54%> (+0.06%)`	⬆️
Linux_2	`56.27% <ø> (ø)`
Linux_3	`37.75% <ø> (-0.01%)`	⬇️
Linux_4	`29.92% <ø> (ø)`
Windows_1	`29.09% <72.54%> (+0.06%)`	⬆️
Windows_2	`56.23% <ø> (ø)`
Windows_3	`37.75% <ø> (-0.01%)`	⬇️
Windows_4	`29.92% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This PR integrates pie charts into VisBuilder using Vega rendering. Issue Resolve: opensearch-project#7752 Signed-off-by: Anan Zhuang <[email protected]>

src/plugins/vis_builder/public/application/utils/validations/validate_aggregations.ts

src/plugins/vis_builder/public/visualizations/index.ts

AMoo-Miki · 2024-08-20T22:06:16Z

src/plugins/vis_builder/public/visualizations/vega/utils/helpers.ts

+      const currentLabels = { ...parentLabels, [`level${level}`]: child.name };
+      levelSet.add(`level${level}`);
+
+      if (child.children && child.children.length > 0) {


Suggested change

if (child.children && child.children.length > 0) {

if (Array.isArray(child.children) && child.children.length > 0) {

k. will modify.

AMoo-Miki · 2024-08-20T22:07:21Z

src/plugins/vis_builder/public/visualizations/vega/utils/helpers.ts

+
+      if (child.children && child.children.length > 0) {
+        flattenSlices(child, split, level + 1, currentLabels);
+      } else {


Is this else safe to happen when child.children = []?

This part of the code is generally safe to execute. Every leaf node, nodes without children, has a size property. However, we could make it safer. I have updated in the code.

src/plugins/vis_builder/public/visualizations/vega/utils/helpers.ts

This PR integrates pie charts into VisBuilder using Vega rendering. Issue Resolve: opensearch-project#7607 Signed-off-by: Anan Zhuang <[email protected]>

Signed-off-by: Anan Zhuang <[email protected]>

LDrago27 · 2024-10-10T22:55:18Z

src/plugins/vis_builder/public/application/components/data_tab/index.tsx

@@ -77,6 +79,17 @@ export const DataTab = () => {
      }

      const panelGroups = Array.from(schemas.all.map((schema) => schema.name));
+      // Check schema order
+      if (destinationSchemaName === 'split') {
+        const validationResult = validateAggregations(aggProps.aggs, schemas.all);


nit: We are only using split schema during validation we can only pass that schema instead of passing in all the schemas

Yeah currently this is an special schema check for pie. It is not appliable for other schemas.

LDrago27

Looks good.

* [VisBuilder-Next] Pie Chart Integration for VisBuilder This PR integrates pie charts into VisBuilder using Vega rendering. Issue Resolve: #7607 --------- Signed-off-by: Anan Zhuang <[email protected]> (cherry picked from commit 615d7d4) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* [VisBuilder-Next] Pie Chart Integration for VisBuilder This PR integrates pie charts into VisBuilder using Vega rendering. Issue Resolve: #7607 --------- (cherry picked from commit 615d7d4) Signed-off-by: Anan Zhuang <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…oject#7752) * [VisBuilder-Next] Pie Chart Integration for VisBuilder This PR integrates pie charts into VisBuilder using Vega rendering. Issue Resolve: opensearch-project#7607 --------- Signed-off-by: Anan Zhuang <[email protected]>

github-actions bot added the distinguished-contributor label Aug 19, 2024

github-actions bot added the failed changeset label Aug 19, 2024

ananzh added v2.17.0 visbuilder-next visbuilder with vega integration vega labels Aug 19, 2024

github-actions bot added Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry and removed failed changeset labels Aug 19, 2024

ananzh force-pushed the vega-slices branch 2 times, most recently from c15a84d to 4a1f149 Compare August 20, 2024 16:28

ananzh force-pushed the vega-slices branch from 4a1f149 to b6f3345 Compare August 20, 2024 17:20

ananzh force-pushed the vega-slices branch from b6f3345 to 8f2c193 Compare August 20, 2024 17:26

opensearch-changeset-bot bot added a commit to ananzh/OpenSearch-Dashboards that referenced this pull request Aug 20, 2024

Changeset file for PR opensearch-project#7752 created/updated

faa2e17

github-actions bot removed the Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry label Aug 20, 2024

ananzh force-pushed the vega-slices branch from faa2e17 to 1829179 Compare August 20, 2024 17:29

ananzh marked this pull request as ready for review August 20, 2024 17:30

ananzh requested review from kavilla, AMoo-Miki, ashwin-pc, joshuarrrr, abbyhu2000, zengyan-amazon, zhongnansu, manasvinibs, ZilongX and Flyingliuhub as code owners August 20, 2024 17:30

ananzh requested review from LDrago27, virajsanghvi, sejli and joshuali925 as code owners August 20, 2024 17:30

opensearch-changeset-bot bot added a commit to ananzh/OpenSearch-Dashboards that referenced this pull request Aug 20, 2024

Changeset file for PR opensearch-project#7752 created/updated

9224249

ananzh force-pushed the vega-slices branch 2 times, most recently from 1309ec9 to 5e59033 Compare August 20, 2024 18:25

opensearch-changeset-bot bot added a commit to ananzh/OpenSearch-Dashboards that referenced this pull request Aug 20, 2024

Changeset file for PR opensearch-project#7752 created/updated

4a9e611

ananzh force-pushed the vega-slices branch from 4a9e611 to 54eb5ca Compare August 20, 2024 19:17

opensearch-changeset-bot bot added a commit to ananzh/OpenSearch-Dashboards that referenced this pull request Aug 20, 2024

Changeset file for PR opensearch-project#7752 created/updated

4c8e90b

AMoo-Miki reviewed Aug 20, 2024

View reviewed changes

src/plugins/vis_builder/public/application/utils/validations/validate_aggregations.ts Outdated Show resolved Hide resolved

AMoo-Miki reviewed Aug 20, 2024

View reviewed changes

src/plugins/vis_builder/public/visualizations/index.ts Outdated Show resolved Hide resolved

AMoo-Miki reviewed Aug 20, 2024

View reviewed changes

src/plugins/vis_builder/public/visualizations/vega/utils/helpers.ts Outdated Show resolved Hide resolved

sejli removed the v2.17.0 label Sep 3, 2024

ananzh added 2 commits October 10, 2024 20:33

[VisBuilder-Next] Pie Chart Integration for VisBuilder

5512675

This PR integrates pie charts into VisBuilder using Vega rendering. Issue Resolve: opensearch-project#7607 Signed-off-by: Anan Zhuang <[email protected]>

fix comments

59d1126

Signed-off-by: Anan Zhuang <[email protected]>

ananzh force-pushed the vega-slices branch from 4c8e90b to 59d1126 Compare October 10, 2024 20:52

ananzh requested a review from huyaboo as a code owner October 10, 2024 20:52

AMoo-Miki approved these changes Oct 10, 2024

View reviewed changes

LDrago27 reviewed Oct 10, 2024

View reviewed changes

LDrago27 approved these changes Oct 10, 2024

View reviewed changes

ananzh added v2.18.0 backport 2.x labels Oct 10, 2024

ananzh merged commit 615d7d4 into opensearch-project:main Oct 10, 2024
72 checks passed

opensearch-trigger-bot bot mentioned this pull request Oct 10, 2024

[Backport 2.x] [VisBuilder-Next] Pie Chart Integration for VisBuilder #8553

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[VisBuilder-Next] Pie Chart Integration for VisBuilder #7752

[VisBuilder-Next] Pie Chart Integration for VisBuilder #7752

ananzh commented Aug 19, 2024 •

edited

Loading

github-actions bot commented Aug 19, 2024

codecov bot commented Aug 19, 2024 •

edited

Loading

AMoo-Miki Aug 20, 2024

ananzh Oct 10, 2024

AMoo-Miki Aug 20, 2024

ananzh Oct 10, 2024

LDrago27 Oct 10, 2024

ananzh Oct 10, 2024

LDrago27 left a comment

	if (child.children && child.children.length > 0) {
	if (Array.isArray(child.children) && child.children.length > 0) {

[VisBuilder-Next] Pie Chart Integration for VisBuilder #7752

[VisBuilder-Next] Pie Chart Integration for VisBuilder #7752

Conversation

ananzh commented Aug 19, 2024 • edited Loading

Description

Issues Resolved

Screenshot

Changelog

Check List

github-actions bot commented Aug 19, 2024

❌ Empty Changelog Section

codecov bot commented Aug 19, 2024 • edited Loading

Codecov Report

AMoo-Miki Aug 20, 2024

Choose a reason for hiding this comment

ananzh Oct 10, 2024

Choose a reason for hiding this comment

AMoo-Miki Aug 20, 2024

Choose a reason for hiding this comment

ananzh Oct 10, 2024

Choose a reason for hiding this comment

LDrago27 Oct 10, 2024

Choose a reason for hiding this comment

ananzh Oct 10, 2024

Choose a reason for hiding this comment

LDrago27 left a comment

Choose a reason for hiding this comment

ananzh commented Aug 19, 2024 •

edited

Loading

codecov bot commented Aug 19, 2024 •

edited

Loading