Fix Rollback feature not creating diffs when properties are moved bet…

…ween tabs in EditorModel events (#10376) * Find the old property by alias instead of by indexes as if you move around properties in the EditorModel events Rollback breaks. * optimise property lookup - avoids iterating entire property set Co-authored-by: Nathan Woulfe <[email protected]>
umbraco · Jun 28, 2021 · 44c814a · 44c814a
1 parent d9f92ec
commit 44c814a
Showing 1 changed file with 44 additions and 31 deletions.
diff --git a/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/rollback/rollback.controller.js b/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/rollback/rollback.controller.js
@@ -112,49 +112,62 @@
          * This will load in a new version
          */
         function createDiff(currentVersion, previousVersion) {
-
             vm.diff = {};
             vm.diff.properties = [];
 
             // find diff in name
             vm.diff.name = JsDiff.diffWords(currentVersion.name, previousVersion.name);
 
             // extract all properties from the tabs and create new object for the diff
-            currentVersion.tabs.forEach((tab, tabIndex) => {
-                tab.properties.forEach((property, propertyIndex) => {
-                    var oldProperty = previousVersion.tabs[tabIndex].properties[propertyIndex];
-
-                    // copy existing properties, so it doesn't manipulate existing properties on page
-                    oldProperty = Utilities.copy(oldProperty);
-                    property = Utilities.copy(property);
-
-                    // we have to make properties storing values as object into strings (Grid, nested content, etc.)
-                    if(property.value instanceof Object) {
-                        property.value = JSON.stringify(property.value, null, 1);
-                        property.isObject = true;
+            currentVersion.tabs.forEach(function (tab) {
+                tab.properties.forEach(function (property) {
+                    let oldTabIndex = -1;
+                    let oldTabPropertyIndex = -1;
+                    const previousVersionTabs = previousVersion.tabs;
+
+                    // find the property by alias, but only search until we find it
+                    for (var oti = 0, length = previousVersionTabs.length; oti < length; oti++) {
+                        const opi = previousVersionTabs[oti].properties.findIndex(p => p.alias === property.alias);
+                        if (opi !== -1) {
+                            oldTabIndex = oti;
+                            oldTabPropertyIndex = opi;
+                            break;
+                        }
                     }
 
-                    if(oldProperty.value instanceof Object) {
-                        oldProperty.value = JSON.stringify(oldProperty.value, null, 1);
-                        oldProperty.isObject = true;
+                    if (oldTabIndex !== -1 && oldTabPropertyIndex !== -1) {
+                        let oldProperty = previousVersion.tabs[oldTabIndex].properties[oldTabPropertyIndex];
+
+                        // copy existing properties, so it doesn't manipulate existing properties on page
+                        oldProperty = Utilities.copy(oldProperty);
+                        property = Utilities.copy(property);
+
+                        // we have to make properties storing values as object into strings (Grid, nested content, etc.)
+                        if (property.value instanceof Object) {
+                            property.value = JSON.stringify(property.value, null, 1);
+                            property.isObject = true;
+                        }
+
+                        if (oldProperty.value instanceof Object) {
+                            oldProperty.value = JSON.stringify(oldProperty.value, null, 1);
+                            oldProperty.isObject = true;
+                        }
+
+                        // diff requires a string
+                        property.value = property.value ? property.value + '' : '';
+                        oldProperty.value = oldProperty.value ? oldProperty.value + '' : '';
+
+                        const diffProperty = {
+                            'alias': property.alias,
+                            'label': property.label,
+                            'diff': property.isObject ? JsDiff.diffJson(property.value, oldProperty.value) : JsDiff.diffWords(property.value, oldProperty.value),
+                            'isObject': property.isObject || oldProperty.isObject
+                        };
+
+                        vm.diff.properties.push(diffProperty);
                     }
-
-                    // diff requires a string
-                    property.value = property.value ? property.value + "" : "";
-                    oldProperty.value = oldProperty.value ? oldProperty.value + "" : "";
-
-                    var diffProperty = {
-                        "alias": property.alias,
-                        "label": property.label,
-                        "diff": (property.isObject) ? JsDiff.diffJson(property.value, oldProperty.value) : JsDiff.diffWords(property.value, oldProperty.value),
-                        "isObject": (property.isObject || oldProperty.isObject) ? true : false
-                    };
-
-                    vm.diff.properties.push(diffProperty);
-
                 });
             });
-
         }
 
         function rollback() {