Do not alter current object when fetching its search options

glpi-project · Feb 10, 2025 · 2a84937 · 2a84937
1 parent 0dd9105
commit 2a84937
Show file tree

Hide file tree

Showing 9 changed files with 71 additions and 31 deletions.
diff --git a/phpunit/abstracts/RuleCommonITILObject.php b/phpunit/abstracts/RuleCommonITILObject.php
@@ -1743,13 +1743,16 @@ public function testITILCategoryCode()
 
     public function testAssignAppliance()
     {
+        $root_entity = \getItemByTypeName(Entity::class, '_test_root_entity', true);
+
         $this->login();
 
         //create appliance "appliance"
         $applianceTest1 = new \Appliance();
         $appliancetest1_id = $applianceTest1->add($applianceTest1_input = [
             "name"                  => "appliance",
-            "is_helpdesk_visible"   => true
+            "is_helpdesk_visible"   => true,
+            "entities_id"           => $root_entity,
         ]);
         $this->checkInput($applianceTest1, $appliancetest1_id, $applianceTest1_input);
 
@@ -1855,13 +1858,16 @@ public function testAssignAppliance()
 
     public function testRegexAppliance()
     {
+        $root_entity = \getItemByTypeName(Entity::class, '_test_root_entity', true);
+
         $this->login();
 
         //create appliance "erp"
         $applianceTest1 = new \Appliance();
         $appliancetest1_id = $applianceTest1->add($applianceTest1_input = [
             "name"                  => "erp",
-            "is_helpdesk_visible"   => true
+            "is_helpdesk_visible"   => true,
+            "entities_id"           => $root_entity,
         ]);
         $this->checkInput($applianceTest1, $appliancetest1_id, $applianceTest1_input);
 
@@ -1965,21 +1971,25 @@ public function testRegexAppliance()
 
     public function testAppendAppliance()
     {
+        $root_entity = \getItemByTypeName(Entity::class, '_test_root_entity', true);
+
         $this->login();
 
         //create appliance "erp"
         $applianceTest1 = new \Appliance();
         $appliancetest1_id = $applianceTest1->add($applianceTest1_input = [
             "name"                  => "erp",
-            "is_helpdesk_visible"   => true
+            "is_helpdesk_visible"   => true,
+            "entities_id"           => $root_entity,
         ]);
         $this->checkInput($applianceTest1, $appliancetest1_id, $applianceTest1_input);
 
         //create appliance "glpi"
         $applianceTest2 = new \Appliance();
         $appliancetest2_id = $applianceTest2->add($applianceTest2_input = [
             "name"                  => "glpi",
-            "is_helpdesk_visible"   => true
+            "is_helpdesk_visible"   => true,
+            "entities_id"           => $root_entity,
         ]);
         $this->checkInput($applianceTest2, $appliancetest2_id, $applianceTest2_input);
 

diff --git a/phpunit/functional/Appliance_ItemTest.php b/phpunit/functional/Appliance_ItemTest.php
@@ -37,6 +37,7 @@
 
 use Appliance_Item;
 use DbTestCase;
+use Entity;
 use Glpi\Asset\Capacity\HasAppliancesCapacity;
 use Glpi\Features\Clonable;
 use Toolbox;
@@ -94,15 +95,19 @@ public function testCountForAppliance()
         /** @var \DBmysql $DB */
         global $DB;
 
+        $entity_id = \getItemByTypeName(Entity::class, '_test_root_entity', true);
+
         $appliance = new \Appliance();
 
         $appliance_1 = (int)$appliance->add([
-            'name'   => 'Test appliance'
+            'name'        => 'Test appliance',
+            'entities_id' => $entity_id,
         ]);
         $this->assertGreaterThan(0, $appliance_1);
 
         $appliance_2 = (int)$appliance->add([
-            'name'   => 'Test appliance'
+            'name'        => 'Test appliance 2',
+            'entities_id' => $entity_id,
         ]);
         $this->assertGreaterThan(0, $appliance_2);
 

diff --git a/phpunit/functional/Appliance_Item_RelationTest.php b/phpunit/functional/Appliance_Item_RelationTest.php
@@ -36,6 +36,7 @@
 namespace tests\units;
 
 use DbTestCase;
+use Entity;
 
 class Appliance_Item_RelationTest extends DbTestCase
 {
@@ -53,10 +54,13 @@ public function testCountForApplianceItem()
         /** @var \DBmysql $DB */
         global $DB;
 
+        $entity_id = \getItemByTypeName(Entity::class, '_test_root_entity', true);
+
         $appliance = new \Appliance();
 
         $appliances_id = (int)$appliance->add([
-            'name'   => 'Test appliance'
+            'name'        => 'Test appliance',
+            'entities_id' => $entity_id,
         ]);
         $this->assertGreaterThan(0, $appliances_id);
 

diff --git a/phpunit/functional/ComputerTest.php b/phpunit/functional/ComputerTest.php
@@ -36,6 +36,7 @@
 namespace tests\units;
 
 use DbTestCase;
+use Entity;
 use Glpi\Asset\Asset_PeripheralAsset;
 use PHPUnit\Framework\Attributes\DataProvider;
 use Psr\Log\LogLevel;
@@ -639,14 +640,17 @@ public function testCloneWithSpecificName()
 
     public function testClonedRelationNamesFromTemplate()
     {
+        $entity_id = \getItemByTypeName(Entity::class, '_test_root_entity', true);
+
         $this->login();
         $this->setEntity('_test_root_entity', true);
 
         /** @var \Computer $computer */
         $computer_template = new \Computer();
         $templates_id = $computer_template->add([
             'template_name' => __FUNCTION__ . '_template',
-            'is_template' => 1
+            'is_template'   => 1,
+            'entities_id'   => $entity_id,
         ]);
         $this->assertGreaterThan(0, $templates_id);
 

diff --git a/phpunit/functional/SearchTest.php b/phpunit/functional/SearchTest.php
@@ -1198,7 +1198,7 @@ public function testDateBeforeOrNot()
      */
     public function testGetSearchOptionsWException()
     {
-        $error = 'Duplicate key 12 (One search option/Any option) in tests\units\DupSearchOpt searchOptions!';
+        $error = 'Duplicate key `12` (`One search option`/`Any option`) in `tests\units\DupSearchOpt` search options.';
 
         $item = new DupSearchOpt();
         $item->searchOptions();

diff --git a/src/CommonDBTM.php b/src/CommonDBTM.php
@@ -3575,11 +3575,14 @@ public function getField($field)
      **/
     public function isField($field)
     {
+        /** @var \DBmysql $DB */
+        global $DB;
 
-        if (!isset($this->fields['id'])) {
-            $this->getEmpty();
+        if (static::$notable === true) {
+            return false;
         }
-        return array_key_exists($field, $this->fields);
+
+        return array_key_exists($field, $DB->listFields(static::getTable()));
     }
 
 
@@ -3920,15 +3923,16 @@ public function getNameID($options = [])
      **/
     final public function searchOptions()
     {
-        $type = $this->getType();
-
-        if (isset(self::$search_options_cache[$type])) {
-            return self::$search_options_cache[$type];
+        if (isset(self::$search_options_cache[static::class])) {
+            return self::$search_options_cache[static::class];
         }
 
-        $options[$type] = [];
+        self::$search_options_cache[static::class] = [];
+
+        // Force usage of a new object, to be sure that the current object will not be altered.
+        $self = new static();
 
-        foreach ($this->rawSearchOptions() as $opt) {
+        foreach ($self->rawSearchOptions() as $opt) {
             // FIXME In GLPI 11.0, trigger a warning on invalid datatype (see `tests\units\Search::testSearchOptionsDatatype()`)
 
             $missingFields = [];
@@ -3954,20 +3958,21 @@ final public function searchOptions()
             $optid = $opt['id'];
             unset($opt['id']);
 
-            if (isset($options[$type][$optid])) {
-                $message = "Duplicate key $optid ({$options[$type][$optid]['name']}/{$opt['name']}) in " .
-                  get_class($this) . " searchOptions!";
-
+            if (isset(self::$search_options_cache[static::class][$optid])) {
+                $message = sprintf(
+                    'Duplicate key `%s` (`%s`/`%s`) in `%s` search options.',
+                    $optid,
+                    self::$search_options_cache[static::class][$optid]['name'],
+                    $opt['name'],
+                    static::class
+                );
                 trigger_error($message, E_USER_WARNING);
             }
 
-            foreach ($opt as $k => $v) {
-                $options[$type][$optid][$k] = $v;
-            }
+            self::$search_options_cache[static::class][$optid] = $opt;
         }
 
-        self::$search_options_cache[$type] = $options[$type];
-        return $options[$type];
+        return self::$search_options_cache[static::class];
     }
 
 

diff --git a/src/CommonDropdown.php b/src/CommonDropdown.php
@@ -932,7 +932,8 @@ public function getLinks($withname = false)
         }
 
         if (
-            $this->isField('knowbaseitemcategories_id')
+            !$this->isNewItem()
+            && $this->isField('knowbaseitemcategories_id')
             && $this->fields['knowbaseitemcategories_id']
         ) {
             $title = __s('FAQ');

diff --git a/src/CommonITILObject.php b/src/CommonITILObject.php
@@ -9412,16 +9412,27 @@ protected function setTechAndGroupFromItilCategory($input)
      */
     protected function setTechAndGroupFromHardware($input, $item)
     {
+        /** @var array $CFG_GLPI */
+        global $CFG_GLPI;
+
         if ($item != null) {
             // Auto assign tech from item
             $has_user_assigned  = $this->hasValidActorInInput($input, User::class, CommonITILActor::ASSIGN);
-            if (!$has_user_assigned && $item->isField('users_id_tech') && $item->fields['users_id_tech'] > 0) {
+            if (
+                !$has_user_assigned
+                && in_array($item::class, $CFG_GLPI['assignable_types'], true)
+                && $item->fields['users_id_tech'] > 0
+            ) {
                 $input['_users_id_assign'] = $item->fields['users_id_tech'];
             }
 
             // Auto assign group from item
             $has_group_assigned = $this->hasValidActorInInput($input, Group::class, CommonITILActor::ASSIGN);
-            if (!$has_group_assigned && $item->isField('groups_id_tech') && $item->fields['groups_id_tech'] > 0) {
+            if (
+                !$has_group_assigned
+                && in_array($item::class, $CFG_GLPI['assignable_types'], true)
+                && $item->fields['groups_id_tech'] > 0
+            ) {
                 $input['_groups_id_assign'] = $item->fields['groups_id_tech'];
             }
         }

diff --git a/src/Glpi/Features/AssetImage.php b/src/Glpi/Features/AssetImage.php
@@ -85,7 +85,7 @@ public function managePictures($input)
 
         $pictures = [];
         $pictures_removed = false;
-        if ($this->isField('pictures')) {
+        if (!$this->isNewItem() && $this->isField('pictures')) {
             $input_keys = array_keys($input);
             $pictures = importArrayFromDB($this->fields['pictures']);
             $to_remove = [];