-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[5.4] Optimize smart search module #45347
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
I have tested this item ✅ successfully on 7826570 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45347. |
I have tested this item ✅ successfully on 7826570 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45347. |
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45347. |
While I agree with these changes, I also don't think it is a bugfix and thus should go into 5.4 |
OK, changed to 5.4 |
As it was a clean rebase without conflicts, the tests and RTC are still valid, and I've just reviewed the changes. They are all ok. |
Summary of Changes
Smart Search module creates new Query and triggers
Joomla\Component\Finder\Administrator\Indexer::processString()
method which loads available taxonomy branch titles via separate SQL query per each module (duplicate query).We should not process empty string, when the module is loaded without any input. Plus,
Taxonomy::getBranchTitles()
load should use static cache and produce only single query to prevent duplicate when we have multiple mod_finder instances (i.e. typical case for Yootheme templates where one module is rendered in header and second in mobile dialog).Testing Instructions
Create two Smart Search modules, apply patch,
Actual result BEFORE applying this Pull Request
Load page with Smart Search modules, see duplicate queries like:
Expected result AFTER applying this Pull Request
No useless queries, the mod_finder render time is also faster now.
Link to documentations