Skip to content

Commit

Permalink
Fix list value for multiselect used as function arg (#997)
Browse files Browse the repository at this point in the history 
* fix

* tests

* Use Immutable 4

* fixes for Immutable 4

* validation - check func in funcs

* chlog
  • Loading branch information
@ukrbublik
ukrbublik authored Oct 28, 2023
1 parent 6d31773 commit d17da01
Show file tree
Hide file tree
Showing 21 changed files with 261 additions and 75 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
# Changelog
- 6.4.3
- Fixed the issue when using func with arg of type `multiselect` (PR #997)
- Updated `immutable` from v3 to v4 (PR #997)
- Fixed issue with "[object Object]" in MUI field autocomplete when item should be bold (PR #997)
- Respect `funcs` in field/arg config during validation of function value (PR #997)
- 6.4.2
- Allow override icons with `renderIcon` (issues #319, #872) (PR #962)
- Support tooltips for MUI (issues #965, #684) (PR #973)
Expand Down
10 changes: 5 additions & 5 deletions packages/core/modules/actions/tree.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import Immutable from "immutable";
import Immutable, {fromJS} from "immutable";
import {toImmutableList} from "../utils/stuff";
import * as constants from "../stores/constants";
import { defaultRuleProperties, defaultGroupProperties } from "../utils/defaultUtils";
Expand Down Expand Up @@ -26,7 +26,7 @@ export const addRule = (config, path, properties, ruleType = "rule", children =
children: children,
path: toImmutableList(path),
id: uuid(),
properties: defaultRuleProperties(config, parentRuleGroupPath).merge(properties || {}),
properties: defaultRuleProperties(config, parentRuleGroupPath).merge(fromJS(properties) || {}),
config: config
});

Expand All @@ -50,7 +50,7 @@ export const addDefaultCaseGroup = (config, path, properties, children = null) =
path: toImmutableList(path),
children: children,
id: uuid(),
properties: defaultGroupProperties(config).merge(properties || {}),
properties: defaultGroupProperties(config).merge(fromJS(properties) || {}),
config: config,
meta: {
isDefaultCase: true
Expand All @@ -67,7 +67,7 @@ export const addCaseGroup = (config, path, properties, children = null) => ({
path: toImmutableList(path),
children: children,
id: uuid(),
properties: defaultGroupProperties(config).merge(properties || {}),
properties: defaultGroupProperties(config).merge(fromJS(properties) || {}),
config: config
});

Expand All @@ -81,7 +81,7 @@ export const addGroup = (config, path, properties, children = null) => ({
path: toImmutableList(path),
children: children,
id: uuid(),
properties: defaultGroupProperties(config).merge(properties || {}),
properties: defaultGroupProperties(config).merge(fromJS(properties) || {}),
config: config
});

Expand Down
2 changes: 1 addition & 1 deletion packages/core/modules/config/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,7 @@ const widgets = {
return vals.map(v => this.utils.SqlString.escape(v));
},
spelFormatValue: function (vals, fieldDef, wgtDef, op, opDef) {
const isCallable = opDef.spelOp && opDef.spelOp.startsWith("${1}");
const isCallable = opDef && opDef.spelOp && opDef.spelOp.startsWith("${1}");
let res = this.utils.spelEscape(vals); // inline list
if (isCallable) {
// `{1,2}.contains(1)` NOT works
Expand Down
6 changes: 5 additions & 1 deletion packages/core/modules/export/jsonLogic.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -299,8 +299,12 @@ const formatFunc = (meta, config, currentValue, parentField = null) => {
const {defaultValue, isOptional} = argConfig;
const defaultValueSrc = defaultValue?.func ? "func" : "value";
const argVal = args ? args.get(argKey) : undefined;
const argValue = argVal ? argVal.get("value") : undefined;
let argValue = argVal ? argVal.get("value") : undefined;
const argValueSrc = argVal ? argVal.get("valueSrc") : undefined;
if (argValueSrc !== "func" && argValue?.toJS) {
// value should not be Immutable
argValue = argValue.toJS();
}
const operator = null;
const widget = getWidgetForFieldOp(config, argConfig, operator, argValueSrc);
const fieldWidgetDef = omit( getFieldWidgetConfig(config, argConfig, operator, widget, argValueSrc), ["factory"] );
Expand Down
6 changes: 5 additions & 1 deletion packages/core/modules/export/mongoDb.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,8 +354,12 @@ const formatFunc = (meta, config, currentValue, parentPath) => {
const {defaultValue, isOptional} = argConfig;
const defaultValueSrc = defaultValue?.func ? "func" : "value";
const argVal = args ? args.get(argKey) : undefined;
const argValue = argVal ? argVal.get("value") : undefined;
let argValue = argVal ? argVal.get("value") : undefined;
const argValueSrc = argVal ? argVal.get("valueSrc") : undefined;
if (argValueSrc !== "func" && argValue?.toJS) {
// value should not be Immutable
argValue = argValue.toJS();
}
const argAsyncListValues = argVal ? argVal.get("asyncListValues") : undefined;
const operator = null;
const widget = getWidgetForFieldOp(config, argConfig, operator, argValueSrc);
Expand Down
6 changes: 5 additions & 1 deletion packages/core/modules/export/queryString.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,8 +307,12 @@ const formatFunc = (config, meta, funcValue, isForDisplay, parentField = null) =
const defaultValueSrc = defaultValue?.func ? "func" : "value";
const argName = isForDisplay && argConfig.label || argKey;
const argVal = args ? args.get(argKey) : undefined;
const argValue = argVal ? argVal.get("value") : undefined;
let argValue = argVal ? argVal.get("value") : undefined;
const argValueSrc = argVal ? argVal.get("valueSrc") : undefined;
if (argValueSrc !== "func" && argValue?.toJS) {
// value should not be Immutable
argValue = argValue.toJS();
}
const argAsyncListValues = argVal ? argVal.get("asyncListValues") : undefined;
const formattedArgVal = formatValue(
config, meta, argValue, argValueSrc, argConfig.type, fieldDef, argConfig, null, null, isForDisplay, parentField, argAsyncListValues
Expand Down
7 changes: 6 additions & 1 deletion packages/core/modules/export/spel.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ const formatSwitch = (item, config, meta, parentField = null) => {
const cases = children
.map((currentChild) => formatCase(currentChild, config, meta, null))
.filter((currentChild) => typeof currentChild !== "undefined")
.valueSeq()
.toArray();

if (!cases.length) return undefined;
Expand Down Expand Up @@ -460,8 +461,12 @@ const formatFunc = (meta, config, currentValue, parentField = null) => {
const {defaultValue, isOptional} = argConfig;
const defaultValueSrc = defaultValue?.func ? "func" : "value";
const argVal = args ? args.get(argKey) : undefined;
const argValue = argVal ? argVal.get("value") : undefined;
let argValue = argVal ? argVal.get("value") : undefined;
const argValueSrc = argVal ? argVal.get("valueSrc") : undefined;
if (argValueSrc !== "func" && argValue?.toJS) {
// value should not be Immutable
argValue = argValue.toJS();
}
const argAsyncListValues = argVal ? argVal.get("asyncListValues") : undefined;
const doEscape = argConfig.spelEscapeForFormat ?? true;
const operator = null;
Expand Down
6 changes: 5 additions & 1 deletion packages/core/modules/export/sql.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,12 @@ const formatFunc = (meta, config, currentValue) => {
const {defaultValue, isOptional} = argConfig;
const defaultValueSrc = defaultValue?.func ? "func" : "value";
const argVal = args ? args.get(argKey) : undefined;
const argValue = argVal ? argVal.get("value") : undefined;
let argValue = argVal ? argVal.get("value") : undefined;
const argValueSrc = argVal ? argVal.get("valueSrc") : undefined;
if (argValueSrc !== "func" && argValue?.toJS) {
// value should not be Immutable
argValue = argValue.toJS();
}
const argAsyncListValues = argVal ? argVal.get("asyncListValues") : undefined;
const formattedArgVal = formatValue(
meta, config, argValue, argValueSrc, argConfig.type, fieldDef, argConfig, null, null, argAsyncListValues
Expand Down
29 changes: 15 additions & 14 deletions packages/core/modules/import/tree.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Immutable, { fromJS, Map } from "immutable";
import {validateTree} from "../utils/validation";
import {extendConfig} from "../utils/configUtils";
import {getTreeBadFields, getLightTree} from "../utils/treeUtils";
import {getTreeBadFields, getLightTree, _fixImmutableValue} from "../utils/treeUtils";
import {isJsonLogic} from "../utils/stuff";

export const getTree = (immutableTree, light = true, children1AsArray = true) => {
Expand Down Expand Up @@ -48,7 +48,16 @@ export const isTree = (tree) => {
export {isJsonLogic};

export function jsToImmutable(tree) {
const imm = fromJS(tree, function (key, value) {
const imm = fromJS(tree, function (key, value, path) {
const isFuncArg = path
&& path.length > 3
&& path[path.length-1] === "value"
&& path[path.length-3] === "args";
const isRuleValue = path
&& path.length > 3
&& path[path.length-1] === "value"
&& path[path.length-2] === "properties";

let outValue;
if (key == "properties") {
outValue = value.toOrderedMap();
Expand All @@ -61,18 +70,10 @@ export function jsToImmutable(tree) {
outValue = outValue.setIn(["value", i], undefined);
}
}
} else if (key == "value" && Immutable.Iterable.isIndexed(value)) {
outValue = value.map(v => {
const vJs = v?.toJS?.();
if (vJs?.func) {
return v.toOrderedMap();
} else if(v?.toJS) {
// for values of multiselect use Array instead of List
return vJs;
} else {
return v;
}
}).toList();
} else if (isFuncArg) {
outValue = _fixImmutableValue(value);
} else if ((path ? isRuleValue : key == "value") && Immutable.Iterable.isIndexed(value)) {
outValue = value.map(_fixImmutableValue).toList();
} else if (key == "asyncListValues") {
// keep in JS format
outValue = value.toJS();
Expand Down
8 changes: 4 additions & 4 deletions packages/core/modules/stores/tree.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import Immutable from "immutable";
import Immutable, { fromJS } from "immutable";
import {
expandTreePath, expandTreeSubpath, getItemByPath, fixPathsInTree,
getTotalRulesCountInTree, fixEmptyGroupsInTree, isEmptyTree, hasChildren, removeIsLockedInTree
Expand Down Expand Up @@ -33,7 +33,7 @@ const addNewGroup = (state, path, type, groupUuid, properties, config, children
const isDefaultCase = !!meta?.isDefaultCase;

const origState = state;
state = addItem(state, path, type, groupUuid, defaultGroupProperties(config).merge(properties || {}), config, children);
state = addItem(state, path, type, groupUuid, defaultGroupProperties(config).merge(fromJS(properties) || {}), config, children);
if (state !== origState) {
if (!children && !isDefaultCase) {
state = state.setIn(expandTreePath(groupPath, "children1"), new Immutable.OrderedMap());
Expand Down Expand Up @@ -160,7 +160,7 @@ const _addChildren1 = (config, item, children) => {
const id1 = uuid();
const it1 = {
...it,
properties: defaultItemProperties(config, it).merge(it.properties || {}),
properties: defaultItemProperties(config, it).merge(fromJS(it.properties) || {}),
id: id1
};
_addChildren1(config, it1, it1.children1);
Expand Down Expand Up @@ -306,7 +306,7 @@ const moveItem = (state, fromPath, toPath, placement, config) => {
}
});
} else if (placement == constants.PLACEMENT_APPEND) {
newTargetChildren = newTargetChildren.merge({[from.get("id")]: from});
newTargetChildren = newTargetChildren.merge(Immutable.OrderedMap({[from.get("id")]: from}));
} else if (placement == constants.PLACEMENT_PREPEND) {
newTargetChildren = Immutable.OrderedMap({[from.get("id")]: from}).merge(newTargetChildren);
}
Expand Down
18 changes: 17 additions & 1 deletion packages/core/modules/utils/treeUtils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ export const getFlatTree = (tree) => {
const id = item.get("id");
const children = item.get("children1");
const isLocked = item.getIn(["properties", "isLocked"]);
const childrenIds = children ? children.map((_child, childId) => childId).toArray() : null;
const childrenIds = children ? children.map((_child, childId) => childId).valueSeq().toArray() : null;
const isRuleGroup = type == "rule_group";
// tip: count rule_group as 1 rule
const isLeaf = !insideRuleGroup && (!children || isRuleGroup);
Expand Down Expand Up @@ -430,3 +430,19 @@ export const getSwitchValues = (tree) => {
export const isEmptyTree = (tree) => (!tree.get("children1") || tree.get("children1").size == 0);

export const hasChildren = (tree, path) => tree.getIn(expandTreePath(path, "children1")).size > 0;


export const _fixImmutableValue = (v) => {
if (v?.toJS) {
const vJs = v?.toJS?.();
if (vJs?.func) {
// `v` is a func arg, keep Immutable
return v.toOrderedMap();
} else {
// for values of multiselect use Array instead of List
return vJs;
}
} else {
return v;
}
};
66 changes: 37 additions & 29 deletions packages/core/modules/utils/validation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -390,39 +390,47 @@ const validateFuncValue = (leftField, field, value, _valueSrc, valueType, asyncL
if (value) {
const funcKey = value.get("func");
if (funcKey) {
const funcConfig = getFuncConfig(config, funcKey);
if (funcConfig) {
if (valueType && funcConfig.returnType != valueType)
return [`Function ${funcKey} should return value of type ${funcConfig.returnType}, but got ${valueType}`, value];
for (const argKey in funcConfig.args) {
const argConfig = funcConfig.args[argKey];
const args = fixedValue.get("args");
const argVal = args ? args.get(argKey) : undefined;
const argDef = getFieldConfig(config, argConfig);
const argValue = argVal ? argVal.get("value") : undefined;
const argValueSrc = argVal ? argVal.get("valueSrc") : undefined;
if (argValue !== undefined) {
const [argValidError, fixedArgVal] = validateValue(
config, leftField, argDef, operator, argValue, argConfig.type, argValueSrc, asyncListValues, canFix, isEndValue, false
);
if (argValidError !== null) {
if (canFix) {
fixedValue = fixedValue.deleteIn(["args", argKey]);
if (argConfig.defaultValue !== undefined) {
fixedValue = fixedValue.setIn(["args", argKey, "value"], argConfig.defaultValue);
fixedValue = fixedValue.setIn(["args", argKey, "valueSrc"], "value");
const fieldDef = getFieldConfig(config, field);
if (fieldDef?.funcs) {
if (!fieldDef.funcs.includes(funcKey)) {
return [`Unsupported function ${funcKey}`, value];
}
}
if (fixedValue) {
const funcConfig = getFuncConfig(config, funcKey);
if (funcConfig) {
if (valueType && funcConfig.returnType != valueType)
return [`Function ${funcKey} should return value of type ${funcConfig.returnType}, but got ${valueType}`, value];
for (const argKey in funcConfig.args) {
const argConfig = funcConfig.args[argKey];
const args = fixedValue.get("args");
const argVal = args ? args.get(argKey) : undefined;
const argDef = getFieldConfig(config, argConfig);
const argValue = argVal ? argVal.get("value") : undefined;
const argValueSrc = argVal ? argVal.get("valueSrc") : undefined;
if (argValue !== undefined) {
const [argValidError, fixedArgVal] = validateValue(
config, leftField, argDef, operator, argValue, argConfig.type, argValueSrc, asyncListValues, canFix, isEndValue, false
);
if (argValidError !== null) {
if (canFix) {
fixedValue = fixedValue.deleteIn(["args", argKey]);
if (argConfig.defaultValue !== undefined) {
fixedValue = fixedValue.setIn(["args", argKey, "value"], argConfig.defaultValue);
fixedValue = fixedValue.setIn(["args", argKey, "valueSrc"], "value");
}
} else {
return [`Invalid value of arg ${argKey} for func ${funcKey}: ${argValidError}`, value];
}
} else {
return [`Invalid value of arg ${argKey} for func ${funcKey}: ${argValidError}`, value];
} else if (fixedArgVal !== argValue) {
fixedValue = fixedValue.setIn(["args", argKey, "value"], fixedArgVal);
}
} else if (fixedArgVal !== argValue) {
fixedValue = fixedValue.setIn(["args", argKey, "value"], fixedArgVal);
} else if (isEndValue && argConfig.defaultValue === undefined && !canFix && !argConfig.isOptional) {
return [`Value of arg ${argKey} for func ${funcKey} is required`, value];
}
} else if (isEndValue && argConfig.defaultValue === undefined && !canFix && !argConfig.isOptional) {
return [`Value of arg ${argKey} for func ${funcKey} is required`, value];
}
}
} else return [`Unknown function ${funcKey}`, value];
} else return [`Unknown function ${funcKey}`, value];
}
} // else it's not function value
} // empty value

Expand Down
2 changes: 1 addition & 1 deletion packages/core/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
},
"dependencies": {
"clone": "^2.1.2",
"immutable": "^3.8.2",
"immutable": "^4.3.4",
"json-logic-js": "^2.0.2",
"lodash": "^4.17.21",
"moment": "^2.29.4",
Expand Down
2 changes: 1 addition & 1 deletion packages/examples/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
"antd": "^5.7.2",
"bootstrap": "^5.1.3",
"clone": "^2.1.2",
"immutable": "^3.8.2",
"immutable": "^4.3.4",
"lodash": "^4.17.21",
"moment": "^2.29.4",
"prop-types": "^15.7.2",
Expand Down
2 changes: 1 addition & 1 deletion packages/mui/modules/widgets/value/MuiAutocomplete.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ export default (props) => {
const isSelected = multiple ? (selectedValue || []).includes(value) : selectedValue == value;
const className = getOptionIsCustom(option) ? "customSelectOption" : undefined;
const prefix = !isFieldAutocomplete && isGrouped ? "\u00A0\u00A0" : "";
const finalTitle = prefix + (renderTitle || title);
const finalTitle = (renderTitle || prefix + title);
let titleSpan = (
<span className={className}>
{finalTitle}
Expand Down
Loading

3 comments on commit d17da01

@vercel
Copy link

@vercel vercel bot commented on d17da01 Oct 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

react-awesome-query-builder-examples – ./packages/examples

react-awesome-query-builder-examples-ukrbublik.vercel.app
react-awesome-query-builder-examples-git-master-ukrbublik.vercel.app
react-awesome-query-builder-examples.vercel.app

@vercel
Copy link

@vercel vercel bot commented on d17da01 Oct 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

react-awesome-query-builder-sandbox-next – ./packages/sandbox_next

react-awesome-query-builder-sandbox-next-git-master-ukrbublik.vercel.app
react-awesome-query-builder-sandbox-next-ukrbublik.vercel.app
react-awesome-query-builder-sandbox-next.vercel.app

@vercel
Copy link

@vercel vercel bot commented on d17da01 Oct 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

react-awesome-query-builder-sandbox – ./packages/sandbox

react-awesome-query-builder-sandbox.vercel.app
react-awesome-query-builder-sandbox-ukrbublik.vercel.app
react-awesome-query-builder-sandbox-git-master-ukrbublik.vercel.app

Please sign in to comment.