Fixed $legacyKey in buildTranslationCatalogueFromLegacyFiles()

[Amazzing]

Fixed $legacyKey to correctly match translations for original strings containing single quotes.

Questions	Answers
Branch?	8.1.x
Description?	Translations for strings with single quotes are not recognised in buildTranslationCatalogueFromLegacyFiles(). It happens because of incorrectly hashed $legacyKey. This fix ensures that $legacyKey is hashed correctly, same as in Translate::getModuleTranslation()
Type?	bug fix
Category?	LO
BC breaks?
Deprecations?
How to test?	1. Find a module that uses legacy translation system 2. Find a translatable text that has a single quote in it. For example $this->l('Don\'t forget to clear cache') 3. Translate that text to FR or any other language, so translation is saved in /fr.php. It doesn't matter if translation has single quotes or not 4. Check output: $container->get('prestashop.translation.external_module_provider')->setModuleName($module_name)->setLocale($locale)->getMessageCatalogue();
Fixed issue or discussion?
Related PRs
Sponsor company

[prestonBot]

Hi, thanks for this contribution!

I found some issues with the Pull Request description:

• Your pull request does not seem to fix any issue, consider creating one (see note below) and linking it by writing Fixes #1234.

Would you mind having a look at it? This will help us understand how interesting your contribution is, thank you very much!

About linked issues

Please consider opening an issue before submitting a Pull Request:

• If it's a bug fix, it helps maintainers verify that the bug is effectively due to a defect in the code, and that it hasn't been fixed already.
• It can help trigger a discussion about the best implementation path before a single line of code is written.
• It may lead the Core Product team to mark that issue as a priority, further attracting the maintainers' attention.

(Note: this is an automated message, but answering it will reach a real human)

[mflasquin]

tests : https://github.com/mflasquin/ga.tests.ui.pr/actions/runs/6052011268

[kpodemski]

It has to be tested by a developer. cc @PrestaShop/committers

[nicosomb]

Hello @Amazzing. Could you send us a module example?

[kpodemski]

@nicosomb you can use https://github.com/PrestaShop/vatnumber or any other module that uses legacy translations...

[M0rgan01]

@Amazzing hello, could you provide more details for the "how to test" part?

Is there an example module available? (Other than https://github.com/PrestaShop/vatnumber, which is archived and contains nothing in /translations)
I tried it on the autoupgrade module, which has old translations, but I don't see any issues without the PR.
Should we see the result directly in the BO under International/Translations and "Modify translations" for a module?

[matks]

@Amazzing hello, could you provide more details for the "how to test" part?

Is there an example module available? (Other than https://github.com/PrestaShop/vatnumber, which is archived and contains nothing in /translations) I tried it on the autoupgrade module, which has old translations, but I don't see any issues without the PR. Should we see the result directly in the BO under International/Translations and "Modify translations" for a module?

@Amazzing What would be really really useful would be a list of accurate steps to reproduce the problem.

Example:

• Install prestashop 8.1.1
• Install module [...] (please submit the module as attachment)
• Go to backoffice page [...]
• Do [...] in page
• Do [...] in page
• See [...] which is not the correct result, it should be [...] instead

😉

[Amazzing]

Actually, exact steps are listed in “How to test”.

To make it clear - the problem is not about displaying translations in FO/BO, but about the result returned by $module_provider->getMessageCatalogue();

Currently the display of legacy module translations is controlled by Translate::getModuleTranslation(). That method works independently from $module_provider->getMessageCatalogue(). I am not sure where $module_provider->getMessageCatalogue() is used. It might be planned for later use, or already is used somewhere in BO. So, this fix ensures that it returns a correct value.

You can follow those steps for better understanding:

1. Install PS 8.1.1
2. Add French + Some other languages
3. Install testmodule33807.zip
4. Review the code of the module and the file structure - You will find that module has 2 translatable strings, and translations for FR are saved in /translations/fr.php
5. Open module configuration page. There you will find the result returned by $module_provider->getMessageCatalogue()

[M0rgan01]

@Amazzing Thank you for the test module as well as the additional explanations. I tested it with and without the fix, it is good for me. Thanks for the contribution 😃

[matks]

Great work @M0rgan01 and @Amazzing 😉

[kpodemski]

thanks @Amazzing! well done 👏🏻