Fix ITIL Item Tabs

[cconard96]

Checklist before requesting a review

• 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

Several issues were seen with various ITIL Item tabs including wrong tab counts, "Show All" button link mixing SQL criteria and search engine criteria, and some redundant/messy code. This aims to resolve #18985 and #19378. This PR may will turn into a rewrite due to the scope of the issues.

[cconard96]

Need an initial review to make sure I haven't missed any expected functionality. Given the large number of deletions vs additions, not including the new tests, it is possible something was accidentally removed rather than there just being a ton of duplicated code.

I also can never seem to figure out how tab counters are supposed to be handled when it comes to some items not being visible within the current entity or not visible because of item-specific permissions. Should tab counters always show the actual count, or visible count? Seems like the actual count would be the right option along with a note in the tab that some items are not currently visible, but that isn't how it is displayed in most places.

The use of CommonItilObject_Item to link items with TicketRecurrent which isn't an ITIL Object complicates things but since it was like this in 10.0, I have left it like this rather than change it within the GLPI 11 beta phase.

I am aware that some E2E tests are needed still.

[cedric-anne]

Need an initial review to make sure I haven't missed any expected functionality. Given the large number of deletions vs additions, not including the new tests, it is possible something was accidentally removed rather than there just being a ton of duplicated code.

Honestly, it is hard to detect it, due to the amount of code changes. The most critical parts are probably tested (rules, for instance).

Seems like the actual count would be the right option along with a note in the tab that some items are not currently visible, but that isn't how it is displayed in most places.

I agree that we should always display the actual count and with a note that indicates how many items are not shown due to visibility restrictions, but this is not how things are done right now. Maybe we should write a RFC about this in order to validate what should be the target. For the moment, I guess counters should just show how many items are visible.

[cconard96]

The use of CommonItilObject_Item to link items with TicketRecurrent which isn't an ITIL Object complicates things but since it was like this in 10.0, I have left it like this rather than change it within the GLPI 11 beta phase.

Actually it looks like this just missed the 10.0 release. So, I guess it could be refactored now.

Honestly, it is hard to detect it, due to the amount of code changes. The most critical parts are probably tested (rules, for instance).

At least the "Add ability to define linked items on TicketRecurrent" feature had no tests added for it. I don't think tests existed for how ITIL items on assets were supposed to be counted/displayed either. It seems like assets are supposed to count tickets on connected peripherals as their own tickets.

Also, it isn't an immediate concern but I am interested in people's thoughts about the TODO comment I added:

        //TODO Costs for changes and problems should probably affect TCO too but there should also be a way to handle costs affecting multiple assets
        //Example, A ticket with a cost of $400 with two computers shouldn't add $400 cost of ownership to both.

[cedric-anne]

Actually it looks like this just missed the 10.0 release. So, I guess it could be refactored now.

If it simplifies something, you can refactor it, but keep in mind that we have to publish the RC release in the next few weeks and we will probably not accept a huge refactoring after that.

[cconard96]

Actually it looks like this just missed the 10.0 release. So, I guess it could be refactored now.

If it simplifies something, you can refactor it, but keep in mind that we have to publish the RC release in the next few weeks and we will probably not accept a huge refactoring after that.

I don't think I'll have time for additional refactoring with this.