Destructured visMetaData and dataConfig Objects in all visualizations #1106

Koustubh5585 · 2022-10-11T13:37:20Z

Description

Destructured visMetaData and dataConfig Objects in all visualizations
Fixed No results found alignment issue in charts view

Issues Resolved

Check List

Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Koustubh Karmalkar <[email protected]>

mengweieric · 2022-10-11T21:35:42Z

dashboards-observability/public/components/visualizations/charts/bar/bar.tsx

@@ -33,6 +29,18 @@ export const Bar = ({ visualizations, layout, config }: any) => {
    },
    vis: visMetaData,
  }: IVisualizationContainerProps = visualizations;
+


You could destruct visMetaData at line 30 directly without renaming. Please apply this code review to all other charts that have this similar problem.

Hi @mengweieric, this is already taken care, can you please check once https://github.com/opensearch-project/observability/pull/1106/files (commit id: 95fef81)

mengweieric · 2022-10-11T21:37:28Z

dashboards-observability/public/components/visualizations/charts/lines/line.tsx

@@ -44,6 +44,8 @@ export const Line = ({ visualizations, layout, config }: any) => {
    },
    vis: visMetaData,
  }: IVisualizationContainerProps = visualizations;
+
+  const { icontype, name } = visMetaData;


similar, please refer to above comments.

mengweieric · 2022-10-11T21:37:44Z

dashboards-observability/public/components/visualizations/charts/pie/pie.tsx

@@ -36,6 +36,7 @@ export const Pie = ({ visualizations, layout, config }: any) => {
    vis: visMetaData,
  }: IVisualizationContainerProps = visualizations;

+  const { mode, icontype, showlegend, legendSize, labelSize, legendposition } = visMetaData;


please refer to the first comment.

mengweieric · 2022-10-11T21:37:56Z

dashboards-observability/public/components/visualizations/charts/stats/stats.tsx

@@ -65,6 +65,7 @@ export const Stats = ({ visualizations, layout, config }: any) => {
    vis: visMetaData,
  }: IVisualizationContainerProps = visualizations;

+  const { charttype, titlesize, valuesize, textmode, orientation, precisionvalue } = visMetaData;


please refer to the first comment.

mengweieric · 2022-10-11T21:38:38Z

Please address failing tests.

Signed-off-by: Koustubh Karmalkar <[email protected]>

codecov-commenter · 2022-10-12T11:30:43Z

Codecov Report

Merging #1106 (e71e35f) into main (249ab50) will increase coverage by 0.22%.
The diff coverage is 71.69%.

@@             Coverage Diff              @@
##               main    #1106      +/-   ##
============================================
+ Coverage     53.49%   53.72%   +0.22%     
  Complexity      291      291              
============================================
  Files           279      279              
  Lines          9454     9475      +21     
  Branches       2215     2278      +63     
============================================
+ Hits           5057     5090      +33     
+ Misses         4227     4215      -12     
  Partials        170      170

Flag	Coverage Δ
dashboards-observability	`47.86% <71.69%> (+0.32%)`	⬆️
opensearch-observability	`71.87% <ø> (ø)`

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

Impacted Files	Coverage Δ
.../components/visualizations/charts/maps/heatmap.tsx	`15.15% <36.36%> (+8.90%)`	⬆️
.../public/components/event_analytics/utils/utils.tsx	`52.00% <50.00%> (-0.39%)`	⬇️
...c/components/visualizations/charts/stats/stats.tsx	`9.58% <52.94%> (-0.42%)`	⬇️
...ts/visualizations/charts/data_table/data_table.tsx	`46.66% <54.54%> (ø)`
...ic/components/visualizations/charts/lines/line.tsx	`40.00% <75.00%> (+4.77%)`	⬆️
...ublic/components/visualizations/charts/bar/bar.tsx	`48.42% <75.60%> (+8.17%)`	⬆️
...ts/visualizations/charts/financial/gauge/gauge.tsx	`30.26% <85.00%> (+2.86%)`	⬆️
...ents/visualizations/charts/histogram/histogram.tsx	`92.10% <85.00%> (+5.99%)`	⬆️
...ublic/components/visualizations/charts/pie/pie.tsx	`45.09% <86.66%> (-1.06%)`	⬇️
...components/visualizations/charts/maps/treemaps.tsx	`54.25% <90.00%> (+4.75%)`	⬆️
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

ps48 · 2022-10-13T00:05:00Z

@Koustubh5585 Can you please resolve the conflicts here.

Signed-off-by: Koustubh Karmalkar <[email protected]>

mengweieric · 2022-10-14T16:09:57Z

dashboards-observability/public/components/event_analytics/utils/utils.tsx

@@ -381,8 +381,12 @@ export const getPropName = (queriedVizObj: {
  name: string;
  label: string;
 }) => {
-  if (queriedVizObj[CUSTOM_LABEL] === '' || queriedVizObj[CUSTOM_LABEL] === undefined) {
-    return `${queriedVizObj.aggregation}(${queriedVizObj.name})`;
+  if (queriedVizObj) {


Can we take look at reviews from previous PRs and having one more reviewer internally to review to avoid spending effort reviewing the same issues? Previously we've reviewed some similar code already like this, below code piece can be further simplified as

if (queriedVizObj && !isEmpty(queriedVizObj[CUSTOM_LABEL])) return `${queriedVizObj.aggregation}(${queriedVizObj.name})`; return '';

@mengweieric Sure, will ensure in next PRs that such small issues can be addressed internally.

Koustubh5585 added 9 commits October 10, 2022 12:39

Bug:1058 destructure of visMetaData object

412c48c

Signed-off-by: Koustubh Karmalkar <[email protected]>

Fixed charts no results found allignment issue

662b3b9

Signed-off-by: Koustubh Karmalkar <[email protected]>

Code optimization

95fef81

Signed-off-by: Koustubh Karmalkar <[email protected]>

Destructured dataConfig object

00f88cf

Signed-off-by: Koustubh Karmalkar <[email protected]>

Bug:1058 destructure of visMetaData object

50fbaed

Signed-off-by: Koustubh Karmalkar <[email protected]>

Fixed charts no results found allignment issue

a706333

Signed-off-by: Koustubh Karmalkar <[email protected]>

Code optimization

9541963

Signed-off-by: Koustubh Karmalkar <[email protected]>

Destructured dataConfig object

e3318cf

Signed-off-by: Koustubh Karmalkar <[email protected]>

Merged latest main

e07a83e

Signed-off-by: Koustubh Karmalkar <[email protected]>

Koustubh5585 requested a review from a team as a code owner October 11, 2022 13:37

mengweieric reviewed Oct 11, 2022

View reviewed changes

Koustubh5585 added 2 commits October 12, 2022 10:58

Code optimized for dataConfig destructuring

55844c4

Signed-off-by: Koustubh Karmalkar <[email protected]>

Merged latest main

676675e

Signed-off-by: Koustubh Karmalkar <[email protected]>

Koustubh5585 added 2 commits October 13, 2022 07:13

Merged latest main

0ebc010

Signed-off-by: Koustubh Karmalkar <[email protected]>

Merged latest main and removed conflicts

e71e35f

Signed-off-by: Koustubh Karmalkar <[email protected]>

mengweieric self-requested a review October 14, 2022 16:04

mengweieric reviewed Oct 14, 2022

View reviewed changes

mengweieric self-requested a review October 14, 2022 21:45

mengweieric approved these changes Oct 14, 2022

View reviewed changes

sejli approved these changes Oct 14, 2022

View reviewed changes

mengweieric merged commit 459289c into opensearch-project:main Oct 14, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Destructured visMetaData and dataConfig Objects in all visualizations #1106

Destructured visMetaData and dataConfig Objects in all visualizations #1106

Koustubh5585 commented Oct 11, 2022

mengweieric Oct 11, 2022

Koustubh5585 Oct 12, 2022 •

edited

Loading

mengweieric Oct 11, 2022

mengweieric Oct 11, 2022

mengweieric Oct 11, 2022

mengweieric commented Oct 11, 2022

codecov-commenter commented Oct 12, 2022 •

edited

Loading

ps48 commented Oct 13, 2022

mengweieric Oct 14, 2022

Koustubh5585 Oct 17, 2022

Destructured visMetaData and dataConfig Objects in all visualizations #1106

Destructured visMetaData and dataConfig Objects in all visualizations #1106

Conversation

Koustubh5585 commented Oct 11, 2022

Description

Issues Resolved

Check List

mengweieric Oct 11, 2022

Choose a reason for hiding this comment

Koustubh5585 Oct 12, 2022 • edited Loading

Choose a reason for hiding this comment

mengweieric Oct 11, 2022

Choose a reason for hiding this comment

mengweieric Oct 11, 2022

Choose a reason for hiding this comment

mengweieric Oct 11, 2022

Choose a reason for hiding this comment

mengweieric commented Oct 11, 2022

codecov-commenter commented Oct 12, 2022 • edited Loading

Codecov Report

ps48 commented Oct 13, 2022

mengweieric Oct 14, 2022

Choose a reason for hiding this comment

Koustubh5585 Oct 17, 2022

Choose a reason for hiding this comment

Koustubh5585 Oct 12, 2022 •

edited

Loading

codecov-commenter commented Oct 12, 2022 •

edited

Loading