diff --git a/CHANGELOG.md b/CHANGELOG.md index e1befc3b..66c8ec2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +### Fixed +- Fixed group reassignment to remove previously assigned groups only when using the Escalade reassignment action + ## [2.10.6] - 2026-07-31 ### Fixed diff --git a/front/ticket.form.php b/front/ticket.form.php index cfd9ad4c..43826575 100644 --- a/front/ticket.form.php +++ b/front/ticket.form.php @@ -44,7 +44,14 @@ throw new AccessDeniedHttpException(); } - PluginEscaladeTicket::timelineClimbAction($group_id, $tickets_id, $_POST); + // Mark this operation as a real escalation from the Escalade form. + $_SESSION['plugin_escalade']['is_escalation'] = true; + + try { + PluginEscaladeTicket::timelineClimbAction($group_id, $tickets_id, $_POST); + } finally { + unset($_SESSION['plugin_escalade']['is_escalation']); + } $track = new Ticket(); diff --git a/inc/history.class.php b/inc/history.class.php index f2e7784d..d76a941f 100644 --- a/inc/history.class.php +++ b/inc/history.class.php @@ -160,12 +160,38 @@ public static function getHistory($tickets_id, $full_history = false) $group = new Group(); $history = new self(); - $found = $history->find(['tickets_id' => $tickets_id], "date_mod DESC"); + $found = $history->find( + ['tickets_id' => $tickets_id], + ['date_mod DESC', 'id DESC'], + ); $nb_histories = count($found); //remove first line (current assign) $first_group = array_shift($found); + // Do not display a group as a previous assignment while it is still + // assigned to the ticket. Its history entry stays in the database and + // becomes visible after a real reassignment removes the group. + $group_ticket = new Group_Ticket(); + $currently_assigned = $group_ticket->find([ + 'tickets_id' => $tickets_id, + 'type' => CommonITILActor::ASSIGN, + ]); + + $currently_assigned_ids = array_map( + static fn(array $actor): int => (int) $actor['groups_id'], + $currently_assigned, + ); + + $found = array_filter( + $found, + static fn(array $history_entry): bool => !in_array( + (int) $history_entry['groups_id'], + $currently_assigned_ids, + true, + ), + ); + if ($full_history) { //show 1st group echo "
"; diff --git a/inc/ticket.class.php b/inc/ticket.class.php index b39f5bc6..93729fc4 100644 --- a/inc/ticket.class.php +++ b/inc/ticket.class.php @@ -502,7 +502,16 @@ public static function processAfterAddGroup(Group_Ticket $item) // getFromDB() is required first so isNewItem() returns false and deleted-actor // detection runs. _plugin_escalade_rules_only skips escalade logic in pre_item_update. // Safety net in case updateActors() above did not already remove old groups. - if ($_SESSION['glpi_plugins']['escalade']['config']['remove_group'] == true) { + if ( + $_SESSION['glpi_plugins']['escalade']['config']['remove_group'] == true + && ( + !empty($_SESSION['plugin_escalade']['is_escalation']) + || !empty($_SESSION['plugin_escalade']['climb_group']) + || !empty($_SESSION['plugin_escalade']['auto_group_assignment']) + || !empty($_SESSION['plugin_escalade']['category_group_reassignment']) + ) + ) { + $all_actors = self::getTicketFieldsWithActors($tickets_id, $groups_id); // Keep only the new group in the assign list (drop old ones). @@ -667,12 +676,22 @@ public static function climb_group($tickets_id, $groups_id, $no_redirect = false // and wipes them from the ticket regardless of the // "Remove requester(s) on escalation" plugin config. $ticket = new Ticket(); - $ticket->update([ - 'id' => $tickets_id, - '_actors' => self::getTicketFieldsWithActors($tickets_id, $groups_id), - 'actortype' => CommonITILActor::ASSIGN, - 'groups_id' => $groups_id, - ]); + + // Mark this assignment as an actual Escalade reassignment. + // processAfterAddGroup() also runs for normal GLPI group assignments, + // so old groups must only be removed for this specific action. + $_SESSION['plugin_escalade']['climb_group'] = true; + + try { + $ticket->update([ + 'id' => $tickets_id, + '_actors' => self::getTicketFieldsWithActors($tickets_id, $groups_id), + 'actortype' => CommonITILActor::ASSIGN, + 'groups_id' => $groups_id, + ]); + } finally { + unset($_SESSION['plugin_escalade']['climb_group']); + } } if (!$no_redirect) { @@ -851,12 +870,19 @@ public static function item_add_user(Ticket_User $item, $type = CommonITILActor: //prevent user removal $_SESSION['plugin_escalade']['keep_users'][$item->fields['users_id']] = $item->fields['users_id']; - //add new group to ticket - $group_ticket->add([ - 'tickets_id' => $tickets_id, - 'groups_id' => $groups_id, - 'type' => CommonITILActor::ASSIGN, - ]); + + // Mark this group assignment as an automatic Escalade assignment. + $_SESSION['plugin_escalade']['auto_group_assignment'] = true; + + try { + $group_ticket->add([ + 'tickets_id' => $tickets_id, + 'groups_id' => $groups_id, + 'type' => CommonITILActor::ASSIGN, + ]); + } finally { + unset($_SESSION['plugin_escalade']['auto_group_assignment']); + } } elseif ($_SESSION['glpi_plugins']['escalade']['config']['remove_tech']) { self::removeAssignGroups($tickets_id); } @@ -952,7 +978,14 @@ public static function qualification(CommonDBTM $item) $group_found = $group_ticket->find($group_condition); if (empty($group_found)) { //add group to ticket - $group_ticket->add($group_condition); + $_SESSION['plugin_escalade']['category_group_reassignment'] = true; + + try { + $group_ticket->add($group_condition); + } finally { + unset($_SESSION['plugin_escalade']['category_group_reassignment']); + } + //remove old group if needed if ($_SESSION['glpi_plugins']['escalade']['config']['remove_group'] && isset($item->input['_groups_id_assign'])) { foreach ($item->input['_groups_id_assign'] as $idActor => $actor) { diff --git a/tests/EscaladeTestCase.php b/tests/EscaladeTestCase.php index d831ddac..6421a125 100644 --- a/tests/EscaladeTestCase.php +++ b/tests/EscaladeTestCase.php @@ -176,7 +176,19 @@ public function escalateWithTimelineButton(Ticket $ticket, Group $group, array $ ], ); $_POST['comment'] = $options['comment'] ?? 'Default comment'; - PluginEscaladeTicket::timelineClimbAction($group->getID(), $ticket->getID(), $options); + + $_SESSION['plugin_escalade']['is_escalation'] = true; + + try { + PluginEscaladeTicket::timelineClimbAction( + $group->getID(), + $ticket->getID(), + $options, + ); + } finally { + unset($_SESSION['plugin_escalade']['is_escalation']); + } + $ticketgroup = new Group_Ticket(); $is_escalate = $ticketgroup->getFromDBByCrit([ 'tickets_id' => $ticket->getID(), diff --git a/tests/Units/GroupEscalationTest.php b/tests/Units/GroupEscalationTest.php index 8db2e4fc..56eff7a5 100644 --- a/tests/Units/GroupEscalationTest.php +++ b/tests/Units/GroupEscalationTest.php @@ -37,6 +37,7 @@ use Notification; use NotificationTarget; use PluginEscaladeHistory; +use PluginEscaladeTicket; use PluginEscaladeNotification; use QueuedNotification; use Ticket; @@ -45,6 +46,153 @@ final class GroupEscalationTest extends EscaladeTestCase { + /** + * Standard GLPI group assignment must not remove previously assigned groups. + */ + public function testStandardGroupAssignmentKeepsExistingGroups(): void + { + $this->initConfig([ + 'remove_group' => 1, + 'show_history' => 1, + ]); + + $group1 = $this->createGroup('standard_group_1_' . uniqid()); + $group2 = $this->createGroup('standard_group_2_' . uniqid()); + + $ticket = $this->createItem(Ticket::class, [ + 'name' => 'Standard group assignment regression test', + 'content' => '', + '_actors' => [ + 'assign' => [ + [ + 'items_id' => $group1->getID(), + 'itemtype' => 'Group', + ], + ], + ], + ]); + + // Simulate adding another group from the standard GLPI actors field. + $this->createItem(Group_Ticket::class, [ + 'tickets_id' => $ticket->getID(), + 'groups_id' => $group2->getID(), + 'type' => CommonITILActor::ASSIGN, + ]); + + $group_ticket = new Group_Ticket(); + $assigned_groups = $group_ticket->find([ + 'tickets_id' => $ticket->getID(), + 'type' => CommonITILActor::ASSIGN, + ]); + + $this->assertCount(2, $assigned_groups); + + $this->assertCount(1, $group_ticket->find([ + 'tickets_id' => $ticket->getID(), + 'groups_id' => $group1->getID(), + 'type' => CommonITILActor::ASSIGN, + ])); + + $this->assertCount(1, $group_ticket->find([ + 'tickets_id' => $ticket->getID(), + 'groups_id' => $group2->getID(), + 'type' => CommonITILActor::ASSIGN, + ])); + } + + /** + * A real Escalade reassignment must remove old groups while preserving + * every previously assigned group in the visual assignment history. + */ + public function testEscaladeReassignmentPreservesAllGroupsInHistory(): void + { + $this->initConfig([ + 'remove_group' => 1, + 'show_history' => 1, + ]); + + $group1 = $this->createGroup('history_group_1_' . uniqid()); + $group2 = $this->createGroup('history_group_2_' . uniqid()); + $group3 = $this->createGroup('history_group_3_' . uniqid()); + $group4 = $this->createGroup('history_destination_' . uniqid()); + + $ticket = $this->createItem(Ticket::class, [ + 'name' => 'Multiple group history regression test', + 'content' => '', + '_actors' => [ + 'assign' => [ + [ + 'items_id' => $group1->getID(), + 'itemtype' => 'Group', + ], + ], + ], + ]); + + // Add groups through the normal GLPI assignment mechanism. + foreach ([$group2, $group3] as $group) { + $this->createItem(Group_Ticket::class, [ + 'tickets_id' => $ticket->getID(), + 'groups_id' => $group->getID(), + 'type' => CommonITILActor::ASSIGN, + ]); + } + + $group_ticket = new Group_Ticket(); + + $this->assertCount(3, $group_ticket->find([ + 'tickets_id' => $ticket->getID(), + 'type' => CommonITILActor::ASSIGN, + ])); + + // Simulate the real Escalade button. + $_SESSION['plugin_escalade']['is_escalation'] = true; + $_POST['comment'] = 'Regression test escalation'; + + try { + PluginEscaladeTicket::timelineClimbAction( + $group4->getID(), + $ticket->getID(), + [ + 'ticket_details' => [ + 'id' => $ticket->getID(), + ], + ], + ); + } finally { + unset($_SESSION['plugin_escalade']['is_escalation']); + unset($_POST['comment']); + } + + // Only the destination group must remain assigned. + $assigned_groups = $group_ticket->find([ + 'tickets_id' => $ticket->getID(), + 'type' => CommonITILActor::ASSIGN, + ]); + + $this->assertCount(1, $assigned_groups); + $assigned_group = reset($assigned_groups); + $this->assertEquals($group4->getID(), $assigned_group['groups_id']); + + // All groups involved in the reassignment must remain visible + // in Escalade history. + $history = new PluginEscaladeHistory(); + + foreach ([$group1, $group2, $group3, $group4] as $group) { + $this->assertGreaterThanOrEqual( + 1, + count($history->find([ + 'tickets_id' => $ticket->getID(), + 'groups_id' => $group->getID(), + ])), + sprintf( + 'Group %d is missing from Escalade history', + $group->getID(), + ), + ); + } + } + public function testTechGroupAttributionUpdateTicket() { $this->initConfig([ diff --git a/tests/Units/TicketTest.php b/tests/Units/TicketTest.php index d3a305a6..8ca189a9 100644 --- a/tests/Units/TicketTest.php +++ b/tests/Units/TicketTest.php @@ -355,14 +355,14 @@ public function testTicketUpdateDoesNotChangeITILCategoryAssignedGroup() ], ]); - $this->assertEquals(0, count($group_ticket->find(['tickets_id' => $ticket_id, 'groups_id' => $group1_id, 'type' => CommonITILActor::ASSIGN]))); + $this->assertEquals(1, count($group_ticket->find(['tickets_id' => $ticket_id, 'groups_id' => $group1_id, 'type' => CommonITILActor::ASSIGN]))); $this->assertEquals(1, count($group_ticket->find(['tickets_id' => $ticket_id, 'groups_id' => $group2_id, 'type' => CommonITILActor::ASSIGN]))); $this->updateItem('Ticket', $ticket_id, [ 'status' => CommonITILObject::WAITING, ]); - $this->assertEquals(0, count($group_ticket->find(['tickets_id' => $ticket_id, 'groups_id' => $group1_id, 'type' => CommonITILActor::ASSIGN]))); + $this->assertEquals(1, count($group_ticket->find(['tickets_id' => $ticket_id, 'groups_id' => $group1_id, 'type' => CommonITILActor::ASSIGN]))); $this->assertEquals(1, count($group_ticket->find(['tickets_id' => $ticket_id, 'groups_id' => $group2_id, 'type' => CommonITILActor::ASSIGN]))); }