OpenMage / magento-lts

Official OpenMage LTS codebase | Migrate easily from Magento Community Edition in minutes! Download the source code for free or contribute to OpenMage LTS | Security vulnerability patches, bug fixes, performance improvements and more.
https://www.openmage.org
Open Software License 3.0
870 stars 436 forks source link

All attributes set to be used for search appear in layering navigation when there are no search results #4055

Closed addison74 closed 4 months ago

addison74 commented 4 months ago

Latest OM version cloned from the repository, DDEV + PHP 8.2 + MariaDB.

Steps to reproduce this issue:

  1. Set a few attributes with the option [Used in Layered Navigation] as Filtrable (with results)
  2. Search in the Frontend for a word that doesn't return any result
  3. Look at the layering navigation block. All your attributes appear with 0 results which is not a Magento correct behavior. If there are no results then no filterable attributes are displayed.

I investigated this issue in detail and I found that the changes made in this file

/app/code/core/Mage/CatalogSearch/Model/Resource/Fulltext/Collection.php

are causing the problem.

IMPORTANT NOTE: I moved forward and I used the file version before the changes and the problem no longer occurs, but I am not able to search for words that returned results before.

I compared the two versions and the changes are many so that I can get to the source of the problem.

fballiano commented 4 months ago

thanks to git bisect I've identified that the problem is generated by commit fc97382ffcf9bb865346de078aafb7996564c7d1, PR https://github.com/OpenMage/magento-lts/pull/2993

@davidhiendl do you have any idea?

davidhiendl commented 4 months ago

It's possible this problem is caused by newer PHP versions, will need some investigation. I cannot reproduce the problem on my 7.4/8.1 test env's.

davidhiendl commented 4 months ago

Also cannot reproduce it on a clean 8.2 instance.

davidhiendl commented 4 months ago

Just to be clear on the reproduction steps:

addison74 commented 4 months ago

It is not related to the PHP version. DDEV allows me to switch in a second any version from 7.4 to 8.3. If I am using a file version before your change in /app/core/local/... the layering navigation doesn't show the 0 values for all searchable attributes but I get another issue, the search feature is not functioning anymore even for words with results.

Steps for reproduction the issue are in the first post. It is not related to products with option.

addison74 commented 4 months ago

It is not related to the PHP version. DDEV allows me to switch in a second any version from 7.4 to 8.3. If I am using a file version before your change in /app/core/local/... the layering navigation doesn't show the 0 values for all searchable attributes but I get another issue, the search feature is not functioning anymore even for words with results. Steps for reproduction the issue are in my initial post. It is not related to products with option.

The screenshot below proves the issue I reported. I have created an attribute that has multiple values. I searched for the word "potato" which does not exist, the result can be seen in the image. What changed that PR is that OM displays the "SHOP BY" block on the left side and lists all the attributes set for the search. This is wrong, that block should not be displayed if there are no results.

screenshot

To reproduce the issue that attribute must have the value for [Use in Layered Navigation] set to Filterable (with results). All the attributes that have this value set are displayed in the side block when there are no results, obviously the displayed number is zero. This is the issue we must fix ASAP, because all the stores in production are affected by.

screenshot_2

davidhiendl commented 4 months ago

Okay I've finally managed to reproduce it but the reason is not directly related to EAV PR. If use set "Use in layered navigation" to "Filterable (with results") it will NOT appear in search unless you also set "use in search results layered navigation" to "no". If you set "Use in search results layered navigation" to "true" it will disappear. But also of note: The "Use in search results layered navigation" true/false value doesn't actually show/hide it in search results.

addison74 commented 4 months ago

I appreciate that you took the time to test and it's good that you were able to reproduce the problem. The issue is that no changes should be made in the attributes as long as they worked in that configuration before a recent upgrade. Something happened over time in the source code because I am upgrading an OM from 2022 that works correctly, but by updating to the latest version I noticed this problem.

As you mentioned there is an issue there that led me to the file Collection.php. Replacing this file with an old one I can hide the layering navigation when there are no results.

davidhiendl commented 4 months ago

/app/code/core/Mage/CatalogSearch/Model/Resource/Fulltext/Collection.php has no functional changes (only codeblock, comments and similar since 2022.

addison74 commented 4 months ago

Indeed, it is a problem related to the combination of "Use in Layered Navigation" and "Use in Search Results Layered Navigation".

Scenario 1 ULN = Filterable (with results) USRLN = No

For a word with results, all values ​​with zero will be displayed for this attribute, which is not correct, because since they are zero, they no longer need to be displayed - they have no links, they consumes layout space on the page, they look terible.

Scenario 2 ULN = Filterable (with results) USRLN = Yes

For a word with results, all zero values ​​from scenario 1 are no longer displayed, which is correct.

Scenario 3 ULN = Filterable (with results) USRLN = Yes

For a word with no results, all attributes are displayed and the options have the value zero. Here again is a problem.

I replaced the entire directory /app/code/core/Mage/CatalogSearch with an older version and the problem disappeared. We have to find out when the OpenMage source code was altered and the initial behavior that we inherited from Magento changed.

davidhiendl commented 4 months ago

The main cause is Mage_Catalog_Model_Layer::getFilterableAttributes used to call _prepareAttributeCollection which applied a is_filterable = 1 condition. The _prepareAttributeCollection method is however overwritten by Mage_CatalogSearch_Model_Layer which is why this happens. There is however a second, unrelated bug regarding the toggling of attribute options I mentioned earlier but I confirmed its present even before.