From cc5e105d7fcb68dbb1b5a656a0b8b76b2ca1de66 Mon Sep 17 00:00:00 2001
From: Eric Espie <eric.espie@combodo.com>
Date: Wed, 17 Jan 2024 15:11:46 +0100
Subject: [PATCH] =?UTF-8?q?N=C2=B07070=20-=20remove=20=5Fdelta=20attribute?=
 =?UTF-8?q?=20from=20datamodel=20and=20avoid=20=5Fdelta=20attributes=20und?=
 =?UTF-8?q?er=20a=20node=20having=20=5Fdelta=20in=20[define,=20define=5Fif?=
 =?UTF-8?q?=5Fnot=5Fexist,=20redefine,=20force]=20in=20deltas?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 .../datamodel.itop-config-mgmt.xml            |  2 +-
 .../2.x/itop-portal/datamodel.itop-portal.xml | 12 +++----
 .../itop-tickets/datamodel.itop-tickets.xml   |  4 +--
 setup/modelfactory.class.inc.php              | 35 ++++++++-----------
 .../unitary-tests/setup/ModelFactoryTest.php  | 28 ++++++++++++++-
 5 files changed, 51 insertions(+), 30 deletions(-)

diff --git a/datamodels/2.x/itop-config-mgmt/datamodel.itop-config-mgmt.xml b/datamodels/2.x/itop-config-mgmt/datamodel.itop-config-mgmt.xml
index 2e42e2e1c..720a067d6 100755
--- a/datamodels/2.x/itop-config-mgmt/datamodel.itop-config-mgmt.xml
+++ b/datamodels/2.x/itop-config-mgmt/datamodel.itop-config-mgmt.xml
@@ -77,7 +77,7 @@
           <default_value/>
           <is_null_allowed>true</is_null_allowed>
         </field>
-        <field id="contacts_list" xsi:type="AttributeLinkedSetIndirect" _delta="define">
+        <field id="contacts_list" xsi:type="AttributeLinkedSetIndirect">
           <linked_class>lnkContactToFunctionalCI</linked_class>
           <ext_key_to_me>functionalci_id</ext_key_to_me>
           <count_min>0</count_min>
diff --git a/datamodels/2.x/itop-portal/datamodel.itop-portal.xml b/datamodels/2.x/itop-portal/datamodel.itop-portal.xml
index 2fe7d8a3c..d8d9c7612 100644
--- a/datamodels/2.x/itop-portal/datamodel.itop-portal.xml
+++ b/datamodels/2.x/itop-portal/datamodel.itop-portal.xml
@@ -41,7 +41,7 @@
         </allowed_portals>
       </properties>
       <classes>
-        <class id="User" _delta="define">
+        <class id="User">
           <!-- Allow <allowed_profile>s to apply the <transition>s below. Flags on <field>s will be merged with those from datamodel -->
           <!-- See example in UserRequest class -->
           <!--<lifecycle/>-->
@@ -56,21 +56,21 @@
             </scope>
           </scopes>
         </class>
-        <class id="Organization" _delta="define">
+        <class id="Organization">
           <scopes>
             <scope id="all">
               <oql_view><![CDATA[SELECT Organization WHERE id = :current_contact->org_id]]></oql_view>
             </scope>
           </scopes>
         </class>
-        <class id="Location" _delta="define">
+        <class id="Location">
           <scopes>
             <scope id="all">
               <oql_view><![CDATA[SELECT Location WHERE org_id = :current_contact->org_id]]></oql_view>
             </scope>
           </scopes>
         </class>
-        <class id="Contact" _delta="define">
+        <class id="Contact">
           <scopes>
             <scope id="all">
               <oql_view><![CDATA[SELECT Contact WHERE org_id = :current_contact->org_id]]></oql_view>
@@ -145,9 +145,9 @@
       </bricks>
       <navigation_rules>
         <!-- Close form (either the modal or the whole page) -->
-        <navigation_rule id="close-form" xsi:type="close" _delta="define"/>
+        <navigation_rule id="close-form" xsi:type="close"/>
         <!-- Go to the homepage -->
-        <navigation_rule id="go-to-homepage" xsi:type="go-to-homepage" _delta="define"/>
+        <navigation_rule id="go-to-homepage" xsi:type="go-to-homepage"/>
       </navigation_rules>
     </module_design>
   </module_designs>
diff --git a/datamodels/2.x/itop-tickets/datamodel.itop-tickets.xml b/datamodels/2.x/itop-tickets/datamodel.itop-tickets.xml
index dc2790b51..201199553 100755
--- a/datamodels/2.x/itop-tickets/datamodel.itop-tickets.xml
+++ b/datamodels/2.x/itop-tickets/datamodel.itop-tickets.xml
@@ -277,7 +277,7 @@
         }
         ]]></code>
         </method>
-        <method id="AddCurrentUserToContacts" _revision_id="294" _delta="define">
+        <method id="AddCurrentUserToContacts" _revision_id="294">
           <comment>/**
             *
             * Add the current contact associated Person to the contacts_list of this Ticket
@@ -305,7 +305,7 @@
 	  }]]></code>
           <arguments/>
         </method>
-        <method id="RemoveCurrentUserFromContacts" _revision_id="296" _delta="define">
+        <method id="RemoveCurrentUserFromContacts" _revision_id="296">
           <comment>/**
             *
             * Remove the current user associated Person from the contacts_list of this Ticket
diff --git a/setup/modelfactory.class.inc.php b/setup/modelfactory.class.inc.php
index a029da479..b25b249b1 100644
--- a/setup/modelfactory.class.inc.php
+++ b/setup/modelfactory.class.inc.php
@@ -2357,29 +2357,24 @@ EOF;
 	 */
 	public function ApplyChanges()
 	{
-		$oNodes = $this->ListChanges();
-		foreach ($oNodes as $oNode)
-		{
-			$sOperation = $oNode->GetAttribute('_alteration');
-			switch ($sOperation)
-			{
-				case 'added':
-				case 'replaced':
-				case 'needed':
-				case 'forced':
-					// marked as added or modified, just reset the flag
-					$oNode->removeAttribute('_alteration');
-					break;
-
-				case 'removed':
-					// marked as deleted, let's remove the node from the tree
-					$oNode->parentNode->removeChild($oNode);
-					break;
+		// Note: omitting the dot will make the query be global to the whole document!!!
+		$oNodes = $this->ownerDocument->GetNodes('.//*[@_alteration or @_old_id or @_delta]', $this, false);;
+		foreach ($oNodes as $oNode) {
+			// _delta must not exist after applying changes
+			if ($oNode->hasAttribute('_delta')) {
+				$oNode->removeAttribute('_delta');
 			}
-			if ($oNode->hasAttribute('_old_id'))
-			{
+			if ($oNode->hasAttribute('_old_id')) {
 				$oNode->removeAttribute('_old_id');
 			}
+			if ($oNode->hasAttribute('_alteration')) {
+				if ('removed' === $oNode->GetAttribute('_alteration')) {
+					$oNode->parentNode->removeChild($oNode);
+				} else {
+						// marked as added or modified, just reset the flag
+						$oNode->removeAttribute('_alteration');
+				}
+			}
 		}
 	}
 }
diff --git a/tests/php-unit-tests/unitary-tests/setup/ModelFactoryTest.php b/tests/php-unit-tests/unitary-tests/setup/ModelFactoryTest.php
index f5eff7661..5f38692af 100644
--- a/tests/php-unit-tests/unitary-tests/setup/ModelFactoryTest.php
+++ b/tests/php-unit-tests/unitary-tests/setup/ModelFactoryTest.php
@@ -727,7 +727,33 @@ XML
   <nodeB id="Banner">
     <nodeE/>
     <nodeC>
-      <nodeD id="Bruce" _delta="define"/>
+      <nodeD id="Bruce"/>
+    </nodeC>
+  </nodeB>
+</nodeA>
+XML
+		];
+		$aDeltas['nested _delta should be cleaned'] = [
+			'sInitialXML' => <<<XML
+<nodeA>
+</nodeA>
+XML
+			,
+			'sDeltaXML' => <<<XML
+<nodeA>
+	<nodeB id="Banner" _delta="define">
+		<nodeC>
+			<nodeD id="Bruce" _delta="define"/>
+		</nodeC>
+	</nodeB>
+</nodeA>
+XML
+			,
+			'sExpectedXML' => <<<XML
+<nodeA>
+  <nodeB id="Banner">
+    <nodeC>
+      <nodeD id="Bruce"/>
     </nodeC>
   </nodeB>
 </nodeA>