Catalogsearch page with catalog permissions throws exception

[kozie]

Issue Brief

The catalogsearch page (with catalog permission active) throws an exception resulting in an error 500 page.

Environment

• PHP Version: 7.2
• Magento Version: 2.3.2 EE
• Tweakwise Version: 1.14.12
• Tweakwise Export Version: 1.3.8
• Magento Deploy Mode: production

Steps to reproduce

1. Configure catalog-permissions accordingly (Magento EE)
2. Search for a product

Actual result

Got error 'PHP message: PHP Fatal error:  Uncaught Error: Call to a member function getAttributeCode() on null in /var/www/vhosts/site/httpdocs/vendor/magento/module-catalog-permissions/Model/Plugin/Catalog/Model/Layer/FilterList.php:123\nStack trace:\n#0 /var/www/vhosts/site/httpdocs/vendor/magento/module-catalog-permissions/Model/Plugin/Catalog/Model/Layer/FilterList.php(89): Magento\\CatalogPermissions\\Model\\Plugin\\Catalog\\Model\\Layer\\FilterList->removePriceFiltersFromList(Array)\n#1 /var/www/vhosts/site/httpdocs/vendor/magento/framework/Interception/Interceptor.php(146): Magento\\CatalogPermissions\\Model\\Plugin\\Catalog\\Model\\Layer\\FilterList->afterGetFilters(Object(Magento\\Catalog\\Model\\Layer\\FilterList\\Interceptor), Array, Object(Magento\\Catalog\\Model\\Layer\\Search\\Interceptor))\n#2 /var/www/vhosts/site/httpdocs/vendor/magento/framework/Interception/Interceptor.php(153): Magento\\Catalog\\Model\\Layer\\FilterList\\Interceptor->Magento\\Framework\\Interception\\{closure}(Object(Magento\\Catalog\\Model\\Layer\\Search\\Intercepto...\n'

Expected result

Working catalogsearch page

Additional information

I understand that this issue is quite specific to the configured filters. However, i noticed that within the Tweakwise specific code it's optional to pass an attribute model along with the filters whereas in the catalog-permission it's necessary.

So on the following line of code the attribute could also be null: https://github.com/EmicoEcommerce/Magento2Tweakwise/blob/v1.4.12/src/Model/Catalog/Layer/FilterList/Tweakwise.php#L83

The plugin code that's triggered afterGetFilters is as follows:

public function afterGetFilters(\Magento\Catalog\Model\Layer\FilterList $subject, array $result)
    {
        if (!$this->permissionsConfig->isEnabled()) {
            return $result;
        }
        $currentCategory = $this->registry->registry('current_category');
        if (!$currentCategory) {
            return $this->isPriceAllowedInConfig() ?
                $result :
                $this->removePriceFiltersFromList($result);
        }
        $currentCategoryId = $currentCategory->getId();
        $permissions = $this->permissionIndex->getIndexForCategory(
            $currentCategoryId,
            $this->customerSession->getCustomerGroupId(),
            $this->storeManager->getStore()->getWebsiteId()
        );
        /**
         * Remove price filters unless prices permissions are allowed for specific category
         * or no specific category permissions set and allowed in config
         */
        if (!empty($permissions[$currentCategoryId]['grant_catalog_product_price'])) {
            if ($permissions[$currentCategoryId]['grant_catalog_product_price'] == Permission::PERMISSION_ALLOW) {
                return $result;
            }
        } elseif ($this->isPriceAllowedInConfig()) {
            return $result;
        }
        return $this->removePriceFiltersFromList($result);
    }

    /**
     * Remove price filters from resulting filters array
     *
     * @param array $filtersArray
     * @return array
     */
    private function removePriceFiltersFromList(array $filtersArray)
    {
        /** @var \Magento\Catalog\Model\Layer\Filter\AbstractFilter $filter */
        foreach ($filtersArray as $key => $filter) {
            try {
                $attribute = $filter->getAttributeModel();
                if ($attribute->getAttributeCode() == ProductInterface::PRICE) {
                    unset($filtersArray[$key]);
                    // no break until all price filters will be removed
                }
            } catch (LocalizedException $e) {
            }
        }
        return $filtersArray;
    }

namely within the function removePriceFiltersFromList the rule $attribute = $filter->getAttributeModel(); does not return an attribute.

[edwinljacobs]

Hi,

The reason why the attribute model is optional is that tweakwise provides filters which are combinations or derived from attribute values but do not directly come from magento attributes. So in general its not clear that a filter has an associated attribute model in magento.

As you correctly point out, the attribute model will be added to the tweakwise filter if it exists (in initFilters) in some cases there is no attribute model. I added a fix for this issue in branch: issue/85-catalog-permissions-exception

The fix involves a plugin on a plugin, I am not entirely happy with the fix but don't see another way. The point is that we want the added code to execute only in the scope of enterprise and not community. I haven't thoroughly tested it but perhaps you can take a look and check if said branch fixes your issue.

If you have any suggestions feel free to add them.

With kind regards

[edwinljacobs]

Could you review my last comment?

With kind regards

[kozie]

Hi @edwinljacobs

Sorry for the late response. The fix seems okay for now as it replicates the hotfix we applied in our project. i'm not able to quickly check your fix in the environment where i had the issue but i reviewed the commits as stated above.

[edwinljacobs]

Hi, also a late response :) We will include the fix as described above in the next release.

[edwinljacobs]

Hi,

Ultimately I decided to remove the plugin and to initialize attribute models on all filters. I do believe that this is the best route as Magento assumes these models exist on all filters. This will problably solve any future issues with missing attribute models.

With kind regards

[edwinljacobs]

The filters which do not correspond to a Magento attribute will have a mocked attribute model.

[kozie]

Hi @edwinljacobs

Thanks for the heads up :) I hope this fix has been thoroughly benchmarked because it feels like it could introduce performance issues. Otherwise i think it's okay and sounds like a better solution than the previous with the plugin on plugin.

[edwinljacobs]

Hi,

I dont think this has a performance hit

**
     * @param string $attributeName
     * @return Attribute
     */
    protected function mockAttributeModel(string $attributeName): Attribute
    {
        /** @var Attribute $attributeModel */
        $attributeModel = $this->attributeFactory->create();
        $attributeModel->setAttributeCode($attributeName);
        return $attributeModel;
    }

This will return an empty attribute instance with an attribute_code. This gets called if we cant find an attribute model for the tweakwise filter https://github.com/EmicoEcommerce/Magento2Tweakwise/commit/47def538656856bb60b9468e2a7f1c3b837a3447. To be fairly honest I did not step into the $this->attributeFactory->create() call to determine what does exactly. I suspect just returning a model and no fancy stuff. I did however expand on the initFilters method in an earlier release. But that is not in scope for this issue I guess. The reason it took a while to get back to this issue was that I wasn't satisfied with the solution and only now got around to revisit and rethink it. If you do think this will cause a performance hit let me know otherwise ill release it.

With kind regards

[edwinljacobs]

Code has been merged and tagged in v2.1.1 https://github.com/EmicoEcommerce/Magento2Tweakwise/releases/tag/v2.1.1