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.

Sign up for GitHub

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

Jump to bottom

Added module psgdpr #27962

Merged
merged 1 commit into from
Apr 7, 2022
Merged

Added module psgdpr #27962

merged 1 commit into from
Apr 7, 2022

Conversation

Progi1984
Copy link
Member

@Progi1984 Progi1984 commented Mar 18, 2022 •
edited by PierreRambaud
Loading

Questions Answers
Branch? develop
Description? Added module psgdpr
Type? bug fix
Category? CO
BC breaks? no
Deprecations? no
Fixed ticket? Relative to #24643
How to test? The module psgdpr is now available in Core.

Ping @PrestaShop/qa-automation if upgrade of automated tests.

This change is Reviewable

@Progi1984 Progi1984 requested a review from a team as a code owner March 18, 2022 10:29
@prestonBot prestonBot added develop Branch Bug fix Type: Bug fix labels Mar 18, 2022
@Progi1984 Progi1984 added this to the 8.0.0 milestone Mar 18, 2022
@Progi1984 Progi1984 mentioned this pull request Mar 18, 2022
Merged
@Progi1984 Progi1984 force-pushed the issue24643_psgdpr branch 2 times, most recently from d46fb18 to 0781e48 Compare March 18, 2022 14:10
saulaski
saulaski previously approved these changes Mar 22, 2022
View reviewed changes
NeOMakinG
NeOMakinG previously approved these changes Mar 24, 2022
View reviewed changes
@Progi1984 Progi1984 added the Waiting for QA Status: action required, waiting for test feedback label Mar 24, 2022
@florine2623 florine2623 self-assigned this Mar 24, 2022
florine2623
florine2623 previously requested changes Mar 24, 2022
View reviewed changes
Copy link
Contributor

@florine2623 florine2623 left a comment

Choose a reason for hiding this comment

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

Hello @Progi1984 ,

Thanks for the PR.

The module is well present in PS 8.0.0.

But I have an exception when I try to configure it :
Screenshot 2022-03-24 at 11 51 37

Then I try to Reset the module, I have an warning :
Screenshot 2022-03-24 at 11 52 43

After that, the module is automatically uninstalled.
Screenshot 2022-03-24 at 11 55 59

So I try to install it again, there's an exception :
Screenshot 2022-03-24 at 11 53 09

Could you check please ?
Thanks!

@florine2623 florine2623 added Waiting for author Status: action required, waiting for author feedback and removed Waiting for QA Status: action required, waiting for test feedback labels Mar 24, 2022
@Progi1984 Progi1984 mentioned this pull request Mar 24, 2022
Merged
@Progi1984
Copy link
Member Author

@florine2623 Bugs fixed on the module : PrestaShop/psgdpr#165

@Progi1984 Progi1984 added the Blocked Status: The issue is blocked by another task label Mar 25, 2022
jolelievre
jolelievre reviewed Mar 31, 2022
View reviewed changes
Copy link
Contributor

@jolelievre jolelievre left a comment

Choose a reason for hiding this comment

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

Nothing to say about the PR but I have one question:

Is it pertinent to add this module in the core release as a composer dependency, meaning it will be included in ALL prestashop shops, when the GDPR is only intended for European countries AFAIK

Also isn't this a duplicate of #27036 (except for the test fixing)

@jolelievre
Copy link
Contributor

Previously this module was installed based on an API that returned appropriate modules based on the default shop's country during installation, here we don't have this feature anymore

But I guess it's a decision that was weighted by @PrestaShop/product-team ?

@Progi1984
Copy link
Member Author

@jolelievre It's @marionf & @MatShir who asks to add psgdpr. 😅 (Check the issue #24643 too).

@jolelievre
Copy link
Contributor

That's why I pinged product team, nothing against your PR 😉

Except maybe this conflict on composer.lock!! :trollface:

@Progi1984 Progi1984 dismissed stale reviews from NeOMakinG, saulaski, and matks via dc297af April 1, 2022 07:34
@Progi1984
Copy link
Member Author

@jolelievre I rebased.

atomiix
atomiix previously approved these changes Apr 1, 2022
View reviewed changes
@MatShir MatShir removed the Blocked Status: The issue is blocked by another task label Apr 1, 2022
MatShir
MatShir previously approved these changes Apr 1, 2022
View reviewed changes
Copy link
Contributor

@MatShir MatShir left a comment

Choose a reason for hiding this comment

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

The module will be accessible to everyone for now. The real need will come from companies that would like to customize an installation depending on the location. We have developed a generic feature that works for most of the time, and that is good enough. If we need to go further, it will be specified and implemented by the concerned devs :)

@Progi1984 Progi1984 dismissed stale reviews from MatShir and atomiix via 8f857b4 April 6, 2022 14:43
@Progi1984 Progi1984 removed Waiting for author Status: action required, waiting for author feedback Blocked Status: The issue is blocked by another task labels Apr 6, 2022
@Progi1984
Copy link
Member Author

@jolelievre & @atomiix The module psgdpr has been released in 1.4.1. So this PR is now unlocked and ready to review.

jolelievre
jolelievre approved these changes Apr 6, 2022
View reviewed changes
Copy link
Contributor

@jolelievre jolelievre left a comment

Choose a reason for hiding this comment

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

Thanks @Progi1984

@NeOMakinG NeOMakinG added the Waiting for QA Status: action required, waiting for test feedback label Apr 6, 2022
florine2623
florine2623 reviewed Apr 7, 2022
View reviewed changes
Copy link
Contributor

@florine2623 florine2623 left a comment

Choose a reason for hiding this comment

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

Hello @Progi1984 ,

Thanks for the PR !

The module is well present in v1.4.1 in PS 8.0.0.
Testing the following features : in BO : customize consent checkboxes, check different languages, check/export customer activity tracking, check new Help tab ✔️
In FO, consent checkboxes are well updated. ✔️
In FO > Customer account, I don't have the GDPR block 🚫
Screenshot 2022-04-07 at 11 14 05
I should have, like ps PS 1.7.8.x :
Screenshot 2022-04-07 at 11 18 34

Could you check ?
Thanks !

@florine2623 florine2623 added Waiting for author Status: action required, waiting for author feedback and removed Waiting for QA Status: action required, waiting for test feedback labels Apr 7, 2022
@Progi1984 Progi1984 mentioned this pull request Apr 7, 2022
Merged
@Progi1984
Copy link
Member Author

@florine2623 Fixed in PrestaShop/classic-theme#22

@Progi1984 Progi1984 added Waiting for QA Status: action required, waiting for test feedback and removed Waiting for author Status: action required, waiting for author feedback labels Apr 7, 2022
florine2623
florine2623 approved these changes Apr 7, 2022
View reviewed changes
Copy link
Contributor

@florine2623 florine2623 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@Progi1984 ,

With this PR, the GDPR - Personal data block is well displayed in FO > Customer account.
Export buttons work well.

It is QA ✅

@florine2623 florine2623 added QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels Apr 7, 2022
@Progi1984 Progi1984 merged commit 7839267 into PrestaShop:develop Apr 7, 2022
@Progi1984 Progi1984 deleted the issue24643_psgdpr branch April 7, 2022 12:07
@Progi1984
Copy link
Member Author

Thanks @florine2623

@PrestaEdit PrestaEdit mentioned this pull request May 2, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PierreRambaud PierreRambaud PierreRambaud approved these changes

@jolelievre jolelievre jolelievre approved these changes

@NeOMakinG NeOMakinG NeOMakinG approved these changes

@florine2623 florine2623 florine2623 approved these changes

@MatShir MatShir MatShir left review comments

@saulaski saulaski saulaski left review comments

@atomiix atomiix Awaiting requested review from atomiix

@matks matks Awaiting requested review from matks

Assignees

@florine2623 florine2623

Labels
Bug fix Type: Bug fix develop Branch QA ✔️ Status: check done, code approved
Projects
None yet
Milestone
8.0.0
Development

Successfully merging this pull request may close these issues.
10 participants
@Progi1984 @jolelievre @MatShir @PierreRambaud @atomiix @matks @NeOMakinG @florine2623 @saulaski @prestonBot