Skip to content

Commit

Permalink
Allow FormulaWidget column to be undefined when using COUNT (#434)
Browse files Browse the repository at this point in the history
  • Loading branch information
Sergio Clebal authored Jun 17, 2022
1 parent 757e654 commit 230e7c7
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 5 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Not released

- Allow FormulaWidget column to be undefined when using COUNT [#434](https://github.com/CartoDB/carto-react/pull/434)

## 1.3

### 1.3.0-beta.1 (2022-06-14)
Expand Down
5 changes: 2 additions & 3 deletions packages/react-widgets/src/widgets/FormulaWidget.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { PropTypes } from 'prop-types';
import { WrapperWidgetUI, FormulaWidgetUI } from '@carto/react-ui';
import { getFormula } from '../models';
import { AggregationTypes } from '@carto/react-core';
import { columnAggregationOn } from './utils/propTypesFns';
import { checkFormulaColumn, columnAggregationOn } from './utils/propTypesFns';
import useWidgetFetch from '../hooks/useWidgetFetch';
import WidgetWithAlert from './utils/WidgetWithAlert';

Expand Down Expand Up @@ -76,8 +76,7 @@ FormulaWidget.propTypes = {
id: PropTypes.string.isRequired,
title: PropTypes.string.isRequired,
dataSource: PropTypes.string.isRequired,
column: PropTypes.oneOfType([PropTypes.string, PropTypes.arrayOf(PropTypes.string)])
.isRequired,
column: checkFormulaColumn,
joinOperation: columnAggregationOn('column'),
operation: PropTypes.oneOf(Object.values(AggregationTypes)).isRequired,
formatter: PropTypes.func,
Expand Down
24 changes: 24 additions & 0 deletions packages/react-widgets/src/widgets/utils/propTypesFns.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,27 @@ export const columnAggregationOn = (columnPropName) => (props, propName) => {
}
}
};

// FormulaWidget
export const checkFormulaColumn = (props, propName) => {
const propValue = props[propName];

const isValidString = !!propValue && typeof propValue === 'string';
const isValidArray = Array.isArray(propValue) && propValue.length;

const validationError = new Error(`Prop ${propName} must be a string or an array`);

if (props.operation === AggregationTypes.COUNT) {
if (propValue && !(isValidString || isValidArray)) {
return validationError;
}
} else {
if (!propValue) {
return new Error(`Prop ${propName} must be defined`);
}

if (!isValidArray || !isValidString) {
return validationError;
}
}
};
10 changes: 8 additions & 2 deletions packages/react-workers/src/workers/features.worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,17 @@ function getFormula({
if (currentFeatures) {
const targetOperation = aggregationFunctions[operation];

assertColumn(column);
const isCount = operation === AggregationTypes.COUNT;

// If the operation isn't count, we need to assert the column
// If the operation is count, the column can be undefined
if (!isCount || (isCount && column)) {
assertColumn(column);
}

const filteredFeatures = getFilteredFeatures(filters, filtersLogicalOperator);

if (filteredFeatures.length === 0 && operation !== AggregationTypes.COUNT) {
if (filteredFeatures.length === 0 && !isCount) {
result = { value: null };
} else {
result = { value: targetOperation(filteredFeatures, column, joinOperation) };
Expand Down

0 comments on commit 230e7c7

Please sign in to comment.