First code review

[Quetzacoalt91]

Hi,

Here are some changes to do in the module:

General

• Module must be written in English first and then translated in your favorite language. Even if if it dedicated to only one country.
• Templates: You can have only one template and translate your text with the following code:

{l s='Text to translate' mod='etranslation'}

• Do not alter core SQL tables ! You can create instead as many tables as you want, but do not modify PrestaShop ones.
• Overrides are forbidden in partner modules

  etranslation.php

• Tools::isSubmit('export')
  • Initial assignment of $$extlgid and $extlgname not compatible with PHP < 5.3. You should have instead:

$explode =  explode('-',Tools::getValue('export_id_lang'));
$extlgid = $explode[0];
$extlgname = $explode[1];

• Functions installModuleTab & uninstallModuleTab are unused, and can be safely removed.

  /templates/admin/server/php/files/UploadHandler.php

• This file should be store in the folder classes/ of the module
• The namespace marks should be removed from this file. This file is not using a namespace and this will allow it to work on PHP 5.1 & 5.2

  Module configuration

• The design is broken by the module. Please do not alter the PrestaShop CSS
• There are 404 errors
• When I try to send a translation request, I get the following error

[faycaldreamit]

Hello Thomas

etranslation First code review.xlsx

We were able to solve some detected problems.

Please find attached ( excel file ) the detail of the actions carried out.

You will find on the following url the updated source code of the etranslation module : https://github.com/faycaldreamit/etranslation

We also hope having more detail on the points mentioned in the Excel file.

Thank you

[Quetzacoalt91]

Module must be written in English first and then translated in your favorite language. Even if if it dedicated to only one country. Est-ce que on peut juste traduire les phrases non encore traduites comme les messages d'erreurs ? Ca nous evite de mettre en cause tout le système actuel d'internalisation

Nope, everything should be written in the code. Because of its open-source status, it will allow anybody to read and understand the code you wrote. Furthermore, if you want to make your module available in another language, you will only have to add one translation file.

Overrides are forbidden in partner modules

In the install() function of etranslation.php:

// en 1.4, recopie de l'override
        $override_file = _PS_ROOT_DIR_ . '/override/classes/CMS.php';
        $source_file = _PS_ROOT_DIR_ . '/modules/etranslation/override_1.4/classes/CMS.php';
        if ( Tools::substr(_PS_VERSION_, 0 , 3) == '1.4' && !file_exists($override_file) ){
            if(!copy($source_file, $override_file)) {
                echo "Erreur lors de la copie $source_file vers $override_file";
                return false;
            }
            chmod($override_file, 0777);
        }

You are trying to override one Core file. If you remove the folder override_1.4, you have to remove this code.

The namespace marks should be removed from this file. This file is not using a namespace and this will allow it to work on PHP 5.1 & 5.2

For this point, we can see namespace marks (antislash):

$background = new \ImagickPixel('none');

But you do not define your class in a namespace, so these antislashes can be removed.

Best regards

[Quetzacoalt91]

Hi,

• There are other namespace marks in your UploadHandler.php. Please remove all of them.
• You have the wrong content in etbusiness.class.php. Instead of additionnal features for the module, you copy pasted the main class content in this file.
• Please check the report of https://validator.prestashop.com. Many comments are returned for this module.
• I found text written in French in the code. As I said, it must be written in English and then translated. AT the same time, having only one template and have its content translated will be easier to manage when updating or adding new languages. You can have more information here: http://doc.prestashop.com/display/PS16/Module+translation
• You module seems to be compatible PrestaShop 1.4. You should include the backward compatibility in it, which will add some 1.5+ functionalities. You can have more information here: http://doc.prestashop.com/display/PS16/Using+the+backward+compatibility+toolkit

etranslation.php

• function _razSql() can be removed
• By including the backward compatibility, your function initContext() can be removed
• Take care to not generate HTML code in your PHP controlleurs. They should be found in tpl files.