From 44c814a550f1060e4255895a5e9515cbf153b6b8 Mon Sep 17 00:00:00 2001
From: Chad <chad.d.currie@gmail.com>
Date: Mon, 28 Jun 2021 12:20:38 +1200
Subject: [PATCH] Fix Rollback feature not creating diffs when properties are
 moved between 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 <nathan@nathanw.com.au>
---
 .../rollback/rollback.controller.js           | 75 +++++++++++--------
 1 file changed, 44 insertions(+), 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
index 0d49c7dd9ce3..4b0dfcb8b47a 100644
--- 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,7 +112,6 @@
          * This will load in a new version
          */
         function createDiff(currentVersion, previousVersion) {
-
             vm.diff = {};
             vm.diff.properties = [];
 
@@ -120,41 +119,55 @@
             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() {