Feature: improve logging of user assignment and deassigment

phpunit/functional/CommonITILActorTest.php

@@ -0,0 +1,172 @@
+<?php
+
+/**
+ * ---------------------------------------------------------------------
+ *
+ * GLPI - Gestionnaire Libre de Parc Informatique
+ *
+ * http://glpi-project.org
+ *
+ * @copyright 2015-2025 Teclib' and contributors.
+ * @licence   https://www.gnu.org/licenses/gpl-3.0.html
+ *
+ * ---------------------------------------------------------------------
+ *
+ * LICENSE
+ *
+ * This file is part of GLPI.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ *
+ * ---------------------------------------------------------------------
+ */
+
+namespace tests\units;
+
+use Change;
+use CommonDBTM;
+use CommonITILActor;
+use DbTestCase;
+use Glpi\DBAL\QueryExpression;
+use Group;
+use Log;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Problem;
+use Supplier;
+use Ticket;
+use User;
+
+class CommonITILActorTest extends DbTestCase
+{
+    public static function addCombinations(): iterable
+    {
+        $classes = [Ticket::class, Change::class, Problem::class ];
+        $positions = [
+            [
+                "id" => CommonITILActor::REQUESTER,
+                "name" => 'requester',
+            ],
+            [
+                "id" => CommonITILActor::OBSERVER,
+                "name" => 'observer',
+            ],
+            [
+                "id" => CommonITILActor::ASSIGN,
+                "name" => "technician",
+                "actor_name" => 'assign',
+            ],
+        ];
+
+        $actor_objs = [
+            getItemByTypeName(User::class, 'post-only', false),
+            getItemByTypeName(Group::class, '_test_group_1', false),
+            getItemByTypeName(Supplier::class, '_suplier01_name', false),
+        ];
+
+        foreach ($classes as $class) {
+            foreach ($actor_objs as $actor_obj) {
+
+                foreach ($positions as $position) {
+                    if ($actor_obj::class == Supplier::class) {
+                        if ($position['id'] != CommonITILActor::ASSIGN) {
+                            continue;  # Supplier can only be used in assign
+                        }
+                        $position['name'] = "supplier";
+                    }
+
+                    yield [
+                        "obj_class" => $class,
+                        "position" => $position,
+                        "actor" => $actor_obj,
+                    ];
+                }
+            }
+        }
+    }
+
+    private function checkLog($obj, $position, $actor, $action)
+    {
+        $expected = [
+            'itemtype'      => $obj::class,
+            'items_id'      => $obj->getId(),
+            'itemtype_link' => $actor::class,
+            'linked_action' => $action,
+            'id_search_option' => ['>', 0],
+        ];
+        # test correct log entry in db
+        $this->assertEquals(1, countElementsInTable(Log::getTable(), $expected));
+        # get log entry
+        $history = Log::getHistoryData($obj, 0, 0, ['itemtype_link' => $actor::class]);
+        $this->assertEquals(1, count($history));
+        $this-> assertArrayHasKey('change', $history[0]);
+        $change = $history[0]['change'];
+        # Ensure position is present in rendered log entry
+        $this->assertStringContainsStringIgnoringCase($position['name'], $change);
+    }
+
+    #[DataProvider(methodName: 'addCombinations')]
+    public function testLogOperationOnAddAndDelete(string $obj_class, array $position, CommonDBTM $actor): void
+    {
+        global $DB;
+
+        $this->login();
+
+        $obj = new $obj_class();
+        $obj_id = $obj->add([
+            'name' => 'testObject',
+            'content' => 'testObject',
+            'entities_id' => getItemByTypeName('Entity', '_test_root_entity', true),
+            '_skip_auto_assign' => true,
+        ]);
+        $this->assertGreaterThan(0, $obj_id);
+        $obj->loadActors();
+        # make sure no actor is Assigned
+        $this->assertEquals(0, $obj->countActors());
+        # clear log
+        $DB->delete(Log::getTable(), [new QueryExpression('true')]);
+        $actor_name = $position['actor_name'] ?? $position['name'];
+        $input = [
+            'id' => $obj_id,
+            '_actors' => [
+                $actor_name => [
+                    [
+                        'itemtype'  => $actor::class,
+                        'items_id'  => $actor -> getID(),
+                        'use_notification' => 0,
+                        'alternative_email' => '',
+                    ],
+                ],
+            ],
+        ];
+
+        $this -> assertTrue($obj -> update($input));
+        $this -> assertEquals(1, $obj->countActors());
+        # check log after adding
+        $this -> checkLog($obj, $position, $actor, Log::HISTORY_ADD_RELATION);
+        # clear log
+        $DB->delete(Log::getTable(), [new QueryExpression('true')]);
+        # remove actor
+        $input = [
+            'id' => $obj_id,
+            '_actors' => [
+                $actor_name => [],
+            ],
+        ];
+
+        $this -> assertTrue($obj -> update($input));
+        $this -> assertEquals(0, $obj->countActors());
+        # check log after removing an actor
+        $this -> checkLog($obj, $position, $actor, Log::HISTORY_DEL_RELATION);
+    }
+}

src/CommonDBRelation.php

@@ -1135,7 +1135,7 @@ public function post_deleteFromDB()
                 && static::$logs_for_item_1
             ) {
                 $changes = [
-                    '0',
+                    $this -> _force_log_option,
                     $this->getHistoryNameForItem1($item2, 'delete'),
                     '',
                 ];

src/CommonITILActor.php

@@ -100,6 +100,47 @@ public function getActors(int $items_id): array
         return $users;
     }
 
+    protected function get_force_log_option()
+    {
+        if ($this->_force_log_option !== 0) {
+            // if _force_log_option is already set by other code
+            // parts just return it to not change existing behavior
+            return $this->_force_log_option;
+        }
+
+        if (!isset($this->input['type'])) {
+            // keep existing behavior if type is not explicitly set
+            return $this->_force_log_option;
+        }
+
+        // Values from CommonITILObject::getSearchOptionsActors()
+        if (static::$itemtype_2 === 'User') {
+            switch ($this->input['type']) {
+                case CommonITILActor::REQUESTER:
+                    return 4;
+                case CommonITILActor::OBSERVER:
+                    return 66;
+                case CommonITILActor::ASSIGN:
+                    return 5;
+            }
+        } elseif (static::$itemtype_2 === 'Group') {
+            switch ($this->input['type']) {
+                case CommonITILActor::REQUESTER:
+                    return 71;
+                case CommonITILActor::OBSERVER:
+                    return 65;
+                case CommonITILActor::ASSIGN:
+                    return 8;
+            }
+        } elseif (static::$itemtype_2 === 'Supplier') {
+            // Suppliers are special the can only be assigned, not observe or request
+            switch ($this->input['type']) {
+                case CommonITILActor::ASSIGN:
+                    return 6;
+            }
+        }
+        return $this->_force_log_option; // again just return default
+    }
     /**
      * @param $items_id
      * @param $email
@@ -171,7 +212,10 @@ public function post_deleteFromDB()
                 }
             }
         }
+        $current_log_option = $this -> _force_log_option;
+        $this -> _force_log_option = $this -> get_force_log_option();
         parent::post_deleteFromDB();
+        $this -> _force_log_option = $current_log_option;
     }
 
     public function prepareInputForAdd($input)
@@ -289,7 +333,9 @@ public function post_addItem()
                 NotificationEvent::raiseEvent($event, $item);
             }
         }
-
+        $current_log_option = $this -> _force_log_option;
+        $this -> _force_log_option = $this -> get_force_log_option();
         parent::post_addItem();
+        $this -> _force_log_option = $current_log_option;
     }
 }

src/CommonITILObject.php

@@ -9351,6 +9351,7 @@ protected function updateActors(bool $disable_notifications = false)
 
         // Process deleted actors
         foreach ($actor_types as $actor_type) {
+            $actor_type_value = constant(CommonITILActor::class . '::' . strtoupper($actor_type));
             foreach ($actor_itemtypes as $actor_itemtype) {
                 $actor_fkey = getForeignKeyFieldForItemType($actor_itemtype);
                 $actors_deleted_input_key = sprintf('_%s_%s_deleted', $actor_fkey, $actor_type);
@@ -9360,7 +9361,7 @@ protected function updateActors(bool $disable_notifications = false)
                     : [];
                 foreach ($deleted as $actor) {
                     $actor_obj = $this->getActorObjectForItem($actor['itemtype']);
-                    $actor_obj->delete(['id' => $actor['id']]);
+                    $actor_obj->delete(['id' => $actor['id'], 'type' => $actor_type_value]);
                 }
             }
         }

src/Group_Ticket.php

@@ -48,22 +48,4 @@ class Group_Ticket extends CommonITILActor
     public static $itemtype_2 = 'Group';
     public static $items_id_2 = 'groups_id';
 
-
-    public function post_addItem()
-    {
-
-        switch ($this->input['type']) {  // Values from CommonITILObject::getSearchOptionsActors()
-            case CommonITILActor::REQUESTER:
-                $this->_force_log_option = 71;
-                break;
-            case CommonITILActor::OBSERVER:
-                $this->_force_log_option = 65;
-                break;
-            case CommonITILActor::ASSIGN:
-                $this->_force_log_option = 8;
-                break;
-        }
-        parent::post_addItem();
-        $this->_force_log_option = 0;
-    }
 }