[Lens] Rename operations to map exposed names for Formula #94710
Conversation
dej611
added
release_note:skip
|
Pinging @elastic/kibana-app (Team:KibanaApp) |
|
@elasticmachine merge upstream |
|
Tested ML related changes and LGTM 👍 |
|
@elasticmachine merge upstream |
| @@ -76,7 +76,6 @@ function buildMetricOperation<T extends MetricColumn<string>>({ | |||
| }, | |||
There was a problem hiding this comment.
There is a problem in this line: https://github.com/elastic/kibana/pull/94710/files#diff-e3c30f7e1cc23b9be6b4b2d689290e96925f48369dfcaa44d641014dfdc7bc80R68
The aggregation restriction mapping logic in
is using the Elasticsearch aggregation names, but now they are not in sync anymore (avg vs average). Aggregation restrictions for the average operation won't be loaded correctly anymore. We have to check aggregation restrictions for avg instead of average.
| @@ -219,7 +219,7 @@ export const createMockedRestrictedIndexPattern = () => { | |||
| interval: 1000, | |||
| }, | |||
| }, | |||
| avg: { | |||
| average: { | |||
There was a problem hiding this comment.
This won't happen - typeMeta is not controlled by Lens and will return avg
| }; | ||
|
|
||
| export const derivativeOperation: OperationDefinition< | ||
| DerivativeIndexPatternColumn, | ||
| 'fullReference' | ||
| > = { | ||
| type: 'derivative', | ||
| type: 'differences', |
There was a problem hiding this comment.
If you create a constant for the name, why not use it here as well?
|
@elasticmachine merge upstream |
|
merge conflict between base and head |
| export const renameOperationsMapping: Record<string, GenericOperationDefinition['type']> = { | ||
| avg: 'average', | ||
| cardinality: 'unique_count', | ||
| derivative: 'differences', |
There was a problem hiding this comment.
Our derivative/differences implementation is not based on the Elasticsearch aggregation, it shouldn't show up here.
There was a problem hiding this comment.
I thought about it, but wanted to create a generic function for the renaming. Do you think this may lead to issues?
There was a problem hiding this comment.
I'm more concerned about this being confusing - Elasticsearch derivative and Lens differences do not share anything, so it doesn't make sense to map one to the other.
dej611
added
the
auto-backport
I forgot to check this before, but it looks working now:
Also it was giving me |
|
I checked it as well and it worked for me. Might be just a bad cache on my side when I made the comment. |
|
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
) Co-authored-by: Kibana Machine <[email protected]>
…95500) Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Marco Liberati <[email protected]>
Summary
This PR renames the
avg,cardinalityandderivativesoperation internally to respectively:average,unique_count,differences.This change will help the user in the upcoming Formula PR to quickly reference functions based on the similarity of new names with the ones shown in the panel.
Changes:
Checklist