Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

RFC: Deleting users with associated orders #3138

Closed
aitbw opened this issue Mar 13, 2019 · 12 comments
Closed

RFC: Deleting users with associated orders #3138

aitbw opened this issue Mar 13, 2019 · 12 comments

Comments

@aitbw
Copy link
Contributor

aitbw commented Mar 13, 2019

After talking with @elia and @jacobherrington about how we should tackle #3126, I think it's better for everybody to have a say on this particular issue. (I think #2221 is also related to this?)

Initially, I plan to submit a PR to hide the delete button for those users that have at least one order associated and then work from there on whatever the community consensus is.

The proposed ideas to completely completely fix the aforementioned issue are the following:

  • Create a ghost / fake user, like Github does with @ghost, to inherit the orders of the deleted users
  • Keep the users, but remove all the sensitive data (i.e.: email address, billing/shipping addresses) and replace their names with something like Deleted user <some_id>

Take into consideration that this is something that stores administrators must be aware of, it should be GDPR compliant and last but not least, orders can't be deleted whatsoever.

Let me know what you think! 🙌

@stem
Copy link
Contributor

stem commented Mar 20, 2019

@epicery, we keep the user object and scramble it's data.
Doing so enable us to keep stats about our average lifetime value, average order interval, average order count, etc. Migrating all orders to a "ghost" user will certainly break that.

@jacobherrington
Copy link
Contributor

I agree with deleting as much PII as possible and renaming the user to "Deleted user <some_id>. I'm not sure on how far we can go with scrambling data before we run into other problems.

@kennyadsl
Copy link
Member

I agree that we should explore the approach proposed by @stem if that's compliant with GDPR. Are there some rules to respect for not being able to reverse scrambled data with real users data?

@seand7565
Copy link
Contributor

We could use SecureRandom to generate a new email - eg."#{SecureRandom.hex}@deleted.com" - to remove unscrambling as a possibility.

@brchristian
Copy link
Contributor

Hi all, have we reached consensus here? We are getting requests from users to delete their accounts but I can’t tell if there is a Solidus best practice for that. Where has this conversation arrived now that we’re 2 years out? I’m happy to submit PRs if needed.

@kennyadsl
Copy link
Member

Hey @brchristian, I think we should go with extracting the current logic that deletes users in a configurable class that contains the logic for that action. In the beginning, it should just be identical to what we have now. After that, we can provide another opt-in class that does something more advanced like scrambling data. I suspect this is not enough because each store can have users' references in other tools (like marketing or OMS integrations) and they will need to add custom logic to remove users from those platforms as well. What do you think?

@brchristian
Copy link
Contributor

Hi @kennyadsl that is a very good suggestion. I would also suggest that we include the logic for whether users are eligible for deletion on the backend, as currently we don’t allow the backend to delete users unless orders.empty?, which will be too restrictive for a lot of stores.

@kennyadsl
Copy link
Member

Makes sense, we can even call this action differently than delete if it makes sense. Like anonymize, can make sense for what you are trying to do?

@tvdeyen
Copy link
Member

tvdeyen commented Jul 26, 2022

We are running into this a lot lately and I think we should provide a solution to be GDPR compliant. Some thoughts:

  1. We should allow to destroy users that have incomplete (not payed) orders. Currently we restrict to delete users with any order in any state. This is wrong imo.
  2. Scrambling the data (especially addresses on complete orders) can cause legal issues with accounting. Ie. an order/invoice might not have been accounted yet and the user already deleted their account.
  3. I just checked the European GDPR "the right to be forgotten" law and this explicitly excludes technical and legal reasons from that law.

That said, I think we should be able to keep addresses that are attached to orders (for legal and accounting reasons), but should remove the address from the users address book (same with payment sources in users wallet). The user record can be deleted, since Solidus does not need an user_id on the order, an email is good enough.

All of this should be good enough to be GDPR compliant.

Obvious disclaimer: I am not a lawyer

tvdeyen added a commit to tvdeyen/solidus that referenced this issue Jul 26, 2022
Currently we restrict to delete a user with an exception if
the user has any orders. Some businesses might want to
allow to delete users with incomplete orders.

This allows to override the can_be_deleted? method in the user
class.

Refs solidusio#3138
@tvdeyen
Copy link
Member

tvdeyen commented Jul 26, 2022

I added a PR that allows to at least change the deletion validation logic. Stores can override the can_be_deleted? method in the user class to allow to delete incomplete users if they like.

tvdeyen added a commit to tvdeyen/solidus that referenced this issue Jul 26, 2022
Currently we restrict to delete a user with an exception if
the user has any orders. Some businesses might want to
allow to delete users with incomplete orders.

This allows to override the can_be_deleted? method in the user
class.

Refs solidusio#3138
tvdeyen added a commit to tvdeyen/solidus that referenced this issue Jul 26, 2022
Currently we restrict to delete a user with an exception if
the user has any orders. Some businesses might want to
allow to delete users with incomplete orders.

This allows to override the can_be_deleted? method in the user
class.

Refs solidusio#3138
tvdeyen added a commit to tvdeyen/solidus that referenced this issue Jul 26, 2022
Currently we restrict to delete a user with an exception if
the user has any orders. Some businesses might want to
allow to delete users with incomplete orders.

This allows to override the can_be_deleted? method in the user
class.

Refs solidusio#3138
@aldesantis
Copy link
Member

aldesantis commented Jul 28, 2022

I remember we did a fair bit of research back in the day and it was distilled into solidus_gdpr, which allows you to define different "data segments" and how to handle them with respect to the different rights afforded by GDPR.

Have you already taken a look at that?

@waiting-for-dev
Copy link
Contributor

The deletion logic can now be configured. See #4471. We can still provide more robust support. Moving it to discussions.

@solidusio solidusio locked and limited conversation to collaborators Sep 7, 2022
@waiting-for-dev waiting-for-dev converted this issue into discussion #4590 Sep 7, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants