Skip to content
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

#7 Nouveau hook permettant la mise à jour du groupe #8

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bastienuh
Copy link
Contributor

suite à la mise à jour d'une adresse en version 1.7 de PS.

Copy link
Owner

@nenes25 nenes25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bonjour,

Super merci pour la pull request !
Par contre je ne peux pas l'accepter en l'état car c'est un changement majeur du comportement du module.
Et le comportement actuel est celui attendu sur prestashop 1.6

Il faudrait :

  • passer en version 0.6.0
  • conditionner l'exécution du hook uniquement à prestashop 1.7,
    ou rajouter une configuration pour demander si les règles doivent être évaluées à chaque modification d'adresse ( pour toutes les versions )

La limite de cette approche étant que si le client a plusieurs adresses, c'est la dernière adresse sauvegardée qui définira le groupe ( ce qui peut être un comportement souhaité mais pas forcément )

Cordialement,
Hervé

@bastienuh
Copy link
Contributor Author

Désolé, je ne peux pas mettre en place tous ces devs malheureusement.

J'ai en fait seulement répondu à la réponse dans l'issue #7 : "Yes i know and this is why this this module is not really interesting for 1.7 versions." en modifiant tout ça pour que cela devienne intéressant pour 1.7.

Je laisse donc cette contribution là où elle en est avec le hook un peu chiant à débuguer (avec ses params & co.), peut-être quelque la reprendra-t-il à son compte plus tard !

@nenes25
Copy link
Owner

nenes25 commented Jun 8, 2021

C'est noté merci pour la contribution en tous cas.
Je verais si j'ai du temps pour faire l'adaption durant les prochaines semaines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants