From 881e1c242412870bf0e23a2648da15ebd60a2b52 Mon Sep 17 00:00:00 2001 From: Benoit VIGNAL Date: Tue, 21 Oct 2025 11:46:54 +0200 Subject: [PATCH 1/6] fix(itil): Ensure readonly logic is enforce in backend --- src/CommonITILObject.php | 55 +++++++++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 12 deletions(-) diff --git a/src/CommonITILObject.php b/src/CommonITILObject.php index 3c93deb5d0d..08c343b1e95 100644 --- a/src/CommonITILObject.php +++ b/src/CommonITILObject.php @@ -1769,16 +1769,7 @@ protected function handleTemplateFields(array $input, bool $show_error_message = { //// check mandatory fields // First get ticket template associated: entity and type/category - $entid = $input['entities_id'] ?? $this->fields['entities_id']; - - $type = null; - if (isset($input['type'])) { - $type = $input['type']; - } elseif (isset($this->fields['type'])) { - $type = $this->fields['type']; - } - - $categid = $input['itilcategories_id'] ?? $this->fields['itilcategories_id']; + $tt = $this->getITILTemplateFromInput($input); $check_allowed_fields_for_template = false; $allowed_fields = []; @@ -1865,8 +1856,6 @@ class_exists($validation_class) } } - $tt = $this->getITILTemplateToUse(0, $type, $categid, $entid); - if (count($tt->mandatory)) { $mandatory_missing = []; $fieldsname = $tt->getAllowedFieldsNames(true); @@ -1976,6 +1965,7 @@ public function prepareInputForUpdate($input) if (!$this->checkFieldsConsistency($input)) { return false; } + $input = $this->handleReadonlyFields($input); // Add document if needed $this->getFromDB($input["id"]); // entities_id field required @@ -2340,6 +2330,23 @@ public function prepareInputForUpdate($input) return $input; } + private function handleReadonlyFields(array $input, bool $isAdd = false): array + { + $tt = $this->getITILTemplateFromInput($input); + foreach (array_keys($tt->readonly) as $read_only_field) { + if ($isAdd && array_key_exists($read_only_field, $tt->predefined)) { + $input[$read_only_field] = $tt->predefined[$read_only_field]; + continue; + } + + if (array_key_exists($read_only_field, $this->fields)) { + $input[$read_only_field] = $this->fields[$read_only_field]; + } else { + unset($input[$read_only_field]); + } + } + return $input; + } public function post_updateItem($history = true) { @@ -2813,6 +2820,7 @@ public function prepareInputForAdd($input) if (!$this->checkFieldsConsistency($input)) { return false; } + $input = $this->handleReadonlyFields($input, true); $input = $this->transformActorsInput($input); @@ -8252,6 +8260,29 @@ public function getITILTemplateToUse( return $tt; } + /** + * Get the template to use + * If the input is not defined, it will get it from the object fields datas + * + * @param array $input + * @return ITILTemplate + * + * @since 11.0.2 + */ + public function getITILTemplateFromInput(array $input = []): ITILTemplate { + $entid = $input['entities_id'] ?? $this->fields['entities_id']; + + $type = null; + if (isset($input['type'])) { + $type = $input['type']; + } elseif (isset($this->fields['type'])) { + $type = $this->fields['type']; + } + + $categid = $input['itilcategories_id'] ?? $this->fields['itilcategories_id']; + return $this->getITILTemplateToUse(0, $type, $categid, $entid); + } + /** * Get template field name * From 435b48bff4c96479abbf99805da02d1e4a9743d7 Mon Sep 17 00:00:00 2001 From: Benoit VIGNAL Date: Tue, 21 Oct 2025 13:47:49 +0200 Subject: [PATCH 2/6] fix(itil): Ensure readonly logic is enforce in backend --- .../AbstractITILTemplateReadonlyFieldTest.php | 184 ++++++++++++++++++ 1 file changed, 184 insertions(+) create mode 100644 tests/src/AbstractITILTemplateReadonlyFieldTest.php diff --git a/tests/src/AbstractITILTemplateReadonlyFieldTest.php b/tests/src/AbstractITILTemplateReadonlyFieldTest.php new file mode 100644 index 00000000000..3c7fd5d102b --- /dev/null +++ b/tests/src/AbstractITILTemplateReadonlyFieldTest.php @@ -0,0 +1,184 @@ +getITILClass(); + $templateClass = $itil_class::getTemplateClass(); + dump($templateClass); + $template = new $templateClass(); + + // ITILTemplate::add expects `_readonly` to be an array with field names as keys and 1 as value. + $readonly_input = array_fill_keys($readonly, 1); + + $template_input = [ + 'name' => 'test_template_' . mt_rand(), + 'is_active' => 1, + 'entities_id' => 0, + 'is_recursive' => 1, + '_readonly' => $readonly_input, + '_predefined' => $predefined, + '_mandatory' => [], + '_hidden' => [], + ]; + + $template_id = $template->add($template_input); + $this->assertGreaterThan(0, $template_id, 'Template creation failed'); + + $category = new ITILCategory(); + $cat_id = $category->add(['name' => 'test_cat_' . mt_rand(), 'entities_id' => 0, 'is_recursive' => 1]); + $this->assertGreaterThan(0, $cat_id, 'Category creation failed'); + + $type = null; + if ($itil_class instanceof Ticket) { + $type = Ticket::INCIDENT_TYPE; + } else { + $type = true; // For Change and Problem + } + $template_field_name = $itil_class->getTemplateFieldName($type); + + $category->update([ + 'id' => $cat_id, + $template_field_name => $template_id, + ]); + + return $cat_id; + } + + public function testHandleReadonlyFieldsOnAddWithPredefined(): void + { + $cat_id = $this->createTemplateAndCategory( + ['name'], // readonly + ['name' => 'Predefined Name Value'] // predefined + ); + + $itilObject = $this->getITILClass(); + $input = [ + 'name' => 'User Input Name', + 'content' => 'Some content', + 'status' => CommonITILObject::INCOMING, + 'itilcategories_id' => $cat_id, + 'entities_id' => 0, + ]; + if ($itilObject instanceof Ticket) { + $input['type'] = Ticket::INCIDENT_TYPE; + } + + $processedInput = $itilObject->prepareInputForAdd($input); + + $this->assertEquals('Predefined Name Value', $processedInput['name']); + $this->assertEquals('Some content', $processedInput['content']); + } + + public function testHandleReadonlyFieldsOnAddWithoutPredefined(): void + { + $cat_id = $this->createTemplateAndCategory( + ['name'] // readonly + ); + + $itilObject = $this->getITILClass(); + $input = [ + 'name' => 'User Input Name', + 'content' => 'Some content', + 'status' => CommonITILObject::INCOMING, + 'itilcategories_id' => $cat_id, + 'entities_id' => 0, + ]; + if ($itilObject instanceof Ticket) { + $input['type'] = Ticket::INCIDENT_TYPE; + } + + $processedInput = $itilObject->prepareInputForAdd($input); + + $this->assertEmpty($processedInput['name']); + $this->assertEquals('Some content', $processedInput['content']); + } + + public function testHandleReadonlyFieldsOnUpdateWithExistingValue(): void + { + $cat_id = $this->createTemplateAndCategory( + ['name'] // readonly + ); + + $itilObject = $this->getITILClass(); + $item_id = $itilObject->add([ + 'name' => 'Existing Name Value', + 'content' => 'Initial content', + 'status' => CommonITILObject::ASSIGNED, + 'itilcategories_id' => $cat_id, + 'entities_id' => 0, + ]); + + $itilObject->getFromDB($item_id); + + $input = [ + 'id' => $item_id, + 'name' => 'User Attempted New Name', + 'content' => 'Updated content', + 'status' => CommonITILObject::ASSIGNED, + 'itilcategories_id' => $cat_id, + 'entities_id' => 0, + ]; + if ($itilObject instanceof Ticket) { + $input['type'] = Ticket::INCIDENT_TYPE; + } + + $processedInput = $itilObject->prepareInputForUpdate($input); + + $this->assertEquals('Existing Name Value', $processedInput['name']); + $this->assertEquals('Updated content', $processedInput['content']); + } + + public function testHandleReadonlyFieldsOnUpdateWithoutExistingValue(): void + { + $cat_id = $this->createTemplateAndCategory( + ['name'] // readonly + ); + + $itilObject = $this->getITILClass(); + $item_id = $itilObject->add([ + 'name' => '', // No initial name + 'content' => 'Initial content', + 'status' => CommonITILObject::ASSIGNED, + 'itilcategories_id' => $cat_id, + 'entities_id' => 0, + ]); + + $itilObject->getFromDB($item_id); + + $input = [ + 'id' => $item_id, + 'name' => 'User Attempted New Name', + 'content' => 'Updated content', + 'status' => CommonITILObject::ASSIGNED, + 'itilcategories_id' => $cat_id, + 'entities_id' => 0, + ]; + if ($itilObject instanceof Ticket) { + $input['type'] = Ticket::INCIDENT_TYPE; + } + + $processedInput = $itilObject->prepareInputForUpdate($input); + + $this->assertArrayNotHasKey('name', $processedInput); + $this->assertEquals('Updated content', $processedInput['content']); + } +} From ace94c01207bbe5f92eccdfceb094f5c94a78c57 Mon Sep 17 00:00:00 2001 From: Benoit VIGNAL Date: Tue, 21 Oct 2025 14:56:32 +0200 Subject: [PATCH 3/6] chore: Update urgency field to use the enum insted of hardcoded value --- src/CommonITILObject.php | 12 ++++++++---- src/Ticket.php | 3 ++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/CommonITILObject.php b/src/CommonITILObject.php index 08c343b1e95..dd1f4385fd0 100644 --- a/src/CommonITILObject.php +++ b/src/CommonITILObject.php @@ -32,6 +32,7 @@ * * --------------------------------------------------------------------- */ + use Glpi\Application\View\TemplateRenderer; use Glpi\ContentTemplates\Parameters\CommonITILObjectParameters; use Glpi\DBAL\QueryExpression; @@ -52,6 +53,7 @@ use Glpi\RichText\UserMention; use Glpi\Search\Output\HTMLSearchOutput; use Glpi\Team\Team; +use Glpi\Urgency; use Safe\Exceptions\DatetimeException; use function Safe\getimagesize; @@ -2333,6 +2335,8 @@ public function prepareInputForUpdate($input) private function handleReadonlyFields(array $input, bool $isAdd = false): array { $tt = $this->getITILTemplateFromInput($input); + $tt->getFromDBWithData($tt->getID()); // We load the fields (predefined and readonly) + foreach (array_keys($tt->readonly) as $read_only_field) { if ($isAdd && array_key_exists($read_only_field, $tt->predefined)) { $input[$read_only_field] = $tt->predefined[$read_only_field]; @@ -2832,7 +2836,7 @@ public function prepareInputForAdd($input) } // save value before clean; - $title = ltrim($input['name']); + $title = ltrim($input['name'] ?? ''); // Set default status to avoid notice if (!isset($input["status"])) { @@ -2843,7 +2847,7 @@ public function prepareInputForAdd($input) !isset($input["urgency"]) || !($CFG_GLPI['urgency_mask'] & (1 << $input["urgency"])) ) { - $input["urgency"] = 3; + $input["urgency"] = Urgency::MEDIUM->value; } if ( !isset($input["impact"]) @@ -2894,8 +2898,8 @@ public function prepareInputForAdd($input) } // No name set name - $input["name"] = ltrim($input["name"]); - $input['content'] = ltrim($input['content']); + $input["name"] = ltrim($input["name"] ?? ''); + $input['content'] = ltrim($input['content'] ?? ''); if (empty($input["name"])) { // Build name based on content diff --git a/src/Ticket.php b/src/Ticket.php index a1b1764302c..8babb00e50f 100644 --- a/src/Ticket.php +++ b/src/Ticket.php @@ -45,6 +45,7 @@ use Glpi\RichText\RichText; use Glpi\RichText\UserMention; use Glpi\Search\DefaultSearchRequestInterface; +use Glpi\Urgency; use Safe\DateTime; use function Safe\preg_match; @@ -3489,7 +3490,7 @@ public static function getDefaultValues($entity = 0) 'name' => '', 'content' => '', 'itilcategories_id' => 0, - 'urgency' => 3, + 'urgency' => Urgency::MEDIUM->value, 'impact' => 3, 'priority' => self::computePriority(3, 3), 'requesttypes_id' => $requesttype, From 17f19a9fd84eae5e2ce789f035b2892d723ab719 Mon Sep 17 00:00:00 2001 From: Benoit VIGNAL Date: Tue, 21 Oct 2025 16:00:52 +0200 Subject: [PATCH 4/6] test(itil): Ensure readonly logic is enforce in backend --- src/CommonITILObject.php | 3 +- .../ChangeITILTemplateReadonlyFieldTest.php | 46 ++++ .../ProblemITILTemplateReadonlyFieldTest.php | 46 ++++ .../TicketITILTemplateReadonlyFieldTest.php | 46 ++++ .../AbstractITILTemplateReadonlyFieldTest.php | 227 ++++++++++++------ 5 files changed, 296 insertions(+), 72 deletions(-) create mode 100644 tests/functional/ChangeITILTemplateReadonlyFieldTest.php create mode 100644 tests/functional/ProblemITILTemplateReadonlyFieldTest.php create mode 100644 tests/functional/TicketITILTemplateReadonlyFieldTest.php diff --git a/src/CommonITILObject.php b/src/CommonITILObject.php index dd1f4385fd0..addb55dc09d 100644 --- a/src/CommonITILObject.php +++ b/src/CommonITILObject.php @@ -8273,7 +8273,8 @@ public function getITILTemplateToUse( * * @since 11.0.2 */ - public function getITILTemplateFromInput(array $input = []): ITILTemplate { + public function getITILTemplateFromInput(array $input = []): ITILTemplate + { $entid = $input['entities_id'] ?? $this->fields['entities_id']; $type = null; diff --git a/tests/functional/ChangeITILTemplateReadonlyFieldTest.php b/tests/functional/ChangeITILTemplateReadonlyFieldTest.php new file mode 100644 index 00000000000..52aaf42f225 --- /dev/null +++ b/tests/functional/ChangeITILTemplateReadonlyFieldTest.php @@ -0,0 +1,46 @@ +. + * + * --------------------------------------------------------------------- + */ + +namespace functional; + +use Change; +use Glpi\Tests\AbstractITILTemplateReadonlyFieldTest; + +class ChangeITILTemplateReadonlyFieldTest extends AbstractITILTemplateReadonlyFieldTest +{ + public function getITILClass(): Change + { + return new Change(); + } +} diff --git a/tests/functional/ProblemITILTemplateReadonlyFieldTest.php b/tests/functional/ProblemITILTemplateReadonlyFieldTest.php new file mode 100644 index 00000000000..84f9979f45b --- /dev/null +++ b/tests/functional/ProblemITILTemplateReadonlyFieldTest.php @@ -0,0 +1,46 @@ +. + * + * --------------------------------------------------------------------- + */ + +namespace functional; + +use Glpi\Tests\AbstractITILTemplateReadonlyFieldTest; +use Problem; + +class ProblemITILTemplateReadonlyFieldTest extends AbstractITILTemplateReadonlyFieldTest +{ + public function getITILClass(): Problem + { + return new Problem(); + } +} diff --git a/tests/functional/TicketITILTemplateReadonlyFieldTest.php b/tests/functional/TicketITILTemplateReadonlyFieldTest.php new file mode 100644 index 00000000000..d574258b326 --- /dev/null +++ b/tests/functional/TicketITILTemplateReadonlyFieldTest.php @@ -0,0 +1,46 @@ +. + * + * --------------------------------------------------------------------- + */ + +namespace functional; + +use Glpi\Tests\AbstractITILTemplateReadonlyFieldTest; +use Ticket; + +class TicketITILTemplateReadonlyFieldTest extends AbstractITILTemplateReadonlyFieldTest +{ + public function getITILClass(): Ticket + { + return new Ticket(); + } +} diff --git a/tests/src/AbstractITILTemplateReadonlyFieldTest.php b/tests/src/AbstractITILTemplateReadonlyFieldTest.php index 3c7fd5d102b..701a3ac9c4a 100644 --- a/tests/src/AbstractITILTemplateReadonlyFieldTest.php +++ b/tests/src/AbstractITILTemplateReadonlyFieldTest.php @@ -1,10 +1,46 @@ . + * + * --------------------------------------------------------------------- + */ + namespace Glpi\Tests; use CommonITILObject; use DbTestCase; +use Glpi\Urgency; use ITILCategory; +use ITILTemplate; +use ITILTemplatePredefinedField; +use ITILTemplateReadonlyField; use Ticket; // For type constants abstract class AbstractITILTemplateReadonlyFieldTest extends DbTestCase @@ -21,39 +57,71 @@ abstract public function getITILClass(): CommonITILObject; */ protected function createTemplateAndCategory(array $readonly = [], array $predefined = []): int { - $itil_class = $this->getITILClass(); - $templateClass = $itil_class::getTemplateClass(); - dump($templateClass); - $template = new $templateClass(); + $itil_object = $this->getITILClass(); + $itil_type = $itil_object->getType(); - // ITILTemplate::add expects `_readonly` to be an array with field names as keys and 1 as value. - $readonly_input = array_fill_keys($readonly, 1); + // Create Template + $template_class = $itil_type . 'Template'; + $template = new $template_class(); + $this->assertInstanceOf(ITILTemplate::class, $template); $template_input = [ 'name' => 'test_template_' . mt_rand(), 'is_active' => 1, 'entities_id' => 0, 'is_recursive' => 1, - '_readonly' => $readonly_input, - '_predefined' => $predefined, - '_mandatory' => [], - '_hidden' => [], ]; - $template_id = $template->add($template_input); $this->assertGreaterThan(0, $template_id, 'Template creation failed'); + // Add Readonly Fields + if (!empty($readonly)) { + $readonly_field_class = $itil_type . 'TemplateReadonlyField'; + $readonly_field = new $readonly_field_class(); + $this->assertInstanceOf(ITILTemplateReadonlyField::class, $readonly_field); + + $foreign_key_field = $readonly_field::$items_id; + + foreach ($readonly as $field_name) { + $result = $readonly_field->add([ + $foreign_key_field => $template_id, + 'num' => $this->getIdFromSearchOptions($field_name), + ]); + $this->assertNotFalse($result, "Failed to add readonly field '$field_name'"); + } + } + + // Add Predefined Fields + if (!empty($predefined)) { + $predefined_field_class = $itil_type . 'TemplatePredefinedField'; + $predefined_field = new $predefined_field_class(); + $this->assertInstanceOf(ITILTemplatePredefinedField::class, $predefined_field); + + $foreign_key_field = $predefined_field::$items_id; + + foreach ($predefined as $field_name => $field_value) { + $result = $predefined_field->add([ + $foreign_key_field => $template_id, + 'num' => $this->getIdFromSearchOptions($field_name), + 'value' => $field_value, + ]); + $this->assertNotFalse($result, "Failed to add predefined field '$field_name'"); + } + } + + // Create Category and associate template $category = new ITILCategory(); $cat_id = $category->add(['name' => 'test_cat_' . mt_rand(), 'entities_id' => 0, 'is_recursive' => 1]); $this->assertGreaterThan(0, $cat_id, 'Category creation failed'); $type = null; - if ($itil_class instanceof Ticket) { + if ($itil_object instanceof Ticket) { $type = Ticket::INCIDENT_TYPE; } else { - $type = true; // For Change and Problem + // For Change and Problem, the template is not type-specific in the same way. + $type = true; } - $template_field_name = $itil_class->getTemplateFieldName($type); + $template_field_name = $itil_object->getTemplateFieldName($type); $category->update([ 'id' => $cat_id, @@ -63,122 +131,139 @@ protected function createTemplateAndCategory(array $readonly = [], array $predef return $cat_id; } + protected function getIdFromSearchOptions(string $field): ?string + { + $item = $this->getITILClass(); + foreach ($item->getSearchOptionsMain() as $option) { + if (isset($option['field']) && $option['field'] === $field) { + return (string) $option['id']; + } + } + return null; + } + + public function setUp(): void + { + parent::setUp(); + $this->login(); + } + + public function testHandleReadonlyFieldsOnAddWithPredefined(): void { - $cat_id = $this->createTemplateAndCategory( - ['name'], // readonly - ['name' => 'Predefined Name Value'] // predefined - ); + $cat_id = $this->createTemplateAndCategory(['urgency'], ['urgency' => Urgency::HIGH->value]); - $itilObject = $this->getITILClass(); + $itil_object = $this->getITILClass(); $input = [ - 'name' => 'User Input Name', - 'content' => 'Some content', + 'urgency' => Urgency::LOW->value, + 'name' => 'Some content', 'status' => CommonITILObject::INCOMING, 'itilcategories_id' => $cat_id, 'entities_id' => 0, ]; - if ($itilObject instanceof Ticket) { + if ($itil_object instanceof Ticket) { $input['type'] = Ticket::INCIDENT_TYPE; } - $processedInput = $itilObject->prepareInputForAdd($input); + $processed_input = $itil_object->prepareInputForAdd($input); - $this->assertEquals('Predefined Name Value', $processedInput['name']); - $this->assertEquals('Some content', $processedInput['content']); + $this->assertEquals(Urgency::HIGH->value, $processed_input['urgency']); + $this->assertEquals('Some content', $processed_input['name']); } public function testHandleReadonlyFieldsOnAddWithoutPredefined(): void { - $cat_id = $this->createTemplateAndCategory( - ['name'] // readonly - ); + $cat_id = $this->createTemplateAndCategory(['urgency']); - $itilObject = $this->getITILClass(); + $itil_object = $this->getITILClass(); $input = [ - 'name' => 'User Input Name', - 'content' => 'Some content', + 'urgency' => Urgency::LOW->value, + 'name' => 'Some content', 'status' => CommonITILObject::INCOMING, 'itilcategories_id' => $cat_id, 'entities_id' => 0, ]; - if ($itilObject instanceof Ticket) { + if ($itil_object instanceof Ticket) { $input['type'] = Ticket::INCIDENT_TYPE; } - $processedInput = $itilObject->prepareInputForAdd($input); + $processed_input = $itil_object->prepareInputForAdd($input); - $this->assertEmpty($processedInput['name']); - $this->assertEquals('Some content', $processedInput['content']); + $this->assertEquals(Urgency::MEDIUM->value, $processed_input['urgency']); // Default value + $this->assertEquals('Some content', $processed_input['name']); } public function testHandleReadonlyFieldsOnUpdateWithExistingValue(): void { - $cat_id = $this->createTemplateAndCategory( - ['name'] // readonly - ); - - $itilObject = $this->getITILClass(); - $item_id = $itilObject->add([ - 'name' => 'Existing Name Value', - 'content' => 'Initial content', + $cat_id = $this->createTemplateAndCategory(['urgency']); + + $itil_object = $this->getITILClass(); + $add_input = [ + 'name' => 'Initial content', 'status' => CommonITILObject::ASSIGNED, 'itilcategories_id' => $cat_id, 'entities_id' => 0, - ]); + ]; + if ($itil_object instanceof Ticket) { + $add_input['type'] = Ticket::INCIDENT_TYPE; + } + $item_id = $itil_object->add($add_input); + $this->assertGreaterThan(0, $item_id); - $itilObject->getFromDB($item_id); + $itil_object->getFromDB($item_id); - $input = [ + $update_input = [ 'id' => $item_id, - 'name' => 'User Attempted New Name', - 'content' => 'Updated content', + 'urgency' => Urgency::HIGH->value, + 'name' => 'Updated content', 'status' => CommonITILObject::ASSIGNED, 'itilcategories_id' => $cat_id, 'entities_id' => 0, ]; - if ($itilObject instanceof Ticket) { - $input['type'] = Ticket::INCIDENT_TYPE; + if ($itil_object instanceof Ticket) { + $update_input['type'] = Ticket::INCIDENT_TYPE; } - $processedInput = $itilObject->prepareInputForUpdate($input); + $processed_input = $itil_object->prepareInputForUpdate($update_input); - $this->assertEquals('Existing Name Value', $processedInput['name']); - $this->assertEquals('Updated content', $processedInput['content']); + $this->assertEquals(Urgency::MEDIUM->value, $processed_input['urgency']); + $this->assertEquals('Updated content', $processed_input['name']); } public function testHandleReadonlyFieldsOnUpdateWithoutExistingValue(): void { - $cat_id = $this->createTemplateAndCategory( - ['name'] // readonly - ); - - $itilObject = $this->getITILClass(); - $item_id = $itilObject->add([ - 'name' => '', // No initial name - 'content' => 'Initial content', + $cat_id = $this->createTemplateAndCategory(['urgency']); + + $itil_object = $this->getITILClass(); + $add_input = [ + 'name' => 'Initial content', 'status' => CommonITILObject::ASSIGNED, 'itilcategories_id' => $cat_id, 'entities_id' => 0, - ]); + ]; + if ($itil_object instanceof Ticket) { + $add_input['type'] = Ticket::INCIDENT_TYPE; + } + $item_id = $itil_object->add($add_input); + $this->assertGreaterThan(0, $item_id); - $itilObject->getFromDB($item_id); + $itil_object->getFromDB($item_id); - $input = [ + $update_input = [ 'id' => $item_id, - 'name' => 'User Attempted New Name', - 'content' => 'Updated content', + 'urgency' => Urgency::LOW->value, + 'name' => 'Updated content', 'status' => CommonITILObject::ASSIGNED, 'itilcategories_id' => $cat_id, 'entities_id' => 0, ]; - if ($itilObject instanceof Ticket) { - $input['type'] = Ticket::INCIDENT_TYPE; + if ($itil_object instanceof Ticket) { + $update_input['type'] = Ticket::INCIDENT_TYPE; } - $processedInput = $itilObject->prepareInputForUpdate($input); + $processed_input = $itil_object->prepareInputForUpdate($update_input); - $this->assertArrayNotHasKey('name', $processedInput); - $this->assertEquals('Updated content', $processedInput['content']); + $this->assertEquals(Urgency::MEDIUM->value, $processed_input['urgency']); // Default value + $this->assertEquals('Updated content', $processed_input['name']); } } From 8a9438d66ae3cd66c8a46ebb2b988e81c0f7160d Mon Sep 17 00:00:00 2001 From: Benoit VIGNAL Date: Wed, 22 Oct 2025 10:37:33 +0200 Subject: [PATCH 5/6] fix(itil): Readonly logic is now only applied on form controller not on all add/update logic --- front/change.form.php | 2 + front/problem.form.php | 2 + front/ticket.form.php | 2 + src/CommonITILObject.php | 39 ++++++++++++++----- .../AbstractITILTemplateReadonlyFieldTest.php | 10 ++--- 5 files changed, 40 insertions(+), 15 deletions(-) diff --git a/front/change.form.php b/front/change.form.php index c9540ed7d43..e66e11fe189 100644 --- a/front/change.form.php +++ b/front/change.form.php @@ -53,6 +53,7 @@ if (isset($_POST["add"])) { $change->check(-1, CREATE, $_POST); + $_POST = $change->enforceReadonlyFields($_POST, true); $newID = $change->add($_POST); Event::log( $newID, @@ -109,6 +110,7 @@ } elseif (isset($_POST["update"])) { $change->check($_POST["id"], UPDATE); + $_POST = $change->enforceReadonlyFields($_POST); $change->update($_POST); Event::log( $_POST["id"], diff --git a/front/problem.form.php b/front/problem.form.php index 046f99b3842..12753211290 100644 --- a/front/problem.form.php +++ b/front/problem.form.php @@ -53,6 +53,7 @@ if (isset($_POST["add"])) { $problem->check(-1, CREATE, $_POST); + $_POST = $problem->enforceReadonlyFields($_POST, true); if ($newID = $problem->add($_POST)) { Event::log( $newID, @@ -108,6 +109,7 @@ } elseif (isset($_POST["update"])) { $problem->check($_POST["id"], UPDATE); + $_POST = $problem->enforceReadonlyFields($_POST); $problem->update($_POST); Event::log( $_POST["id"], diff --git a/front/ticket.form.php b/front/ticket.form.php index f2fefafa232..c50d4202ba2 100644 --- a/front/ticket.form.php +++ b/front/ticket.form.php @@ -74,6 +74,7 @@ if (isset($_POST["add"])) { $track->check(-1, CREATE, $_POST); + $_POST = $track->enforceReadonlyFields($_POST, true); if ($track->add($_POST)) { if ($_SESSION['glpibackcreated']) { @@ -85,6 +86,7 @@ if (!$track::canUpdate()) { throw new AccessDeniedHttpException(); } + $_POST = $track->enforceReadonlyFields($_POST); $track->update($_POST); if (isset($_POST['kb_linked_id'])) { diff --git a/src/CommonITILObject.php b/src/CommonITILObject.php index addb55dc09d..6dcf03b3346 100644 --- a/src/CommonITILObject.php +++ b/src/CommonITILObject.php @@ -32,7 +32,6 @@ * * --------------------------------------------------------------------- */ - use Glpi\Application\View\TemplateRenderer; use Glpi\ContentTemplates\Parameters\CommonITILObjectParameters; use Glpi\DBAL\QueryExpression; @@ -1770,8 +1769,6 @@ public function cleanDBonPurge() protected function handleTemplateFields(array $input, bool $show_error_message = true) { //// check mandatory fields - // First get ticket template associated: entity and type/category - $tt = $this->getITILTemplateFromInput($input); $check_allowed_fields_for_template = false; $allowed_fields = []; @@ -1858,7 +1855,9 @@ class_exists($validation_class) } } - if (count($tt->mandatory)) { + // First get ticket template associated: entity and type/category + $tt = $this->getITILTemplateFromInput($input); + if ($tt && count($tt->mandatory)) { $mandatory_missing = []; $fieldsname = $tt->getAllowedFieldsNames(true); foreach ($tt->mandatory as $key => $val) { @@ -1967,7 +1966,6 @@ public function prepareInputForUpdate($input) if (!$this->checkFieldsConsistency($input)) { return false; } - $input = $this->handleReadonlyFields($input); // Add document if needed $this->getFromDB($input["id"]); // entities_id field required @@ -2332,9 +2330,25 @@ public function prepareInputForUpdate($input) return $input; } - private function handleReadonlyFields(array $input, bool $isAdd = false): array + + /** + * Processes readonly fields in the input array based on the ITIL template data. + * + * @param array $input The user input data to process (often $_POST). + * @param bool $isAdd true if we are in a creation, will force to apply the template predefined field. + * + * @return array The modified user input array after processing readonly fields. + * + * @since 11.0.2 + */ + public function enforceReadonlyFields(array $input, bool $isAdd = false): array { $tt = $this->getITILTemplateFromInput($input); + if (!$tt) { + dump('Template not found'); + return $input; + } + $tt->getFromDBWithData($tt->getID()); // We load the fields (predefined and readonly) foreach (array_keys($tt->readonly) as $read_only_field) { @@ -2824,7 +2838,6 @@ public function prepareInputForAdd($input) if (!$this->checkFieldsConsistency($input)) { return false; } - $input = $this->handleReadonlyFields($input, true); $input = $this->transformActorsInput($input); @@ -8269,13 +8282,16 @@ public function getITILTemplateToUse( * If the input is not defined, it will get it from the object fields datas * * @param array $input - * @return ITILTemplate + * @return ITILTemplate|null * * @since 11.0.2 */ - public function getITILTemplateFromInput(array $input = []): ITILTemplate + public function getITILTemplateFromInput(array $input = []): ?ITILTemplate { - $entid = $input['entities_id'] ?? $this->fields['entities_id']; + $entid = $input['entities_id'] ?? $this->fields['entities_id'] ?? $input['id'] ?? null; + if (is_null($entid)) { + return null; + } $type = null; if (isset($input['type'])) { @@ -8285,6 +8301,9 @@ public function getITILTemplateFromInput(array $input = []): ITILTemplate } $categid = $input['itilcategories_id'] ?? $this->fields['itilcategories_id']; + if (is_null($categid)) { + return null; + } return $this->getITILTemplateToUse(0, $type, $categid, $entid); } diff --git a/tests/src/AbstractITILTemplateReadonlyFieldTest.php b/tests/src/AbstractITILTemplateReadonlyFieldTest.php index 701a3ac9c4a..7deb3dc2a87 100644 --- a/tests/src/AbstractITILTemplateReadonlyFieldTest.php +++ b/tests/src/AbstractITILTemplateReadonlyFieldTest.php @@ -165,7 +165,7 @@ public function testHandleReadonlyFieldsOnAddWithPredefined(): void $input['type'] = Ticket::INCIDENT_TYPE; } - $processed_input = $itil_object->prepareInputForAdd($input); + $processed_input = $itil_object->enforceReadonlyFields($input, true); $this->assertEquals(Urgency::HIGH->value, $processed_input['urgency']); $this->assertEquals('Some content', $processed_input['name']); @@ -187,9 +187,9 @@ public function testHandleReadonlyFieldsOnAddWithoutPredefined(): void $input['type'] = Ticket::INCIDENT_TYPE; } - $processed_input = $itil_object->prepareInputForAdd($input); + $processed_input = $itil_object->enforceReadonlyFields($input, true); - $this->assertEquals(Urgency::MEDIUM->value, $processed_input['urgency']); // Default value + $this->assertArrayNotHasKey('urgency', $processed_input); // Default value $this->assertEquals('Some content', $processed_input['name']); } @@ -224,7 +224,7 @@ public function testHandleReadonlyFieldsOnUpdateWithExistingValue(): void $update_input['type'] = Ticket::INCIDENT_TYPE; } - $processed_input = $itil_object->prepareInputForUpdate($update_input); + $processed_input = $itil_object->enforceReadonlyFields($update_input); $this->assertEquals(Urgency::MEDIUM->value, $processed_input['urgency']); $this->assertEquals('Updated content', $processed_input['name']); @@ -261,7 +261,7 @@ public function testHandleReadonlyFieldsOnUpdateWithoutExistingValue(): void $update_input['type'] = Ticket::INCIDENT_TYPE; } - $processed_input = $itil_object->prepareInputForUpdate($update_input); + $processed_input = $itil_object->enforceReadonlyFields($update_input); $this->assertEquals(Urgency::MEDIUM->value, $processed_input['urgency']); // Default value $this->assertEquals('Updated content', $processed_input['name']); From 3ba3f6dca83dd0a253dbc3791fbd09da5a7dfce3 Mon Sep 17 00:00:00 2001 From: Benoit VIGNAL Date: Wed, 22 Oct 2025 12:27:54 +0200 Subject: [PATCH 6/6] test(itil): Add cypress test on the readonly field logic --- src/CommonITILObject.php | 3 +- .../cypress/e2e/ITILObject/ticket_form.cy.js | 48 +++++++++++++++++++ .../AbstractITILTemplateReadonlyFieldTest.php | 20 +++++++- 3 files changed, 68 insertions(+), 3 deletions(-) diff --git a/src/CommonITILObject.php b/src/CommonITILObject.php index 6dcf03b3346..7827e82a440 100644 --- a/src/CommonITILObject.php +++ b/src/CommonITILObject.php @@ -2345,7 +2345,6 @@ public function enforceReadonlyFields(array $input, bool $isAdd = false): array { $tt = $this->getITILTemplateFromInput($input); if (!$tt) { - dump('Template not found'); return $input; } @@ -8300,7 +8299,7 @@ public function getITILTemplateFromInput(array $input = []): ?ITILTemplate $type = $this->fields['type']; } - $categid = $input['itilcategories_id'] ?? $this->fields['itilcategories_id']; + $categid = $input['itilcategories_id'] ?? $this->fields['itilcategories_id'] ?? null; if (is_null($categid)) { return null; } diff --git a/tests/cypress/e2e/ITILObject/ticket_form.cy.js b/tests/cypress/e2e/ITILObject/ticket_form.cy.js index 5124954f55a..d629780d9b9 100644 --- a/tests/cypress/e2e/ITILObject/ticket_form.cy.js +++ b/tests/cypress/e2e/ITILObject/ticket_form.cy.js @@ -334,4 +334,52 @@ describe("Ticket Form", () => { cy.findByRole('cell').should('contain.text', 'No results found'); }); }); + + it('Create/update a ticket using a template with readonly fields', () => { + const ticket_template_name = `test template ${rand}`; + cy.createWithAPI('TicketTemplate', { + 'name': ticket_template_name, + }).as('ticket_template_id'); + + cy.get('@ticket_template_id').then((ticket_template_id) => { + cy.createWithAPI('TicketTemplatePredefinedField', { + 'tickettemplates_id': ticket_template_id, // Default template + 'num': 10, // Urgency + 'value': 4, // High + }); + + cy.createWithAPI('TicketTemplateReadonlyField', { + 'tickettemplates_id': ticket_template_id, + 'num': 10, + }); + + cy.createWithAPI('ITILCategory', { + 'name':ticket_template_name, + 'tickettemplates_id': ticket_template_id, + 'tickettemplates_id_incident': ticket_template_id, + 'tickettemplates_id_demand': ticket_template_id, + 'changetemplates_id': ticket_template_id, + 'problemtemplates_id': ticket_template_id, + }); + }); + + // Create form + cy.visit(`/front/ticket.form.php`); + + // intercept form submit + cy.intercept('POST', '/front/ticket.form.php').as('submit'); + + cy.getDropdownByLabelText('Category').selectDropdownValue(`ยป${ticket_template_name}`); + + // We change the value of a readonly field, it should be ignored + cy.get('input[name="urgency"]').invoke('val', '1'); + cy.findByRole('button', {'name': 'Add'}).click(); + cy.wait('@submit').its('response.statusCode').should('eq', 200); + cy.get('input[name="urgency"]').should('have.value', '4'); // Should be the template 4 value + + // We try updating it + cy.get('input[name="urgency"]').invoke('val', '1'); + cy.findByRole('button', {'name': 'Save'}).click(); + cy.get('input[name="urgency"]').should('have.value', '4'); // Should be the template 4 value + }); }); diff --git a/tests/src/AbstractITILTemplateReadonlyFieldTest.php b/tests/src/AbstractITILTemplateReadonlyFieldTest.php index 7deb3dc2a87..cd637445606 100644 --- a/tests/src/AbstractITILTemplateReadonlyFieldTest.php +++ b/tests/src/AbstractITILTemplateReadonlyFieldTest.php @@ -41,7 +41,7 @@ use ITILTemplate; use ITILTemplatePredefinedField; use ITILTemplateReadonlyField; -use Ticket; // For type constants +use Ticket; abstract class AbstractITILTemplateReadonlyFieldTest extends DbTestCase { @@ -148,6 +148,24 @@ public function setUp(): void $this->login(); } + public function testHandleReadonlyFieldsWithNoTemplate(): void + { + $itil_object = $this->getITILClass(); + $input = [ + 'urgency' => Urgency::LOW->value, + 'name' => 'Some content', + 'status' => CommonITILObject::INCOMING, + 'entities_id' => 0, + ]; + if ($itil_object instanceof Ticket) { + $input['type'] = Ticket::INCIDENT_TYPE; + } + + $processed_input = $itil_object->enforceReadonlyFields($input, true); + + $this->assertEquals(Urgency::LOW->value, $processed_input['urgency']); + $this->assertEquals('Some content', $processed_input['name']); + } public function testHandleReadonlyFieldsOnAddWithPredefined(): void {