Reports Feature Refactor + UI

[cconard96]

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	-

This wasn't meant to be a rewrite of the report feature, but the required refactoring is extensive.

1. Removal of data retrieval and UI from front files.
2. Separation of data retrieval and UI (Prerequisite for HL API).
3. Twig templates for UI.
4. Addition of some generic report rendering methods for the various types of reports.
5. Cleanup of the display of some reports.
   • Default report had counts with potentially confusing labels. It would list all assets by type, OS by name and then asset types (ComputerType, etc) by the itemtype name (Computer) in the same list. Now, the labels are like "Assets > Computers", "Operating Systems > Fedora Linux 38", "Types > Computers > QEMU", etc.
6. Conversion of forms to use GET instead of POST (both were used in different reports).
7. Unification of the report and its criteria (some reports have a URL just for the criteria and another just for the result).

[cconard96]

Some help is needed regarding the graphs that are supposed to show for the two Infocom reports.

[trasher]

When trying to change report from the dropdown, it seems to rely on configured URI instead of current one (as it's done in master). My GLPI URI is http://glpi.loclahost, changing report goes to http://localhost/glpi

[trasher]

I think I've never used that part before; I really do not know how it should work.

On default report, I have quite a different result: on current main:

On your PR:

Event if the new display may be correct (as I do not understand the old one at all); at least the last line seems strange.

Also, please rebase, I had to switch between main and your branch, and I had to rerun dependencies install each time.

[cconard96]

Event if the new display may be correct (as I do not understand the old one at all); at least the last line seems strange.

Also, please rebase, I had to switch between main and your branch, and I had to rerun dependencies install each time.

The new format still isn't great, but better. The last line indicates that it is the count for Computers with no type (----- being the display value for an empty selection). In the old report, it is the ----- on its own under the "Computer" line which is supposed to be a section heading, but there is 0 indication of that in the old report.

All the lines without a count that were in the old report are skipped in the new one to reduce noise.

[trasher]

"----" is the empty default for dropdowns, I'm really not sure that make sense for anyone outside this context -but I understand it was present before. Maybe it's the good time to change for something more explicit?

[cconard96]

"----" is the empty default for dropdowns, I'm really not sure that make sense for anyone outside this context -but I understand it was present before. Maybe it's the good time to change for something more explicit?

N/A is used in other places. Perhaps that but also in italics?

[trasher]

"----" is the empty default for dropdowns, I'm really not sure that make sense for anyone outside this context -but I understand it was present before. Maybe it's the good time to change for something more explicit?

N/A is used in other places. Perhaps that but also in italics?

It seems more easy to understand to me, yes

[trasher]

On report.infocom.php, I have the following error:

[2024-05-16 12:58:34] glpiphplog.CRITICAL:   *** Uncaught Exception TypeError: Unsupported operand types: int + string in /var/www/html/private/glpi/src/Report.php at line 1652

[trasher]

On report.infocom.php, I have the following error:

[2024-05-16 12:58:34] glpiphplog.CRITICAL:   *** Uncaught Exception TypeError: Unsupported operand types: int + string in /var/www/html/private/glpi/src/Report.php at line 1652

Problem is $item['anv'] can be '-'

[cconard96]

On report.infocom.php, I have the following error:

[2024-05-16 12:58:34] glpiphplog.CRITICAL:   *** Uncaught Exception TypeError: Unsupported operand types: int + string in /var/www/html/private/glpi/src/Report.php at line 1652

Problem is $item['anv'] can be '-'

Ah. The phpdoc for Infocom::Amort is wrong.

[trasher]

Ah. The phpdoc for Infocom::Amort is wrong.

Well.. Most of "amort" part is wrong :D

What I do not understand is I have not that issue on main, but I also do not have the "ANV" column displayed:

[cconard96]

What I do not understand is I have not that issue on main, but I also do not have the "ANV" column displayed

In French the label is VNC.

[trasher]

In French the label is VNC.

Indeed, I did not pay attention lang has switched (I usually work in english :D).

Another issue: I do not have the search form (Start/End date) with "Other financial and ..." (no error in logs).

[trasher]

On network report:

There is no "Network socket":

I have no location in my test DB, so no idea if anything wrong in this part, I only can use "By hardware". On main branch, I have data in the table for all equipements I can choose, but on your PR, I always have "No data".

Once search has been done, on main, dropdowns are no longer displayed. They are in your PR (good point I think), but current entry is not selected (maybe it's related to previous point).

[trasher]

End of my tests:

• Loan report: seems I do not have any data, and I do no know what's it's related to,
• Status report: OK!

[cconard96]

On network report:

There is no "Network socket"

I have no location in my test DB, so no idea if anything wrong in this part, I only can use "By hardware". On main branch, I have data in the table for all equipements I can choose, but on your PR, I always have "No data".

Once search has been done, on main, dropdowns are no longer displayed. They are in your PR (good point I think), but current entry is not selected (maybe it's related to previous point).

Old report hid criteria if there were no items. Seemed confusing and an extra check that wasn't needed, so criteria is always visible now. All reports now have a single page for the search and display, so no more of the criteria going away and making it difficult to generate new reports.

Loan report is related to reservations (bad name).

[trasher]

Loan report is related to reservations (bad name).

OK, I have an SQL error (empty in are not allowed) trying to add a new reservation :/ I'll try that later

[trasher]

Loan report is related to reservations (bad name).

OK, I have an SQL error (empty in are not allowed) trying to add a new reservation :/ I'll try that later

ON another DB, I do not have the problem. Loan report is OK too.

[trasher]

So, remaining issues:

• [ ] I do not have the search form (Start/End date) with "Other financial and ..." (no error in logs).
• [x] Network report, "By hardware" I have data in the table for all equipements I can choose, but on your PR, I always have "No data".
• [x] Network report, "By hardware" searched entry is not selected
• [x] when changing report from the dropdown, we should not rely on configured URL

[cconard96]

So, remaining issues:

* [ ]  I do not have the search form (Start/End date) with "Other financial and ..." (no error in logs).

Report not implemented at all because it had no tabular data. The only thing it showed are the graphs which already seemed broken and I have no idea how they are supposed to work.

* [ ]  Network report, "By hardware" I have data in the table for all equipements I can choose, but on your PR, I always have "No data".
* [ ]  Network report, "By hardware" searched entry is not selected

Maybe fixed.

* [ ]  when changing report from the dropdown, we should not rely on configured URL

Fixed

[trasher]

So, remaining issues:

* [ ]  I do not have the search form (Start/End date) with "Other financial and ..." (no error in logs).

Report not implemented at all because it had no tabular data. The only thing it showed are the graphs which already seemed broken and I have no idea how they are supposed to work.

Unfortunately, I cannot help :/

All other issues are fixed; LGTM.

[trasher]

If the "Other financial and ..." report is broken; I propose to just drop it. ping @orthagh?

[orthagh]

If the thing is broken since a long time, you may remove it. I would want a bit more investigation on the history of the report

[trasher]

I'll take an eye

[trasher]

OK, it seems my first tests were wrong. I've just tested again on a 10.0/bf fresh install; and graphs are working

[trasher]

I still have an empty page for "Other financial" report

[cconard96]

I don't recreate that issue. Do you have some purchase data for non-assets in the selected time range?

[trasher]

I cannot select any time range ^^

[cconard96]

I guess there should be a PHP error in the logs for this.

[trasher]

Unfortunately no; there is absolutely nothing in logs :( I'll try again today; I'll take care of remonving all possible cache to be sure.