-
-
Notifications
You must be signed in to change notification settings - Fork 196
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
Close; Anonymise; Erase #7611
Merged
Merged
Close; Anonymise; Erase #7611
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For use in other specs. Had to change `after_initialize` to `before_validation` so that records built via FactoryBot were valid. This also affects normal records: data = load_file_fixture('parrot.jpg') a = ProfilePhoto.new(data: data, user: User.first) a.valid? # => true data = load_file_fixture('parrot.jpg') a = ProfilePhoto.new(user: User.first) a.data = data a.valid? # => false With this change, both examples now return `true`.
Tweak the spec such that it'll be easier to add forthcoming examples.
mysociety-pusher
force-pushed
the
close-anonymise-erase
branch
2 times, most recently
from
March 2, 2023 12:34
f756b50
to
4ad05e1
Compare
gbp
approved these changes
Mar 16, 2023
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.
Looks good. Left some minor comments.
Closing your account should stop all communications from the app. This is likely not completely true in practice [1] as some messages may ignore this flag, but this sets the intent. [1] #4603
Add a version of `#close` that raises on failure, and define `#close` in terms of `#close!` as per convention.
Method to only erase PII account data to enable forthcoming composition of various close / anonymise / erase options.
Simple method to handle anonymisation via CensorRule if the user has any content to anonymise.
Compose from new task-specific methods to make clearer. Had to tweak the censor rule creation since closing currently appends "(Account suspended)", which then gets added to the censor rule `text`. We only want to redact the raw attribute content. Drop `User#redact_name!` since we now have `anonymise!` to do the same thing. Fixes #5600 as the use of `#erase!` for this method now clears the profile photo.
mysociety-pusher
force-pushed
the
close-anonymise-erase
branch
from
March 17, 2023 14:47
4ad05e1
to
8c25e2b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add some task-specific methods for better composability and future ability to add a UI button for each action to give us more control over how we handle various user requests.
A precursor to: