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

[Visualizations] Adds visConfig.title and uiState to build pipeline function #2192

Merged
merged 1 commit into from
Sep 6, 2022

Conversation

fbaligand
Copy link
Contributor

In pipeline building, this PR adds visConfig.title and uiState properties for "visualization" function.

This aims 2 goals:

  • To be consistent with pipelines built with "buildPipelineVisFunction".
  • To provide "title" and "uiState" informations for visualizations.

This is currently a missing information for community plugins.

@fbaligand fbaligand requested a review from a team as a code owner August 23, 2022 22:16
@ashwin-pc
Copy link
Member

Hey @fbaligand can you open an issue to discuss your usecase and how you intend on using this additional information to build community plugins? The change looks good to me, but i will need to make sure that it does not break anything since the visualization renderer cant handle a changing UIState and adding this might cause issues with that.

@codecov-commenter
Copy link

Codecov Report

Merging #2192 (6e5112d) into main (6120cf5) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #2192      +/-   ##
==========================================
- Coverage   67.23%   67.22%   -0.01%     
==========================================
  Files        3100     3100              
  Lines       59566    59567       +1     
  Branches     9063     9063              
==========================================
- Hits        40047    40043       -4     
- Misses      17332    17336       +4     
- Partials     2187     2188       +1     
Impacted Files Coverage Δ
...ins/visualizations/public/legacy/build_pipeline.ts 48.92% <0.00%> (-0.27%) ⬇️
...s/osd-optimizer/src/node/node_auto_tranpilation.ts 83.67% <0.00%> (-4.09%) ⬇️
packages/osd-optimizer/src/node/cache.ts 50.00% <0.00%> (-2.78%) ⬇️

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

@fbaligand
Copy link
Contributor Author

fbaligand commented Aug 24, 2022

Hey @fbaligand can you open an issue to discuss your usecase and how you intend on using this additional information to build community plugins? The change looks good to me, but i will need to make sure that it does not break anything since the visualization renderer cant handle a changing UIState and adding this might cause issues with that.

Hi @ashwin-pc, actually, I need this fix so that community plugins (like mine: https://github.com/fbaligand/kibana-enhanced-table) can do "CSV export" with filename "[visualization name].csv" (instead of "export.csv").
This issue has been requested 2 times:
fbaligand/kibana-enhanced-table#155
fbaligand/kibana-enhanced-table#281

And especially, I have done exactly the same PR on Elastic Kibana, that has been merged in 2020 for Kibana 7.11:
elastic/kibana#84456

Actually, this is a bug fix, because, in previous Kibana versions (7.7 and earlier), "CSV export" was already saved as "[visualization name].csv".

@ashwin-pc
Copy link
Member

@fbaligand Thanks for the context! I agree with the change, it was just backwards compatibility that had me a little concerned, specifically with UIState. But given what you said, i don't think that should be a problem. Thanks the fix and the explanation!

…unction

In pipeline building, this PR adds `visConfig.title` and `uiState` properties for "visualization" function.

This aims 2 goals:
-    To be consistent with pipelines built with "buildPipelineVisFunction".
-    To provide "title" and "uiState" informations for visualizations.

This is currently a missing information for community plugins.

Signed-off-by: fbaligand <[email protected]>
@joshuarrrr joshuarrrr merged commit 140c56f into opensearch-project:main Sep 6, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 6, 2022
…unction (#2192)

In pipeline building, this PR adds `visConfig.title` and `uiState` properties for "visualization" function.

This aims 2 goals:
-    To be consistent with pipelines built with "buildPipelineVisFunction".
-    To provide "title" and "uiState" informations for visualizations.

This is currently a missing information for community plugins.

Signed-off-by: fbaligand <[email protected]>

Signed-off-by: fbaligand <[email protected]>
(cherry picked from commit 140c56f)
@fbaligand fbaligand deleted the patch-1 branch September 6, 2022 19:47
@fbaligand
Copy link
Contributor Author

Thanks for having merged the PR!

joshuarrrr pushed a commit that referenced this pull request Sep 6, 2022
…unction (#2192)

In pipeline building, this PR adds `visConfig.title` and `uiState` properties for "visualization" function.

This aims 2 goals:
-    To be consistent with pipelines built with "buildPipelineVisFunction".
-    To provide "title" and "uiState" informations for visualizations.

This is currently a missing information for community plugins.

Signed-off-by: fbaligand <[email protected]>

Signed-off-by: fbaligand <[email protected]>
(cherry picked from commit 140c56f)
@ananzh ananzh added the v2.3.0 label Sep 7, 2022
ananzh pushed a commit that referenced this pull request Sep 7, 2022
…unction (#2192) (#2272)

In pipeline building, this PR adds `visConfig.title` and `uiState` properties for "visualization" function.

This aims 2 goals:
-    To be consistent with pipelines built with "buildPipelineVisFunction".
-    To provide "title" and "uiState" informations for visualizations.

This is currently a missing information for community plugins.

Signed-off-by: fbaligand <[email protected]>

Signed-off-by: fbaligand <[email protected]>
(cherry picked from commit 140c56f)

Co-authored-by: Fabien Baligand <[email protected]>
Co-authored-by: Josh Romero <[email protected]>
sipopo pushed a commit to sipopo/OpenSearch-Dashboards that referenced this pull request Sep 14, 2022
…unction (opensearch-project#2192)

In pipeline building, this PR adds `visConfig.title` and `uiState` properties for "visualization" function.

This aims 2 goals:
-    To be consistent with pipelines built with "buildPipelineVisFunction".
-    To provide "title" and "uiState" informations for visualizations.

This is currently a missing information for community plugins.

Signed-off-by: fbaligand <[email protected]>

Signed-off-by: fbaligand <[email protected]>
Signed-off-by: Sergey V. Osipov <[email protected]>
sipopo pushed a commit to sipopo/OpenSearch-Dashboards that referenced this pull request Dec 16, 2022
…unction (opensearch-project#2192)

In pipeline building, this PR adds `visConfig.title` and `uiState` properties for "visualization" function.

This aims 2 goals:
-    To be consistent with pipelines built with "buildPipelineVisFunction".
-    To provide "title" and "uiState" informations for visualizations.

This is currently a missing information for community plugins.

Signed-off-by: fbaligand <[email protected]>

Signed-off-by: fbaligand <[email protected]>
Signed-off-by: Sergey V. Osipov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants