Idea to improve removeOverride()

[JPresta.com]

Hi,

I was facing a problem in my module, when upgrading it, the removeOverride() method was not removing my previous override. I analyzed why and found out that if you remove an override to install a new version of it then it does not work. New override must be the same because of this:

if (md5($module_content) != md5($orig_content)) {
    $override_file = $override_file_orig;
}

This is done to know if the module is the owner of the installed override (I guess).

 

My suggestion is: we should add an information like

/** @author module_that_installed_override */

so we could check the owner, plus people reading code could know the origin of the override.

 

What do you think?

[bellini13]

I think the problem really boils down to, what if 2 or more modules override the same function.  I believe right now Prestashop will not attempt to install the override, because it cannot merge the 2 functions together.

 

So if the md5 check fails, this means

1) Someone manually changed the override file

2) Another module created an override using a different function.  I believe Prestashop will attempt to merge these overrides since there is not a conflicting function

 

So I'm not entirely sure adding the author comment solves anything really.  Either your module is the only module and the md5 checksum should match, or "something" changed this override file and it is no longer safe to assume that removing the file, or the function will result in things working properly.  You might break another module, or you might break some other functionality the merchant/developer manually added.

 

So the best thing, and what I recommend to all, is to ensure that any override you create, that you always check that your module is installed and enabled, before you execute your override code.  Otherwise if it is not installed, or it is disabled, then you should just execute the parent function and return.  This is the safest way to implement an override.

Edited September 3, 2014 by bellini13 (see edit history)

[JPresta.com]

My case is this one:

• Version 1 of my module installed an override on method A
• Version 2 fix a bug in this override

The upgrade system try to remove the override, then to install the new one.

Problem, new override is different so it's not removed and installation fails because method A is already overriden.

 

If @author is the module that is removing the override we should trust him; the module know what it is doing.

 

Do you have another way to upgrade an override?

[El Patron]

My case is this one:

• Version 1 of my module installed an override on method A
• Version 2 fix a bug in this override

The upgrade system try to remove the override, then to install the new one.

Problem, new override is different so it's not removed and installation fails because method A is already overriden.

 

If @author is the module that is removing the override we should trust him; the module know what it is doing.

 

Do you have another way to upgrade an override?

 

if version 1 is uninstalled (normal process) before installing version 2, then version 1 override is removed before version 2 is installed.  yes?

 

I do love the idea of ps commenting what module is installing the override, this  would really help troubleshoot as so often on the forum there are issues with an override but shop owner can not relate to a module.

[JPresta.com]

if version 1 is uninstalled (normal process) before installing version 2, then version 1 override is removed before version 2 is installed.  yes?

Are you sure about that? I think there is no uninstall process before upgrading. New files are copied in module directory, then upgrade is run.

[El Patron]

Are you sure about that? I think there is no uninstall process before upgrading. New files are copied in module directory, then upgrade is run.

 

prestashop upgrade or internal auto module upgrade like addon modules?

[JPresta.com]

prestashop upgrade or internal auto module upgrade like addon modules?

I add new version via admin with button "Add a new module". It runs the file /upgrade/Upgrade_2.php in module for example.

[El Patron]

I add new version via admin with button "Add a new module". It runs the file /upgrade/Upgrade_2.php in module for example.

 

oh, so you are using modulename_version yes?  i.e. you are changing module name with new version?  sorry so many questions.

[JPresta.com]

oh, so you are using modulename_version yes?  i.e. you are changing module name with new version?  sorry so many questions.

No, why? I'm using this feature: http://doc.prestashop.com/display/PS16/Enabling+the+Auto-Update

[El Patron]

No, why? I'm using this feature: http://doc.prestashop.com/display/PS16/Enabling+the+Auto-Update

 

ok, thanks for patience Joe,  Now I can see how this can be an issue.

 

have you opened a forge report?

[JPresta.com]

have you opened a forge report?

No, I wanted your opinion before.

[El Patron]

No, I wanted your opinion before.

 

well, if one can not during upgrade process using function to remove existing override during upgrade, then I think this is a bug.  With forge report, the developers can provide guidance. 

[JPresta.com]

OK, I will create a report.

[bellini13]

I might also suggest that you add your own md5 checksum to your module.  This way, in your use case, you can inform the admin/merchant that the install/upgrade failed, because the override was not properly installed.

 

You can place this logic in the construct method.  The downside is that again, if the override is then later changed, the checksum compare will fail.  So maybe only do this during your install and/or upgrade function within your module.

 

Another approach is to add a static variable to the override file.  something like...

static public $mymodule_variable = '123123123123';

 

Then within your module, you can check if this variable exists and the value matches so pre-determined value.  This is another way to ensure your override was installed properly.  If the variable does not exist, or the value does not match, then you know it failed to install/upgrade.  Perhaps use a version number as the value, so that when you change the override, you would increment the version

 

The problem with waiting for PS to address this type of issue, is your module will be used on Prestashop versions that do not have Prestashops 'fixes'.  So you should protect your coding assuming that Prestashop cannot install/remove/upgrade overrides in some proper fashion.

[El Patron]

OK, I will create a report.

 

please post here the forge report link so others can follow/comment and vote up. It certainly appears to be valid point and it would be nice to have resolved for all autoupgrade module logic.thanks

[JPresta.com]

The problem with waiting for PS to address this type of issue, is your module will be used on Prestashop versions that do not have Prestashops 'fixes'.  So you should protect your coding assuming that Prestashop cannot install/remove/upgrade overrides in some proper fashion.

I found a way to handle it in my module, I just want to share it so PS1.7 will fix this issue.

[JPresta.com]

http://forge.prestashop.com/browse/PSCSX-3241

[JPresta.com]

Fixed in 1.6.0.10