Fixed a solution added even though the required fields were not filled in

hook.php

@@ -31,43 +31,8 @@
  * -------------------------------------------------------------------------
  */
 
-use Glpi\Inventory\Conf;
 use GlpiPlugin\Moreoptions\Config;
 
-/**
- * -------------------------------------------------------------------------
- * MoreOptions plugin for GLPI
- * -------------------------------------------------------------------------
- *
- * MIT License
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in all
- * copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
- * SOFTWARE.
- * -------------------------------------------------------------------------
- * @copyright Copyright (C) 2025 by the MoreOptions plugin team.
- * @copyright Copyright (C) 2022-2024 by More Options plugin team.
- * @license   MIT https://opensource.org/licenses/mit-license.php
- * @license   GPLv3 https://www.gnu.org/licenses/gpl-3.0.html
- * @link      https://github.com/pluginsGLPI/moreoptions
- * @link      https://gitlab.teclib.com/glpi-network/cancelsend/
- * -------------------------------------------------------------------------
- */
-
 function plugin_moreoptions_install(): bool
 {
     $migration = new Migration(PLUGIN_MOREOPTIONS_VERSION);

setup.php

@@ -96,6 +96,13 @@ function plugin_init_moreoptions(): void
         Controller::class, 'beforeCloseITILObject',
     ];
 
+    $PLUGIN_HOOKS[Hooks::PRE_ITEM_ADD]['moreoptions'][ITILSolution::class] = [
+        Controller::class, 'beforeCloseITILObject',
+    ];
+    $PLUGIN_HOOKS[Hooks::PRE_ITEM_UPDATE]['moreoptions'][ITILSolution::class] = [
+        Controller::class, 'beforeCloseITILObject',
+    ];
+
     $PLUGIN_HOOKS[Hooks::PRE_ITEM_UPDATE]['moreoptions'][Config::class] = [
         Config::class, 'preItemUpdate',
     ];

src/Controller.php

@@ -48,6 +48,7 @@
 use CommonITILObject;
 use CommonITILValidation;
 use Glpi\Form\Category;
+use GlpiPlugin\Behaviors\Common;
 use GlpiPlugin\Moreoptions\Config;
 use Group_Item;
 use Group_Problem;
@@ -271,27 +272,60 @@ private static function addGroupsForActorType(CommonDBTM $item, Config $moconfig
         }
     }
 
-    public static function beforeCloseITILObject(CommonITILObject $item): void
+    public static function beforeCloseITILObject(CommonDBTM $item): void
     {
         if (!is_array($item->input)) {
             return;
         }
 
+        $closed = true;
+
+        if ($item instanceof ITILSolution) {
+            $itemtype = $item->input['itemtype'] ?? null;
+
+            switch ($itemtype) {
+                case 'Ticket':
+                    $parent_item = new Ticket();
+                    break;
+                case 'Change':
+                    $parent_item = new Change();
+                    break;
+                case 'Problem':
+                    $parent_item = new Problem();
+                    break;
+                default:
+                    return;
+            }
+
+            if (!$parent_item->getFromDB($item->input['items_id'])) {
+                return;
+            }
+            $closed = self::requireFieldsToClose($parent_item);
+            $closed = self::preventClosure($parent_item) && $closed;
+        }
+
         if (
-            (isset($item->input['status']) && ($item->input['status'] == CommonITILObject::CLOSED || $item->input['status'] == CommonITILObject::SOLVED))
-            || $item->fields['status'] == CommonITILObject::CLOSED
-            || $item->fields['status'] == CommonITILObject::SOLVED
+            $item instanceof CommonITILObject
+            && (
+                (isset($item->input['status']) && ($item->input['status'] == CommonITILObject::CLOSED || $item->input['status'] == CommonITILObject::SOLVED))
+                || $item->fields['status'] == CommonITILObject::CLOSED
+                || $item->fields['status'] == CommonITILObject::SOLVED
+            )
         ) {
-            self::requireFieldsToClose($item);
-            self::preventClosure($item);
+            $closed = self::requireFieldsToClose($item);
+            $closed = self::preventClosure($item) && $closed;
+        }
+
+        if (!$closed) {
+            $item->input = false;
         }
     }
 
-    public static function preventClosure(CommonDBTM $item): void
+    public static function preventClosure(CommonDBTM $item): bool
     {
         $conf = Config::getConfig();
         if ($conf->fields['is_active'] != 1) {
-            return;
+            return true;
         }
 
         $tasks = [];
@@ -319,21 +353,24 @@ public static function preventClosure(CommonDBTM $item): void
             if (is_array($t) && isset($t['state']) && $t['state'] == Planning::TODO) {
                 Session::addMessageAfterRedirect(__s('The ticket you wish to close has tasks that need to be completed.', 'moreoptions'), false, ERROR);
                 $item->input = false;
-                return;
+                return false;
             }
         }
+        return true;
     }
 
-    public static function requireFieldsToClose(CommonDBTM $item): void
+    public static function requireFieldsToClose(CommonDBTM $item, bool $is_solution = false): bool
     {
         $conf = Config::getConfig();
         if ($conf->fields['is_active'] != 1) {
-            return;
+            return true;
         }
 
         $message = '';
         $itemtype = get_class($item);
 
+        $data = empty($item->input) ? $item->fields : $item->input;
+
         // Determine the configuration suffix and actor classes based on item type
         $configSuffix = '_' . strtolower($itemtype);
         $userClass = $item->userlinkclass ?? '';
@@ -346,10 +383,10 @@ public static function requireFieldsToClose(CommonDBTM $item): void
                 $tech = new $userClass();
             } else {
                 // If the user class is not valid, skip this check
-                return;
+                return false;
             }
             $techs = $tech->find([
-                $itemIdField => $item->fields['id'],
+                $itemIdField => $data['id'],
                 'type'       => CommonITILActor::ASSIGN,
             ]);
             if (count($techs) == 0) {
@@ -363,10 +400,10 @@ public static function requireFieldsToClose(CommonDBTM $item): void
                 $group = new $groupClass();
             } else {
                 // If the group class is not valid, skip this check
-                return;
+                return false;
             }
             $groups = $group->find([
-                $itemIdField => $item->fields['id'],
+                $itemIdField => $data['id'],
                 'type'       => CommonITILActor::ASSIGN,
             ]);
             if (count($groups) == 0) {
@@ -376,27 +413,30 @@ public static function requireFieldsToClose(CommonDBTM $item): void
 
         // Check for required category
         if ($conf->fields['require_category_to_close' . $configSuffix] == 1) {
-            if ((!isset($item->input['itilcategories_id']) || empty($item->input['itilcategories_id']))) {
+            if ((!isset($data['itilcategories_id']) || empty($data['itilcategories_id']))) {
                 $message .= '- ' . __s('Category') . '<br>';
             }
         }
 
         // Check for required location
         if ($conf->fields['require_location_to_close' . $configSuffix] == 1) {
-            if ((!isset($item->input['locations_id']) || empty($item->input['locations_id']))) {
+            if ((!isset($data['locations_id']) || empty($data['locations_id']))) {
                 $message .= '- ' . __s('Location') . '<br>';
             }
         }
 
         // Check if solution exists before closing
-        if ($conf->fields['require_solution_to_close' . $configSuffix] == 1
-            && is_array($item->input)
-            && isset($item->input['status'])
-            && $item->input['status'] == CommonITILObject::CLOSED) {
+        if (
+            !$is_solution
+            && $conf->fields['require_solution_to_close' . $configSuffix] == 1
+            && is_array($data)
+            && isset($data['status'])
+            && $data['status'] == CommonITILObject::CLOSED
+        ) {
             $solution = new ITILSolution();
             $solutions = $solution->find([
                 'itemtype' => $itemtype,
-                'items_id' => $item->fields['id'],
+                'items_id' => $data['id'],
                 'NOT' => [
                     'status' => CommonITILValidation::REFUSED,
                 ],
@@ -411,9 +451,9 @@ public static function requireFieldsToClose(CommonDBTM $item): void
 
             $message = sprintf(__s('To close this %s, you must fill in the following fields:', 'moreoptions'), $itemTypeLabel) . '<br>' . $message;
             Session::addMessageAfterRedirect($message, false, ERROR);
-            $item->input = false;
-            return;
+            return false;
         }
+        return true;
     }
 
     public static function checkTaskRequirements(CommonDBTM $item): CommonDBTM

tests/Units/ConfigTest.php

@@ -306,6 +306,94 @@ public function testTicketMandatoryFieldsBeforeCloseTicket(): void
         $this->assertTrue($resetResult);
     }
 
+    /**
+     * Test mandatory fields before adding a solution
+     */
+    public function testCannotAddSolutionWhenMissingMandatoryFields(): void
+    {
+        $this->login();
+
+        $conf = $this->getCurrentConfig();
+
+        // Configure mandatory fields before closing (which impacts solutions too)
+        $result = $this->updateTestConfig($conf, [
+            'is_active'                          => 1,
+            'entities_id'                        => 0,
+            'require_technician_to_close_ticket' => 1,
+            'require_category_to_close_ticket'   => 1,
+        ]);
+        $this->assertTrue($result);
+
+        // Create a ticket without mandatory fields
+        $ticket = $this->createItem(
+            \Ticket::class,
+            [
+                'name'    => 'Test ticket solution',
+                'content' => 'Test content',
+            ],
+        );
+        $tid = $ticket->getID();
+
+        // Attempt to add a solution (Expected to fail because missing tech and category)
+        $solution = new \ITILSolution();
+        $resultFields = $solution->add([
+            'itemtype' => \Ticket::class,
+            'items_id' => $tid,
+            'content'  => 'My test solution',
+            'status'   => \CommonITILObject::SOLVED,
+        ]);
+
+        $this->clearSessionMessages();
+        $this->assertFalse($resultFields);
+
+        // Add technician to the ticket
+        $user = new \User();
+        $this->assertTrue($user->getFromDBByCrit(['name' => 'glpi']));
+
+        $this->createItem(
+            \Ticket_User::class,
+            [
+                'tickets_id' => $tid,
+                'users_id'   => $user->getID(),
+                'type'       => \Ticket_User::ASSIGN,
+            ],
+        );
+
+        // Create category and update ticket
+        $category = $this->createItem(
+            \ITILCategory::class,
+            [
+                'name' => 'Test category for solution test',
+            ],
+        );
+        $this->updateItem(
+            \Ticket::class,
+            $tid,
+            [
+                'itilcategories_id' => $category->getID(),
+            ],
+        );
+
+        // Attempt to add solution with all mandatory fields present (Expected to succeed)
+        $solution2 = new \ITILSolution();
+        $resultOk = $solution2->add([
+            'itemtype'   => \Ticket::class,
+            'items_id'   => $tid,
+            'solutiontypes_id' => 0,
+            'content'    => 'My test solution with fields ok',
+            'status'     => \CommonITILObject::SOLVED,
+        ]);
+        $this->assertIsInt($resultOk);
+        $this->clearSessionMessages();
+
+        // Reset config
+        $resetResult = $this->updateTestConfig($conf, [
+            'require_technician_to_close_ticket' => 0,
+            'require_category_to_close_ticket'   => 0,
+        ]);
+        $this->assertTrue($resetResult);
+    }
+
     /**
      * Test mandatory fields before closing a change
      */