From a3c911e93eaacfb48f0aa7ae3ad9463345d9bc2f Mon Sep 17 00:00:00 2001
From: Eric Espie <eric.espie@combodo.com>
Date: Wed, 29 Jan 2025 09:32:21 +0100
Subject: [PATCH] =?UTF-8?q?N=C2=B08139=20-=20:white=5Fcheck=5Fmark:=20Avoi?=
 =?UTF-8?q?d=20double=20writing=20in=20lifecycle=20action?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 core/dbobject.class.php                       |   2 +
 .../DBObjectWithModifiedDataModelTest.php     |  84 ++++++++++
 .../core/DBObject/Delta/dbobjecttest.xml      | 155 +++++++++++++++++-
 3 files changed, 239 insertions(+), 2 deletions(-)
 create mode 100644 tests/php-unit-tests/unitary-tests/core/DBObject/DBObjectWithModifiedDataModelTest.php

diff --git a/core/dbobject.class.php b/core/dbobject.class.php
index 869957265..f0030a860 100644
--- a/core/dbobject.class.php
+++ b/core/dbobject.class.php
@@ -4538,6 +4538,8 @@ abstract class DBObject implements iDisplay
 				} else {
 					$aBackupValues[$sAttCode] = $value;
 				}
+			} else {
+				$aBackupValues[$sAttCode] = $oAttDef->GetNullValue();
 			}
 		}
 
diff --git a/tests/php-unit-tests/unitary-tests/core/DBObject/DBObjectWithModifiedDataModelTest.php b/tests/php-unit-tests/unitary-tests/core/DBObject/DBObjectWithModifiedDataModelTest.php
new file mode 100644
index 000000000..cda459023
--- /dev/null
+++ b/tests/php-unit-tests/unitary-tests/core/DBObject/DBObjectWithModifiedDataModelTest.php
@@ -0,0 +1,84 @@
+<?php
+/**
+ * @copyright   Copyright (C) 2010-2025 Combodo SARL
+ * @license     http://opensource.org/licenses/AGPL-3.0
+ */
+
+namespace Combodo\iTop\Test\UnitTest\Core\DBObject;
+
+use Combodo\iTop\Test\UnitTest\ItopCustomDatamodelTestCase;
+use IssueLog;
+use LogChannels;
+use MetaModel;
+use utils;
+use const EVENT_DB_LINKS_CHANGED;
+
+class DBObjectWithModifiedDataModelTest extends ItopCustomDatamodelTestCase
+{
+	public function GetDatamodelDeltaAbsPath(): string
+	{
+		return __DIR__.'/Delta/dbobjecttest.xml';
+	}
+
+	const USE_TRANSACTION = true;
+	const CREATE_TEST_ORG = false;
+
+
+	private static string $sLogFile = 'log/test_error_CRUDEventTest.log';
+
+	protected function setUp(): void
+	{
+		parent::setUp();
+		static::$DEBUG_UNIT_TEST = false;
+
+		if (static::$DEBUG_UNIT_TEST) {
+			echo '--- logging in '.APPROOT.static::$sLogFile."\n\n";
+			@unlink(APPROOT.static::$sLogFile);
+			IssueLog::Enable(APPROOT.static::$sLogFile);
+			$oConfig = utils::GetConfig();
+			$oConfig->Set('log_level_min', [LogChannels::DM_CRUD => 'Debug', LogChannels::EVENT_SERVICE => 'Trace']);
+		}
+	}
+
+	protected function tearDown(): void
+	{
+		if (is_file(APPROOT.static::$sLogFile)) {
+			$sLog = file_get_contents(APPROOT.static::$sLogFile);
+			echo "--- error.log\n$sLog\n\n";
+			@unlink(APPROOT.static::$sLogFile);
+		}
+
+		parent::tearDown();
+	}
+
+	public function testStimulusStoppingActionPreventObjectModification()
+	{
+		// Given
+		$sObjectKey = $this->GivenObjectInDB('TestDBObject', ['name' => 'parent', 'status' => 'new']);
+		$oParent = MetaModel::GetObject('TestDBObject', $sObjectKey);
+
+		// When actions ApplyStimulus then next action fails
+		$oParent->ApplyStimulus('ev_assign');
+		$oParent->Reload();
+
+		// Then
+		// Check status...
+		$this->assertEquals('new', $oParent->Get('status'), 'The status should have remained unmodified due to action failure');
+	}
+
+	public function testCallingApplyStimulusWithinActionsWorks()
+	{
+		// Given
+		$sObjectKey = $this->GivenObjectInDB('TestDBObject', ['name' => 'parent', 'status' => 'assigned']);
+		$oParent = MetaModel::GetObject('TestDBObject', $sObjectKey);
+
+		// When action ApplyStimulus
+		$oParent->ApplyStimulus('ev_reassign');
+		$oParent->Reload();
+
+		// Then
+		// Check that status has changed to the final status
+		$this->assertEquals('resolved', $oParent->Get('status'), 'The status should have been modified to resolved (the final state after a nested stimulus)');
+	}
+}
+
diff --git a/tests/php-unit-tests/unitary-tests/core/DBObject/Delta/dbobjecttest.xml b/tests/php-unit-tests/unitary-tests/core/DBObject/Delta/dbobjecttest.xml
index 20c31528e..2b2a9d5ca 100644
--- a/tests/php-unit-tests/unitary-tests/core/DBObject/Delta/dbobjecttest.xml
+++ b/tests/php-unit-tests/unitary-tests/core/DBObject/Delta/dbobjecttest.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<itop_design xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.7">
+<itop_design xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="3.0">
   <classes>
     <class id="TestDBObject" _delta="define">
       <parent>cmdbAbstractObject</parent>
@@ -12,7 +12,77 @@
             <attribute id="name"/>
           </attributes>
         </reconciliation>
+        <fields_semantic>
+          <state_attribute>status</state_attribute>
+        </fields_semantic>
       </properties>
+      <lifecycle>
+        <highlight_scale>
+          <item id="closed">
+            <rank>3</rank>
+            <color>HIGHLIGHT_CLASS_NONE</color>
+            <icon>images/user-request-closed.svg</icon>
+          </item>
+        </highlight_scale>
+        <stimuli>
+          <stimulus id="ev_assign" xsi:type="StimulusUserAction"/>
+          <stimulus id="ev_reassign" xsi:type="StimulusUserAction"/>
+          <stimulus id="ev_resolve" xsi:type="StimulusUserAction"/>
+        </stimuli>
+        <states>
+          <state id="new">
+            <flags/>
+            <transitions>
+              <transition id="ev_assign">
+                <target>assigned</target>
+                <actions>
+                  <action>
+                    <verb>Assign</verb>
+                    <params/>
+                  </action>
+                  <action>
+                    <verb>Assign2</verb>
+                    <params/>
+                  </action>
+                </actions>
+              </transition>
+            </transitions>
+          </state>
+          <state id="assigned">
+            <inherit_flags_from>new</inherit_flags_from>
+            <flags/>
+            <transitions>
+              <transition id="ev_resolve">
+                <target>resolved</target>
+                <actions>
+                  <action>
+                    <verb>Resolve</verb>
+                    <params/>
+                  </action>
+                </actions>
+              </transition>
+              <transition id="ev_reassign">
+                <target>assigned</target>
+                <actions>
+                  <action>
+                    <verb>Reassign</verb>
+                    <params/>
+                  </action>
+                </actions>
+                <flags/>
+              </transition>
+            </transitions>
+          </state>
+          <state id="resolved">
+            <highlight>
+              <code>closed</code>
+            </highlight>
+            <inherit_flags_from>assigned</inherit_flags_from>
+            <flags/>
+            <transitions/>
+          </state>
+        </states>
+      </lifecycle>
       <fields>
         <field id="name" xsi:type="AttributeString">
           <sql>name</sql>
@@ -38,9 +108,90 @@
           <count_max>0</count_max>
           <with_php_computation>true</with_php_computation>
         </field>
+        <field id="status" xsi:type="AttributeEnum">
+          <always_load_in_tables>true</always_load_in_tables>
+          <sort_type>rank</sort_type>
+          <values>
+            <value id="new">
+              <code>new</code>
+              <rank>10</rank>
+            </value>
+            <value id="assigned">
+              <code>assigned</code>
+              <rank>60</rank>
+            </value>
+            <value id="resolved">
+              <code>resolved</code>
+              <rank>100</rank>
+            </value>
+          </values>
+          <sql>status</sql>
+          <default_value>new</default_value>
+          <is_null_allowed>false</is_null_allowed>
+        </field>
       </fields>
       <presentation/>
-      <methods/>
+      <methods>
+        <method id="Assign">
+          <comment><![CDATA[/**
+	 * @return true (returning false would stop the ongoing transition)
+	 */]]></comment>
+          <static>false</static>
+          <access>public</access>
+          <type>LifecycleAction</type>
+          <arguments>
+          </arguments>
+          <code><![CDATA[	public function Assign()
+	{
+	  $this->ApplyStimulus('ev_assign');
+		return true;
+	}]]></code>
+        </method>
+        <method id="Assign2">
+          <comment><![CDATA[/**
+	 * @return true (returning false would stop the ongoing transition)
+	 */]]></comment>
+          <static>false</static>
+          <access>public</access>
+          <type>LifecycleAction</type>
+          <arguments>
+          </arguments>
+          <code><![CDATA[	public function Assign2()
+	{
+		return false;
+	}]]></code>
+        </method>
+        <method id="Resolve">
+          <comment><![CDATA[/**
+	 * @return true (returning false would stop the ongoing transition)
+	 */]]></comment>
+          <static>false</static>
+          <access>public</access>
+          <type>LifecycleAction</type>
+          <arguments>
+          </arguments>
+          <code><![CDATA[	public function Resolve()
+	{
+	  $this->ApplyStimulus('ev_resolve');
+		return true;
+	}]]></code>
+        </method>
+        <method id="Reassign">
+          <comment><![CDATA[/**
+	 * @return true (returning false would stop the ongoing transition)
+	 */]]></comment>
+          <static>false</static>
+          <access>public</access>
+          <type>LifecycleAction</type>
+          <arguments>
+          </arguments>
+          <code><![CDATA[	public function Reassign()
+	{
+	  $this->ApplyStimulus('ev_resolve');
+		return true;
+	}]]></code>
+        </method>
+      </methods>
     </class>
   </classes>
 </itop_design>
\ No newline at end of file