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

NEXT-10849 - Add console command to change the system default language #1864

Conversation

breaker92
Copy link

add console command to change the system default language
add selection of a default language when installing Shopware using the cli
shopwareBoostDay#202

1. Why is this change necessary?

Currently the only way to change the system default language is by the install GUI.

2. What does this change do, exactly?

add a console command to change the system default language

3. Describe each step to reproduce the issue or behaviour.

try to configure the system default language , via cli.

4. Please link to the relevant issues (if any).

5. Checklist

  • I have written tests and verified that they fail without my change
  • I have squashed any insignificant commits
  • I have created a changelog file with all necessary information about my changes
  • I have written or adjusted the documentation according to my changes
  • This change has comments for package types, values, functions, and non-obvious lines of code
  • I have read the contribution requirements and fulfil them.

@CLAassistant
Copy link

CLAassistant commented May 17, 2021

CLA assistant check
All committers have signed the CLA.

@breaker92
Copy link
Author

@J-Rahe as mentined in the boostday repo

@J-Rahe
Copy link
Contributor

J-Rahe commented May 17, 2021

@breaker92, thanks : )

@J-Rahe
Copy link
Contributor

J-Rahe commented May 18, 2021

Hey @breaker92, would you mind adding an changelog and add to /change the notice about possible data loss to tell the user to make an database backup before?

@breaker92 breaker92 force-pushed the NEXT-10849/allow_the_selection_of_a_default_language_when_installing_Shopware_using_the_cli branch from ab36a70 to d01092a Compare May 27, 2021 06:56
@breaker92
Copy link
Author

Hey @breaker92, would you mind adding an changelog and add to /change the notice about possible data loss to tell the user to make an database backup before?

@J-Rahe done

@shopwareBot
Copy link

Hello,

thank you for creating this pull request.
I have opened an issue on our Issue Tracker for you. See the issue link: https://issues.shopware.com/issues/NEXT-15556

Please use this issue to track the state of your pull request.

@NiklasLimberg NiklasLimberg assigned Dominik28111 and unassigned J-Rahe Jul 29, 2021
@breaker92 breaker92 force-pushed the NEXT-10849/allow_the_selection_of_a_default_language_when_installing_Shopware_using_the_cli branch from d01092a to 62f9d75 Compare August 10, 2021 12:16
@Dominik28111
Copy link
Contributor

Hi @breaker92,

To merge this PR you have to the following things first:

  • test if this command works on the different hosts (the INFORMATION_SCHEMA DB is accessed).
  • much more tests. You test only the output of the command. Whether the data is migrated is not tested at all.
  • the code must be completely formatted and optimized (early-returns, no one line criterias, ...).
  • please have a look at the FieldResolver in the DAL then you see how you should build "plain" sql together.
  • transaction safety, i.e. what happens if a table cannot be migrated?
  • why did you disable foreign key checks?
  • fetch the entities via plain sql if you want to return a name => code map

@Dominik28111
Copy link
Contributor

Hi @breaker92 , please complete your PR otherwise I will close it within the next days.

$tmpId = Uuid::randomBytes();

// assign new uuid to old DEFAULT
$stmt->execute([
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may destroy the translation chain, as it is not enforced that all translations are defined in the new default language, maybe we can add a check that verifies that all translations also exist in the new language


$this->swapDefaultLanguageId($newDefaultLanguageId);

$output->writeln(sprintf('<info>system default language changed to %s</info>', $newLocale->getCode()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just swapping the default language in the DB is probably not enough, as the old stuff may still be cached or saved in some denormalized structures

So we should clear the cache and rebuild all indexes after swapping the default language

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants