Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lruozzi9
Copy link
Contributor

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.

@lruozzi9 lruozzi9 requested a review from stefandoorn as a code owner January 14, 2022 16:21
@stefandoorn
Copy link
Owner

@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
Copy link
Contributor Author

Sure! Will add them as soon as possible 🚀

@lruozzi9
Copy link
Contributor Author

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.

@lruozzi9 lruozzi9 force-pushed the use-channel-default-locale branch from 4fc5448 to 1a181d3 Compare January 2, 2023 10:58
@lruozzi9 lruozzi9 force-pushed the use-channel-default-locale branch from 1a181d3 to a4c7e52 Compare January 2, 2023 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants