[Conditionals] Ensure correct conditions are updated after reordering (#3336)

* Use id of condition instead of index to ensure

src/plugins/condition/ConditionManager.js

@@ -125,11 +125,17 @@ export default class ConditionManager extends EventEmitter {
         }
     }
 
-    updateCondition(conditionConfiguration, index) {
-        let condition = this.conditions[index];
-        this.conditionSetDomainObject.configuration.conditionCollection[index] = conditionConfiguration;
-        condition.update(conditionConfiguration);
-        this.persistConditions();
+    updateCondition(conditionConfiguration) {
+        let condition = this.findConditionById(conditionConfiguration.id);
+        if (condition) {
+            condition.update(conditionConfiguration);
+        }
+
+        let index = this.conditionSetDomainObject.configuration.conditionCollection.findIndex(item => item.id === conditionConfiguration.id);
+        if (index > -1) {
+            this.conditionSetDomainObject.configuration.conditionCollection[index] = conditionConfiguration;
+            this.persistConditions();
+        }
     }
 
     updateConditionDescription(condition) {
@@ -202,12 +208,18 @@ export default class ConditionManager extends EventEmitter {
         this.persistConditions();
     }
 
-    removeCondition(index) {
-        let condition = this.conditions[index];
-        condition.destroy();
-        this.conditions.splice(index, 1);
-        this.conditionSetDomainObject.configuration.conditionCollection.splice(index, 1);
-        this.persistConditions();
+    removeCondition(id) {
+        let index = this.conditions.findIndex(item => item.id === id);
+        if (index > -1) {
+            this.conditions[index].destroy();
+            this.conditions.splice(index, 1);
+        }
+
+        let conditionCollectionIndex = this.conditionSetDomainObject.configuration.conditionCollection.findIndex(item => item.id === id);
+        if (conditionCollectionIndex > -1) {
+            this.conditionSetDomainObject.configuration.conditionCollection.splice(conditionCollectionIndex, 1);
+            this.persistConditions();
+        }
     }
 
     findConditionById(id) {
@@ -220,8 +232,8 @@ export default class ConditionManager extends EventEmitter {
         reorderPlan.forEach((reorderEvent) => {
             let item = oldConditions[reorderEvent.oldIndex];
             newCollection.push(item);
-            this.conditionSetDomainObject.configuration.conditionCollection = newCollection;
         });
+        this.conditionSetDomainObject.configuration.conditionCollection = newCollection;
         this.persistConditions();
     }
 

src/plugins/condition/ConditionManagerSpec.js

@@ -27,13 +27,32 @@ describe('ConditionManager', () => {
     let conditionMgr;
     let mockListener;
     let openmct = {};
-    let mockCondition = {
+    let mockDefaultCondition = {
         isDefault: true,
         id: '1234-5678',
         configuration: {
             criteria: []
         }
     };
+    let mockCondition1 = {
+        id: '2345-6789',
+        configuration: {
+            criteria: []
+        }
+    };
+    let updatedMockCondition1 = {
+        id: '2345-6789',
+        configuration: {
+            trigger: 'xor',
+            criteria: []
+        }
+    };
+    let mockCondition2 = {
+        id: '3456-7890',
+        configuration: {
+            criteria: []
+        }
+    };
     let conditionSetDomainObject = {
         identifier: {
             namespace: "",
@@ -43,7 +62,9 @@ describe('ConditionManager', () => {
         location: "mine",
         configuration: {
             conditionCollection: [
-                mockCondition
+                mockCondition1,
+                mockCondition2,
+                mockDefaultCondition
             ]
         }
     };
@@ -59,7 +80,7 @@ describe('ConditionManager', () => {
 
         let mockDomainObject = {
             useCapability: function () {
-                return mockCondition;
+                return mockDefaultCondition;
             }
         };
         mockInstantiate.and.callFake(function () {
@@ -107,7 +128,11 @@ describe('ConditionManager', () => {
         openmct.objects.get.and.returnValues(new Promise(function (resolve, reject) {
             resolve(conditionSetDomainObject);
         }), new Promise(function (resolve, reject) {
-            resolve(mockCondition);
+            resolve(mockCondition1);
+        }), new Promise(function (resolve, reject) {
+            resolve(mockCondition2);
+        }), new Promise(function (resolve, reject) {
+            resolve(mockDefaultCondition);
         }));
         openmct.objects.makeKeyString.and.returnValue(conditionSetDomainObject.identifier.key);
         openmct.objects.observe.and.returnValue(function () {});
@@ -126,9 +151,65 @@ describe('ConditionManager', () => {
     });
 
     it('creates a conditionCollection with a default condition', function () {
-        expect(conditionMgr.conditionSetDomainObject.configuration.conditionCollection.length).toEqual(1);
-        let defaultConditionId = conditionMgr.conditions[0].id;
-        expect(defaultConditionId).toEqual(mockCondition.id);
+        expect(conditionMgr.conditionSetDomainObject.configuration.conditionCollection.length).toEqual(3);
+        let defaultConditionId = conditionMgr.conditions[2].id;
+        expect(defaultConditionId).toEqual(mockDefaultCondition.id);
+    });
+
+    it('reorders a conditionCollection', function () {
+        let reorderPlan = [{
+            oldIndex: 1,
+            newIndex: 0
+        },
+        {
+            oldIndex: 0,
+            newIndex: 1
+        },
+        {
+            oldIndex: 2,
+            newIndex: 2
+        }];
+        conditionMgr.reorderConditions(reorderPlan);
+        expect(conditionMgr.conditionSetDomainObject.configuration.conditionCollection.length).toEqual(3);
+        expect(conditionMgr.conditionSetDomainObject.configuration.conditionCollection[0].id).toEqual(mockCondition2.id);
+        expect(conditionMgr.conditionSetDomainObject.configuration.conditionCollection[1].id).toEqual(mockCondition1.id);
+    });
+
+    it('updates the right condition after reorder', function () {
+        let reorderPlan = [{
+            oldIndex: 1,
+            newIndex: 0
+        },
+        {
+            oldIndex: 0,
+            newIndex: 1
+        },
+        {
+            oldIndex: 2,
+            newIndex: 2
+        }];
+        conditionMgr.reorderConditions(reorderPlan);
+        conditionMgr.updateCondition(updatedMockCondition1);
+        expect(conditionMgr.conditions[1].trigger).toEqual(updatedMockCondition1.configuration.trigger);
+    });
+
+    it('removes the right condition after reorder', function () {
+        let reorderPlan = [{
+            oldIndex: 1,
+            newIndex: 0
+        },
+        {
+            oldIndex: 0,
+            newIndex: 1
+        },
+        {
+            oldIndex: 2,
+            newIndex: 2
+        }];
+        conditionMgr.reorderConditions(reorderPlan);
+        conditionMgr.removeCondition(mockCondition1.id);
+        expect(conditionMgr.conditions.length).toEqual(2);
+        expect(conditionMgr.conditionSetDomainObject.configuration.conditionCollection[0].id).toEqual(mockCondition2.id);
     });
 
 });

src/plugins/condition/components/Condition.vue

@@ -308,15 +308,15 @@ export default {
             };
             this.condition.configuration.criteria.push(criteriaObject);
         },
-        dragStart(e) {
-            e.dataTransfer.setData('dragging', e.target); // required for FF to initiate drag
-            e.dataTransfer.effectAllowed = "copyMove";
-            e.dataTransfer.setDragImage(e.target.closest('.c-condition-h'), 0, 0);
+        dragStart(event) {
+            event.dataTransfer.clearData();
+            event.dataTransfer.setData('dragging', event.target); // required for FF to initiate drag
+            event.dataTransfer.effectAllowed = "copyMove";
+            event.dataTransfer.setDragImage(event.target.closest('.c-condition-h'), 0, 0);
             this.$emit('setMoveIndex', this.conditionIndex);
         },
-        dragEnd(event) {
+        dragEnd() {
             this.dragStarted = false;
-            event.dataTransfer.clearData();
             this.$emit('dragComplete');
         },
         dropCondition(event, targetIndex) {
@@ -359,10 +359,10 @@ export default {
         },
         destroy() {
         },
-        removeCondition(ev) {
-            this.$emit('removeCondition', this.conditionIndex);
+        removeCondition() {
+            this.$emit('removeCondition', this.condition.id);
         },
-        cloneCondition(ev) {
+        cloneCondition() {
             this.$emit('cloneCondition', {
                 condition: this.condition,
                 index: this.conditionIndex
@@ -380,8 +380,7 @@ export default {
         },
         persist() {
             this.$emit('updateCondition', {
-                condition: this.condition,
-                index: this.conditionIndex
+                condition: this.condition
             });
         },
         initCap(str) {

src/plugins/condition/components/ConditionCollection.vue

@@ -223,10 +223,10 @@ export default {
             this.conditionManager.addCondition();
         },
         updateCondition(data) {
-            this.conditionManager.updateCondition(data.condition, data.index);
+            this.conditionManager.updateCondition(data.condition);
         },
-        removeCondition(index) {
-            this.conditionManager.removeCondition(index);
+        removeCondition(id) {
+            this.conditionManager.removeCondition(id);
         },
         reorder(reorderPlan) {
             this.conditionManager.reorderConditions(reorderPlan);