Skip to content

Commit

Permalink
Refactor User#close_and_anonymise
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
garethrees committed Mar 17, 2023
1 parent 0717a94 commit 8c25e2b
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 25 deletions.
27 changes: 4 additions & 23 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -444,29 +444,17 @@ def erase!
def anonymise!
return if info_requests.none?

censor_rules.create!(text: name,
censor_rules.create!(text: read_attribute(:name),
replacement: _('[Name Removed]'),
last_edit_editor: 'User#anonymise!',
last_edit_comment: 'User#anonymise!')
end

def close_and_anonymise
sha = Digest::SHA1.hexdigest(rand.to_s)

transaction do
redact_name! if info_requests.any?

sign_ins.destroy_all

update(
name: _('[Name Removed]'),
email: "#{sha}@invalid",
url_name: sha,
about_me: '',
password: MySociety::Util.generate_token,
receive_email_alerts: false,
closed_at: Time.zone.now
)
close!
anonymise!
erase!
end
end

Expand Down Expand Up @@ -671,13 +659,6 @@ def flipper_id

private

def redact_name!
censor_rules.create!(text: name,
replacement: _('[Name Removed]'),
last_edit_editor: 'User#close_and_anonymise',
last_edit_comment: 'User#close_and_anonymise')
end

def set_defaults
return unless new_record?

Expand Down
1 change: 1 addition & 0 deletions app/views/admin_user/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
<li>Disables email alerts</li>
<li>Removes user from search index</li>
<li>Hides requests on the profile page</li>
<li>Clears sign ins and profile photo</li>
<li><strong>Does not</strong> delete or set prominence of the user's requests</li>
<li>Makes a naive attempt to redact <tt>name</tt> from requests<sup>1</sup></li>
</ul>
Expand Down
5 changes: 3 additions & 2 deletions spec/models/user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1105,7 +1105,8 @@ def create_user(options = {})

before do
allow(Digest::SHA1).to receive(:hexdigest).and_return('1234')
allow(MySociety::Util).to receive(:generate_token).and_return('ABCD')
allow(MySociety::Util).
to receive(:generate_token).and_return('r@nd0m-pa$$w0rd')
allow(AlaveteliConfiguration).
to receive(:user_sign_in_activity_retention_days).and_return(1)
FactoryBot.create(:user_sign_in, user: user)
Expand Down Expand Up @@ -1152,7 +1153,7 @@ def create_user(options = {})

it 'should anonymise user password' do
expect { user.close_and_anonymise }.
to change(user, :password).to('ABCD')
to change(user, :password).to('r@nd0m-pa$$w0rd')
end

it 'should set user to not receive email alerts' do
Expand Down

0 comments on commit 8c25e2b

Please sign in to comment.