Fix back-office translations when multishop and multiple languages #28392

MeKeyCool · 2022-05-02T15:05:43Z

Questions	Answers
Branch?	1.7.8.x
Description?	Multishop translations interfered with BO ones
Type?	bug fix
Category?	BO
BC breaks?	no
Deprecations?	no
Fixed ticket?	Fixes #27617
Related PRs	Na.
How to test?	1.- Make a fresh installation of prestashop with two languages, for example English and Spanish. 2.- configure the user's language in Spanish. All BO menus appear in Spanish. 3.- activate the multistore 4.- Set up a second store. 5.- configure English only for store 1 6.- configure Spanish only for store 2 7.- You should have all translations Ok (menus, pages, etc. for BO and every shops).
Possible impacts?	This change could impact translations for multishop (FO and BO)

Thanks @AureRita for finding another issue

When you setup multishop, default Prestashop language is applied to every shop even if it is not available to those shops.
As no "default language" attribute is defined for shops in multishop, I solved this inconsistency by taking the first language available I find for the shop. This way,

If you have only one language available, it will be used.
If you have several languages allowed,
- If default Prestashop language is one of them, it will be used.
- If default Prestashop language is not allowed for this shop, the first time you visit, it may be "random" language but once you select your language, it will be taken from your session.

@PrestaShop/product-team , are you Ok with this behavior ?

PululuK · 2022-05-02T15:22:10Z

config/config.inc.php

+$use_default_language = !isset($language) ||                    // if language not defined
+    !Validate::isLoadedObject($language) ||                     // or if language object is not valide
+    ( !$language->isAssociatedToShop() && !isset($employee) );  // or if ( language is not associated to shop [FO] AND employee not defined [BO] )
+


I suggest to use variables name instead comments, something like :

$isNotValidLanguage = !isset($language) || !Validate::isLoadedObject($language); $isNotAssociatedToShop = !$language->isAssociatedToShop(); $isEmployeeUndefined = !Validate::isLoadedObjet($employee); $useDefaultLanguage = $isNotValidLanguage || ($isNotAssociatedToShop && $isEmployeeUndefined);

Looks good for me ^^
Let me change this :)

☝️ I think I'll keep some comments cause I think the key pain point is about multishop and BO mixed behavior.

atomiix · 2022-05-03T15:55:11Z

config/config.inc.php

+$isLanguageDefinedFromCustomSession = (isset($language) && $language->isAssociatedToShop()) || isset($employee);
+
+$useDefaultLanguage = $isNotValidLanguage || !$isLanguageDefinedFromCustomSession;
+if ( $useDefaultLanguage ) {


I'm surprised we don't have a code styling rule for this, but as we usually don't put spaces for simple if, I suggest you to remove them.

Suggested change

if ( $useDefaultLanguage ) {

if ($useDefaultLanguage) {

@atomiix ,

I took some time to search if I could configure something to help (cause I'll never be able to be attentive to this alone ^^') and I found that :
PHP-CS-Fixer/PHP-CS-Fixer#5709
and
https://packagist.org/packages/superdj/spaces-in-parentheses-php-cs-fixer

But it seems this fixer isn't compatible with version we used ^^'

Should be had the link to some roadmap ? Where can I find this ?

Actually, the rule is used, but the config of php-cs-fixer omit the config/ folder.

Oh ! I'll check this then ^^
Thanks for information ☺️

matks

Thank you for exploring the bug and for the nice and clear bugfix @MeKeyCool well done!

config/config.inc.php

Progi1984

Small feedback

AureRita · 2022-05-06T12:48:41Z

Hi @MeKeyCool

Thank you for your PR, I tested it and all pages on the BO seems to be translated as we wish except merchant expertise (maybe because it's a module ?) as you can see :

(I put it in french, it's better for me)

but there is another issue following this pr, you can see it on this video :

Untitled_.May.6.2022.2_42.PM.mp4

Currently, my shop name "deuxième" should be in french, and when I go on it, it's in english, even if some words are already translated on BO like the catogories or anything else. Moreover, you can't change the languages on the website because you doesn't allow it

MeKeyCool · 2022-05-06T14:28:47Z

Hi @MeKeyCool

Thank you for your PR, I tested it and all pages on the BO seems to be translated as we wish except merchant expertise (maybe because it's a module ?) as you can see :

(I put it in french, it's better for me)

but there is another issue following this pr, you can see it on this video :
Untitled_.May.6.2022.2_42.PM.mp4

Currently, my shop name "deuxième" should be in french, and when I go on it, it's in english, even if some words are already translated on BO like the catogories or anything else. Moreover, you can't change the languages on the website because you doesn't allow it

Well done @AureRita , I did those tests but not from BO link 🤔 Maybe as you are still logged in BO it is detected.
I'll investigate more ^^

AureRita · 2022-05-06T15:07:00Z

It's maybe because of this option select :

(it's on another pr, that why the default country is Canada)

Currently my browser is in Canadian ( that allow me to have the english languages select in first and the french in the second part ) that maybe have a priority on the selection of the store's language ?

MeKeyCool · 2022-05-06T15:11:59Z

It's maybe because of this option select :

(it's on another pr, that why the default country is Canada)

Currently my browser is in Canadian ( that allow me to have the english languages select in first and the french in the second part ) that maybe have a priority on the selection of the store's language ?

Hmmm 🤔 ...
Yes this option looks suspect ... How should we manage if we have to set language from browser with default language en and shop is configured to take french or spanish language ?

MeKeyCool · 2022-05-06T15:14:09Z

Does the bug happen if no multishop configured ?

AureRita · 2022-05-06T15:44:01Z

As you can see, on a single shop, you can't disable languages, if you have two of them, they could be selected on the FO :

Untitled_.May.6.2022.5_40.PM.mp4

Progi1984 · 2022-05-10T06:47:35Z

config/config.inc.php

+$useDefaultLanguage = $isNotValidLanguage || !$isLanguageDefinedFromSession;
+if ($useDefaultLanguage) {


Suggested change

$useDefaultLanguage = $isNotValidLanguage || !$isLanguageDefinedFromSession;

if ($useDefaultLanguage) {

if ($isNotValidLanguage || !$isLanguageDefinedFromSession) {

Since the conditions are quite complex (double negation, OR with isNot on one side and Is on the other) splitting this into multiple lines with clear naming seems more human-readable to me

Progi1984

A last comment

matks · 2022-06-14T13:10:57Z

@MeKeyCool what's the status of this PR?

MeKeyCool · 2022-06-15T07:29:22Z

@MeKeyCool what's the status of this PR?

We reviewed it with @jolelievre yesterday and he added few comments. I think he forgot to apply his review ^^
For me the PR is okay, I juste received some comments about wording that I want to fix.

jolelievre · 2022-06-14T09:03:35Z

config/config.inc.php

+
+$isNotValidLanguage = !isset($language) || !Validate::isLoadedObject($language);
+// `true` if language is defined from multishop or backoffice (`$employee` variable defined) session
+$isLanguageDefinedFromSession = (isset($language) && $language->isAssociatedToShop()) || isset($employee);


Suggested change

$isLanguageDefinedFromSession = (isset($language) && $language->isAssociatedToShop()) || isset($employee);

$isLanguageDefinedFromContext = (isset($language) && $language->isAssociatedToShop()) || defined('_PS_ADMIN_DIR_');

@MeKeyCool friendly reminder for this replacement and then I will approve the PR 😉

Sorry, I forgot this one ^^'

jolelievre · 2022-06-14T09:05:02Z

config/config.inc.php

+$useDefaultLanguage = $isNotValidLanguage || !$isLanguageDefinedFromSession;
+if ($useDefaultLanguage) {


Since the conditions are quite complex (double negation, OR with isNot on one side and Is on the other) splitting this into multiple lines with clear naming seems more human-readable to me

jolelievre · 2022-06-16T09:26:51Z

config/config.inc.php

+
+    // if `PS_LANG_DEFAULT` not a valid language for current shop then 
+    // use first valid language of the shop as default language.
+    if($language->isMultishop() && !$language->isAssociatedToShop()) {


This fallback is legit in the current context because we have inconsistent configuration, we allow to define a default language which can be different from the languages associated to the shop, hence this forced fallback

We need to open an issue to avoid this behaviour, in the language edition we should prevent unassociating the default language, thus forcing the user to change it accordingly before he unassociates it

jolelievre

Thanks @MeKeyCool

AureRita · 2022-06-28T10:01:09Z

hi @MeKeyCool,

Thank you for your modification, currently the bad behaviour is corrected, but I find another issue thank to that, as you can see on this video :

Untitled_.Jun.28.2022.11_50.AM.mp4

at the end, when you return to a single shop and you can't modiy if a languages is enabled or not

MeKeyCool · 2022-06-29T12:04:28Z

hi @MeKeyCool,

Thank you for your modification, currently the bad behaviour is corrected, but I find another issue thank to that, as you can see on this video :
Untitled_.Jun.28.2022.11_50.AM.mp4

at the end, when you return to a single shop and you can't modiy if a languages is enabled or not

Hi @AureRita , is it possible that you apply all scenari you think are necessary to describe #27617 issue and to share them all in one raw ?
Then I'll fix them all in one round.

I agree it is important to clean the wall feature but it is really a waste of time and energy to wait for your tests one by one.
(And IMO it is not a good way to promote contributions)

AureRita · 2022-06-29T13:56:27Z

Hi @MeKeyCool,

As I said "I find another issue thank to that". Sorry to not have all possible tests in my mind.

Well I tested if I find this issue without your PR and it's currently not because of your PR, so I can say that your PR is QA Approved

Thank you and sorry for wasting your time

prestonBot · 2022-06-29T13:57:10Z

QA approved, well done! Message to the maintainers: do not forget to milestone it before the merge.

Progi1984 · 2022-06-29T13:59:40Z

Thanks @MeKeyCool & @AureRita

MeKeyCool · 2022-06-29T14:15:05Z

Hi @MeKeyCool,

As I said "I find another issue thank to that". Sorry to not have all possible tests in my mind.

Well I tested if I find this issue without your PR and it's currently not because of your PR, so I can say that your PR is QA Approved

Thank you and sorry for wasting your time

My bad, sorry if I have been rude, I thought you asked me to fix another problem ^^' ...
Thanks for your precise analysis ^^

MeKeyCool requested a review from a team as a code owner May 2, 2022 15:05

prestonBot added 1.7.8.x Branch Bug fix Type: Bug fix labels May 2, 2022

PululuK reviewed May 2, 2022

View reviewed changes

MeKeyCool force-pushed the bugfix/27617_fix_bo_translations branch 2 times, most recently from 1c9c2b4 to 106ca04 Compare May 2, 2022 15:46

atomiix reviewed May 3, 2022

View reviewed changes

MeKeyCool force-pushed the bugfix/27617_fix_bo_translations branch from 106ca04 to 193d423 Compare May 4, 2022 11:03

atomiix previously approved these changes May 4, 2022

View reviewed changes

matks changed the title ~~Fix bo translations~~ Fix back-office translations when multishop and multiple languages May 4, 2022

matks previously approved these changes May 4, 2022

View reviewed changes

matks added the Waiting for QA Status: action required, waiting for test feedback label May 4, 2022

Progi1984 reviewed May 5, 2022

View reviewed changes

config/config.inc.php Outdated Show resolved Hide resolved

Progi1984 reviewed May 5, 2022

View reviewed changes

AureRita self-assigned this May 5, 2022

Fix translations for legacy parts of BO.

1d151ce

MeKeyCool dismissed stale reviews from matks and atomiix via 1d151ce May 9, 2022 07:51

MeKeyCool force-pushed the bugfix/27617_fix_bo_translations branch from 193d423 to 1d151ce Compare May 9, 2022 07:51

MeKeyCool mentioned this pull request May 9, 2022

Multistore - wrong BO language #27617

Closed

2 tasks

Progi1984 removed the Waiting for QA Status: action required, waiting for test feedback label May 10, 2022

Progi1984 reviewed May 10, 2022

View reviewed changes

jolelievre reviewed Jun 16, 2022

View reviewed changes

Fix default language for shops in multishop.

226bc4f

MeKeyCool force-pushed the bugfix/27617_fix_bo_translations branch from 1db61f7 to 226bc4f Compare June 23, 2022 09:49

jolelievre approved these changes Jun 23, 2022

View reviewed changes

matthieu-rolland approved these changes Jun 24, 2022

View reviewed changes

matthieu-rolland added the Waiting for QA Status: action required, waiting for test feedback label Jun 24, 2022

AureRita added Waiting for author Status: action required, waiting for author feedback and removed Waiting for QA Status: action required, waiting for test feedback labels Jun 28, 2022

AureRita approved these changes Jun 29, 2022

View reviewed changes

AureRita added QA ✔️ Status: check done, code approved and removed Waiting for author Status: action required, waiting for author feedback labels Jun 29, 2022

Progi1984 added this to the 1.7.8.7 milestone Jun 29, 2022

Progi1984 merged commit 02d4bb7 into PrestaShop:1.7.8.x Jun 29, 2022

bullmade mentioned this pull request Oct 15, 2022

Language in multistore #28221

Closed

2 tasks

HanaRebaiQA mentioned this pull request Nov 2, 2022

Multistore - It is possible to select a default language for a shop that does not belong to the shop #30171

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix back-office translations when multishop and multiple languages #28392

Fix back-office translations when multishop and multiple languages #28392

MeKeyCool commented May 2, 2022 •

edited

PululuK May 2, 2022 •

edited

MeKeyCool May 2, 2022 •

edited

atomiix May 3, 2022

MeKeyCool May 4, 2022

atomiix May 4, 2022

MeKeyCool May 4, 2022 •

edited

matks left a comment

Progi1984 left a comment

AureRita commented May 6, 2022

MeKeyCool commented May 6, 2022

AureRita commented May 6, 2022 •

edited

MeKeyCool commented May 6, 2022

MeKeyCool commented May 6, 2022 •

edited

AureRita commented May 6, 2022

Progi1984 May 10, 2022

jolelievre Jun 14, 2022

Progi1984 left a comment

matks commented Jun 14, 2022

MeKeyCool commented Jun 15, 2022

jolelievre Jun 14, 2022

jolelievre Jun 22, 2022

MeKeyCool Jun 23, 2022

jolelievre Jun 14, 2022

jolelievre Jun 16, 2022

jolelievre left a comment

AureRita commented Jun 28, 2022

MeKeyCool commented Jun 29, 2022

AureRita commented Jun 29, 2022

prestonBot commented Jun 29, 2022

Progi1984 commented Jun 29, 2022

MeKeyCool commented Jun 29, 2022 •

edited

		$useDefaultLanguage = $isNotValidLanguage \|\| !$isLanguageDefinedFromSession;
		if ($useDefaultLanguage) {

	$useDefaultLanguage = $isNotValidLanguage \|\| !$isLanguageDefinedFromSession;
	if ($useDefaultLanguage) {
	if ($isNotValidLanguage \|\| !$isLanguageDefinedFromSession) {

	$isLanguageDefinedFromSession = (isset($language) && $language->isAssociatedToShop()) \|\| isset($employee);
	$isLanguageDefinedFromContext = (isset($language) && $language->isAssociatedToShop()) \|\| defined('_PS_ADMIN_DIR_');

Fix back-office translations when multishop and multiple languages #28392

Fix back-office translations when multishop and multiple languages #28392

Conversation

MeKeyCool commented May 2, 2022 • edited

PululuK May 2, 2022 • edited

Choose a reason for hiding this comment

MeKeyCool May 2, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MeKeyCool May 4, 2022 • edited

Choose a reason for hiding this comment

matks left a comment

Choose a reason for hiding this comment

Progi1984 left a comment

Choose a reason for hiding this comment

AureRita commented May 6, 2022

MeKeyCool commented May 6, 2022

AureRita commented May 6, 2022 • edited

MeKeyCool commented May 6, 2022

MeKeyCool commented May 6, 2022 • edited

AureRita commented May 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Progi1984 left a comment

Choose a reason for hiding this comment

matks commented Jun 14, 2022

MeKeyCool commented Jun 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jolelievre left a comment

Choose a reason for hiding this comment

AureRita commented Jun 28, 2022

MeKeyCool commented Jun 29, 2022

AureRita commented Jun 29, 2022

prestonBot commented Jun 29, 2022

Progi1984 commented Jun 29, 2022

MeKeyCool commented Jun 29, 2022 • edited

MeKeyCool commented May 2, 2022 •

edited

PululuK May 2, 2022 •

edited

MeKeyCool May 2, 2022 •

edited

MeKeyCool May 4, 2022 •

edited

AureRita commented May 6, 2022 •

edited

MeKeyCool commented May 6, 2022 •

edited

MeKeyCool commented Jun 29, 2022 •

edited