Manage newsletter subscription in a modern ajax way

[jf-viguier]

This is more elegant and avoid full page reload, see PrestaShop/PrestaShop#17054

Newsletter subscription uses an old fashion "post to home page" form with several issues :

• After subscribing, the user is redirected to the home page.
• If the module is hooked several time, a buggy message tel the user that his email has already been used (see Bugs for v2.5.0 ps_emailsubscription, when hooked twice or on left column PrestaShop#15629)

Notice : I've changed the version number to 2.5.2 to launch upgrade : I must register a new hook actionFrontControllerSetMedia to load my new js file.

[jf-viguier]

I've made a new commit to replace ajaxRender by ajaxDie for Prestashop < 1.7.5 support

[jf-viguier]

Any news from QA ?

[jf-viguier]

Any news from QA ?

Doing it rn

[NeOMakinG]

Hey @jf-viguier

Please configure your IDE so it auto-add new line at the end of the file :)

[jf-viguier]

Hello @SD1982, I don't have this behavior. I'm in 1.7.6.2 and you ?
I've changed message position to match other ps behaviors like in account creation page.

[matks]

@jf-viguier I tested your PR also (on 1.7.7.x). I have the same issue:

I checked the AJAX query, the data.msg data returned by backend is empty

{"value":"mathieu.fermentb@prestashop.comsss","msg":"","conditions":"Vous pouvez vous d\u00e9sinscrire \u00e0 tout moment. Vous trouverez pour cela nos informations de contact dans les conditions d'utilisation du site."}

EDIT: oh, same answer from @SD1982

[SD1982]

Hello @jf-viguier This behavior appears when i test on version 1.7.7 but I just tested on version 1.7.6 like you and indeed it works correctly and as you can see the message is diplayed on top this time and without disabling javascript:

Version 1.7.6.x

so it probably does not come from your PR what do you think of that @matks , @NeOMakinG

[jf-viguier]

@SD1982 please use a stable version ;-) You've certainly found a bug for 1.7.7 but I've no time to check this for now.
Certainly a bug in ajax request that doesn't have correct context of global vars stuff. @matks ? Other ajax messages works on 1.7.7 ?

[Robin-Fischer-PS]

I think we need @PrestaShop/prestashop-core-developers opinion on this ! If this PR works in 176 but not in 177, is this link to the PR or to the 177 version ?

Thanks

[jf-viguier]

@Robin-Fischer-PS I've already answered this : it's linked to 177 version.
I just use old messages in ajax return array.
Tt's certainly a bug in ajax request that doesn't have correct context of global vars stuff.

[matks]

@Robin-Fischer-PS I've already answered this : it's linked to 177 version.
I just use old messages in ajax return array.
Tt's certainly a bug in ajax request that doesn't have correct context of global vars stuff.

It does not matter. We are not going to accept a Pull Request that we know it will fail with 177 😉. What kind of professionnals would we be if we accept something that we know will fail with next release when it's scheduled for 1 or 2 months in the future ?

Until we know why it fails with 177 we cannot merge this PR.

It does not mean we will not merge this PR. It means we need more exploration.

[jf-viguier]

Thanks @matks I understand. I just means that my pr is not the source of the bug. I guess that other ajax requests have this bug in 177 ?

[matks]

Found the culprit
https://github.com/PrestaShop/ps_emailsubscription/pull/37/files#diff-cfb1aa2304713070e111aaa57e5c8ef5R350

[matks]

@jf-viguier Fixed with commit da17e14

This PR can now go through QA again, for both 176 and 177 checks 💪

[jf-viguier]

Thanks @matks for the fix, you rocks

[SD1982]

LGTM in 1.7.6.x and 1.7.7.x Thanks @jf-viguier

[Progi1984]

Thanks @jf-viguier

[jf-viguier]

@Progi1984 when this will pr be released ?

[jf-viguier]

Newsletter subscription seems no more in ajax in hummingbird theme. Can someone confirm ?