Code review comments

[Quetzacoalt91]

While I am proceeding with the functionnal tests, here are the comments related to the code:

General:

• The text should be written in english, even if the module is made for only one country. It allows people to understand your module and eventually add contributions.

  /mercadopago.php:

• function createStates():
  • Avoid SQL requests if you can have PHP code to do the same job (SQL structure can change between 2 versions)
  • l.60: You can use Language::getLanguages()
  • In the foreach, please use the ObjectModel OrderState to create your data.
• Your configuration data should have the prefix MERCADOPAGO_ in capital letters.
• function install():
  • Because createStates() returns a value, you should add a test on it.
  • You should use https for shops with SSL enabled or it may throw mixed-content errors.
• function getContent():
  • At the end, you could define an array like this

            $conf_data = array('mercadopago_CATEGORY',
                'mercadopago_CREDITCARD_ACTIVE', [...]
                );

and then foreach on it:

            foreach ($conf_data as $conf_name)
                Configuration::updateValue($conf_name, Tools::getValue($conf_name));

• function valideCredentials() & validatePublicKey():
  • Temporary var $result not really useful.
• function hookPayment():
  • if hasCredential() is false, you should not show anything in the payment page (even not an error), because the customer cannot do anything to fix the issue. However, you can display a warning on the back-office.
• hooks:
  • You should add a check for your hooks:

if (!$this->active)
    return;

• function getPrestaShopPreferences():
  • $customer has multiple assigments, for unrealted data. You should define it this way:
  • keep $customer for the object
  • create a new variable $customer_data to define your array
  • you do not need to reload the variable $customer with Context::getContext()->customer. Just reuse the first variable.
  • URL with http which should accept https when ssl is enabled on the shop.
  • URL stored in back_urls array may won't work when URL rewriting is enabled.
• function retrieveOrder() is the same as Order::getOrderByCartId()
• function updateOrderStatus() is the same as OrderHistory::chageIdOrderState(...)

  /controllers/front/notification.php

• Please write a ModuleFrontController in this file, which will implements the function displayAjax()

  Mails

• Why did you copy pasted the content of the folder mails of PrestaShop ?

  TPL files

• CSS files and JS files should be included addCss() and addJs().
• Style inline is forbidden (style="[...]"). Please move your style definitions in a CSS file.
• Some translation tags are missing
• Text must be written in English and then translated in your favorite language.

[ricardobrito]

Hi,

So how should I proceed to fix "URL stored in back_urls array may won't work when URL rewriting is enabled."?

Thanks

[Quetzacoalt91]

Hi @ricardobrito,

You should use the function

Link::getModuleLink($module, $controller = 'default', array $params = array(), $ssl = null, $id_lang = null, $id_shop = null, $relative_protocol = false)

It will generate the properly URL, regarding your URL rewriting configuration in the BO.

[ricardobrito]

Hi @Quetzacoalt91 ,

I pushed all changes asked.

Just the following issue I didn't do since the new code checks some settings and that would be repetitive inside a foreach:

function getContent(): At the end, you could define an array like this

Please let me know asap if the module needs more changes.

Thanks!