New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Advanced EU Compliance module issue #104

Closed
MockoB opened this Issue Feb 14, 2017 · 11 comments

Comments

5 participants
@MockoB

MockoB commented Feb 14, 2017

At first when installed module looks almost good. Just the info on hover is hard to read because the font color. After some time here is how it looks http://imgur.com/a/VpWKs and here is how it should look http://imgur.com/H1BxVsA . After uninstalling the module the info is still there http://imgur.com/F7zqmUw and here is how it should look like http://imgur.com/w0iM1HD Here is on bees after some addition of products, refreshes etc. http://imgur.com/Rpx8f4k it looks almost perfect. After uninstalling the module and clearing the cache WITH "recompile template files if the files have been updated" option switched on, the info is cleared. With just clear cache they are still there. No caching optipons were switched on except ccc and smarty cache.

@BoBBer446

This comment has been minimized.

Show comment
Hide comment
@BoBBer446

BoBBer446 Feb 19, 2017

All images 404 not found, thats not okay ^^

BoBBer446 commented Feb 19, 2017

All images 404 not found, thats not okay ^^

@MockoB

This comment has been minimized.

Show comment
Hide comment
@MockoB

MockoB Feb 19, 2017

I replaced all the images on another host but it is git issue or just me don't know how to upload decent link maybe :) Just copy the link and paste it in your browser and you should be able to view the pictures.

MockoB commented Feb 19, 2017

I replaced all the images on another host but it is git issue or just me don't know how to upload decent link maybe :) Just copy the link and paste it in your browser and you should be able to view the pictures.

@Nobodaddy

This comment has been minimized.

Show comment
Hide comment
@Nobodaddy

Nobodaddy Feb 20, 2017

Contributor

This cache error of AEUC is a real old hat and known since August 2015: https://www.prestashop.com/forums/topic/451323-retour-advanced-eu-compliance/page-2#entry211914 and discussed on Github too, e. g. here []PrestaShop/advancedeucompliance#54.
As a workaround you can avoid it by disabling the smarty cache. Not the best solution but it works. An attempt to fix this bug was made in ps_legalcompliance for PS 1.7.

Contributor

Nobodaddy commented Feb 20, 2017

This cache error of AEUC is a real old hat and known since August 2015: https://www.prestashop.com/forums/topic/451323-retour-advanced-eu-compliance/page-2#entry211914 and discussed on Github too, e. g. here []PrestaShop/advancedeucompliance#54.
As a workaround you can avoid it by disabling the smarty cache. Not the best solution but it works. An attempt to fix this bug was made in ps_legalcompliance for PS 1.7.

@MockoB

This comment has been minimized.

Show comment
Hide comment
@MockoB

MockoB Feb 20, 2017

It is really sad to hear that! Unfortunately bees project is based on 1.6.1.11 and as I can see the issue is after version 1.6.1.8. So basically we are stuck waiting the good will of the developers to solve it. Unfortunately it is impossible to start shop on German and Austrian market without that module and may be in other countries also. I hope the issue will be handled more responsibly by the bees team than the other ...

MockoB commented Feb 20, 2017

It is really sad to hear that! Unfortunately bees project is based on 1.6.1.11 and as I can see the issue is after version 1.6.1.8. So basically we are stuck waiting the good will of the developers to solve it. Unfortunately it is impossible to start shop on German and Austrian market without that module and may be in other countries also. I hope the issue will be handled more responsibly by the bees team than the other ...

@firstred

This comment has been minimized.

Show comment
Hide comment
@firstred

firstred Feb 20, 2017

Contributor

Has anyone tried the ps_legalcompliance module?
Would it be better to backport that module, rather than upgrading the advanced eu compliance module?

Contributor

firstred commented Feb 20, 2017

Has anyone tried the ps_legalcompliance module?
Would it be better to backport that module, rather than upgrading the advanced eu compliance module?

@Nobodaddy

This comment has been minimized.

Show comment
Hide comment
@Nobodaddy

Nobodaddy Feb 20, 2017

Contributor

Nope, the solution is really poor, because it is more of another workaround because they didn't find out what caused the bug. Have a look at the hooks and you will know.
https://github.com/PrestaShop/ps_legalcompliance/tree/master/views/templates/hook

Contributor

Nobodaddy commented Feb 20, 2017

Nope, the solution is really poor, because it is more of another workaround because they didn't find out what caused the bug. Have a look at the hooks and you will know.
https://github.com/PrestaShop/ps_legalcompliance/tree/master/views/templates/hook

@firstred

This comment has been minimized.

Show comment
Hide comment
@firstred

firstred Feb 20, 2017

Contributor

Which leaves me wondering... How the heck did PrestaShop 1.7 get the EHI seal? https://ehi-siegel.de/shopbetreiber/ehi-siegel/pruef-kriterien-bedingungen/pruef-kriterien/

Contributor

firstred commented Feb 20, 2017

Which leaves me wondering... How the heck did PrestaShop 1.7 get the EHI seal? https://ehi-siegel.de/shopbetreiber/ehi-siegel/pruef-kriterien-bedingungen/pruef-kriterien/

@Nobodaddy

This comment has been minimized.

Show comment
Hide comment
@Nobodaddy

Nobodaddy Feb 21, 2017

Contributor

Ha, ha, you must be kidding! None of those cerification companies (EHI, Trusted Shops, Haendlerbund etc.) proof if a certain software works properly or complies to whatever law. These are not the criteria for any certification. It's just about a catalog of formalities, a monitoring of guidelines that often the "candidate" himself sets. This is the certification process those companies are paid for. It's just business!

Contributor

Nobodaddy commented Feb 21, 2017

Ha, ha, you must be kidding! None of those cerification companies (EHI, Trusted Shops, Haendlerbund etc.) proof if a certain software works properly or complies to whatever law. These are not the criteria for any certification. It's just about a catalog of formalities, a monitoring of guidelines that often the "candidate" himself sets. This is the certification process those companies are paid for. It's just business!

@firstred firstred added this to the 1.0.4 milestone Aug 16, 2017

@firstred firstred added the Bug label Aug 16, 2017

@Traumflug

This comment has been minimized.

Show comment
Hide comment
@Traumflug

Traumflug Nov 21, 2017

Collaborator

I wish I could reproduce this. Can't read this thread in french language, but I tried the instructions here: PrestaShop/advancedeucompliance#54 (comment)

Step one: Install module Advanced EU Compliance with default settings.

Step two: turn on caching in Backoffice -> Advanced Parameters -> Performance:

bildschirmfoto von 2017-11-21 21-33-53

Step three: login as a customer, no employee privileges. Done.

Step four: go to the shop front page. Looks perfectly fine:

bildschirmfoto von 2017-11-21 21-38-38

Step five: go to a product page. Looks perfect as well:

bildschirmfoto von 2017-11-21 21-39-44

Collaborator

Traumflug commented Nov 21, 2017

I wish I could reproduce this. Can't read this thread in french language, but I tried the instructions here: PrestaShop/advancedeucompliance#54 (comment)

Step one: Install module Advanced EU Compliance with default settings.

Step two: turn on caching in Backoffice -> Advanced Parameters -> Performance:

bildschirmfoto von 2017-11-21 21-33-53

Step three: login as a customer, no employee privileges. Done.

Step four: go to the shop front page. Looks perfectly fine:

bildschirmfoto von 2017-11-21 21-38-38

Step five: go to a product page. Looks perfect as well:

bildschirmfoto von 2017-11-21 21-39-44

@Traumflug

This comment has been minimized.

Show comment
Hide comment
@Traumflug

Traumflug Nov 25, 2017

Collaborator

Mabe this is the reason why these duplicates are so hard to reproduce: thirtybees/advancedeucompliance@46ad6f9

Note how {nocache} was added to the template, one of the recommended fixes.

Collaborator

Traumflug commented Nov 25, 2017

Mabe this is the reason why these duplicates are so hard to reproduce: thirtybees/advancedeucompliance@46ad6f9

Note how {nocache} was added to the template, one of the recommended fixes.

@Traumflug

This comment has been minimized.

Show comment
Hide comment
@Traumflug

Traumflug Nov 25, 2017

Collaborator

Now that the bug was found to be fixed already, let me describe for the records, what the problem was and how it can be fixed in various ways.

As @eleazar/@Nobodaddy described in the PS forum correctly, module advancedeucompliance re-uses a single template for multiple purposes. Re-using code see usages of dumpHookDisplayProductPriceBlock() in advancedeucompliance.php. Re-usages see e.g. here: https://github.com/thirtybees/community-theme-default/blob/1.0.x/product-list-item.tpl#L62-L64 (yes, that's part of the Frontoffice template).

As soon as Smarty cache is turned on, this template gets compiled only once for all uses. All subsequent uses just replay the cached one. That's why messages apparently get duplicated: first copy is the correct one, next one means to be a different label, but gets rendered exactly like the first one.

As far as I can see, there are three ways to fix this:

  1. Turn off caching for this template. One way to do this is wrapping the template into a {nocache}...{/nocache} pair. That's what the fix for thirty bees does. One could do about the same from within PHP, see https://www.smarty.net/docsv2/en/caching.tpl.

  2. Don't re-use the template. Simply make a distinct copy for each use. That's what PS 1.7 did: PrestaShop/ps_legalcompliance@af78a2a I'd do it, too, if there were not more urgent things to do on other bugs. Generally, code ( = template) re-usage is a good idea, but in this use case it's pretty pointless, the template outputs entirely independent content for each use case. Worse, it needs quite some code to distinguish all the use cases.

  3. Take advantage of Smartys $compile_id parameter to fetch(). Basically one sets a distinct ID for each usage, so the one template ends up in many cached versions, one cached version for each usage. See https://www.smarty.net/docsv2/en/api.fetch. Caveat: while Smarty supports this, FrontController->display() doesn't forward this parameter, so it'd either require an override or changes to core code.

Collaborator

Traumflug commented Nov 25, 2017

Now that the bug was found to be fixed already, let me describe for the records, what the problem was and how it can be fixed in various ways.

As @eleazar/@Nobodaddy described in the PS forum correctly, module advancedeucompliance re-uses a single template for multiple purposes. Re-using code see usages of dumpHookDisplayProductPriceBlock() in advancedeucompliance.php. Re-usages see e.g. here: https://github.com/thirtybees/community-theme-default/blob/1.0.x/product-list-item.tpl#L62-L64 (yes, that's part of the Frontoffice template).

As soon as Smarty cache is turned on, this template gets compiled only once for all uses. All subsequent uses just replay the cached one. That's why messages apparently get duplicated: first copy is the correct one, next one means to be a different label, but gets rendered exactly like the first one.

As far as I can see, there are three ways to fix this:

  1. Turn off caching for this template. One way to do this is wrapping the template into a {nocache}...{/nocache} pair. That's what the fix for thirty bees does. One could do about the same from within PHP, see https://www.smarty.net/docsv2/en/caching.tpl.

  2. Don't re-use the template. Simply make a distinct copy for each use. That's what PS 1.7 did: PrestaShop/ps_legalcompliance@af78a2a I'd do it, too, if there were not more urgent things to do on other bugs. Generally, code ( = template) re-usage is a good idea, but in this use case it's pretty pointless, the template outputs entirely independent content for each use case. Worse, it needs quite some code to distinguish all the use cases.

  3. Take advantage of Smartys $compile_id parameter to fetch(). Basically one sets a distinct ID for each usage, so the one template ends up in many cached versions, one cached version for each usage. See https://www.smarty.net/docsv2/en/api.fetch. Caveat: while Smarty supports this, FrontController->display() doesn't forward this parameter, so it'd either require an override or changes to core code.

@Traumflug Traumflug closed this Nov 25, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment