[-] CORE : Fix bad dependency injection on module upgrade

[tchauviere]

No description provided.

[tchauviere]

Hi @Nobodaddy,

Thanks for your feedback. I'm not sure to get what you mean by this fix "does not refresh module page".
Your fix does pretty much the same thing as mine except yours make one more method call than mine.
Module::getInstanceByName()does the same thing as Adapter_ServiceLocator::get()as you can see here:
https://github.com/PrestaShop/PrestaShop/blob/1.6.1.x/classes/module/Module.php#L1107-L1109

These return values are filled with this:
https://github.com/PrestaShop/PrestaShop/blob/1.6.1.x/classes/module/Module.php#L1142

You can see here the call to Adapter_ServiceLocator::get() which is what I do directly with my fix.

[Nobodaddy]

In theory, maybe. In point of fact there is currently a 500 reported, or a blank page, because the database table ps_module is not updated (e.g. to 2.0.0 for AdvancedEuCompliance). And you know from the French and American forum that I'm not the only one who can reproduce this error.

The other misbehaviour (page not refreshed) may have other reasons, because I copied the whole php file of the dev version of AdminModulesController.php into my release 1.6.1.3.

[tchauviere]

Hi @Nobodaddy,

Thanks for your feedback, another point in favor of not using Module::getInstanceByNameis that it uses cache, which may lead to some strange behavior at some point, which Adapter_ServiceLocator::get()doesn't do.

To avoid any error once you applied my fix on AdminModuleController(which contains call to Adapter_ServiceLocator::get()) you should remove completly the module and then reinstall it. Just tried on my fresh 1.6.1.3 now, doesn't seem to cause any 500 (Apache with PHP 5.4)

[Nobodaddy]

Maybe you can avoid errors by previously deleting the modules you want to update, but this cannot be the solution of the problem. We're talking about module updates, not fresh installations!
Are you really suggesting to delete a module first before you can attempt an upgrade?
Should I really give users the advice to avoid automatic updates in future and and always delete the module first. Are you serious?

[kpodemski]

@Nobodaddy I cannot reproduce this error, maybe try to edit advancedeucompilant.php, change version to 1.5.1, enable developer mode and try to update module from back-office again? It should show some errors if there will be some, I successfully updated module without any issues so I think that problem can be somewhere else

[tchauviere]

This is not what I'm saying here.

I give this advice only for AdvancedEUCompliance since it has dependencies injection directly in its constructor, and this is the root of the issue here. (no other module but this one works that way, it was a prequel to new architecture changes)
Without this fix on AdminModuleController I made, when AdvancedEUCompliance tried to auto-update (or just manual update), it failed in the middle of the process because there was a bug caused by PrestaShop Core (from AdminModuleController). My fix resolve this issue BUT, since AdvancedEUCompliance already tried to update once (and that failed) you need to clean everything up (once my fix is applied) in order to get everything running smoothly again.

And no, you shouldn't advice users to avoid automatic updates, it was a bug that is now fixed with this patch, no need to do so in the future. But you should advice only users that don't have applied this fix yet and already had auto-updated or manually updated AvancedEUCompliance.

Hope this is a clearer answer :)

Best regards,

[Nobodaddy]

Yes, it is! ;)
And what will be your answer to the users of the PrestaShop cloud version?
'Wait, till we implement the architectures changes to advancedeucompliance?' ;)
Those guys can not use your fix!
btw you can't even install this module subsequently. In this case you get the following message: https://www.prestashop.com/forums/topic/489539-eu-advanced-compliance-modul-200/#entry2212222
(click the image in this post!)
Funny, isn't it?

[tchauviere]

For cloud, we take care of upgrading core files ourselves for all our users.

Your pic isn't related to this issue. The picture is just a warning about untrusted modules, which is another bug we recently discovered and we are working on it.

Best regards,

[Nobodaddy]

Just a hint for your search: In the cloud PrestaShop displays this warning even when you try to install a native module. When you switch Disable all overrides to yes, this does the trick.