From bdcf5238920ef9ab2b606413540dbd638d607fc2 Mon Sep 17 00:00:00 2001 From: Lainow Date: Wed, 27 May 2026 14:47:17 +0200 Subject: [PATCH 1/4] Change config UI --- src/Config.php | 107 +++++++----- src/Controller.php | 22 --- templates/config.html.twig | 337 ++++++------------------------------- 3 files changed, 113 insertions(+), 353 deletions(-) diff --git a/src/Config.php b/src/Config.php index 8e900ca..adf9377 100644 --- a/src/Config.php +++ b/src/Config.php @@ -47,6 +47,7 @@ class Config extends CommonDBTM { public $dohistory = true; public static $rightname = 'config'; + public const CONFIG_PARENT = \Entity::CONFIG_PARENT; public static function getMenuName(): string { return __('More options', 'moreoptions'); @@ -105,10 +106,8 @@ public static function preItemUpdate(CommonDBTM $item): CommonDBTM } foreach (self::getItilConfigFields() as $field) { - if (!isset($item->input[$field])) { - $item->input[$field] = 0; - } elseif ($item->input[$field] == 'on') { - $item->input[$field] = 1; + if (isset($item->input[$field])) { + $item->input[$field] = (int) $item->input[$field]; } } @@ -121,7 +120,6 @@ public static function preItemUpdate(CommonDBTM $item): CommonDBTM public static function getItilConfigFields(): array { return [ - 'use_parent_entity', 'take_item_group_ticket', 'take_item_group_change', 'take_item_group_problem', @@ -153,6 +151,12 @@ public static function getItilConfigFields(): array 'mandatory_task_duration', 'mandatory_task_user', 'mandatory_task_group', + 'take_requester_group_ticket', + 'take_requester_group_change', + 'take_requester_group_problem', + 'take_technician_group_ticket', + 'take_technician_group_change', + 'take_technician_group_problem', ]; } @@ -175,24 +179,31 @@ public static function showForEntity(Entity $item): void 'entities_id' => $item->getID(), ]); - // Get effective configuration to show which entity's config is actually used - $effectiveConfig = self::getConfig($item->getID(), true); - $parentEntityInfo = null; - - if (($moconfig->fields['use_parent_entity'] ?? false) && ($effectiveConfig->fields['entities_id'] != $item->getID())) { - $parentEntity = new Entity(); - if ($parentEntity->getFromDB($effectiveConfig->fields['entities_id'])) { - $parentEntityInfo = $parentEntity->getName(); + $inheritance_labels = []; + if ($item->getID() > 0) { + $parentConfig = self::getConfig($item->fields['entities_id'], true); + foreach (self::getItilConfigFields() as $field) { + $inheritance_labels[$field] = self::getInheritedValueBadge($parentConfig->fields[$field] ?? 0); + } + foreach ([ + 'take_requester_group_ticket', + 'take_requester_group_change', + 'take_requester_group_problem', + 'take_technician_group_ticket', + 'take_technician_group_change', + 'take_technician_group_problem', + ] as $field) { + $inheritance_labels[$field] = self::getInheritedValueBadgeForActorGroup($parentConfig->fields[$field] ?? 0); } } TemplateRenderer::getInstance()->display( '@moreoptions/config.html.twig', [ - 'item' => $moconfig, - 'dropdown_options' => self::getSelectableActorGroup(), - 'parent_entity_info' => $parentEntityInfo, - 'params' => [ + 'item' => $moconfig, + 'dropdown_options' => self::getSelectableActorGroup(), + 'inheritance_labels' => $inheritance_labels, + 'params' => [ 'canedit' => true, ], ], @@ -204,11 +215,26 @@ public static function getIcon(): string return "ti ti-send"; } + private static function getInheritedValueBadge(mixed $value): string + { + $text = match ((int) $value) { + 1 => __('Yes'), + default => __('No'), + }; + return Entity::inheritedValue(htmlescape($text), false, false); + } + + private static function getInheritedValueBadgeForActorGroup(mixed $value): string + { + $options = self::getSelectableActorGroup(); + $text = $options[(int) $value] ?? __('No'); + return Entity::inheritedValue(htmlescape($text), false, false); + } + public static function addConfig(CommonDBTM $item): void { $moconfig = new self(); $moconfig->add([ - 'is_active' => 0, 'entities_id' => $item->getID(), ]); } @@ -222,7 +248,6 @@ public static function addConfig(CommonDBTM $item): void */ public static function getConfig(?int $entityId = null, bool $useInheritance = true): self { - // Use current entity if not specified if ($entityId === null) { $entityId = Session::getActiveEntity(); } @@ -232,12 +257,13 @@ public static function getConfig(?int $entityId = null, bool $useInheritance = t 'entities_id' => $entityId, ]); - // If inheritance is enabled, use_parent_entity is set, and we're not at root entity - if ($useInheritance && ($moconfig->fields['use_parent_entity'] ?? false) && $entityId > 0) { + if ($useInheritance && $entityId > 0) { $entity = new Entity(); if ($entity->getFromDB($entityId)) { - $parentId = $entity->fields['entities_id']; - return self::getConfig($parentId, true); + $parentConfig = self::getConfig($entity->fields['entities_id'], true); + foreach (self::getItilConfigFields() as $field) { + $moconfig->fields[$field] = $parentConfig->fields[$field] ?? 0; + } } } @@ -253,18 +279,16 @@ public static function install(Migration $migration): void $migration->displayMessage("Installing $table"); $query = "CREATE TABLE IF NOT EXISTS `$table` ( `id` int unsigned NOT NULL AUTO_INCREMENT, - `is_active` tinyint NOT NULL DEFAULT '1', `entities_id` int unsigned NOT NULL DEFAULT '0', - `use_parent_entity` tinyint NOT NULL DEFAULT '0', `take_item_group_ticket` tinyint NOT NULL DEFAULT '-2', `take_item_group_change` tinyint NOT NULL DEFAULT '0', `take_item_group_problem` tinyint NOT NULL DEFAULT '0', - `take_requester_group_ticket` int unsigned NOT NULL DEFAULT '0', - `take_requester_group_change` int unsigned NOT NULL DEFAULT '0', - `take_requester_group_problem` int unsigned NOT NULL DEFAULT '0', - `take_technician_group_ticket` int unsigned NOT NULL DEFAULT '0', - `take_technician_group_change` int unsigned NOT NULL DEFAULT '0', - `take_technician_group_problem` int unsigned NOT NULL DEFAULT '0', + `take_requester_group_ticket` tinyint NOT NULL DEFAULT '0', + `take_requester_group_change` tinyint NOT NULL DEFAULT '0', + `take_requester_group_problem` tinyint NOT NULL DEFAULT '0', + `take_technician_group_ticket` tinyint NOT NULL DEFAULT '0', + `take_technician_group_change` tinyint NOT NULL DEFAULT '0', + `take_technician_group_problem` tinyint NOT NULL DEFAULT '0', `prevent_closure_ticket` tinyint NOT NULL DEFAULT '0', `prevent_closure_change` tinyint NOT NULL DEFAULT '0', `prevent_closure_problem` tinyint NOT NULL DEFAULT '0', @@ -294,22 +318,23 @@ public static function install(Migration $migration): void `mandatory_task_user` tinyint NOT NULL DEFAULT '0', `mandatory_task_group` tinyint NOT NULL DEFAULT '0', PRIMARY KEY (`id`), - KEY `entities_id` (`entities_id`), - KEY `is_active` (`is_active`) + KEY `entities_id` (`entities_id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci ROW_FORMAT=DYNAMIC; "; $DB->doQuery($query); } - // Migration: Add use_parent_entity column if it doesn't exist - if (!$DB->fieldExists($table, 'use_parent_entity')) { - $migration->displayMessage("Adding use_parent_entity field to $table"); - $migration->addField($table, 'use_parent_entity', 'tinyint', [ - 'after' => 'entities_id', - 'value' => 0, - 'nodefault' => false, - ]); + foreach ([ + 'take_requester_group_ticket', + 'take_requester_group_change', + 'take_requester_group_problem', + 'take_technician_group_ticket', + 'take_technician_group_change', + 'take_technician_group_problem', + ] as $field) { + $migration->changeField($table, $field, $field, 'tinyint', ['value' => 0]); } + $migration->executeMigration(); $entities = new Entity(); foreach ($entities->find() as $entity) { diff --git a/src/Controller.php b/src/Controller.php index f18da82..6ce8f6e 100644 --- a/src/Controller.php +++ b/src/Controller.php @@ -86,10 +86,6 @@ public static function useConfig(CommonDBTM $item): void } $moconfig = Config::getConfig(); - if ($moconfig->fields['is_active'] != 1) { - return; - } - switch ($item) { case $item instanceof Ticket_User: if ($item->fields['type'] == \CommonITILActor::REQUESTER) { @@ -132,9 +128,6 @@ public static function useConfig(CommonDBTM $item): void public static function addItemGroups(CommonDBTM $item): void { $conf = Config::getConfig(); - if ($conf->fields['is_active'] != 1) { - return; - } // Mapping of item types to their configuration fields and group classes $itemMappings = [ @@ -311,9 +304,6 @@ public static function beforeCloseITILObject(CommonDBTM $item): void public static function preventClosure(CommonDBTM $item): bool { $conf = Config::getConfig(); - if ($conf->fields['is_active'] != 1) { - return true; - } $tasks = []; @@ -349,9 +339,6 @@ public static function preventClosure(CommonDBTM $item): bool public static function requireFieldsToClose(CommonDBTM $item, bool $is_solution = false): bool { $conf = Config::getConfig(); - if ($conf->fields['is_active'] != 1) { - return true; - } $message = ''; $itemtype = get_class($item); @@ -447,9 +434,6 @@ public static function requireFieldsToClose(CommonDBTM $item, bool $is_solution public static function checkTaskRequirements(CommonDBTM $item): CommonDBTM { $conf = Config::getConfig(); - if ($conf->fields['is_active'] != 1) { - return $item; - } $message = ''; if ($conf->fields['mandatory_task_category'] == 1) { @@ -488,9 +472,6 @@ public static function checkTaskRequirements(CommonDBTM $item): CommonDBTM public static function updateItemActors(CommonITILObject $item): CommonITILObject { $conf = Config::getConfig(); - if ($conf->fields['is_active'] != 1) { - return $item; - } switch (get_class($item)) { case 'Ticket': @@ -557,9 +538,6 @@ public static function updateItemActors(CommonITILObject $item): CommonITILObjec public static function assignTechnicianFromTask(\CommonITILTask $item): void { $conf = Config::getConfig(Session::getActiveEntity()); - if ($conf->fields['is_active'] != 1) { - return; - } // Check if a technician is assigned to the task if (empty($item->fields['users_id_tech'])) { diff --git a/templates/config.html.twig b/templates/config.html.twig index 7d088c6..984a9b8 100644 --- a/templates/config.html.twig +++ b/templates/config.html.twig @@ -33,10 +33,13 @@ #} {% import 'components/form/fields_macros.html.twig' as fields %} -{% import 'components/alerts_macros.html.twig' as alerts %} {# Layout options for fields #} {% set field_options = {'label_class': 'col-8', 'input_class': 'col-4', 'field_class': 'col-6', 'full_width': false} %} +{% set inherit_option = item.fields['entities_id'] != 0 ? {(constant('Entity::CONFIG_PARENT')): __('Inheritance of the parent entity')} : {} %} +{% set yes_no_options = inherit_option + {0: __('No'), 1: __('Yes')} %} +{% set dropdown_options_with_inherit = inherit_option + dropdown_options %} +{% set cell_options = {'no_label': true, 'field_class': 'col-12'} %}
@@ -53,194 +56,110 @@ {{ __('Itil items') }}
- {{ fields.checkboxField( - 'is_active', - item.fields['is_active'], - __('Active on this entity', 'moreoptions') - ) }} - - {% if item.fields['entities_id'] != 0 %} - {{ fields.checkboxField( - 'use_parent_entity', - item.fields['use_parent_entity'], - __('Use parent entity configuration', 'moreoptions'), - {'helper': __('When enabled, this entity will inherit configuration from its parent entity', 'moreoptions')} - ) }} - - {% if parent_entity_info %} - {{ alerts.alert_info(__('Configuration is inherited from parent entity: %s', 'moreoptions')|format(parent_entity_info)) }} - {% endif %} - {% endif %} -
- +
- - - - - + + + - - - - + + + - - - - + + + - - - - + + + - - - - + + + - - - - + + +
  {{ __('Ticket') }} {{ __('Change') }} {{ __('Problem') }}{{ __('All') }}
{{ __('Take the group of associated item', 'moreoptions') }}{{ fields.dropdownArrayField('take_item_group_ticket', item.fields['take_item_group_ticket'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['take_item_group_ticket']|default(null)})) }}{{ fields.dropdownArrayField('take_item_group_change', item.fields['take_item_group_change'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['take_item_group_change']|default(null)})) }}{{ fields.dropdownArrayField('take_item_group_problem', item.fields['take_item_group_problem'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['take_item_group_problem']|default(null)})) }}
{{ __('Take the requester group', 'moreoptions') }} - {{ fields.dropdownArrayField( - 'take_requester_group_ticket', - item.fields['take_requester_group_ticket'], - dropdown_options, - '', - field_options|merge({'field_class': 'col-12', 'no_label': true}) - ) }} - - {{ fields.dropdownArrayField( - 'take_requester_group_change', - item.fields['take_requester_group_change'], - dropdown_options, - '', - field_options|merge({'field_class': 'col-12', 'no_label': true}) - ) }} - - {{ fields.dropdownArrayField( - 'take_requester_group_problem', - item.fields['take_requester_group_problem'], - dropdown_options, - '', - field_options|merge({'field_class': 'col-12', 'no_label': true}) - ) }} - {{ fields.dropdownArrayField('take_requester_group_ticket', item.fields['take_requester_group_ticket'], dropdown_options_with_inherit, '', cell_options|merge({'add_field_html': inheritance_labels['take_requester_group_ticket']|default(null)})) }}{{ fields.dropdownArrayField('take_requester_group_change', item.fields['take_requester_group_change'], dropdown_options_with_inherit, '', cell_options|merge({'add_field_html': inheritance_labels['take_requester_group_change']|default(null)})) }}{{ fields.dropdownArrayField('take_requester_group_problem', item.fields['take_requester_group_problem'], dropdown_options_with_inherit, '', cell_options|merge({'add_field_html': inheritance_labels['take_requester_group_problem']|default(null)})) }}
{{ __('Take the technician group', 'moreoptions') }} - {{ fields.dropdownArrayField( - 'take_technician_group_ticket', - item.fields['take_technician_group_ticket'], - dropdown_options, - '', - field_options|merge({'field_class': 'col-12', 'no_label': true}) - ) }} - - {{ fields.dropdownArrayField( - 'take_technician_group_change', - item.fields['take_technician_group_change'], - dropdown_options, - '', - field_options|merge({'field_class': 'col-12', 'no_label': true}) - ) }} - - {{ fields.dropdownArrayField( - 'take_technician_group_problem', - item.fields['take_technician_group_problem'], - dropdown_options, - '', - field_options|merge({'field_class': 'col-12', 'no_label': true}) - ) }} - {{ fields.dropdownArrayField('take_technician_group_ticket', item.fields['take_technician_group_ticket'], dropdown_options_with_inherit, '', cell_options|merge({'add_field_html': inheritance_labels['take_technician_group_ticket']|default(null)})) }}{{ fields.dropdownArrayField('take_technician_group_change', item.fields['take_technician_group_change'], dropdown_options_with_inherit, '', cell_options|merge({'add_field_html': inheritance_labels['take_technician_group_change']|default(null)})) }}{{ fields.dropdownArrayField('take_technician_group_problem', item.fields['take_technician_group_problem'], dropdown_options_with_inherit, '', cell_options|merge({'add_field_html': inheritance_labels['take_technician_group_problem']|default(null)})) }}
{{ __('Prevent closure with tasks in To Do status', 'moreoptions') }}{{ fields.dropdownArrayField('prevent_closure_ticket', item.fields['prevent_closure_ticket'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['prevent_closure_ticket']|default(null)})) }}{{ fields.dropdownArrayField('prevent_closure_change', item.fields['prevent_closure_change'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['prevent_closure_change']|default(null)})) }}{{ fields.dropdownArrayField('prevent_closure_problem', item.fields['prevent_closure_problem'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['prevent_closure_problem']|default(null)})) }}
{{ __('Assign technical manager when changing category', 'moreoptions') }}{{ fields.dropdownArrayField('assign_technical_manager_when_changing_category_ticket', item.fields['assign_technical_manager_when_changing_category_ticket'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['assign_technical_manager_when_changing_category_ticket']|default(null)})) }}{{ fields.dropdownArrayField('assign_technical_manager_when_changing_category_change', item.fields['assign_technical_manager_when_changing_category_change'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['assign_technical_manager_when_changing_category_change']|default(null)})) }}{{ fields.dropdownArrayField('assign_technical_manager_when_changing_category_problem', item.fields['assign_technical_manager_when_changing_category_problem'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['assign_technical_manager_when_changing_category_problem']|default(null)})) }}
{{ __('Assign technical group when changing category', 'moreoptions') }}{{ fields.dropdownArrayField('assign_technical_group_when_changing_category_ticket', item.fields['assign_technical_group_when_changing_category_ticket'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['assign_technical_group_when_changing_category_ticket']|default(null)})) }}{{ fields.dropdownArrayField('assign_technical_group_when_changing_category_change', item.fields['assign_technical_group_when_changing_category_change'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['assign_technical_group_when_changing_category_change']|default(null)})) }}{{ fields.dropdownArrayField('assign_technical_group_when_changing_category_problem', item.fields['assign_technical_group_when_changing_category_problem'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['assign_technical_group_when_changing_category_problem']|default(null)})) }}
- {{ __('Solve and Close Items') }} + {{ __('Mandatory fields to Solve and Close ITILs', 'moreoptions') }}
- {{ alerts.alert_danger(__('The options below relate to the mandatory fields for resolving and closing ITIL items.', 'moreoptions')) }} - - +
- - - - - + + + - - - - + + + - - - - + + + - - - - + + + - - - - + + +
  {{ __('Ticket') }} {{ __('Change') }} {{ __('Problem') }}{{ __('All') }}
{{ __('Technician') }}{{ fields.dropdownArrayField('require_technician_to_close_ticket', item.fields['require_technician_to_close_ticket'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['require_technician_to_close_ticket']|default(null)})) }}{{ fields.dropdownArrayField('require_technician_to_close_change', item.fields['require_technician_to_close_change'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['require_technician_to_close_change']|default(null)})) }}{{ fields.dropdownArrayField('require_technician_to_close_problem', item.fields['require_technician_to_close_problem'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['require_technician_to_close_problem']|default(null)})) }}
{{ __('Technicians group') }}{{ fields.dropdownArrayField('require_technicians_group_to_close_ticket', item.fields['require_technicians_group_to_close_ticket'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['require_technicians_group_to_close_ticket']|default(null)})) }}{{ fields.dropdownArrayField('require_technicians_group_to_close_change', item.fields['require_technicians_group_to_close_change'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['require_technicians_group_to_close_change']|default(null)})) }}{{ fields.dropdownArrayField('require_technicians_group_to_close_problem', item.fields['require_technicians_group_to_close_problem'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['require_technicians_group_to_close_problem']|default(null)})) }}
{{ __('Category') }}{{ fields.dropdownArrayField('require_category_to_close_ticket', item.fields['require_category_to_close_ticket'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['require_category_to_close_ticket']|default(null)})) }}{{ fields.dropdownArrayField('require_category_to_close_change', item.fields['require_category_to_close_change'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['require_category_to_close_change']|default(null)})) }}{{ fields.dropdownArrayField('require_category_to_close_problem', item.fields['require_category_to_close_problem'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['require_category_to_close_problem']|default(null)})) }}
{{ __('Location') }}{{ fields.dropdownArrayField('require_location_to_close_ticket', item.fields['require_location_to_close_ticket'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['require_location_to_close_ticket']|default(null)})) }}{{ fields.dropdownArrayField('require_location_to_close_change', item.fields['require_location_to_close_change'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['require_location_to_close_change']|default(null)})) }}{{ fields.dropdownArrayField('require_location_to_close_problem', item.fields['require_location_to_close_problem'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['require_location_to_close_problem']|default(null)})) }}
{{ __('Solution') }}{{ fields.dropdownArrayField('require_solution_to_close_ticket', item.fields['require_solution_to_close_ticket'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['require_solution_to_close_ticket']|default(null)})) }}{{ fields.dropdownArrayField('require_solution_to_close_change', item.fields['require_solution_to_close_change'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['require_solution_to_close_change']|default(null)})) }}{{ fields.dropdownArrayField('require_solution_to_close_problem', item.fields['require_solution_to_close_problem'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['require_solution_to_close_problem']|default(null)})) }}
- {{ __('Tasks') }} + {{ __('Mandatory fields for Tasks creation', 'moreoptions') }}
- {{ alerts.alert_danger(__('The options below relate to the mandatory fields for creating a task.', 'moreoptions')) }} - - +
@@ -251,10 +170,10 @@ - - - - + + + +
{{ __('Category') }}
{{ fields.dropdownArrayField('mandatory_task_category', item.fields['mandatory_task_category'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['mandatory_task_category']|default(null)})) }}{{ fields.dropdownArrayField('mandatory_task_duration', item.fields['mandatory_task_duration'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['mandatory_task_duration']|default(null)})) }}{{ fields.dropdownArrayField('mandatory_task_user', item.fields['mandatory_task_user'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['mandatory_task_user']|default(null)})) }}{{ fields.dropdownArrayField('mandatory_task_group', item.fields['mandatory_task_group'], yes_no_options, '', cell_options|merge({'add_field_html': inheritance_labels['mandatory_task_group']|default(null)})) }}
@@ -266,166 +185,4 @@
{{ include('components/form/buttons.html.twig') }} - - - \ No newline at end of file + \ No newline at end of file From cd2851c4a1fbcdf31e3e32405e531527e9c9ca65 Mon Sep 17 00:00:00 2001 From: Lainow Date: Wed, 27 May 2026 16:43:20 +0200 Subject: [PATCH 2/4] Fix tests and inheritance --- src/Config.php | 29 +++++++++---- tests/Units/ConfigTest.php | 84 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 8 deletions(-) diff --git a/src/Config.php b/src/Config.php index adf9377..52cac93 100644 --- a/src/Config.php +++ b/src/Config.php @@ -234,9 +234,14 @@ private static function getInheritedValueBadgeForActorGroup(mixed $value): strin public static function addConfig(CommonDBTM $item): void { $moconfig = new self(); - $moconfig->add([ - 'entities_id' => $item->getID(), - ]); + $entity_id = $item->getID(); + $data = ['entities_id' => $entity_id]; + if ($entity_id > 0) { + foreach (self::getItilConfigFields() as $field) { + $data[$field] = self::CONFIG_PARENT; + } + } + $moconfig->add($data); } /** @@ -262,7 +267,9 @@ public static function getConfig(?int $entityId = null, bool $useInheritance = t if ($entity->getFromDB($entityId)) { $parentConfig = self::getConfig($entity->fields['entities_id'], true); foreach (self::getItilConfigFields() as $field) { - $moconfig->fields[$field] = $parentConfig->fields[$field] ?? 0; + if (($moconfig->fields[$field] ?? 0) == self::CONFIG_PARENT) { + $moconfig->fields[$field] = $parentConfig->fields[$field] ?? 0; + } } } } @@ -334,14 +341,20 @@ public static function install(Migration $migration): void ] as $field) { $migration->changeField($table, $field, $field, 'tinyint', ['value' => 0]); } - $migration->executeMigration(); $entities = new Entity(); foreach ($entities->find() as $entity) { if (is_array($entity) && isset($entity['id'])) { - $data = [ - 'entities_id' => $entity['id'], - ]; + $entity_id = (int) $entity['id']; + if ($DB->numrows($DB->request(['FROM' => self::getTable(), 'WHERE' => ['entities_id' => $entity_id]])) > 0) { + continue; + } + $data = ['entities_id' => $entity_id]; + if ($entity_id > 0) { + foreach (self::getItilConfigFields() as $field) { + $data[$field] = self::CONFIG_PARENT; + } + } $DB->insert( self::getTable(), $data, diff --git a/tests/Units/ConfigTest.php b/tests/Units/ConfigTest.php index 824d39b..49cc775 100644 --- a/tests/Units/ConfigTest.php +++ b/tests/Units/ConfigTest.php @@ -1974,4 +1974,88 @@ public function testAssignTechnicianFromProblemTask(): void $this->assertEquals($tech_id, $assignedUser['users_id']); $this->assertEquals(\CommonITILActor::ASSIGN, $assignedUser['type']); } + + /** + * Test that CONFIG_PARENT values are resolved through multiple entity levels. + * + * Hierarchy: Root(0) → A → B → C + * + * Root : take_item_group_ticket=1, take_requester_group_ticket=2 + * A : take_item_group_ticket=CONFIG_PARENT, take_requester_group_ticket=1 (own) + * B : take_item_group_ticket=0 (own), take_requester_group_ticket=CONFIG_PARENT + * C : take_item_group_ticket=CONFIG_PARENT, take_requester_group_ticket=CONFIG_PARENT + * + * Expected resolved values: + * A effective: take_item_group_ticket=1 (from root), take_requester_group_ticket=1 (own) + * B effective: take_item_group_ticket=0 (own), take_requester_group_ticket=1 (from A) + * C effective: take_item_group_ticket=0 (from B), take_requester_group_ticket=1 (from B→A) + */ + public function testMultiLevelInheritanceResolvesConfigParentThroughChain(): void + { + $this->initEntitySession(); + + $entity_a = $this->createItem( + \Entity::class, + ['name' => 'Inheritance Test A', 'entities_id' => 0], + ['name'], + ); + $this->clearLogEntriesContaining('glpiactiveentities_string'); + + $entity_b = $this->createItem( + \Entity::class, + ['name' => 'Inheritance Test B', 'entities_id' => $entity_a->getID()], + ['name', 'entities_id'], + ); + $this->clearLogEntriesContaining('glpiactiveentities_string'); + + $entity_c = $this->createItem( + \Entity::class, + ['name' => 'Inheritance Test C', 'entities_id' => $entity_b->getID()], + ['name', 'entities_id'], + ); + $this->clearLogEntriesContaining('glpiactiveentities_string'); + + // Root: explicit values + $root_conf = Config::getConfig(0, false); + $this->updateItem(Config::class, $root_conf->getID(), [ + 'take_item_group_ticket' => 1, + 'take_requester_group_ticket' => 2, + ]); + + // A: inherit take_item_group_ticket from root, own take_requester_group_ticket=1 + $conf_a = Config::getConfig($entity_a->getID(), false); + $this->updateItem(Config::class, $conf_a->getID(), [ + 'take_item_group_ticket' => Config::CONFIG_PARENT, + 'take_requester_group_ticket' => 1, + ]); + + // B: own take_item_group_ticket=0, inherit take_requester_group_ticket from A + $conf_b = Config::getConfig($entity_b->getID(), false); + $this->updateItem(Config::class, $conf_b->getID(), [ + 'take_item_group_ticket' => 0, + 'take_requester_group_ticket' => Config::CONFIG_PARENT, + ]); + + // C: inherit everything + $conf_c = Config::getConfig($entity_c->getID(), false); + $this->updateItem(Config::class, $conf_c->getID(), [ + 'take_item_group_ticket' => Config::CONFIG_PARENT, + 'take_requester_group_ticket' => Config::CONFIG_PARENT, + ]); + + // Entity A: CONFIG_PARENT resolves to root's value + $resolved_a = Config::getConfig($entity_a->getID()); + $this->assertEquals(1, $resolved_a->fields['take_item_group_ticket'], 'A should inherit take_item_group_ticket=1 from root'); + $this->assertEquals(1, $resolved_a->fields['take_requester_group_ticket'], 'A should keep its own take_requester_group_ticket=1'); + + // Entity B: own value wins over parent, CONFIG_PARENT resolves through A + $resolved_b = Config::getConfig($entity_b->getID()); + $this->assertEquals(0, $resolved_b->fields['take_item_group_ticket'], 'B should keep its own take_item_group_ticket=0'); + $this->assertEquals(1, $resolved_b->fields['take_requester_group_ticket'], 'B should inherit take_requester_group_ticket=1 from A'); + + // Entity C: CONFIG_PARENT resolves to B's effective values (not root's) + $resolved_c = Config::getConfig($entity_c->getID()); + $this->assertEquals(0, $resolved_c->fields['take_item_group_ticket'], 'C should inherit take_item_group_ticket=0 from B (not root=1)'); + $this->assertEquals(1, $resolved_c->fields['take_requester_group_ticket'], 'C should inherit take_requester_group_ticket=1 through B→A'); + } } From 717346cfed32431a594682df6a93c2387be4f36e Mon Sep 17 00:00:00 2001 From: Lainow Date: Fri, 29 May 2026 10:45:20 +0200 Subject: [PATCH 3/4] Fix tests --- src/Config.php | 10 +++-- tests/Units/ConfigTest.php | 84 +++++++++----------------------------- 2 files changed, 26 insertions(+), 68 deletions(-) diff --git a/src/Config.php b/src/Config.php index 52cac93..3188ca7 100644 --- a/src/Config.php +++ b/src/Config.php @@ -265,7 +265,7 @@ public static function getConfig(?int $entityId = null, bool $useInheritance = t if ($useInheritance && $entityId > 0) { $entity = new Entity(); if ($entity->getFromDB($entityId)) { - $parentConfig = self::getConfig($entity->fields['entities_id'], true); + $parentConfig = self::getConfig((int) $entity->fields['entities_id'], true); foreach (self::getItilConfigFields() as $field) { if (($moconfig->fields[$field] ?? 0) == self::CONFIG_PARENT) { $moconfig->fields[$field] = $parentConfig->fields[$field] ?? 0; @@ -287,7 +287,7 @@ public static function install(Migration $migration): void $query = "CREATE TABLE IF NOT EXISTS `$table` ( `id` int unsigned NOT NULL AUTO_INCREMENT, `entities_id` int unsigned NOT NULL DEFAULT '0', - `take_item_group_ticket` tinyint NOT NULL DEFAULT '-2', + `take_item_group_ticket` tinyint NOT NULL DEFAULT '0', `take_item_group_change` tinyint NOT NULL DEFAULT '0', `take_item_group_problem` tinyint NOT NULL DEFAULT '0', `take_requester_group_ticket` tinyint NOT NULL DEFAULT '0', @@ -339,9 +339,13 @@ public static function install(Migration $migration): void 'take_technician_group_change', 'take_technician_group_problem', ] as $field) { - $migration->changeField($table, $field, $field, 'tinyint', ['value' => 0]); + if ($DB->fieldExists($table, $field)) { + $migration->changeField($table, $field, $field, 'bool', ['value' => '0']); + } } + $migration->executeMigration(); + $entities = new Entity(); foreach ($entities->find() as $entity) { if (is_array($entity) && isset($entity['id'])) { diff --git a/tests/Units/ConfigTest.php b/tests/Units/ConfigTest.php index 49cc775..a379b62 100644 --- a/tests/Units/ConfigTest.php +++ b/tests/Units/ConfigTest.php @@ -49,7 +49,6 @@ public function testTaskMandatoryField(): void $conf = $this->getCurrentConfig(); $result = $this->updateTestConfig($conf, [ - 'is_active' => 1, 'entities_id' => 0, 'mandatory_task_category' => 1, 'mandatory_task_duration' => 1, @@ -189,7 +188,6 @@ public function testTicketMandatoryFieldsBeforeCloseTicket(): void // Configure mandatory fields before closing $result = $this->updateTestConfig($conf, [ - 'is_active' => 1, 'entities_id' => 0, 'require_technician_to_close_ticket' => 1, 'require_technicians_group_to_close_ticket' => 1, @@ -317,7 +315,6 @@ public function testCannotAddSolutionWhenMissingMandatoryFields(): void // 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, @@ -405,7 +402,6 @@ public function testChangeMandatoryFieldsBeforeCloseChange(): void // Configure mandatory fields before closing $result = $this->updateTestConfig($conf, [ - 'is_active' => 1, 'entities_id' => 0, 'require_technician_to_close_change' => 1, 'require_technicians_group_to_close_change' => 1, @@ -534,7 +530,6 @@ public function testProblemMandatoryFieldsBeforeCloseProblem(): void // Configure mandatory fields before closing $result = $this->updateTestConfig($conf, [ - 'is_active' => 1, 'entities_id' => 0, 'require_technician_to_close_problem' => 1, 'require_technicians_group_to_close_problem' => 1, @@ -661,7 +656,6 @@ public function testTakeTheRequesterGroup(): void // Configure to take all groups of the requester $result = $this->updateTestConfig($conf, [ - 'is_active' => 1, 'entities_id' => 0, 'take_requester_group_ticket' => 2, // All ]); @@ -736,7 +730,6 @@ public function testTakeTheRequesterGroup(): void $config = new Config(); // Configurer pour ne prendre que le groupe principal du demandeur $result = $this->updateTestConfig($conf, [ - 'is_active' => 1, 'entities_id' => 0, 'take_requester_group_ticket' => 1, // Default ]); @@ -783,7 +776,6 @@ public function testTakeTheRequesterGroup(): void // Reset config // Réinitialiser la configuration $resetResult = $this->updateTestConfig($conf, [ - 'is_active' => 1, 'entities_id' => 0, 'take_requester_group_ticket' => 0, // Default ]); @@ -799,7 +791,6 @@ public function testTakeTheTechnicianGroup(): void // Setup to take all groups of the technician $result = $this->updateTestConfig($conf, [ - 'is_active' => 1, 'entities_id' => 0, 'take_technician_group_ticket' => 2, // All ]); @@ -873,7 +864,6 @@ public function testTakeTheTechnicianGroup(): void // Setup to take only the main group of the technician $result = $this->updateTestConfig($conf, [ - 'is_active' => 1, 'entities_id' => 0, 'take_technician_group_ticket' => 1, // Default ]); @@ -927,7 +917,6 @@ public function testTakeItemGroups(): void // Setup to take the groups of the items $result = $this->updateTestConfig($conf, [ - 'is_active' => 1, 'entities_id' => 0, 'take_item_group_ticket' => 1, ]); @@ -1006,7 +995,6 @@ public function testUpdateTicketActorsOnCategoryChange(): void // Configure to assign technical manager and group when changing category $result = $this->updateTestConfig($conf, [ - 'is_active' => 1, 'entities_id' => 0, 'assign_technical_manager_when_changing_category_ticket' => 1, 'assign_technical_group_when_changing_category_ticket' => 1, @@ -1100,7 +1088,6 @@ public function testUpdateChangeActorsOnCategoryChange(): void // Configure to assign technical manager and group when changing category $result = $this->updateTestConfig($conf, [ - 'is_active' => 1, 'entities_id' => 0, 'assign_technical_manager_when_changing_category_change' => 1, 'assign_technical_group_when_changing_category_change' => 1, @@ -1194,7 +1181,6 @@ public function testUpdateProblemActorsOnCategoryChange(): void // Configure to assign technical manager and group when changing category $result = $this->updateTestConfig($conf, [ - 'is_active' => 1, 'entities_id' => 0, 'assign_technical_manager_when_changing_category_problem' => 1, 'assign_technical_group_when_changing_category_problem' => 1, @@ -1288,7 +1274,6 @@ public function testUpdateActorsDisabledConfiguration(): void // Ensure configuration is disabled $result = $this->updateTestConfig($conf, [ - 'is_active' => 1, 'entities_id' => 0, 'assign_technical_manager_when_changing_category_ticket' => 0, 'assign_technical_group_when_changing_category_ticket' => 0, @@ -1365,59 +1350,50 @@ public function testUpdateActorsDisabledConfiguration(): void */ public function testParentEntityConfigInheritance(): void { - $this->initEntitySession(); + $this->login(); - $parent_entity_id = false; - // Create child entity + // Create child entity under root (entities_id=0) $child_entity = $this->createItem( \Entity::class, [ 'name' => 'Child Entity Test', - 'entities_id' => 0, // Parent entity as parent + 'entities_id' => 0, ], - ['name'], // Entity uses 'completename' not 'name' + ['name'], ); $child_entity_id = $child_entity->getID(); $this->clearLogEntriesContaining('glpiactiveentities_string'); - // Configure parent entity with specific settings + // Configure root entity with specific settings $conf = Config::getConfig(0, false); $this->updateItem( Config::class, $conf->getID(), [ - 'entities_id' => $parent_entity_id, - 'is_active' => true, - 'use_parent_entity' => false, // This is the source config - 'take_item_group_ticket' => true, - 'prevent_closure_ticket' => true, - 'require_technician_to_close_ticket' => true, - 'mandatory_task_category' => true, + 'take_item_group_ticket' => 1, + 'prevent_closure_ticket' => 1, + 'require_technician_to_close_ticket' => 1, + 'mandatory_task_category' => 1, ], ); - // Configure child entity to use parent configuration + // Configure child entity to inherit from parent (CONFIG_PARENT) $this->assertIsInt($child_entity_id); $child_conf = Config::getConfig($child_entity_id, false); $this->updateItem( Config::class, $child_conf->getID(), [ - 'entities_id' => $child_entity_id, - 'is_active' => 1, - 'use_parent_entity' => 1, // Enable inheritance - 'take_item_group_ticket' => 0, // These values should be ignored - 'prevent_closure_ticket' => 0, - 'require_technician_to_close_ticket' => 0, - 'mandatory_task_category' => 0, + 'take_item_group_ticket' => Config::CONFIG_PARENT, + 'prevent_closure_ticket' => Config::CONFIG_PARENT, + 'require_technician_to_close_ticket' => Config::CONFIG_PARENT, + 'mandatory_task_category' => Config::CONFIG_PARENT, ], ); - // Test effective configuration for child entity + // Test effective configuration for child entity — should inherit root values $effective_config = Config::getConfig($child_entity_id, true); - // Should return parent config - $this->assertEquals($parent_entity_id, $effective_config->fields['entities_id']); $this->assertEquals(1, $effective_config->fields['take_item_group_ticket']); $this->assertEquals(1, $effective_config->fields['prevent_closure_ticket']); $this->assertEquals(1, $effective_config->fields['require_technician_to_close_ticket']); @@ -1474,8 +1450,6 @@ public function testMultiLevelParentEntityConfigInheritance(): void $grandparent_conf->getID(), [ 'entities_id' => $grandparent_entity_id, - 'is_active' => 1, - 'use_parent_entity' => 0, // This is the source config 'take_item_group_ticket' => 1, 'prevent_closure_ticket' => 1, 'require_technician_to_close_ticket' => 1, @@ -1490,8 +1464,6 @@ public function testMultiLevelParentEntityConfigInheritance(): void $parent_conf->getID(), [ 'entities_id' => $parent_entity_id, - 'is_active' => 1, - 'use_parent_entity' => 1, // Cascade to grandparent 'take_item_group_ticket' => 0, // Should be ignored 'prevent_closure_ticket' => 0, 'require_technician_to_close_ticket' => 0, @@ -1506,8 +1478,6 @@ public function testMultiLevelParentEntityConfigInheritance(): void $child_conf->getID(), [ 'entities_id' => $child_entity_id, - 'is_active' => 1, - 'use_parent_entity' => 1, // Should cascade to grandparent 'take_item_group_ticket' => 0, // Should be ignored 'prevent_closure_ticket' => 0, 'require_technician_to_close_ticket' => 0, @@ -1525,7 +1495,7 @@ public function testMultiLevelParentEntityConfigInheritance(): void } /** - * Test that child entity without use_parent_entity uses its own config + * Test that child entity without inheritance uses its own config */ public function testChildEntityWithoutInheritanceUsesOwnConfig(): void { @@ -1562,8 +1532,6 @@ public function testChildEntityWithoutInheritanceUsesOwnConfig(): void $parent_conf->getID(), [ 'entities_id' => $parent_entity_id, - 'is_active' => 1, - 'use_parent_entity' => 0, 'take_item_group_ticket' => 1, 'prevent_closure_ticket' => 1, ], @@ -1577,8 +1545,6 @@ public function testChildEntityWithoutInheritanceUsesOwnConfig(): void $child_conf->getID(), [ 'entities_id' => $child_entity_id, - 'is_active' => 1, - 'use_parent_entity' => 0, // NO inheritance 'take_item_group_ticket' => 0, // Different from parent 'prevent_closure_ticket' => 0, ], @@ -1619,8 +1585,6 @@ public function testGetEffectiveConfigUsesCurrentSession(): void $test_conf->getID(), [ 'entities_id' => $test_entity_id, - 'is_active' => 1, - 'use_parent_entity' => 0, 'take_item_group_ticket' => 1, ], ); @@ -1656,8 +1620,6 @@ public function testRootEntityCannotInheritFromParent(): void Config::class, [ 'entities_id' => 0, - 'is_active' => 1, - 'use_parent_entity' => 1, // This should be ignored for root entity 'take_item_group_ticket' => 1, ], ); @@ -1668,7 +1630,6 @@ public function testRootEntityCannotInheritFromParent(): void Config::class, $root_config->getID(), [ - 'use_parent_entity' => 1, // This should be ignored 'take_item_group_ticket' => 1, ], ); @@ -1725,8 +1686,6 @@ public function testControllerUsesEffectiveConfigWithInheritance(): void $parent_conf->getID(), [ 'entities_id' => $parent_entity_id, - 'is_active' => 1, - 'use_parent_entity' => 0, 'mandatory_task_category' => 1, // Enable mandatory task category 'mandatory_task_duration' => 1, ], @@ -1740,8 +1699,6 @@ public function testControllerUsesEffectiveConfigWithInheritance(): void $child_conf->getID(), [ 'entities_id' => $child_entity_id, - 'is_active' => 1, - 'use_parent_entity' => 1, // Inherit from parent 'mandatory_task_category' => 0, // Should be ignored 'mandatory_task_duration' => 0, ], @@ -1787,7 +1744,6 @@ public function testAssignTechnicianFromTask(): void $conf = $this->getCurrentConfig(); $result = $this->updateTestConfig($conf, [ - 'is_active' => 1, 'entities_id' => 0, ]); $this->assertTrue($result); @@ -1852,7 +1808,6 @@ public function testAssignTechnicianFromChangeTask(): void $conf = $this->getCurrentConfig(); $result = $this->updateTestConfig($conf, [ - 'is_active' => 1, 'entities_id' => 0, ]); $this->assertTrue($result); @@ -1917,7 +1872,6 @@ public function testAssignTechnicianFromProblemTask(): void $conf = $this->getCurrentConfig(); $result = $this->updateTestConfig($conf, [ - 'is_active' => 1, 'entities_id' => 0, ]); $this->assertTrue($result); @@ -1992,7 +1946,7 @@ public function testAssignTechnicianFromProblemTask(): void */ public function testMultiLevelInheritanceResolvesConfigParentThroughChain(): void { - $this->initEntitySession(); + $this->login(); $entity_a = $this->createItem( \Entity::class, @@ -2004,14 +1958,14 @@ public function testMultiLevelInheritanceResolvesConfigParentThroughChain(): voi $entity_b = $this->createItem( \Entity::class, ['name' => 'Inheritance Test B', 'entities_id' => $entity_a->getID()], - ['name', 'entities_id'], + ['name'], ); $this->clearLogEntriesContaining('glpiactiveentities_string'); $entity_c = $this->createItem( \Entity::class, ['name' => 'Inheritance Test C', 'entities_id' => $entity_b->getID()], - ['name', 'entities_id'], + ['name'], ); $this->clearLogEntriesContaining('glpiactiveentities_string'); From 7b7c868811a5fcb9a422437616b32282271c387a Mon Sep 17 00:00:00 2001 From: Lainow Date: Fri, 29 May 2026 11:04:09 +0200 Subject: [PATCH 4/4] Fix lint --- src/Config.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Config.php b/src/Config.php index 3188ca7..de7fdd9 100644 --- a/src/Config.php +++ b/src/Config.php @@ -350,7 +350,7 @@ public static function install(Migration $migration): void foreach ($entities->find() as $entity) { if (is_array($entity) && isset($entity['id'])) { $entity_id = (int) $entity['id']; - if ($DB->numrows($DB->request(['FROM' => self::getTable(), 'WHERE' => ['entities_id' => $entity_id]])) > 0) { + if (countElementsInTable(self::getTable(), ['entities_id' => $entity_id]) > 0) { continue; } $data = ['entities_id' => $entity_id];