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

Add for_associations selector for anonymising related records #105

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

Conversation

Tabby
Copy link
Contributor

@Tabby Tabby commented Jun 13, 2024

It is rare that records need to be anonymised in isolation, and this makes it easy to configure the associations on a model which should be anonymised at the same time. This should greatly simplify finding and anonymising all records for a given subject across large, complex database structures

It is rare that records need to be anonymised in isolation, and this
makes it easy to configure the associations on a model which should be
anonymised at the same time. This should greatly simplify finding and
anonymising all records for a given subject across large, complex
database structures
@Tabby
Copy link
Contributor Author

Tabby commented Jun 13, 2024

Needs #106 to be approved and merged for the rubocop checks to pass

@Tabby Tabby marked this pull request as draft June 13, 2024 11:50
@Tabby
Copy link
Contributor Author

Tabby commented Jun 13, 2024

Changed to a draft because it's not actually fully ready yet, I just want to seek opinions early

I know I need to handle circular references, which should just be a case of checking if anonymised? is true at the beginning of anonymise!

@xmjw pointed out that I should check that we won't run into potential issues with stack depth which is a good shout and I will look into that

I would also like thoughts on the return value. I wanted to ensure that if there are no associations being anonymised, that we don't change what we return for compatibility, but I wasn't sure whether to have some kind of MergedResult that includes information about all the changes, or to use a hash with the model name as the key and the result as the value, so I just did the simple thing for now and wanted to ask for thoughts from others

@Tabby
Copy link
Contributor Author

Tabby commented Jun 13, 2024

See https://github.com/orgs/gocardless/discussions/505 for more info on why I think we should make this change

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.

1 participant