Merge branch 'release/2.0' into release/2.1

alexandersch · Aug 17, 2020 · 89d38b2 · 89d38b2
2 parents 4bc4030 + bd876de
commit 89d38b2
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 11 deletions.
diff --git a/src/Sulu/Bundle/AdminBundle/Resources/js/containers/Form/fields/Select.js b/src/Sulu/Bundle/AdminBundle/Resources/js/containers/Form/fields/Select.js
@@ -1,5 +1,6 @@
 // @flow
 import React from 'react';
+import {computed} from 'mobx';
 import MultiSelectComponent from '../../../components/MultiSelect';
 import type {FieldTypeProps} from '../../../types';
 
@@ -38,24 +39,32 @@ export default class Select extends React.Component<Props> {
         }
     }
 
+    @computed get values() {
+        const {values} = this.props.schemaOptions;
+
+        if (!values || !Array.isArray(values.value)) {
+            throw new Error('The "values" option has to be set for the Select FieldType');
+        }
+
+        return values.value;
+    }
+
     handleChange = (value: Array<string | number>) => {
         const {onChange, onFinish} = this.props;
 
-        onChange(value.length > 0 ? value : undefined);
+        const allowedValues = this.values.map((value) => value.name);
+        const filteredValue = value.filter((v) => allowedValues.includes(v));
+
+        onChange(filteredValue.length > 0 ? filteredValue : undefined);
         onFinish();
     };
 
     render() {
-        const {schemaOptions, disabled, value} = this.props;
-        const {values} = schemaOptions;
-
-        if (!values || !Array.isArray(values.value)) {
-            throw new Error('The "values" option has to be set for the Select FieldType');
-        }
+        const {disabled, value} = this.props;
 
         return (
             <MultiSelectComponent disabled={!!disabled} onChange={this.handleChange} values={value || []}>
-                {values.value.map(({name: value, title}) => {
+                {this.values.map(({name: value, title}) => {
                     if (typeof value !== 'string' && typeof value !== 'number') {
                         throw new Error('The children of "values" must only contain values of type string or number!');
                     }

diff --git a/src/Sulu/Bundle/AdminBundle/Resources/js/containers/Form/tests/fields/Select.test.js b/src/Sulu/Bundle/AdminBundle/Resources/js/containers/Form/tests/fields/Select.test.js
@@ -144,6 +144,42 @@ test('Should call onChange with undefined if value is changed to an empty array'
     expect(finishSpy).toBeCalledWith();
 });
 
+test('Should call onChange with allowed values only if value contains old values', () => {
+    const formInspector = new FormInspector(new ResourceFormStore(new ResourceStore('test'), 'test'));
+    const changeSpy = jest.fn();
+    const finishSpy = jest.fn();
+    const schemaOptions = {
+        values: {
+            name: 'values',
+            value: [
+                {
+                    name: 'mr',
+                    title: 'Mister',
+                },
+                {
+                    name: 'ms',
+                    title: 'Miss',
+                },
+            ],
+        },
+    };
+
+    const select = shallow(
+        <Select
+            {...fieldTypeDefaultProps}
+            formInspector={formInspector}
+            onChange={changeSpy}
+            onFinish={finishSpy}
+            schemaOptions={schemaOptions}
+        />
+    );
+
+    select.simulate('change', ['mr', 'removed-value']);
+
+    expect(changeSpy).toBeCalledWith(['mr']);
+    expect(finishSpy).toBeCalledWith();
+});
+
 test('Should call onFinish callback on every onChange', () => {
     const formInspector = new FormInspector(new ResourceFormStore(new ResourceStore('test'), 'test'));
     const finishSpy = jest.fn();

diff --git a/src/Sulu/Bundle/AdminBundle/Resources/js/containers/Toolbar/Toolbar.js b/src/Sulu/Bundle/AdminBundle/Resources/js/containers/Toolbar/Toolbar.js
@@ -1,6 +1,6 @@
 // @flow
 import {observer} from 'mobx-react';
-import {action, computed} from 'mobx';
+import {action, computed, toJS} from 'mobx';
 import React, {Fragment} from 'react';
 import ToolbarComponent from '../../components/Toolbar';
 import Snackbar from '../../components/Snackbar';
@@ -153,7 +153,11 @@ class Toolbar extends React.Component<ToolbarProps> {
                     <ToolbarComponent.Controls>
                         {iconsConfig.length > 0 &&
                             <ToolbarComponent.Icons>
-                                {iconsConfig.map((icon) => icon)}
+                                {iconsConfig.map((icon) => {
+                                    // TODO this creates a deep copy and has a performance impact.
+                                    // Would be better to return functional components in withToolbar.
+                                    return toJS(icon);
+                                })}
                             </ToolbarComponent.Icons>
                         }
                         {!!localeConfig &&

diff --git a/src/Sulu/Bundle/AdminBundle/Resources/js/containers/Toolbar/types.js b/src/Sulu/Bundle/AdminBundle/Resources/js/containers/Toolbar/types.js
@@ -36,7 +36,7 @@ export type ToolbarConfig = {
     backButton?: Button,
     disableAll?: boolean,
     errors?: Array<string>,
-    icons?: Array<Node>,
+    icons?: Array<Node>, // TODO would be better to be typed as Array<React.ComponentType>
     items?: Array<ToolbarItemConfig<*>>,
     locale?: Select<string>,
     showSuccess?: IObservableValue<boolean>,