Fix rule clone losing criteria and actions

[TarekRemo]

Checklist before requesting a review

Please delete options that are not relevant.

• I have read the CONTRIBUTING document.
• I have performed a self-review of my code.
• I have added tests that prove my fix is effective or that my feature works.
• This change requires a documentation update.

Description

• It fixes !45508
• Cloning a business rule (from the massive actions of "Business rules for tickets") sometimes creates a clone without any criteria nor action, and only one clone is created even when several copies are requested.
• The following exception is raised (it aborts the clone loop right after the rule row has been inserted, and before its relations are copied):

Uncaught PHP Exception RuntimeException: "Cannot guess field for itemtype Rule on RuleAction"
./src/CommonDBChild.php:1039
./src/Glpi/Features/Clonable.php:121   CommonDBChild::getItemField()
./src/Glpi/Features/Clonable.php:299   Rule->cloneRelations()
./src/Glpi/Features/Clonable.php:229   Rule->clone()
./src/MassiveAction.php:1657           Rule->cloneMultiple()

• The cause is most likely due to DbUtils::getTableForItemType() registering both directions of the itemtype/table mapping:

$CFG_GLPI['glpitablesitemtype'][$itemtype] = $table;   // RuleTicket => glpi_rules   OK
$CFG_GLPI['glpiitemtypetables'][$table]    = $itemtype; // glpi_rules => RuleTicket  wrong

Rule::getTable() returns glpi_rules for every subclass (example: RuleTicket). So when the table of such a subclass is resolved, getItemTypeForTable('glpi_rules') returns RuleTicket instead of Rule for the rest of the request.

CommonDBChild::getItemField() then compares getItemtypeForForeignKeyField('rules_id')
(now RuleTicket) with Rule::class, does not match so it throws an exception.

Fix :

In CommonDBChild::getItemField() , compare tables rather than itemtypes.

[cconard96]

Rule::getTable() returns glpi_rules for every subclass (example: RuleTicket). So when the table of such a subclass is resolved, getItemTypeForTable('glpi_rules') returns RuleTicket instead of Rule for the rest of the request.

If this is this underlying issue, couldn't this come up in other places besides CommonDBChild::getItemField?

[TarekRemo]

Rule::getTable() returns glpi_rules for every subclass (example: RuleTicket). So when the table of such a subclass is resolved, getItemTypeForTable('glpi_rules') returns RuleTicket instead of Rule for the rest of the request.

If this is this underlying issue, couldn't this come up in other places besides CommonDBChild::getItemField?

searched and found that it could come up in CommonDBTM::cleanRelationData() (line 927) where getItemTypeForTable() could return the cached itemtype which can be one of the sub-classes of the actual owner of the table.

TO fix it I extracted the name resolving logic out of getItemTypeForTable() into a public method getExpectedItemTypeForTable(string $table) and used it in CommonDBTM::cleanRelationData().

[cconard96]

I don't think it is easy to assume we know every path the code may take for every user input to replicate the underlying issue (AJAX endpoints, legacy front files, controllers, legacy API, new API, plugins, etc).

[TarekRemo]

I don't think it is easy to assume we know every path the code may take for every user input to replicate the underlying issue (AJAX endpoints, legacy front files, controllers, legacy API, new API, plugins, etc).

True, but I think this fix is necessary here anyway (especially in CommonDBChild::getItemField). If the problem comes up elsewhere in the future, we'll handle it then.