createAggConfig
should allow order-specific additions
#3159
Labels
enhancement
New feature or request
unified visualization UX
v2.5.0
'Issues and PRs related to version v2.5.0'
vis builder
visualizations
Issues and PRs related to visualizations
Is your feature request related to a problem? Please describe.
When creating a new
AggConfig
, the default behavior is to also add the newly createdAggConfig
to the existing list ofaggConfigs
:OpenSearch-Dashboards/src/plugins/data/common/search/aggs/agg_configs.ts
Line 149 in bfc59d4
However, order matters when specifying aggregations, and the aggregation
Schema
supports the propertymustBeFirst
, which is used by the pie chart to ensure that slices are calculated accurately. The simplest, and likely best, fix is to updatecreateAggConfig
to take an additional parameter that allows the new AggConfig to be added to the beginning of the aggConfigs array rather than the end.Describe the solution you'd like
Maintain existing behavior of
createAggConfig
, but add a new parameter that can switch the insertion point from the end of the array to the beginning.Describe alternatives you've considered
The existing logic to enforce
mustBeFirst
constraints is in thevis_default_editor
plugin. But for adding pie charts to VisBuilder, we'd have to recreate this logic, since VisBuilder doesn't use thevis_default_editor
. In addition, we'd just like to always order the aggs correctly, rather than displaying and error message and forcing the user to reorder (as the current Pie chart visualization does)Doing so downstream of the agg service means separating the creation of the new
AggConfig
from the process of adding it to the existingaggConfigs
, and ends up with lots of duplication of the order checking logic and direct manipulation of theaggConfigs
array. For example, one implementation might look something like this:Conceptually, the agg service should provide the right methods and tooling to add, remove, and reorder agg configs without requiring lots of manual array manipulation and cloning.
Additional context
Blocks #1619
The text was updated successfully, but these errors were encountered: