Merge pull request #3482 from grebaldi/bugfix/2553/identity-crisis

BUGFIX: Properly handle object identity DTOs in select box editor

Author: markusguenther

packages/neos-ui-editors/src/Editors/SelectBox/DataSourceBasedSelectBoxEditor.js

@@ -7,6 +7,7 @@ import MultiSelectBox from '@neos-project/react-ui-components/src/MultiSelectBox
 import {selectors} from '@neos-project/neos-ui-redux-store';
 import {neos} from '@neos-project/neos-ui-decorators';
 import {shouldDisplaySearchBox, searchOptions, processSelectBoxOptions} from './SelectBoxHelpers';
+import {createSelectBoxValueStringFromPossiblyStrangeNodePropertyValue} from './createSelectBoxValueStringFromPossiblyStrangeNodePropertyValue';
 import PreviewOption from '../../Library/PreviewOption';
 
 const getDataLoaderOptionsForProps = props => ({
@@ -30,7 +31,17 @@ export default class DataSourceBasedSelectBoxEditor extends PureComponent {
         className: PropTypes.string,
         value: PropTypes.oneOfType([
             PropTypes.string,
-            PropTypes.arrayOf(PropTypes.string)
+            PropTypes.arrayOf(PropTypes.string),
+            PropTypes.shape({
+                __identity: PropTypes.string.isRequired,
+                __type: PropTypes.string
+            }),
+            PropTypes.arrayOf(
+                PropTypes.shape({
+                    __identity: PropTypes.string.isRequired,
+                    __type: PropTypes.string
+                })
+            )
         ]),
         options: PropTypes.shape({
             allowEmpty: PropTypes.bool,
@@ -101,8 +112,23 @@ export default class DataSourceBasedSelectBoxEditor extends PureComponent {
             });
     }
 
+    get valueForSingleSelect() {
+        const {value} = this.props;
+        return createSelectBoxValueStringFromPossiblyStrangeNodePropertyValue(value);
+    }
+
+    get valueForMultiSelect() {
+        const {value} = this.props;
+
+        if (Array.isArray(value)) {
+            return value.map(createSelectBoxValueStringFromPossiblyStrangeNodePropertyValue);
+        }
+
+        return value ? [createSelectBoxValueStringFromPossiblyStrangeNodePropertyValue(value)] : [];
+    }
+
     render() {
-        const {commit, value, i18nRegistry, className} = this.props;
+        const {commit, i18nRegistry, className} = this.props;
         const options = Object.assign({}, this.constructor.defaultOptions, this.props.options);
 
         const processedSelectBoxOptions = processSelectBoxOptions(i18nRegistry, this.state.selectBoxOptions);
@@ -115,7 +141,7 @@ export default class DataSourceBasedSelectBoxEditor extends PureComponent {
             return (<MultiSelectBox
                 className={className}
                 options={processedSelectBoxOptions}
-                values={value || []}
+                values={this.valueForMultiSelect}
                 onValuesChange={commit}
                 loadingLabel={loadingLabel}
                 ListPreviewElement={PreviewOption}
@@ -136,7 +162,7 @@ export default class DataSourceBasedSelectBoxEditor extends PureComponent {
         return (<SelectBox
             className={className}
             options={this.state.searchTerm ? searchOptions(this.state.searchTerm, processedSelectBoxOptions) : processedSelectBoxOptions}
-            value={value}
+            value={this.valueForSingleSelect}
             onValueChange={commit}
             loadingLabel={loadingLabel}
             ListPreviewElement={PreviewOption}

packages/neos-ui-editors/src/Editors/SelectBox/createSelectBoxValueStringFromPossiblyStrangeNodePropertyValue.spec.ts

@@ -0,0 +1,24 @@
+import {createSelectBoxValueStringFromPossiblyStrangeNodePropertyValue} from './createSelectBoxValueStringFromPossiblyStrangeNodePropertyValue';
+
+describe('createSelectBoxValueStringFromPossiblyStrangeNodePropertyValue', () => {
+    it('accepts value of type "string" and returns a "string"', () => {
+        const value =
+            createSelectBoxValueStringFromPossiblyStrangeNodePropertyValue(
+                'I am already a valid select box value, believe it or not.'
+            );
+
+        expect(value).toEqual(
+            'I am already a valid select box value, believe it or not.'
+        );
+    });
+
+    it('accepts an object identity DTO and returns a "string"', () => {
+        const value =
+            createSelectBoxValueStringFromPossiblyStrangeNodePropertyValue({
+                __identity: 'de93b358-cb77-422e-b295-2f219bfc4dfb',
+                __type: 'Neos\\Media\\Domain\\Model\\Tag',
+            });
+
+        expect(value).toEqual('de93b358-cb77-422e-b295-2f219bfc4dfb');
+    });
+});

packages/neos-ui-editors/src/Editors/SelectBox/createSelectBoxValueStringFromPossiblyStrangeNodePropertyValue.ts

@@ -0,0 +1,36 @@
+/**
+ * @TODO I am a cry for help!
+ *
+ * This is an ad-hoc solution to the problem that properties of a PHP class type
+ * (like "Neos\Media\Domain\Model\Tag" for example) may or may not be persisted
+ * as object identity DTOs.
+ *
+ * The function name is intentionally kept vague to allow bugfixes to capture
+ * more, potentially obscure cases in which the persisted property value needs
+ * to be filtered before the select box receives it.
+ *
+ * A proper way to handle this would be to define precisely what kind of values
+ * the select box editor is going to accept and simply reject everything that
+ * violates that definition. Errors would need to be handled in way that
+ * indicates to editors that there's a problem that an integrator needs to fix.
+ * Furthermore the error handling should make it easy for integrators to figure
+ * out what value has been provided to the select box and why it has been
+ * rejected.
+ *
+ * That however would constitute a breaking change and that's how we end up
+ * with with this function.
+ */
+export const createSelectBoxValueStringFromPossiblyStrangeNodePropertyValue = (
+    value: unknown
+) => {
+    if (typeof value === 'object' && value !== null) {
+        if (
+            '__identity' in value &&
+            typeof (value as Record<'__identity', any>).__identity === 'string'
+        ) {
+            return (value as Record<'__identity', string>).__identity;
+        }
+    }
+
+    return value;
+};