Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix list value for multiselect used as function arg #997

Merged
merged 6 commits into from
Oct 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
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
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -299,8 +299,12 @@
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();

Check warning on line 306 in packages/core/modules/export/jsonLogic.js

View check run for this annotation

Codecov / codecov/patch

packages/core/modules/export/jsonLogic.js#L306

Added line #L306 was not covered by tests
}
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
Original file line number Diff line number Diff line change
Expand Up @@ -354,8 +354,12 @@
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();

Check warning on line 361 in packages/core/modules/export/mongoDb.js

View check run for this annotation

Codecov / codecov/patch

packages/core/modules/export/mongoDb.js#L361

Added line #L361 was not covered by tests
}
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
Original file line number Diff line number Diff line change
Expand Up @@ -307,8 +307,12 @@
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();

Check warning on line 314 in packages/core/modules/export/queryString.js

View check run for this annotation

Codecov / codecov/patch

packages/core/modules/export/queryString.js#L314

Added line #L314 was not covered by tests
}
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
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
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 {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();

Check warning on line 468 in packages/core/modules/export/spel.js

View check run for this annotation

Codecov / codecov/patch

packages/core/modules/export/spel.js#L468

Added line #L468 was not covered by tests
}
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
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,12 @@
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();

Check warning on line 278 in packages/core/modules/export/sql.js

View check run for this annotation

Codecov / codecov/patch

packages/core/modules/export/sql.js#L278

Added line #L278 was not covered by tests
}
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
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
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 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 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 @@
}
});
} else if (placement == constants.PLACEMENT_APPEND) {
newTargetChildren = newTargetChildren.merge({[from.get("id")]: from});
newTargetChildren = newTargetChildren.merge(Immutable.OrderedMap({[from.get("id")]: from}));

Check warning on line 309 in packages/core/modules/stores/tree.js

View check run for this annotation

Codecov / codecov/patch

packages/core/modules/stores/tree.js#L309

Added line #L309 was not covered by tests
} 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
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
Original file line number Diff line number Diff line change
Expand Up @@ -390,39 +390,47 @@
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];

Check warning on line 396 in packages/core/modules/utils/validation.js

View check run for this annotation

Codecov / codecov/patch

packages/core/modules/utils/validation.js#L395-L396

Added lines #L395 - L396 were not covered by tests
}
}
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];

Check warning on line 403 in packages/core/modules/utils/validation.js

View check run for this annotation

Codecov / codecov/patch

packages/core/modules/utils/validation.js#L403

Added line #L403 was not covered by tests
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");

Check warning on line 420 in packages/core/modules/utils/validation.js

View check run for this annotation

Codecov / codecov/patch

packages/core/modules/utils/validation.js#L416-L420

Added lines #L416 - L420 were not covered by tests
}
} else {
return [`Invalid value of arg ${argKey} for func ${funcKey}: ${argValidError}`, value];

Check warning on line 423 in packages/core/modules/utils/validation.js

View check run for this annotation

Codecov / codecov/patch

packages/core/modules/utils/validation.js#L423

Added line #L423 was not covered by tests
}
} 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];

Check warning on line 429 in packages/core/modules/utils/validation.js

View check run for this annotation

Codecov / codecov/patch

packages/core/modules/utils/validation.js#L429

Added line #L429 was not covered by tests
}
} 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];

Check warning on line 432 in packages/core/modules/utils/validation.js

View check run for this annotation

Codecov / codecov/patch

packages/core/modules/utils/validation.js#L432

Added line #L432 was not covered by tests
}
} // else it's not function value
} // empty value

Expand Down
2 changes: 1 addition & 1 deletion packages/core/package.json
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
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
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