Closed JoshuaBehrens closed 3 years ago
hey @JoshuaBehrens
could you please rebase your branch against master
?
Feels like a simple rebase is not the solution :D
Additionally this can never fully work after the fact: E.g. we want to switch the default language from englisch to spanish. After we ran this command every translation in english that is already in the system will actually be in spanish and vice versa, as the translated values are not migrated between the languages.
We should clearly state that shortcomming in the description of the command and maybe even prefix that command with dev:
or danger:
so it doesn't get executed lightly in a prod env.
It totally makes sense to add the locale option to the system:install command, adding a command in general that allows changing the system default language at any time needs some more consideration.
Hey @keulinho I am on it https://github.com/shopware/platform/issues/860 with the guys from wexo for the command that can be run anytime you want. That is capable of changing every translation at runtime. I run 5 or 6 installations with this pull request and German as default language. At the time of this pull request the boost days were far away. I just need this command to easily integrate it into the setup command.
@keulinho I did some changes to prevent unintended execution. With this lock the cache will get cleared by the followed command in any case so that is not a problem either anymore. My locking might be just a bit... exotic :)
Language creation is missing when it's not existing by default Fix: https://github.com/FriendsOfShopware/production/commit/ff7ea09967c08d8cec336c84bce0077b9714c2d9
What about currency etc.? Also why not use the ShopService from Recovery for this? might require it to be moved elsewhere?
Without adjusting the runCommands
method like this, it fails for me on the second cache clear, as that one requires the command argument - probably a Symfony bug but this is an easy workaround:
/**
* @param array<string, array<string, string>> $commands
*
* @return int
*/
private function runCommands(array $commands, OutputInterface $output): int
{
foreach ($commands as $parameters) {
$output->writeln('');
$command = $this->getApplication()->find($parameters['command']);
if (!$command->getDefinition()->hasArgument('command')) {
unset($parameters['command']);
}
$returnCode = $command->run(new ArrayInput($parameters, $command->getDefinition()), $output);
if ($returnCode !== 0) {
return $returnCode;
}
}
return 0;
}
@AndreasA this is a problem of this repository. There is already a pull request solving this. I like your approach to exit early when a command fails.
OK. Thanks for the informatin.
One other thing. In the shopService the swap language command does not disable foreign key checks which is important as otherwise, e.g. the mail template contents will not be switched.
It is done like this:
private function swapDefaultLanguageId(string $newLanguageId): void
{
$stmt = $this->connection->prepare(
'UPDATE language
SET id = :newId
WHERE id = :oldId'
);
// assign new uuid to old DEFAULT
$stmt->execute(
[
'newId' => Uuid::randomBytes(),
'oldId' => Uuid::fromHexToBytes(Defaults::LANGUAGE_SYSTEM),
]
);
// change id to DEFAULT
$stmt->execute(
[
'newId' => Uuid::fromHexToBytes(Defaults::LANGUAGE_SYSTEM),
'oldId' => $newLanguageId,
]
);
}
One more question: In case the system default language is part of a language pack. How would one ensure the language pack is loaded prior to executing the system default change command? Although I guess it does not matter if it is installed afterwards? Or I would need to install it by modifying the command to activate prior to switching languages? Hmm.. OK for now e.g. the English templates would be used then. would be nice to determine base language for templates - not specific to this command and also to install language packs prior to setting system default which should also setup the mail templates accordingly.
This PR was closed due to inactivity. If this change is still important to you, feel free to create a new pull request. For more information about our contribution guidelines, see https://docs.shopware.com/en/shopware-platform-dev-en/contribution/contribution-guideline
@mitelg @bneumann97 Why was this closed, this is a must have for the setup command.
Personally, I think just closing the PR due to inactivity is not really a nice way to do this. Especially, as the inactivity was mostly you need providing additional feedback.
The PR already works and @JoshuaBehrens already put a lot of effort into it. There might be some remaining issues but you could point them out or maybe merge the PR as is, so the functionality is in the main branch and create another PR to fix any remaining issues with it.
Or at least provide an alternative PR before closing this one - to ensure there is an alternative at least - but as this one is mostly working already, I think it would be better to have this one merged as soon as possible. And if necessary add additional PRs to fix any remaining issues.
Hey @AndreasA!
Thank you for your comment. We are re-opening this PR. Closing it was indeed an oversight on our part.
By default a PR marked as "Incomplete" is closed after two weeks, in this case that rule doesn't apply since it is still alive.
We already discussed that this timeframe of two weeks might be a bit short and will re-evaluate the topic.
I won't go into detail about the PR itself and just leave it open for discussion. :)
Hi @JoshuaBehrens , as Niklas said, we'd like to integrate this change, do you think you'll find the time to complete this?
Hi @philipgatzka Not before Saturday. Should this make it into the 6.4?
Glad to hear that, merging this before 6.4 is not a requirement and too short of a timeframe either way I think. I didn't intend to build up pressure here, just wanted to know whether/how we can proceed 🙂 As far as I can tell there's one suggestion still open, just let us know if you need any information or assistance with this or any other roadblocks.
I see this pull request is labeled as "scheduled", when can I expect this PR to merged and make it into a future Shopware update? I'm also in need of this feature! :sunglasses:
Hi @dfsoeten , as mentioned above, this PR is still incomplete (I forgot to set the label correctly the last time, sorry for that 🙈). We intend to include the change as soon as it's finished.
Ah, that's a mistake on my part then. Haven't read the entire conversation carefully enough. Thank you for clarifying @philipgatzka !
SO adding the last suggestion from @keulinho, and PR will be completed? @JoshuaBehrens @shyim
AFAIK:
Change this
if (!empty($input->getOption('locale'))) {
$this->getApplication()->find('system:locale-destructive')->activateCommand();
$commands[] = [
'command' => 'system:locale-destructive',
'locale' => $input->getOption('locale'),
];
$commands[] = [
'command' => 'cache:clear',
];
}
To:
if (!empty($input->getOption('locale'))) {
$this->getApplication()->find('system:locale-destructive')->activateCommand();
$commands[] = [
'command' => 'system:locale-destructive',
'locale' => $input->getOption('locale'),
];
}
ANd than use the cache clearer, since the cache clearer is is clearing in the SystemLocaleCommand?
$this->setDefaultLanguage($locale);
$this->cacheClearer->clear();
@unugiyen I'll have a look at that :) thank you very much
Hi @JoshuaBehrens, are you still working on this issue or are you currently not working on it?
I had a look at @unugiyen changes and they were good but I didn't integrate them yet. Shall I? :)
Hi @JoshuaBehrens, that would be great. :blue_heart:
Please i need this commit in the original shopware production template git <3 @JoshuaBehrens
We will provide an option to do this "shortly" :tm: in the shopware platform, i'm not sure if switching the default language and currencies afterwards will be supported directly, but we will definetly add support to change the default language and currency during the installation process, which is the most pressing concern i guess.
@keulinho I think it is now, so I guess this PR could be closed?
After all with 6.4.6.0, there is a command to change it:
and it can also be set at the beginning: https://github.com/shopware/platform/blob/v6.4.6.0/src/Core/Maintenance/System/Command/SystemInstallCommand.php#L41
@AndreasA Yep v6.4.6.0 already includes a command to change the language afterward, and you can set it during CLI set up, so i'm closing this PR.
If you have any issues or anything is missing in those commands, please open another issue or another PR.
WE HAVE A COMMAND NOW :O 💙 💙 💙 💙 💙 💙 💙 💙
I had a look at it and yes, I will amend it :) nothing troublesome so far. We will see how well it does in our next projects. Thank you so much @keulinho for this command!
For all that are running into this. The command is not perfect yet! Change your default language ASAP after installation
Now you can run and get an administration using German as system language. Useful for German only shops that don't force the users to enter English data first. Can be applied to any other locale that shopware ships.
Lots of it is 📋🍝 from recovery.