Skip to content

Commit

Permalink
[Bug] Fix map saved with empty filter cannt be load; validate empty f…
Browse files Browse the repository at this point in the history
…ilter.name when merging (#1909)
  • Loading branch information
igorDykhta committed Aug 9, 2022
1 parent 26b5f84 commit fae2058
Show file tree
Hide file tree
Showing 9 changed files with 84 additions and 17 deletions.
2 changes: 1 addition & 1 deletion src/reducers/provider-state-updaters.ts
Expand Up @@ -19,7 +19,7 @@
// THE SOFTWARE.

import {withTask} from 'react-palm/tasks';
import {default as Console} from 'global/console';
import Console from 'global/console';
import {generateHashId, getError, isPlainObject, toArray} from '../utils/utils';
import {
EXPORT_FILE_TO_CLOUD_TASK,
Expand Down
5 changes: 2 additions & 3 deletions src/schemas/vis-state-schema.ts
Expand Up @@ -20,7 +20,7 @@

import pick from 'lodash.pick';
import {VERSIONS} from './versions';
import {isValidFilterValue} from 'utils/filter-utils';
import {isFilterValidToSave} from 'utils/filter-utils';
import {LAYER_VIS_CONFIGS} from '@kepler.gl/constants';
import Schema from './schema';
import cloneDeep from 'lodash.clonedeep';
Expand Down Expand Up @@ -610,8 +610,7 @@ export class FilterSchemaV0 extends Schema {
save(filters: Filter[]): {filters: SavedFilter[]} {
return {
filters: filters
// @ts-expect-error should pass type of the layer instead?
.filter(isValidFilterValue)
.filter(isFilterValidToSave)
.map(filter => this.savePropertiesOrApplySchema(filter).filters)
};
}
Expand Down
12 changes: 6 additions & 6 deletions src/utils/dom-to-image.ts
Expand Up @@ -25,7 +25,7 @@

import window from 'global/window';
import document from 'global/document';
import console from 'global/console';
import Console from 'global/console';
import svgToMiniDataURI from 'mini-svg-data-uri';
import {IMAGE_EXPORT_ERRORS} from '@kepler.gl/constants';
import {
Expand Down Expand Up @@ -361,8 +361,8 @@ function newFontFaces() {
// Handle any error that occurred in any of the previous
// promises in the chain. stylesheet failed to load should not stop
// the process, hence result in only a warning, instead of reject
console.warn(IMAGE_EXPORT_ERRORS.styleSheet, sheet.href);
console.log(err);
Console.warn(IMAGE_EXPORT_ERRORS.styleSheet, sheet.href);
Console.log(err);
return;
});
}
Expand Down Expand Up @@ -431,19 +431,19 @@ function newFontFaces() {
try {
rules = sheet.rules || sheet.cssRules;
} catch (e) {
console.log(`'Can't read the css rules of: ${sheet.href}`, e);
Console.log(`'Can't read the css rules of: ${sheet.href}`, e);
return;
}

if (rules && typeof rules === 'object') {
try {
asArray(rules || []).forEach(cssRules.push.bind(cssRules));
} catch (e) {
console.log(`Error while reading CSS rules from ${sheet.href}`, e);
Console.log(`Error while reading CSS rules from ${sheet.href}`, e);
return;
}
} else {
console.log('getCssRules can not find cssRules');
Console.log('getCssRules can not find cssRules');
return;
}
});
Expand Down
6 changes: 3 additions & 3 deletions src/utils/dom-utils.ts
Expand Up @@ -18,7 +18,7 @@
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.

import console from 'global/console';
import Console from 'global/console';
import window from 'global/window';
import document from 'global/document';
import {IMAGE_EXPORT_ERRORS} from '@kepler.gl/constants';
Expand Down Expand Up @@ -166,7 +166,7 @@ export function makeImage(uri) {
};
image.onerror = err => {
const message = IMAGE_EXPORT_ERRORS.dataUri;
console.log(uri);
Console.log(uri);
// error is an Event Object
// https://www.w3schools.com/jsref/obj_event.asp
reject({event: err, message});
Expand Down Expand Up @@ -347,7 +347,7 @@ export function getAndEncode(url, options) {
}

function fail(message) {
console.error(message);
Console.error(message);
resolve('');
}
});
Expand Down
18 changes: 16 additions & 2 deletions src/utils/filter-utils.ts
Expand Up @@ -20,7 +20,7 @@

import {ascending, extent, histogram as d3Histogram, ticks} from 'd3-array';
import keyMirror from 'keymirror';
import {console as Console} from 'global/console';
import Console from 'global/console';
import get from 'lodash.get';
import isEqual from 'lodash.isequal';

Expand Down Expand Up @@ -240,7 +240,7 @@ export function validateFilter(
const filterDataId = toArray(filter.dataId);

const filterDatasetIndex = filterDataId.indexOf(dataset.id);
if (filterDatasetIndex < 0) {
if (filterDatasetIndex < 0 || !toArray(filter.name)[filterDatasetIndex]) {
// the current filter is not mapped against the current dataset
return failed;
}
Expand Down Expand Up @@ -829,6 +829,20 @@ export function getTimeWidgetHintFormatter(domain: [number, number]): string | u

/**
* Sanity check on filters to prepare for save
* @type {typeof import('./filter-utils').isFilterValidToSave}
*/
export function isFilterValidToSave(filter: any): boolean {
return (
filter?.type &&
Array.isArray(filter?.name) &&
filter?.name.length &&
isValidFilterValue(filter?.type, filter?.value)
);
}

/**
* Sanity check on filters to prepare for save
* @type {typeof import('./filter-utils').isValidFilterValue}
*/
/* eslint-disable complexity */
export function isValidFilterValue(type: string | null, value: any): boolean {
Expand Down
2 changes: 1 addition & 1 deletion src/utils/locale-utils.ts
Expand Up @@ -19,7 +19,7 @@
// THE SOFTWARE.

import {isObject} from './utils';
import {console as Console} from 'global/console';
import Console from 'global/console';

// Flat messages since react-intl does not seem to support nested structures
// Adapted from https://medium.com/siren-apparel-press/internationalization-and-localization-of-sirenapparel-eu-sirenapparel-us-and-sirenapparel-asia-ddee266066a2
Expand Down
2 changes: 1 addition & 1 deletion src/utils/table-utils/kepler-table.ts
Expand Up @@ -18,7 +18,7 @@
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.

import {console as Console} from 'global/console';
import Console from 'global/console';
import {TRIP_POINT_FIELDS, SORT_ORDER, ALL_FIELD_TYPES, SCALE_TYPES} from '@kepler.gl/constants';
import {ascending, descending} from 'd3-array';

Expand Down
39 changes: 39 additions & 0 deletions test/node/reducers/vis-state-merger-test.js
Expand Up @@ -249,6 +249,45 @@ test('VisStateMerger.v1 -> mergeFilters -> toWorkingState', t => {
t.end();
});

test('VisStateMerger.v1 -> mergeFilters -> empty filter', t => {
const savedConfig = cloneDeep(savedStateV1);
// set an empty filter
savedConfig.config.config.visState.filters[0].name = [];
const oldState = cloneDeep(InitialState);

const oldVisState = oldState.visState;
const oldFilters = [...oldState.visState.filters];

const parsedConfig = SchemaManager.parseSavedConfig(savedConfig.config, oldState);
const parsedFilters = parsedConfig.visState.filters;

const mergedState = mergeFilters(oldState.visState, parsedFilters);

Object.keys(oldVisState).forEach(key => {
if (key === 'filterToBeMerged') {
t.deepEqual(
mergedState.filterToBeMerged,
parsedFilters,
'Should save filters to filterToBeMerged before data loaded'
);
} else {
t.deepEqual(mergedState[key], oldVisState[key], 'Should keep the rest of state same');
}
});

const parsedData = SchemaManager.parseSavedData(savedConfig.datasets);

// load data into reducer
const stateWData = visStateReducer(mergedState, updateVisData(parsedData));

// test parsed filters
cmpFilters(t, oldFilters, stateWData.filters);
t.deepEqual(stateWData.filterToBeMerged, [], 'should not pass fiter validate if tiler is empty');

// should filter data
t.end();
});

test('VisStateMerger -> mergeLayers -> invalid layer config', t => {
const oldState = cloneDeep(InitialState);
const oldVisState = oldState.visState;
Expand Down
15 changes: 15 additions & 0 deletions test/node/schemas/vis-state-schema-test.js
Expand Up @@ -37,6 +37,8 @@ import {
testCsvDataId,
testGeoJsonDataId
} from 'test/helpers/mock-state';
import keplerGlReducer from 'reducers/core';
import * as VisStateActions from 'actions/vis-state-actions';

test('#visStateSchema -> v1 -> save layers', t => {
const initialState = cloneDeep(StateWFilesFiltersLayerColor);
Expand Down Expand Up @@ -124,6 +126,19 @@ test('#visStateSchema -> v1 -> save load filters', t => {
t.end();
});

test('#visStateSchema -> v1 -> save and validate filters', t => {
const initialState = cloneDeep(StateWFilesFiltersLayerColor);

// add empty filter
const nextStte = keplerGlReducer(initialState, VisStateActions.addFilter(testCsvDataId));
const savedState = SchemaManager.getConfigToSave(nextStte);

t.equal(nextStte.visState.filters.length, 3, 'should have 3 filters');
t.equal(savedState.config.visState.filters.length, 2, 'should only save 2 filters');

t.end();
});

test('#visStateSchema -> v1 -> save load interaction', t => {
const initialState = cloneDeep(StateWFilesFiltersLayerColor);
const savedState = SchemaManager.getConfigToSave(initialState);
Expand Down

0 comments on commit fae2058

Please sign in to comment.