code improvements

elastic · Jun 16, 2022 · 3ca5e4f · 3ca5e4f
1 parent 7c5eb48
commit 3ca5e4f
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 22 deletions.
diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts
@@ -383,7 +383,7 @@ interface BaseOperationDefinitionProps<C extends BaseIndexPatternColumn, P = {}>
    */
   helpComponentTitle?: string;
   /**
-   * Optimizes EsAggs expression. Invoked only once per operation type. May mutate arguments.
+   * Optimizes EsAggs expression. Invoked only once per operation type.
    */
   optimizeEsAggs?: (
     aggs: ExpressionAstExpressionBuilder[],

diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.tsx
@@ -148,8 +148,10 @@ export const percentileOperation: OperationDefinition<
       }
     ).toAst();
   },
-  // TODO - there may be a way to get around including aggExpressionToEsAggsIdMap
-  optimizeEsAggs: (aggs, esAggsIdMap, aggExpressionToEsAggsIdMap) => {
+  optimizeEsAggs: (_aggs, _esAggsIdMap, aggExpressionToEsAggsIdMap) => {
+    let aggs = [..._aggs];
+    const esAggsIdMap = { ..._esAggsIdMap };
+
     const percentileExpressionsByArgs: Record<string, ExpressionAstExpressionBuilder[]> = {};
 
     // group percentile dimensions by differentiating parameters
@@ -171,8 +173,8 @@ export const percentileOperation: OperationDefinition<
 
     // collapse them into a single esAggs expression builder
     Object.values(percentileExpressionsByArgs).forEach((expressionBuilders) => {
-      if (!(expressionBuilders.length > 1)) {
-        // don't need to optimize if there's only one
+      if (expressionBuilders.length <= 1) {
+        // don't need to optimize if there aren't more than one
         return;
       }
 
@@ -203,18 +205,15 @@ export const percentileOperation: OperationDefinition<
 
           const duplicateExpressionBuilder = percentileToBuilder[percentile];
 
-          if (
-            !aggExpressionToEsAggsIdMap.has(builder) ||
-            !aggExpressionToEsAggsIdMap.has(duplicateExpressionBuilder)
-          ) {
+          const idForDuplicate = aggExpressionToEsAggsIdMap.get(duplicateExpressionBuilder);
+          const idForThisOne = aggExpressionToEsAggsIdMap.get(builder);
+
+          if (!idForDuplicate || !idForThisOne) {
             throw new Error(
               "Couldn't find esAggs ID for percentile expression builder... this should never happen."
             );
           }
 
-          const idForDuplicate = aggExpressionToEsAggsIdMap.get(duplicateExpressionBuilder)!;
-          const idForThisOne = aggExpressionToEsAggsIdMap.get(builder)!;
-
           esAggsIdMap[idForDuplicate].push(...esAggsIdMap[idForThisOne]);
 
           delete esAggsIdMap[idForThisOne];

diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts b/x-pack/plugins/lens/public/indexpattern_datasource/to_expression.ts
@@ -38,6 +38,15 @@ declare global {
   }
 }
 
+// esAggs column ID manipulation functions
+const extractEsAggId = (id: string) => id.split('.')[0].split('-')[2];
+const updatePositionIndex = (currentId: string, newIndex: number) => {
+  const [fullId, percentile] = currentId.split('.');
+  const idParts = fullId.split('-');
+  idParts[1] = String(newIndex);
+  return idParts.join('-') + (percentile ? `.${percentile}` : '');
+};
+
 function getExpressionForLayer(
   layer: IndexPatternLayer,
   indexPattern: IndexPattern,
@@ -219,19 +228,12 @@ function getExpressionForLayer(
     */
 
     const updatedEsAggsIdMap: Record<string, OriginalColumn[]> = {};
-    const extractEsAggId = (id: string) => id.split('.')[0].split('-')[2];
-    const updatePositionIndex = (currentId: string, newIndex: number) => {
-      const [fullId, percentile] = currentId.split('.');
-      const idParts = fullId.split('-');
-      idParts[1] = String(newIndex);
-      return idParts.join('-') + (percentile ? `.${percentile}` : '');
-    };
     let counter = 0;
+
+    const esAggsIds = Object.keys(esAggsIdMap);
     aggs.forEach((builder) => {
       const esAggId = builder.functions[0].getArgument('id')?.[0];
-      const matchingEsAggColumnIds = Object.keys(esAggsIdMap).filter(
-        (id) => extractEsAggId(id) === esAggId
-      );
+      const matchingEsAggColumnIds = esAggsIds.filter((id) => extractEsAggId(id) === esAggId);
 
       matchingEsAggColumnIds.forEach((currentId) => {
         const currentColumn = esAggsIdMap[currentId][0];