EmicoEcommerce / Magento2Tweakwise

Magento 2 module for Tweakwise integration
Other
4 stars 16 forks source link

AttributeSlug::getSlug() must return a string #190

Closed evs-xsarus closed 1 month ago

evs-xsarus commented 3 months ago

Issue Brief

What is the purpose of this issue? Explain the background context.

Environment

Steps to reproduce

[2024-06-05T07:50:32.855005+00:00] main.CRITICAL: TypeError: Tweakwise\Magento2Tweakwise\Model\AttributeSlug::getSlug(): Return value must be of type string, null returned in /home/svc-tsf_prod/releases/80de182d/vendor/tweakwise/magento2-tweakwise/Model/AttributeSlug.php:51
Stack trace:
#0 /home/svc-tsf_prod/releases/80de182d/vendor/tweakwise/magento2-tweakwise/Model/AttributeSlugRepository.php(89): Tweakwise\Magento2Tweakwise\Model\AttributeSlug->getSlug()
#1 /home/svc-tsf_prod/releases/80de182d/vendor/tweakwise/magento2-tweakwise/Model/Catalog/Layer/Url/Strategy/FilterSlugManager.php(97): Tweakwise\Magento2Tweakwise\Model\AttributeSlugRepository->save()
#2 /home/svc-tsf_prod/releases/80de182d/vendor/tweakwise/magento2-tweakwise/Model/Catalog/Layer/Url/Strategy/PathSlugStrategy.php(439): Tweakwise\Magento2Tweakwise\Model\Catalog\Layer\Url\Strategy\FilterSlugManager->getSlugForFilterItem()
#3 /home/svc-tsf_prod/releases/80de182d/vendor/tweakwise/magento2-tweakwise/Model/Catalog/Layer/Url/Strategy/PathSlugStrategy.php(379): Tweakwise\Magento2Tweakwise\Model\Catalog\Layer\Url\Strategy\PathSlugStrategy->buildFilterSlugPath()
#4 /home/svc-tsf_prod/releases/80de182d/vendor/tweakwise/magento2-tweakwise/Model/Catalog/Layer/Url/Strategy/PathSlugStrategy.php(167): Tweakwise\Magento2Tweakwise\Model\Catalog\Layer\Url\Strategy\PathSlugStrategy->buildFilterUrl()
#5 /home/svc-tsf_prod/releases/80de182d/vendor/magento/framework/Interception/Interceptor.php(58): Tweakwise\Magento2Tweakwise\Model\Catalog\Layer\Url\Strategy\PathSlugStrategy->getAttributeSelectUrl()
#6 /home/svc-tsf_prod/releases/80de182d/vendor/magento/framework/Interception/Interceptor.php(138): Tweakwise\Magento2Tweakwise\Model\Catalog\Layer\Url\Strategy\PathSlugStrategy\Interceptor->___callParent()
#7 /home/svc-tsf_prod/releases/80de182d/vendor/tweakwise/magento2-attributelanding-tweakwise/src/Plugin/PathSlugStrategyPlugin.php(74): Tweakwise\Magento2Tweakwise\Model\Catalog\Layer\Url\Strategy\PathSlugStrategy\Interceptor->Magento\Framework\Interception\{closure}()
#8 /home/svc-tsf_prod/releases/80de182d/vendor/magento/framework/Interception/Interceptor.php(135): Tweakwise\AttributeLandingTweakwise\Plugin\PathSlugStrategyPlugin->aroundGetAttributeSelectUrl()

Solution:

    public function getSlug(): string
    {
        return $this->getData(self::SLUG) ?? '';
    }
ah-net commented 3 months ago

@evs-xsarus The slug should never be null. This indicates that there is something wrong with your data. Empty slugs may cause issues with filters not working. The bug fix should be preventing an NULL value from occurring.

Can you check the tweakwise_attribute_slug table and see if there is an entry with an slug value NULL? If there is, what is the attribute value of that row?

evs-xsarus commented 3 months ago

@ah-net there are no entries in tweakwise_attribute_slug where the slug is NULL. Could this be a situation that occured in previous versions of Magento2Tweakwise?

ah-net commented 3 months ago

@evs-xsarus It's possible, but when i look in the code i see nothing to prevent empty slugs from being created. Looking at the exception. I think this may occur if you request an filter without an value. For example magento.test/category/color without /black

I'll see if I can add an check on an empty slug in the code to prevent this error.

evs-xsarus commented 3 months ago

Thank you.