Simplify fetchVisLayers() and pipeline helper fns

Signed-off-by: Tyler Ohlsen <ohltyler@amazon.com>
opensearch-project · Mar 8, 2023 · 4de53b3 · 4de53b3
1 parent c0b8771
commit 4de53b3
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 29 deletions.
diff --git a/src/plugins/vis_augmenter/public/utils/utils.ts b/src/plugins/vis_augmenter/public/utils/utils.ts
@@ -4,7 +4,7 @@
  */
 
 import { get } from 'lodash';
-import { Vis } from '../../../visualizations/public';
+import { Vis } from '../../../../plugins/visualizations/public';
 import {
   formatExpression,
   buildExpressionFunction,
@@ -45,19 +45,13 @@ export const getAugmentVisSavedObjs = async (
  * contains the results from each expression function that was ran.
  */
 export const buildPipelineFromAugmentVisSavedObjs = (objs: ISavedAugmentVis[]): string => {
-  const visLayerExpressionFns = [] as Array<
-    ExpressionAstFunctionBuilder<VisLayerFunctionDefinition>
-  >;
-
   try {
-    objs.forEach((obj: ISavedAugmentVis) => {
-      visLayerExpressionFns.push(
-        buildExpressionFunction<VisLayerFunctionDefinition>(
-          obj.visLayerExpressionFn.name,
-          obj.visLayerExpressionFn.args
-        )
-      );
-    });
+    const visLayerExpressionFns = objs.map((obj: ISavedAugmentVis) =>
+      buildExpressionFunction<VisLayerFunctionDefinition>(
+        obj.visLayerExpressionFn.name,
+        obj.visLayerExpressionFn.args
+      )
+    ) as Array<ExpressionAstFunctionBuilder<VisLayerFunctionDefinition>>;
     const ast = buildExpression(visLayerExpressionFns).toAst();
     return formatExpression(ast);
   } catch (e) {

diff --git a/src/plugins/visualizations/public/embeddable/visualize_embeddable.ts b/src/plugins/visualizations/public/embeddable/visualize_embeddable.ts
@@ -404,20 +404,13 @@ export class VisualizeEmbeddable
     this.abortController = new AbortController();
     const abortController = this.abortController;
 
-    let exprVisLayers = {} as ExprVisLayers;
-    // TODO: final eligibility will be defined as part of a separate effort.
-    // This includes not fetching any layers / not showing any layers, when in the
-    // edit context of the vis
-    // See https://github.com/opensearch-project/OpenSearch-Dashboards/issues/3268
-    if (isEligibleForVisLayers(this.vis)) {
-      exprVisLayers = await this.fetchVisLayers(expressionParams, abortController);
-    }
+    const visLayers = await this.fetchVisLayers(expressionParams, abortController);
 
     this.expression = await buildPipeline(this.vis, {
       timefilter: this.timefilter,
       timeRange: this.timeRange,
       abortSignal: this.abortController!.signal,
-      visLayers: isEmpty(exprVisLayers) ? ([] as VisLayers) : exprVisLayers.layers,
+      visLayers: visLayers,
     });
 
     if (this.handler && !abortController.signal.aborted) {
@@ -490,18 +483,26 @@ export class VisualizeEmbeddable
   /**
    * Collects any VisLayers from plugin expressions functions
    * by fetching all AugmentVisSavedObjects that match the vis
-   * saved object ID
+   * saved object ID.
+   *
+   * TODO: final eligibility will be defined as part of a separate effort.
+   * Right now we have a placeholder function isEligibleForVisLayers() which
+   * is used below. For more details, see
+   * https://github.com/opensearch-project/OpenSearch-Dashboards/issues/3268
    */
   fetchVisLayers = async (
     expressionParams: IExpressionLoaderParams,
     abortController: AbortController
-  ): Promise<ExprVisLayers> => {
-    let exprVisLayers = {} as ExprVisLayers;
+  ): Promise<VisLayers> => {
     const augmentVisSavedObjs = await getAugmentVisSavedObjs(
       this.vis.id,
       this.savedAugmentVisLoader
     );
-    if (!isEmpty(augmentVisSavedObjs) && !abortController.signal.aborted) {
+    if (
+      !isEmpty(augmentVisSavedObjs) &&
+      !abortController.signal.aborted &&
+      isEligibleForVisLayers(this.vis)
+    ) {
       const visLayersPipeline = buildPipelineFromAugmentVisSavedObjs(augmentVisSavedObjs);
       // The initial input for the pipeline will just be an empty arr of VisLayers. As plugin
       // expression functions are ran, they will incrementally append their generated VisLayers to it.
@@ -511,12 +512,13 @@ export class VisualizeEmbeddable
       };
       // We cannot use this.handler in this case, since it does not support the run() cmd
       // we need here. So, we consume the expressions service to run this instead.
-      exprVisLayers = (await getExpressions().run(
+      const exprVisLayers = (await getExpressions().run(
         visLayersPipeline,
         visLayersPipelineInput,
         expressionParams as Record<string, unknown>
       )) as ExprVisLayers;
+      return exprVisLayers.layers;
     }
-    return exprVisLayers;
+    return [] as VisLayers;
   };
 }
diff --git a/src/plugins/visualizations/public/legacy/build_pipeline.ts b/src/plugins/visualizations/public/legacy/build_pipeline.ts
@@ -33,7 +33,7 @@ import moment from 'moment';
 import { formatExpression, SerializedFieldFormat } from '../../../../plugins/expressions/public';
 import { IAggConfig, search, TimefilterContract } from '../../../../plugins/data/public';
 import { Vis, VisParams } from '../types';
-import { VisLayers } from '../../../vis_augmenter/public';
+import { VisLayers } from '../../../../plugins/vis_augmenter/public';
 
 const { isDateHistogramBucketAggConfig } = search.aggs;