Fix splits in resolvers

src/common/utils/rules/split-validators.ts

@@ -0,0 +1,84 @@
+/*
+ * Copyright 2017-2021 Allegro.pl
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { NORMAL_COLORS } from "../../models/colors/colors";
+import { Dimension } from "../../models/dimension/dimension";
+import { SeriesList } from "../../models/series-list/series-list";
+import { DimensionSort, SeriesSort, SortDirection } from "../../models/sort/sort";
+import { Split, SplitType } from "../../models/split/split";
+import { thread } from "../functional/functional";
+
+export function fixColorSplit(split: Split, dimension: Dimension, series: SeriesList): Split {
+  return thread(
+    split,
+    fixSort(dimension, series),
+    // TODO: This magic 5 will disappear in #756
+    fixLimit([5, NORMAL_COLORS.length])
+  );
+}
+
+export function fixContinuousTimeSplit(split: Split): Split {
+  const { reference } = split;
+  return split
+    .changeLimit(null)
+    .changeSort(new DimensionSort({
+      reference,
+      direction: SortDirection.ascending
+    }));
+}
+
+export function fixLimit(availableLimits: number[], defaultLimit = availableLimits[0]) {
+  return function(split: Split): Split {
+    const { limit } = split;
+    return availableLimits.indexOf(limit) === -1
+      ? split.changeLimit(defaultLimit)
+      : split;
+  };
+}
+
+export function fixSort(dimension: Dimension, series: SeriesList, availableDimensions = [dimension.name]) {
+  return function(split: Split): Split {
+    const { sort } = split;
+    if (sort instanceof SeriesSort) return split;
+    if (availableDimensions.indexOf(sort.reference) !== -1) return split;
+    const direction = SortDirection.descending;
+    const { sortStrategy } = dimension;
+    if (sortStrategy) {
+      if (sortStrategy === "self" || split.reference === sortStrategy) {
+        return split.changeSort(new DimensionSort({
+          reference: split.reference,
+          direction
+        }));
+      }
+      if (series.hasMeasureSeries(sortStrategy)) {
+        return split.changeSort(new DimensionSort({
+          reference: sortStrategy,
+          direction
+        }));
+      }
+    }
+    if (split.type === SplitType.string) {
+      split = split.changeSort(new SeriesSort({
+        reference: series.series.first().reference,
+        direction: SortDirection.descending
+      }));
+    }
+    return split.changeSort(new DimensionSort({
+      reference: split.reference,
+      direction
+    }));
+  };
+}

src/common/visualization-manifests/bar-chart/bar-chart.ts

@@ -16,135 +16,96 @@
  */
 
 import { List } from "immutable";
-import { clamp } from "../../../client/utils/dom/dom";
-import { NORMAL_COLORS } from "../../models/colors/colors";
-import { canBucketByDefault, Dimension } from "../../models/dimension/dimension";
+import { Dimension } from "../../models/dimension/dimension";
 import { allDimensions, findDimensionByName } from "../../models/dimension/dimensions";
-import { DimensionSort, SortDirection } from "../../models/sort/sort";
-import { Split } from "../../models/split/split";
+import { Split, SplitType } from "../../models/split/split";
 import { Splits } from "../../models/splits/splits";
-import { NORMAL_PRIORITY_ACTION, Resolve, VisualizationManifest } from "../../models/visualization-manifest/visualization-manifest";
+import {
+  NORMAL_PRIORITY_ACTION,
+  Resolve,
+  VisualizationManifest
+} from "../../models/visualization-manifest/visualization-manifest";
 import { emptySettingsConfig } from "../../models/visualization-settings/empty-settings-config";
+import { thread } from "../../utils/functional/functional";
 import { Actions } from "../../utils/rules/actions";
 import { Predicates } from "../../utils/rules/predicates";
+import { fixColorSplit, fixContinuousTimeSplit, fixLimit, fixSort } from "../../utils/rules/split-validators";
 import { visualizationDependentEvaluatorBuilder } from "../../utils/rules/visualization-dependent-evaluator";
 
-function isNominalSplitValid(nominalSplit: Split): boolean {
-  return nominalSplit.limit !== null && nominalSplit.limit <= NORMAL_COLORS.length;
-}
-
-function isTimeSplitValid(timeSplit: Split): boolean {
-  const sortByTime = new DimensionSort({
-    reference: timeSplit.reference,
-    direction: SortDirection.ascending
-  });
-
-  const isTimeSortValid = timeSplit.sort.equals(sortByTime);
-  const isTimeLimitValid = timeSplit.limit === null;
-
-  return isTimeSortValid && isTimeLimitValid;
-}
-
-// TODO: This magic 5 will disappear in #756
-const clampNominalSplitLimit = (split: Split) => split
-  .update("limit", limit =>
-    clamp(limit, 5, NORMAL_COLORS.length));
-
-const fixTimeSplit = (split: Split) => {
-  const { reference } = split;
-  return split
-    .changeLimit(null)
-    .changeSort(new DimensionSort({
-      reference,
-      direction: SortDirection.ascending
-    }));
-};
-
 const rulesEvaluator = visualizationDependentEvaluatorBuilder
   .when(Predicates.noSplits())
   .then(Actions.manualDimensionSelection("The Bar Chart requires at least one split"))
 
   .when(Predicates.areExactSplitKinds("time"))
   .then(({ splits, isSelectedVisualization }) => {
     const timeSplit = splits.getSplit(0);
-    if (isTimeSplitValid(timeSplit)) return Resolve.ready(isSelectedVisualization ? 10 : 3);
+    const newTimeSplit = fixContinuousTimeSplit(timeSplit);
+    if (timeSplit.equals(newTimeSplit)) return Resolve.ready(isSelectedVisualization ? 10 : 3);
     return Resolve.automatic(6, {
       splits: new Splits({
-        splits: List([
-          fixTimeSplit(timeSplit)
-        ])
+        splits: List([newTimeSplit])
       })
     });
   })
 
   .when(Predicates.areExactSplitKinds("time", "*"))
-  .then(({ splits }) => {
+  .then(({ splits, series, dataCube }) => {
     const timeSplit = splits.getSplit(0);
     const nominalSplit = splits.getSplit(1);
+    const nominalDimension = findDimensionByName(dataCube.dimensions, nominalSplit.reference);
 
     return Resolve.automatic(6, {
       // Switch splits in place and conform
       splits: new Splits({
         splits: List([
-          clampNominalSplitLimit(nominalSplit),
-          fixTimeSplit(timeSplit)
+          fixColorSplit(nominalSplit, nominalDimension, series),
+          fixContinuousTimeSplit(timeSplit)
         ])
       })
     });
   })
   .when(Predicates.areExactSplitKinds("*", "time"))
-  .then(({ splits, isSelectedVisualization }) => {
-    const nominalSplit = splits.getSplit(0);
+  .then(({ splits, series, dataCube, isSelectedVisualization }) => {
     const timeSplit = splits.getSplit(1);
+    const nominalSplit = splits.getSplit(0);
+    const nominalDimension = findDimensionByName(dataCube.dimensions, nominalSplit.reference);
+
+    const newSplits = new Splits({
+      splits: List([
+        fixColorSplit(nominalSplit, nominalDimension, series),
+        fixContinuousTimeSplit(timeSplit)
+      ])
+    });
 
-    if (isTimeSplitValid(timeSplit) && isNominalSplitValid(nominalSplit)) return Resolve.ready(isSelectedVisualization ? 10 : 3);
+    const changed = !splits.equals(newSplits);
+    if (!changed) return Resolve.ready(isSelectedVisualization ? 10 : 3);
     return Resolve.automatic(6, {
-      splits: new Splits({
-        splits: List([
-          clampNominalSplitLimit(nominalSplit),
-          fixTimeSplit(timeSplit)
-        ])
-      })
+      splits: newSplits
     });
 
   })
 
   .when(Predicates.areExactSplitKinds("*"))
   .or(Predicates.areExactSplitKinds("*", "*"))
-  .then(({ splits, dataCube, isSelectedVisualization }) => {
-    let continuousBoost = 0;
-
-    // Auto adjustment
-    let autoChanged = false;
+  .then(({ splits, series, dataCube, isSelectedVisualization }) => {
+    const hasNumberSplits = splits.splits.some(split => split.type === SplitType.number);
+    const continuousBoost = hasNumberSplits ? 4 : 0;
 
     const newSplits = splits.update("splits", splits => splits.map((split: Split) => {
       const splitDimension = findDimensionByName(dataCube.dimensions, split.reference);
-      if (canBucketByDefault(splitDimension) && split.sort.reference !== splitDimension.name) {
-        split = split.changeSort(new DimensionSort({
-          reference: splitDimension.name,
-          direction: split.sort.direction
-        }));
-        autoChanged = true;
-      }
-
-      if (splitDimension.kind === "number") {
-        continuousBoost = 4;
-      }
-
-      // ToDo: review this
-      if (!split.limit && (autoChanged || splitDimension.kind !== "time")) {
-        split = split.changeLimit(25);
-        autoChanged = true;
-      }
-
-      return split;
+      return thread(
+        split,
+        fixLimit(splitDimension.limits),
+        fixSort(splitDimension, series)
+      );
     }));
 
-    if (autoChanged) {
+    const changed = !splits.equals(newSplits);
+    if (changed) {
       return Resolve.automatic(5 + continuousBoost, { splits: newSplits });
     }
 
-    return Resolve.ready(isSelectedVisualization ? 10 : (7 + continuousBoost));
+    return Resolve.ready(isSelectedVisualization ? 10 : 7 + continuousBoost);
   })
 
   .otherwise(({ dataCube }) => {

src/common/visualization-manifests/grid/grid.ts

@@ -14,10 +14,13 @@
  * limitations under the License.
  */
 
+import { findDimensionByName } from "../../models/dimension/dimensions";
 import { Resolve, VisualizationManifest } from "../../models/visualization-manifest/visualization-manifest";
 import { emptySettingsConfig } from "../../models/visualization-settings/empty-settings-config";
+import { thread } from "../../utils/functional/functional";
 import { Actions } from "../../utils/rules/actions";
 import { Predicates } from "../../utils/rules/predicates";
+import { fixLimit, fixSort } from "../../utils/rules/split-validators";
 import { visualizationDependentEvaluatorBuilder } from "../../utils/rules/visualization-dependent-evaluator";
 
 export const GRID_LIMITS = [50, 100, 200, 500, 1000, 10000];
@@ -29,12 +32,17 @@ const rulesEvaluator = visualizationDependentEvaluatorBuilder
   .when(Predicates.noSelectedMeasures())
   .then(Actions.manualMeasuresSelection())
 
-  .otherwise(({ isSelectedVisualization, splits }) => {
+  .otherwise(({ isSelectedVisualization, series, dataCube, splits }) => {
     const firstSplit = splits.getSplit(0);
-    const { limit: firstLimit } = firstSplit;
-    const safeFirstLimit = GRID_LIMITS.indexOf(firstLimit) === -1 ? GRID_LIMITS[0] : firstLimit;
 
-    const newSplits = splits.replace(firstSplit, firstSplit.changeLimit(safeFirstLimit));
+    const splitReferences = splits.splits.toArray().map(split => split.reference);
+    const dimension = findDimensionByName(dataCube.dimensions, firstSplit.reference);
+    const fixedFirstSplit = thread(
+      firstSplit,
+      fixLimit(GRID_LIMITS),
+      fixSort(dimension, series, splitReferences)
+    );
+    const newSplits = splits.replace(firstSplit, fixedFirstSplit);
 
     if (splits.equals(newSplits)) {
       return Resolve.ready(isSelectedVisualization ? 10 : 4);

src/common/visualization-manifests/heat-map/heat-map.ts

@@ -18,11 +18,12 @@
 import { allDimensions, findDimensionByName } from "../../models/dimension/dimensions";
 import { allMeasures } from "../../models/measure/measures";
 import { MeasureSeries } from "../../models/series/measure-series";
-import { DimensionSort, isSortEmpty, SeriesSort, SortDirection } from "../../models/sort/sort";
-import { Split, SplitType } from "../../models/split/split";
+import { Split } from "../../models/split/split";
 import { Resolve, VisualizationManifest } from "../../models/visualization-manifest/visualization-manifest";
 import { emptySettingsConfig } from "../../models/visualization-settings/empty-settings-config";
+import { thread } from "../../utils/functional/functional";
 import { Predicates } from "../../utils/rules/predicates";
+import { fixLimit, fixSort } from "../../utils/rules/split-validators";
 import {
   ActionVariables,
   visualizationDependentEvaluatorBuilder
@@ -42,49 +43,20 @@ const rulesEvaluator = visualizationDependentEvaluatorBuilder
     variables.series.series.size === 0 ? suggestAddingMeasure(variables) : suggestRemovingMeasures(variables)
   ))
   .otherwise(({ splits, dataCube, series }) => {
-    let autoChanged = false;
     const newSplits = splits.update("splits", splits => splits.map(split => {
       const splitDimension = findDimensionByName(dataCube.dimensions, split.reference);
-      const sortStrategy = splitDimension.sortStrategy;
 
-      if (isSortEmpty(split.sort)) {
-        if (sortStrategy) {
-          if (sortStrategy === "self" || split.reference === sortStrategy) {
-            split = split.changeSort(new DimensionSort({
-              reference: splitDimension.name,
-              direction: SortDirection.descending
-            }));
-          } else {
-            split = split.changeSort(new SeriesSort({
-              reference: sortStrategy,
-              direction: SortDirection.descending
-            }));
-          }
-        } else {
-          if (split.type === SplitType.string) {
-            split = split.changeSort(new SeriesSort({
-              reference: series.series.first().reference,
-              direction: SortDirection.descending
-            }));
-          } else {
-            split = split.changeSort(new DimensionSort({
-              reference: splitDimension.name,
-              direction: SortDirection.descending
-            }));
-          }
-          autoChanged = true;
-        }
-      }
-
-      if (!split.limit && splitDimension.kind !== "time") {
-        split = split.changeLimit(25);
-        autoChanged = true;
-      }
-
-      return split;
+      return thread(
+        split,
+        fixLimit(splitDimension.limits),
+        fixSort(splitDimension, series)
+      );
     }));
 
-    return autoChanged ? Resolve.automatic(10, { splits: newSplits }) : Resolve.ready(10);
+    const changed = !newSplits.equals(splits);
+    return changed
+      ? Resolve.automatic(10, { splits: newSplits })
+      : Resolve.ready(10);
   })
   .build();