stefandoorn / sitemap-plugin

Sitemap Plugin for Sylius eCommerce platform
MIT License
79 stars 45 forks source link

Use channel default locale as main localization to generate url #197

Open lruozzi9 opened 2 years ago

lruozzi9 commented 2 years ago

Hi @stefandoorn! IMHO using the LocaleContext in a CLI context is not always a good choice. It will pick the locale code from the default locale parameter. Our use case is with two channels with respectively one locale different from the other. So, the generate command creates without problems the sitemap for the channel that has the same locale of the locale parameter, but it throws errors for the other Channel. The error is due to the missing of the slug in the default locale that is used as the main localization!

So, this PR will use the Channel's default Locale as the main localization. The LocaleContext is now only a "fallback" if it is not set.

stefandoorn commented 2 years ago

@lruozzi9 Thanks - looks interesting! I will have a proper look soon. Would you also be able to provide a PHPUnit test that covers this including a fixture XML output file? Would be great. I highly value the amount of tests in this repo, to be sure we don't repeat mistakes in the future.

lruozzi9 commented 2 years ago

Sure! Will add them as soon as possible 🚀

lruozzi9 commented 2 years ago

Unfortunately, it is not as easy as I thought. The test is ready but making a request to a channel without the locale EN makes a redirect. I can't think of anything at the moment to avoid overwriting the Symfony local variable.