[GLPI 10.0.0] BR for Tickets does not receive correct Actors

[VladoTTX]

Code of Conduct

• I agree to follow this project's Code of Conduct

Is there an existing issue for this?

• I have searched the existing issues

Version

10.0.0

Bug description

There were changes to code how Actors are handled in version 10.0.0 on frontend.
BR for tickets were not fully updated to handle actors in input as _actors array.
Previously they were defined for each type as array (example_groups_id_assign) which was submitted directly by form.

This causes that BR for Tickets do not receive complete information about current actors on Tickets.
If there is rule with has criteria based on actors like _groups_id_assign or others, rule is not working correctly.
Code will need probably to copy the _actors arrays to old arrays users_id* groups_id* if we want to make rules working and keep backward compatibility at the same time.

Relevant log output

No response

Page URL

No response

Steps To reproduce

1. Create BR for Tickets rule with criteria based on Actors (example "Technician Group" does not exist)
2. Create Ticket and Add Technician Group during ticket creation before "Save"
3. Rule will match even when there is Technician Group defined

Your GLPI setup information

Stock 10.0.0 without plugins

Anything else?

Current BR For tickets still uses _groups_id_assign for example here:

glpi/src/RuleTicket.php

Line 581 in dfe8065

$criterias['_groups_id_assign']['table'] = 'glpi_groups';

[trasher]

Please try with latests nightly build

[VladoTTX]

Tried with latest 10.0 nightly from 24.5.2022:
https://nightly.glpi-project.org/glpi/10.0-e26f2f2.tar.gz

Behavior is still the same. Also checked repository and there does not seem to be fixes for this part of code.

[VladoTTX]

Thanks for the fix.
Is it really necessary to update actors here at prepareInputDataForProcess?
It gets called for each Rule so the indexes _{users|groups}id* get multiplicated with same data.
If yes it should be probably changed to check for existence of items_id in array like:
if(!in_array($actor['items_id'],$input[$input_key])) $input[$input_key][] = $actor['items_id'];

[VladoTTX]

Guys, there is one issue with fix #11728 and I would like to hear your opinion on this..
The issue is, actions on Actors are called now two times.
Once in CommonITILObject::prepareInputForUpdate() is calling addAdditionalActors() which will act on _addtional arrays.
And second time actors are processed in CommonITILObject::post_updateItem() where this time CommonITILObject::updateAc]tors() is called, which works on _actors array.
I am not sure whether this needs to be done in different part of processing as it leads to issues..
Example it breaks Escalade plugin functionality "Remove old assign group on new group assign" as first actors are added in prepareInputForUpdate() which is catched by Escalade plugin on Group_Ticket which removes 1st group in DB.
Later on it is revered by CommonITILObject::updateAc]tors() as it founds that group is present in _actors array and adds it back as it was submitted on form.

Would it be possible to move updateActors call to prepareInputforUpdate method before addAdditionalActors() call?
Or maybe update addAdditionalActors in way, it will update _actors arrays which will be processed later on, instead of calling add() directly on linking classes?

[cedric-anne]

@VladoTTX

Can you test #11957 ? With proposed changes, all actors update will be made at the same moment, in CommonITILObject::updateAc]tors() which is call on CommonITILObject::post_addItem()/CommonITILObject::post_updateItem().

[VladoTTX]

Hi,
Thanks much for rework of the whole thing!
Works fine except one issue..
New code seems to break the assignment by category or HW (eg. setTechAndGroupFromItilCategory() or setTechAndGroupFromHardware()).
Arrays seems to be initialized to empty value:
https://github.com/cedric-anne/glpi/blob/93e8850847351d2ace08c40e2d75903ccfa92324/src/CommonITILObject.php#L8961

Also question is what should we do in plugins if we want to assign Ticket to some actor? Should we alter input['_actors'] or input['_(groups|user)_id_assign'] in pre_item_add?

[cedric-anne]

Hi, Thanks much for rework of the whole thing! Works fine except one issue.. New code seems to break the assignment by category or HW (eg. setTechAndGroupFromItilCategory() or setTechAndGroupFromHardware()). Arrays seems to be initialized to empty value: https://github.com/cedric-anne/glpi/blob/93e8850847351d2ace08c40e2d75903ccfa92324/src/CommonITILObject.php#L8961

#12130 should fix this issue.

Also question is what should we do in plugins if we want to assign Ticket to some actor? Should we alter input['_actors'] or input['_(groups|user)_id_assign'] in pre_item_add?

You should add an entry in input['_(groups|user)_id_assign'].

[cedric-anne]

Also question is what should we do in plugins if we want to assign Ticket to some actor? Should we alter input['_actors'] or input['_(groups|user)_id_assign'] in pre_item_add?

You should add an entry in input['_(groups|user)_id_assign'].

My bad. If _actors key is present (from UI), you should alter it, but if it is not present, you should alter input['_(groups|user)_id_assign']. Indeed, code expect _actors to contains all actors (both existing and new actors, for all roles), so it will erase input['_(groups|user)_id_assign'], but if it is not present, you will have to rebuilt it with existing actors if you do not want them to be erased all, so it is preferable to use input['_(groups|user)_id_assign'] in this case.

[VladoTTX]

Thanks the patch works (possibly assignment from category worked also before, my fault..)
what was not working was our plugin which modified _groups_id_assign based on some conditions in pre_item_add.
According to your comment we should alter _actors if it is present, if not we should construct _groups_id_assign.
I am OK with that will modify our plugin, so this should be the case with all the plugins which want to be compatible with GLPI10..

Maybe would not be better to not reset old index keys here for backward compatibility or you want to get rid of these arrays in near future?
https://github.com/cedric-anne/glpi/blob/93e8850847351d2ace08c40e2d75903ccfa92324/src/CommonITILObject.php#L8961