Fixes to detecting changes in forms.

client/src/campaigns/triggers/CUD.js

@@ -98,7 +98,7 @@ export default class CUD extends Component {
 
     componentDidMount() {
         if (this.props.entity) {
-            this.getFormValuesFromEntity(this.props.entity, ::this.getFormValuesMutator);
+            this.getFormValuesFromEntity(this.props.entity);
 
         } else {
             this.populateFormValues({

client/src/lib/form.js

@@ -983,6 +983,19 @@ const withForm = createComponentMixin([], [], (TargetClass, InnerClass) => {
         isServerValidationRunning: false
     });
 
+    const getSaveData = (self, formStateData) => {
+        let data = formStateData.map(attr => attr.get('value')).toJS();
+
+        if (self.submitFormValuesMutator) {
+            const newData = self.submitFormValuesMutator(data);
+            if (newData !== undefined) {
+                data = newData;
+            }
+        }
+
+        return data;
+    };
+
     // formValidateResolve is called by "validateForm" once client receives validation response from server that does not
     // trigger another server validation
     let formValidateResolve = null;
@@ -1100,7 +1113,7 @@ const withForm = createComponentMixin([], [], (TargetClass, InnerClass) => {
         delete data.hash;
 
         if (this.getFormValuesMutator) {
-            this.getFormValuesMutator(data);
+            this.getFormValuesMutator(data, this.getFormValues());
         }
 
         this.populateFormValues(data);
@@ -1126,7 +1139,7 @@ const withForm = createComponentMixin([], [], (TargetClass, InnerClass) => {
         delete data.hash;
 
         if (this.getFormValuesMutator) {
-            const newData = this.getFormValuesMutator(data);
+            const newData = this.getFormValuesMutator(data, this.getFormValues());
 
             if (newData !== undefined) {
                 data = newData;
@@ -1167,7 +1180,7 @@ const withForm = createComponentMixin([], [], (TargetClass, InnerClass) => {
             if (settings.leaveConfirmation) {
                 await new Promise((resolve, reject) => {
                     this.setState(previousState => ({
-                        formState: previousState.formState.set('savedData', previousState.formState.get('data'))
+                        formState: previousState.formState.set('savedData', getSaveData(this, previousState.formState.get('data')))
                     }), resolve);
                 });
             }
@@ -1197,7 +1210,7 @@ const withForm = createComponentMixin([], [], (TargetClass, InnerClass) => {
                 }));
 
                 if (settings.leaveConfirmation) {
-                    mutState.set('savedData', mutState.get('data'));
+                    mutState.set('savedData', getSaveData(this, mutState.get('data')));
                 }
 
                 validateFormState(this, mutState);
@@ -1304,6 +1317,7 @@ const withForm = createComponentMixin([], [], (TargetClass, InnerClass) => {
     };
 
     proto.getFormValues = function(name) {
+        if (!this.state || !this.state.formState) return undefined;
         return this.state.formState.get('data').map(attr => attr.get('value')).toJS();
     };
 
@@ -1324,21 +1338,8 @@ const withForm = createComponentMixin([], [], (TargetClass, InnerClass) => {
     };
 
     const _isFormChanged = self => {
-        const settings = self.state.formSettings;
+        const currentData = getSaveData(self, self.state.formState.get('data'));
-
+        const savedData = self.state.formState.get('savedData');
-        const mutateData = data => {
-            if (self.submitFormValuesMutator) {
-                const newData = self.submitFormValuesMutator(data);
-                if (newData !== undefined) {
-                    data = newData;
-                }
-            }
-
-            return data;
-        };
-
-        const currentData = mutateData(self.state.formState.get('data').map(attr => attr.get('value')).toJS());
-        const savedData = mutateData(self.state.formState.get('savedData').map(attr => attr.get('value')).toJS());
 
         return !deepEqual(currentData, savedData);
     };

client/src/lists/forms/CUD.js

@@ -81,10 +81,8 @@ export default class CUD extends Component {
                 changed: this.serverValidatedFields
             },
             onChange: {
-                previewList: () => {
+                previewList: (newState, key, oldValue, newValue) => {
-                    this.setState({
+                    newState.formState.setIn(['data', 'previewContents', 'value'], null);
-                        previewContents: null
-                    });
                 }
             }
         });
@@ -293,9 +291,9 @@ export default class CUD extends Component {
         }
     }
 
-    getFormValuesMutator(data) {
+    getFormValuesMutator(data, originalData) {
         this.supplyDefaults(data);
-        data.selectedTemplate = data.selectedTemplate || 'layout';
+        data.selectedTemplate = (originalData && originalData.selectedTemplate) || 'layout';
     }
 
     submitFormValuesMutator(data) {

client/src/lists/segments/CUD.js

@@ -27,6 +27,7 @@ import {ActionLink, Button, Icon} from "../../lib/bootstrap-components";
 import {getRuleHelpers} from "./helpers";
 import RuleSettingsPane from "./RuleSettingsPane";
 import {withComponentMixins} from "../../lib/decorator-helpers";
+import clone from "clone";
 
 // https://stackoverflow.com/a/4819886/1601953
 const isTouchDevice = !!('ontouchstart' in window || navigator.maxTouchPoints);
@@ -41,7 +42,7 @@ const isTouchDevice = !!('ontouchstart' in window || navigator.maxTouchPoints);
 ])
 export default class CUD extends Component {
     // The code below keeps the segment settings in form value. However, it uses it as a mutable datastructure.
-    // After initilization, segment settings is never set using setState. This is OK we update the state.rulesTree
+    // After initilization, segment settings is never set using setState. This is OK since we update the state.rulesTree
     // from the segment settings on relevant events (changes in the tree and closing the rule settings pane).
 
     constructor(props) {
@@ -104,9 +105,9 @@ export default class CUD extends Component {
         return tree;
     }
 
-    getFormValuesMutator(data) {
+    getFormValuesMutator(data, originalData) {
         data.rootRuleType = data.settings.rootRule.type;
-        data.selectedRule = null; // Validation errors of the selected rule are attached to this which makes sure we don't submit the segment if the opened rule has errors
+        data.selectedRule = (originalData && originalData.selectedRule) || null; // Validation errors of the selected rule are attached to this which makes sure we don't submit the segment if the opened rule has errors
 
         this.setState({
             rulesTree: this.getTreeFromRules(data.settings.rootRule.rules)
@@ -115,6 +116,10 @@ export default class CUD extends Component {
 
     submitFormValuesMutator(data) {
         data.settings.rootRule.type = data.rootRuleType;
+
+        // We have to clone the data here otherwise the form change detection doesn't work. This is because we use the state as a mutable structure.
+        data = clone(data);
+
         return filterData(data, ['name', 'settings']);
     }
 

client/src/lists/segments/RuleSettingsPane.js

@@ -31,8 +31,7 @@ export default class RuleSettingsPane extends PureComponent {
 
         this.initForm({
             leaveConfirmation: false,
-            onChangeBeforeValidation: ::this.populateRuleDefaults,
+            onChangeBeforeValidation: ::this.populateRuleDefaults
-            onChange: ::this.onFormChange
         });
     }
 
@@ -45,7 +44,8 @@ export default class RuleSettingsPane extends PureComponent {
         forceShowValidation: PropTypes.bool.isRequired
     }
 
-    updateStateFromProps(props, populateForm) {
+    updateStateFromProps(populateForm) {
+        const props = this.props;
         if (populateForm) {
             const rule = props.rule;
             const ruleHelpers = this.ruleHelpers;
@@ -74,15 +74,32 @@ export default class RuleSettingsPane extends PureComponent {
         if (props.forceShowValidation) {
             this.showFormValidation();
         }
-        
     }
     
     componentDidMount() {
-        this.updateStateFromProps(this.props, true);
+        this.updateStateFromProps(true);
     }
 
-    componentWillReceiveProps(nextProps) {
+    componentDidUpdate(prevProps) {
-        this.updateStateFromProps(nextProps, this.props.rule !== nextProps.rule);
+        this.updateStateFromProps(this.props.rule !== prevProps.rule);
+
+        if (this.isFormWithoutErrors()) {
+            const rule = this.props.rule;
+            const ruleHelpers = this.ruleHelpers;
+
+            rule.type = this.getFormValue('type');
+
+            if (!ruleHelpers.isCompositeRuleType(rule.type)) {
+                rule.column = this.getFormValue('column');
+
+                const settings = this.ruleHelpers.getRuleTypeSettings(rule);
+                settings.assignRuleSettings(rule, key => this.getFormValue(key));
+            }
+
+            this.props.onChange(false);
+        } else {
+            this.props.onChange(true);
+        }
     }
 
     localValidateFormValues(state) {
@@ -95,16 +112,17 @@ export default class RuleSettingsPane extends PureComponent {
 
         const ruleType = state.getIn(['type', 'value']);
         if (!ruleHelpers.isCompositeRuleType(ruleType)) {
-            const column = state.getIn(['column', 'value']);
+            if (!ruleType) {
+                state.setIn(['type', 'error'], t('typeMustBeSelected'));
+            }
 
+            const column = state.getIn(['column', 'value']);
             if (column) {
                 const colType = ruleHelpers.getColumnType(column);
 
                 if (ruleType) {
                     const settings = ruleHelpers.primitiveRuleTypes[colType][ruleType];
                     settings.validate(state);
-                } else {
-                    state.setIn(['type', 'error'], t('typeMustBeSelected'));
                 }
             } else {
                 state.setIn(['column', 'error'], t('fieldMustBeSelected'));
@@ -133,28 +151,6 @@ export default class RuleSettingsPane extends PureComponent {
         }
     }
 
-    onFormChange(newState) {
-        const noErrors = !newState.formState.get('data').find(attr => attr.get('error'));
-
-        if (noErrors) {
-            const rule = this.props.rule;
-            const ruleHelpers = this.ruleHelpers;
-
-            rule.type = newState.formState.getIn(['data','type','value']);
-
-            if (!ruleHelpers.isCompositeRuleType(rule.type)) {
-                rule.column = newState.formState.getIn(['data','column','value']);
-
-                const settings = this.ruleHelpers.getRuleTypeSettings(rule);
-                settings.assignRuleSettings(rule, key => newState.formState.getIn(['data', key, 'value']));
-            }
-
-            this.props.onChange(false);
-        } else {
-            this.props.onChange(true);
-        }
-    }
-
     async closeForm() {
         if (this.isFormWithoutErrors()) {
             this.props.onClose();

client/src/lists/subscriptions/List.js

@@ -67,8 +67,8 @@ export default class List extends Component {
         this.updateSegmentSelection(this.props);
     }
 
-    componentWillReceiveProps(nextProps) {
+    componentDidUpdate() {
-        this.updateSegmentSelection(nextProps);
+        this.updateSegmentSelection(this.props);
     }
 
     render() {