Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG]: Visualizations not appearing in Operational Panels fixed #1170

Conversation

SivaprasadAluri
Copy link
Contributor

Signed-off-by: SivaprasadAluri [email protected]

Description

Fixed the Saved visualizations will appear in operational panel after creating the panel

Issues Resolved

[BUG]: Visualizations not appearing in Operational Panels (#907)

@SivaprasadAluri SivaprasadAluri requested a review from a team as a code owner October 20, 2022 14:26
@mengweieric mengweieric requested a review from ps48 October 20, 2022 16:31
return <></>;
}
return (
<Visualization
visualizations={getVizContainerProps({
vizId: type,
rawVizData: data,
query: {},
query: { rawQuery: metaData.query },
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to handle a corner case when metaData does not contain query field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it has been handled when metaData is empty object, it wont render Visualization, and also we can not save the data without query. I believe it is not required!.

@@ -289,15 +289,15 @@ export const isPPLFilterValid = (

// Renders visualization in the vizualization container component
export const displayVisualization = (metaData: any, data: any, type: string) => {
if (metaData === undefined || metaData === {}) {
if (metaData === undefined || Object.keys(metaData).length === 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

isEmpty(metaData) might look cleaner

Copy link
Contributor

Choose a reason for hiding this comment

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

+1, please adjust.

@SivaprasadAluri SivaprasadAluri force-pushed the bug/operationalPanel_Issue_907 branch from 7fea280 to 51e5067 Compare October 21, 2022 12:25
@codecov-commenter
Copy link

codecov-commenter commented Oct 21, 2022

Codecov Report

Merging #1170 (51e5067) into main (e486d06) will increase coverage by 2.10%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main    #1170      +/-   ##
============================================
+ Coverage     53.70%   55.81%   +2.10%     
  Complexity      291      291              
============================================
  Files           279      279              
  Lines          9490     9490              
  Branches       2244     2244              
============================================
+ Hits           5097     5297     +200     
+ Misses         4223     4021     -202     
- Partials        170      172       +2     
Flag Coverage Δ
dashboards-observability 50.64% <100.00%> (+2.78%) ⬆️
opensearch-observability 71.87% <ø> (ø)

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

Impacted Files Coverage Δ
.../public/components/custom_panels/helpers/utils.tsx 82.85% <100.00%> (ø)
...ic/components/visualizations/charts/lines/line.tsx 37.89% <0.00%> (-2.11%) ⬇️
.../public/components/event_analytics/utils/utils.tsx 52.66% <0.00%> (+0.66%) ⬆️
...ublic/components/visualizations/charts/bar/bar.tsx 69.47% <0.00%> (+21.05%) ⬆️
.../query_manager/ast/expression/AggregateFunction.ts 33.33% <0.00%> (+33.33%) ⬆️
...lity/common/query_manager/ast/tree/aggragations.ts 35.29% <0.00%> (+35.29%) ⬆️
...ponents/visualizations/charts/helpers/viz_types.ts 66.94% <0.00%> (+35.59%) ⬆️
...mmon/query_manager/ast/expression/AggregateTerm.ts 44.44% <0.00%> (+44.44%) ⬆️
...vability/common/query_manager/ppl_query_manager.ts 50.00% <0.00%> (+50.00%) ⬆️
...rds-observability/common/query_manager/ast/node.ts 55.55% <0.00%> (+55.55%) ⬆️
... and 8 more

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

joshuali925
joshuali925 previously approved these changes Oct 21, 2022
anirudha
anirudha previously approved these changes Oct 21, 2022
@SivaprasadAluri SivaprasadAluri dismissed stale reviews from anirudha and joshuali925 via 1c37c5f October 24, 2022 12:16
@SivaprasadAluri SivaprasadAluri force-pushed the bug/operationalPanel_Issue_907 branch from 51e5067 to 1c37c5f Compare October 24, 2022 12:16
Signed-off-by: SivaprasadAluri <[email protected]>
@SivaprasadAluri SivaprasadAluri requested review from pjfitzgibbons and anirudha and removed request for mengweieric and pjfitzgibbons October 24, 2022 14:04
@mengweieric mengweieric self-requested a review October 24, 2022 16:26
@mengweieric mengweieric merged commit edbc7cb into opensearch-project:main Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants