search

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
866 stars 436 forks source link

Fixed regression introduced in #2993 where attributes are not correctly pre-filtered for the layered navigation #4063

Closed davidhiendl closed 3 months ago

davidhiendl commented 3 months ago

Description (*)

Fix a regression introduced with the EAV overhaul where the filterable and filterable in search attribute options are not honored correctly.

Notes: For some reason the Mage_CatalogSearch_Model_Layer::_prepareAttributeCollection used to include a is_visible condition but not the parent for regular catalog layered nav. I've reproduced this behavior in the interest of having a usable fix quickly but I think it might cause the other issues found during the debugging of #4055

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

See issue #4055

fballiano commented 3 months ago

I can confirm this PR fixed the problem

ADDISON74 commented 3 months ago

@davidhiendl - Thank you for your fast reaction in solving this issue.

"Use in Layered Navigation" = USL "Use in Search Results Layered Navigation" = USRLN

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

For a word with results, all values ​​with zero will not be displayed for the attribute, which is correct, because since they are zero, they no longer need to be displayed.

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

The same as before which is correct.

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

For a word with no results, there are no attributes displayed in the layering navigation, which is correct.

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

The same as before which is correct.

I'll let you decide if those changes proposed by @fbaliano are requested, Otherwise PR solves the problem reported by me.

fballiano commented 3 months ago

since we have many instances of the same kind of typing in the main branch, I'll commit and merge those so that I can release 20.10 ;-)