coreshop / CoreShop

CoreShop - Pimcore enhanced eCommerce
http://www.coreshop.org
Other
274 stars 154 forks source link

[FrontendBundle] Prevent PHP error for coreshop_locale_switcher() if being called from non-Coreshop site #2641

Open BlackbitDevs opened 1 month ago

BlackbitDevs commented 1 month ago
Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no

We had this code in our layout.html.twig:

{% for document in coreshop_locale_switcher(mainDocument) %}
        {% if document.language != locale %}
            <link rel="alternate" hreflang="{{ document.language }}" href="{{ app.request.getSchemeAndHttpHost() ~ document.target }}" />
        {% endif %}
{% endfor %}

This works as it should. But this is a multi-site project. And when this layout is being used in a document of a site which does not have a coreshop store, an exception gets thrown

app.EMERGENCY: CoreShop\Component\Store\Context\StoreNotFoundException: Store could not be found! in /var/www/html/vendor/coreshop/core-shop/src/CoreShop/Component/Store/Context/CompositeStoreContext.php:53

The cause is in https://github.com/coreshop/CoreShop/blob/52781b3bb4e6f8f6a50a1333b022d2873cf2b342/src/CoreShop/Bundle/FrontendBundle/Twig/LocaleSwitcherExtension.php#L57 - there is no check if there actually is a store for current document. This PR adds this check and returns [] if there is no store.

dpfaffenbauer commented 1 month ago

@BlackbitDevs Please check the tests, they are failing

BlackbitDevs commented 1 month ago

Sorry, forgot the use statement... is fixed now.

sonarcloud[bot] commented 1 month ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

dpfaffenbauer commented 1 month ago

@BlackbitDevs I think there is a better solution to this. Rather than catching the error and return an empty array, we should add a check to the twig template that calls this function to check if we have a store. WDYT?

BlackbitDevs commented 1 month ago

@dpfaffenbauer The problem with checking it in the twig template is that every project has to remember this again and again. But if we fix it in coreshop_locale_switcher(), it is fixed once and for all time.

Is there actually a function to check if current site has a corresponding store?

dpfaffenbauer commented 1 month ago

coreshop.hasStore

I believe that certain functions should throw errors in order to show you that you mis-configured something.