search

glpi-project / glpi

GLPI is a Free Asset and IT Management Software package, Data center management, ITIL Service Desk, licenses tracking and software auditing.
https://glpi-project.org
GNU General Public License v3.0
4.16k stars 1.28k forks source link

Improve FilterableTrait performances #17382

Closed AdrienClairembault closed 3 months ago

AdrienClairembault commented 3 months ago

(Requested by @orthagh).

Filters on webhook/notifications are hard to manage on big databases due to potential slow search queries that are executed when previewing the filter results.

A quick image to remind you what this feature look like: image

Save button behavior

The first issue is that saving a filter also update the preview; thus forcing execution of a potential slow query.

Now, the preview is no longer updated when a filter is saved. It is instead hidden (thus avoiding displaying preview information that may be outdated), if the user want to confirm the results again he is free to manually click the preview button and check the results.

Prevent useless background query

The second issue is that an useless preview search query is executed when the page is loaded. This was not meant to be this way and is just a side effect of using the search engine in our page.

The default behavior of the search engine is to run its query when it is rendered, so its content was manually set to d-none on the filter page until the user clicked on "preview" for the first time.

This was great at hiding the results but the query is still executed when the page is loaded (and is never actually displayed since clicking "preview" will trigger a new independent ajax search) so it is bad for performances.

To fix this, I've added a specific parameter to the search engine that disable the query execution. This mean we can still load the search engine output (as its container need to be in the page for ajax searches to work) without any real content.

This new parameter is only supported by the HtmlTableOutput output type to be safe, we could enable it for others outputs (map ?) later if needed.

This solution seems the simpler given the actual state of our search engine. I don't think we can do better until it is rewritten.

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets !33503
cconard96 commented 3 months ago

The preview display seems broken. No rows/columns are shown and no error is shown either.

AdrienClairembault commented 3 months ago

The preview display seems broken. No rows/columns are shown and no error is shown either.

What item and filter are you using ? I don't reproduce the issue on my side :/

cconard96 commented 3 months ago

The preview display seems broken. No rows/columns are shown and no error is shown either.

What item and filter are you using ? I don't reproduce the issue on my side :/

Under a webhook with the itemtype set to Computer (and tested with Followups) I saved a blank filter and then clicked preview. Selection_347

Same thing happens even if I add more specific criteria to the filter.

cconard96 commented 3 months ago

The preview button isn't triggering any AJAX request for me. It looks like it is just showing the search engine output that already existed on the page.

orthagh commented 3 months ago

Under a webhook with the itemtype set to Computer (and tested with Followups) I saved a blank filter and then clicked preview.

It's strange, with the same scenario, I have the preview displaying correctly. I set itemtype=Computer, event=update

Edit: I tested also with event=new and chrome browser (previously firefox), everything seems ok

AdrienClairembault commented 3 months ago

The only error and my side was already there before my changes :

PHP Warning (2): Undefined array key "as_map" in /glpi/main/src/Glpi/Search/Output/HTMLSearchOutput.php at line 163

Even if it not related to this PR I can fix it to see if it helps you.

AdrienClairembault commented 3 months ago

Could it also be some cache issue on your side ?

cconard96 commented 3 months ago

Could it also be some cache issue on your side ?

Indeed 🙃. All good on my end now.