-
Notifications
You must be signed in to change notification settings - Fork 72
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
Fix hook registration if needed #183
Fix hook registration if needed #183
Conversation
kpodemski
commented
Dec 9, 2022
•
edited by matks
Loading
edited by matks
Questions | Answers |
---|---|
Description? | Sometimes hook wasn't registered. This PR adds a migration script from v1.4.2 to v1.4.3 that performs the following logic: "if the hook displayGDPRConsent was not registered for this module, I register it" |
Type? | bug fix |
BC breaks? | no |
Deprecations? | no |
Fixed ticket? | Fixes PrestaShop/PrestaShop#30348. |
How to test? | Please see ticket. You need a usecase where GDPR module is installed (v1.4.2) and the hook displayGDPRConsent is not registered for the module. See if checkboxes are displayed after GDPR upgrade module to 1.4.3. |
d49a8a5
to
2f37402
Compare
@kpodemski I improved a bit the PR description and How to test ;) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @kpodemski
Thanks for your PR 🚀
LGTM except the creation of account, where the consent checkbox is not displayed 😞
Even thought, in BO the option is enabled, see the attached screenshot below:
See the attached screen record below:
psgdpr_183.webm
Please check and feedback.
Thanks!
Thanks @hibatallahAouadni ! I've fixed it, it was indeed a problem as the id of the page is not |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @hibatallahAouadni ! |
…boxes Fix hook registration if needed