[Fix] incorrect multi-dataset filter domain

Signed-off-by: Ihor Dykhta <[email protected]>

Author: igorDykhta

src/reducers/src/vis-state-updaters.ts

@@ -66,7 +66,6 @@ import {
   parseFieldValue,
   removeLayerFromSplitMaps,
   set,
-  mergeFilterDomainStep,
   updateFilterPlot,
   removeFilterPlot,
   isLayerAnimatable
@@ -142,7 +141,8 @@ import {
   updateTimeFilterPlotType,
   getDefaultTimeFormat,
   LayerToFilterTimeInterval,
-  TIME_INTERVALS_ORDERED
+  TIME_INTERVALS_ORDERED,
+  mergeFilterDomain
 } from '@kepler.gl/utils';
 import {createEffect} from '@kepler.gl/effects';
 import {PayloadAction} from '@reduxjs/toolkit';
@@ -1192,20 +1192,12 @@ function _removeFilterDataIdAtValueIndex(filter, valueIndex, datasets) {
   }
 
   // mergeFieldDomain for the remaining fields
-  // @ts-expect-error figure out correct types to use with mergeFilterDomainStep
-  let domainSteps: Filter & {step?: number | undefined} = {};
-  filter.dataId.forEach((filterDataId, idx) => {
-    const dataset = datasets[filterDataId];
-    const filterProps = dataset.getColumnFilterProps(filter.name[idx]);
-    // @ts-expect-error figure out correct types to use with mergeFilterDomainStep
-    domainSteps = mergeFilterDomainStep(domainSteps, filterProps);
-  });
+  const domainSteps = mergeFilterDomain(filter, datasets);
 
   const nextFilter = {
     ...filter,
     // value: nextValue,
-    domain: domainSteps.domain,
-    step: domainSteps.step
+    ...(domainSteps ? {domain: domainSteps?.domain, step: domainSteps?.step} : {})
   };
 
   const nextValue = adjustValueToFilterDomain(nextFilter.value, nextFilter);
@@ -1237,7 +1229,8 @@ function _updateFilterProp(state, filter, prop, value, valueIndex, datasetIds?)
       const datasetId = filter.dataId[valueIndex];
       const {filter: updatedFilter, dataset: newDataset} = applyFilterFieldName(
         filter,
-        state.datasets[datasetId],
+        state.datasets,
+        datasetId,
         value,
         valueIndex,
         {mergeDomain: valueIndex > 0}

src/utils/src/filter-utils.ts

@@ -207,14 +207,17 @@ const filterValidators = {
 
 /**
  * Default validate filter function
- * @param dataset
+ * @param datasets
+ * @param datasetId
  * @param filter
  * @return - {filter, dataset}
  */
 export function validateFilter<K extends KeplerTableModel<K, L>, L>(
-  dataset: K,
+  datasets: Record<string, K>,
+  datasetId: string,
   filter: ParsedFilter
 ): {filter: Filter | null; dataset: K} {
+  const dataset = datasets[datasetId];
   // match filter.dataId
   const failed = {dataset, filter: null};
   const filterDataId = toArray(filter.dataId);
@@ -235,7 +238,8 @@ export function validateFilter<K extends KeplerTableModel<K, L>, L>(
   const fieldName = initializeFilter.name[filterDatasetIndex];
   const {filter: updatedFilter, dataset: updatedDataset} = applyFilterFieldName(
     initializeFilter,
-    dataset,
+    datasets,
+    datasetId,
     fieldName,
     filterDatasetIndex,
     {mergeDomain: true}
@@ -262,19 +266,21 @@ export function validateFilter<K extends KeplerTableModel<K, L>, L>(
 /**
  * Validate saved filter config with new data
  *
- * @param dataset
+ * @param datasets
+ * @param datasetId
  * @param filter - filter to be validate
  * @param layers - layers
  * @return validated filter
  */
 export function validateFilterWithData<K extends KeplerTableModel<K, L>, L>(
-  dataset: K,
+  datasets: Record<string, K>,
+  datasetId: string,
   filter: ParsedFilter,
   layers: L[]
 ): {filter: Filter; dataset: K} {
   return filter.type && Object.prototype.hasOwnProperty.call(filterValidators, filter.type)
-    ? filterValidators[filter.type](dataset, filter, layers)
-    : validateFilter(dataset, filter);
+    ? filterValidators[filter.type](datasets[datasetId], filter, layers)
+    : validateFilter(datasets, datasetId, filter);
 }
 
 /**
@@ -913,15 +919,17 @@ export function applyFiltersToDatasets<
 /**
  * Applies a new field name value to filter and update both filter and dataset
  * @param filter - to be applied the new field name on
- * @param dataset - dataset the field belongs to
+ * @param datasets - All datasets
+ * @param datasetId - Id of the dataset the field belongs to
  * @param fieldName - field.name
  * @param filterDatasetIndex - field.name
  * @param option
  * @return - {filter, datasets}
  */
 export function applyFilterFieldName<K extends KeplerTableModel<K, L>, L>(
   filter: Filter,
-  dataset: K,
+  datasets: Record<string, K>,
+  datasetId: string,
   fieldName: string,
   filterDatasetIndex = 0,
   option?: {mergeDomain: boolean}
@@ -935,9 +943,10 @@ export function applyFilterFieldName<K extends KeplerTableModel<K, L>, L>(
       ? option.mergeDomain
       : false;
 
-  const fieldIndex = dataset.getColumnFieldIdx(fieldName);
+  const dataset = datasets[datasetId];
+  const fieldIndex = dataset?.getColumnFieldIdx(fieldName);
   // if no field with same name is found, move to the next datasets
-  if (fieldIndex === -1) {
+  if (!dataset || fieldIndex === -1) {
     // throw new Error(`fieldIndex not found. Dataset must contain a property with name: ${fieldName}`);
     return {filter: null, dataset};
   }
@@ -946,7 +955,8 @@ export function applyFilterFieldName<K extends KeplerTableModel<K, L>, L>(
   const filterProps = dataset.getColumnFilterProps(fieldName);
 
   let newFilter = {
-    ...(mergeDomain ? mergeFilterDomainStep(filter, filterProps) : {...filter, ...filterProps}),
+    ...filter,
+    ...filterProps,
     name: Object.assign([...toArray(filter.name)], {[filterDatasetIndex]: fieldName}),
     fieldIdx: Object.assign([...toArray(filter.fieldIdx)], {
       [filterDatasetIndex]: fieldIndex
@@ -955,6 +965,18 @@ export function applyFilterFieldName<K extends KeplerTableModel<K, L>, L>(
     ...(filter.plotType ? {plotType: filter.plotType} : {})
   };
 
+  if (mergeDomain) {
+    const domainSteps: (Filter & {step?: number}) | null =
+      mergeFilterDomain(newFilter, datasets) ?? ({} as Filter);
+    if (domainSteps) {
+      const {domain, step} = domainSteps;
+      newFilter.domain = domain;
+      if (newFilter.step !== step) {
+        newFilter.step = step;
+      }
+    }
+  }
+
   // TODO: if we don't set filter value in filterProps, we don't need to do this
   if (filterDatasetIndex > 0) {
     // don't reset the filter value if we are just adding a synced dataset
@@ -970,12 +992,31 @@ export function applyFilterFieldName<K extends KeplerTableModel<K, L>, L>(
   };
 }
 
+/**
+ * Merge the domains of a filter in case it applies to multiple datasets
+ */
+export function mergeFilterDomain(
+  filter: Filter,
+  datasets: Record<string, KeplerTableModel<any, any>>
+): (Filter & {step?: number}) | null {
+  let domainSteps: (Filter & {step?: number}) | null = null;
+  if (!filter?.dataId?.length) {
+    return filter;
+  }
+  filter.dataId.forEach((filterDataId, idx) => {
+    const dataset = datasets[filterDataId];
+    const filterProps = dataset.getColumnFilterProps(filter.name[idx]);
+    domainSteps = mergeFilterDomainStep(domainSteps ?? ({} as Filter), filterProps);
+  });
+  return domainSteps;
+}
+
 /**
  * Merge one filter with other filter prop domain
  */
 /* eslint-disable complexity */
 export function mergeFilterDomainStep(
-  filter: Filter,
+  filter: Filter | null,
   filterProps?: Partial<Filter>
 ): (Filter & {step?: number}) | null {
   if (!filter) {
@@ -1134,7 +1175,12 @@ export function validateFiltersUpdateDatasets<
           const toValidate = acc.validatedFilter || filterToValidate;
 
           const {filter: updatedFilter, dataset: updatedDataset} = validateFilterWithData(
-            acc.augmentedDatasets[datasetId] || dataset,
+            {
+              ...updatedDatasets,
+              ...acc.augmentedDatasets,
+              [datasetId]: acc.augmentedDatasets[datasetId] || dataset
+            },
+            datasetId,
             toValidate,
             datasetLayers
           );

test/node/reducers/vis-state-test.js

@@ -6007,8 +6007,13 @@ test('#visStateReducer -> applyFilterFieldName', t => {
   const stateToSave = CloneDeep(StateWFilters);
 
   const oldFilter = stateToSave.visState.filters[0];
-  const dataset = stateToSave.visState.datasets[oldFilter.dataId];
-  const {filter: newFilter} = applyFilterFieldName(oldFilter, dataset, oldFilter.name[0]);
+  const datasets = stateToSave.visState.datasets;
+  const {filter: newFilter} = applyFilterFieldName(
+    oldFilter,
+    datasets,
+    oldFilter.dataId,
+    oldFilter.name[0]
+  );
   t.deepEqual(
     oldFilter.plotType,
     newFilter.plotType,