Merge branch 'close-anonymise-erase' into develop

app/models/profile_photo.rb

@@ -32,7 +32,7 @@ class ProfilePhoto < ApplicationRecord
   attr_accessor :x, :y, :w, :h
   attr_accessor :image
 
-  after_initialize :convert_data_to_image
+  before_validation :convert_data_to_image
 
   # make image valid format and size
   def convert_image

app/models/user.rb

@@ -406,33 +406,61 @@ def banned?
   end
 
   def close
-    update(closed_at: Time.zone.now)
+    close!
+  rescue ActiveRecord::RecordInvalid
+    false
+  end
+
+  def close!
+    update!(closed_at: Time.zone.now, receive_email_alerts: false)
   end
 
   def closed?
     closed_at.present?
   end
 
-  def close_and_anonymise
+  def erase
+    erase!
+  rescue ActiveRecord::RecordInvalid
+    false
+  end
+
+  def erase!
+    raise ActiveRecord::RecordInvalid unless closed?
+
     sha = Digest::SHA1.hexdigest(rand.to_s)
 
     transaction do
-      redact_name! if info_requests.any?
-
       sign_ins.destroy_all
+      profile_photo&.destroy!
 
-      update(
+      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
+        password: MySociety::Util.generate_token
       )
     end
   end
 
+  def anonymise!
+    return if info_requests.none?
+
+    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
+    transaction do
+      close!
+      anonymise!
+      erase!
+    end
+  end
+
   def active?
     !banned? && !closed?
   end
@@ -636,13 +664,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?
 

app/views/admin_user/edit.html.erb

@@ -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>

spec/factories/profile_photos.rb

@@ -0,0 +1,23 @@
+# == Schema Information
+# Schema version: 20230223145243
+#
+# Table name: profile_photos
+#
+#  id         :integer          not null, primary key
+#  data       :binary           not null
+#  user_id    :integer
+#  draft      :boolean          default(FALSE), not null
+#  created_at :datetime
+#  updated_at :datetime
+#
+FactoryBot.define do
+  factory :profile_photo do
+    user
+    data { load_file_fixture('parrot.jpg') }
+    draft { false }
+
+    trait :draft do
+      draft { true }
+    end
+  end
+end

spec/factories/users.rb

@@ -100,6 +100,7 @@
 
     trait :closed do
       closed_at { Time.zone.now }
+      receive_email_alerts { false }
     end
   end
 end

spec/models/user_spec.rb

@@ -1098,7 +1098,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)
@@ -1145,7 +1146,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
@@ -1161,11 +1162,66 @@ def create_user(options = {})
   end
 
   describe '#close' do
+    subject { user.close }
+
     let(:user) { FactoryBot.build(:user) }
 
-    it 'closes the user account' do
-      user.close
-      expect(user).to be_closed
+    context 'the update is successful' do
+      before do
+        expect(user).to receive(:close!).and_call_original
+        subject
+      end
+
+      it { is_expected.to eq(true) }
+
+      it 'closes the account' do
+        expect(user).to be_closed
+      end
+    end
+
+    context 'the update is unsuccessful' do
+      before do
+        expect(user).to receive(:close!).and_raise(ActiveRecord::RecordInvalid)
+        subject
+      end
+
+      it { is_expected.to eq(false) }
+
+      it 'does not close the account' do
+        expect(user).not_to be_closed
+      end
+    end
+  end
+
+  describe '#close!' do
+    subject { user.close! }
+
+    let(:user) { FactoryBot.build(:user) }
+
+    context 'the update is successful' do
+      before { subject }
+
+      it 'closes the account' do
+        expect(user).to be_closed
+      end
+
+      it 'sets closed_at' do
+        expect(user.closed_at).to be_present
+      end
+
+      it 'disables email alerts' do
+        expect(user.receive_email_alerts).to eq(false)
+      end
+    end
+
+    context 'the update is unsuccessful' do
+      before do
+        expect(user).to receive(:update!).and_raise(ActiveRecord::RecordInvalid)
+      end
+
+      it 'raises an ActiveRecord::RecordInvalid error' do
+        expect { subject }.to raise_error(ActiveRecord::RecordInvalid)
+      end
     end
   end
 
@@ -1184,6 +1240,138 @@ def create_user(options = {})
 
   end
 
+  describe '#erase' do
+    subject { user.erase }
+
+    let(:user) { FactoryBot.build(:user) }
+
+    context 'the update is successful' do
+      before do
+        user.close!
+        expect(user).to receive(:erase!).and_call_original
+        subject
+      end
+
+      it { is_expected.to eq(true) }
+
+      it 'erases the account' do
+        expect(user.name).to match(/Name Removed/)
+      end
+    end
+
+    context 'the update is unsuccessful' do
+      before do
+        expect(user).to receive(:erase!).and_raise(ActiveRecord::RecordInvalid)
+        subject
+      end
+
+      it { is_expected.to eq(false) }
+
+      it 'does not erase the account' do
+        expect(user.name).not_to match(/Name Removed/)
+      end
+    end
+  end
+
+  describe '#erase!' do
+    subject { user.erase! }
+
+    context 'the user account is not closed' do
+      let(:user) { FactoryBot.build(:user, about_me: 'Hi') }
+
+      it 'raises an ActiveRecord::RecordInvalid' do
+        expect { subject }.to raise_error(ActiveRecord::RecordInvalid)
+      end
+    end
+
+    context 'the update is successful' do
+      let(:user) { FactoryBot.build(:user, :closed, about_me: 'Hi') }
+
+      before do
+        allow(AlaveteliConfiguration).
+          to receive(:user_sign_in_activity_retention_days).and_return(1)
+        FactoryBot.create(:user_sign_in, user: user)
+        FactoryBot.create(:profile_photo, user: user)
+
+        allow(Digest::SHA1).to receive(:hexdigest).and_return('a1b2c3d4')
+        allow(MySociety::Util).
+          to receive(:generate_token).and_return('r@nd0m-pa$$w0rd')
+
+        subject
+
+        user.reload
+      end
+
+      it 'erases the name' do
+        # #name currently appends "(Account suspended)". Here we specifically
+        # only care about the data we hold.
+        expect(user.read_attribute(:name)).to eq('[Name Removed]')
+      end
+
+      it 'erases the url_name' do
+        expect(user.url_name).to eq('a1b2c3d4')
+      end
+
+      it 'erases the email' do
+        expect(user.email).to eq('a1b2c3d4@invalid')
+      end
+
+      it 'erases the password' do
+        expect(user.password).to eq('r@nd0m-pa$$w0rd')
+      end
+
+      it 'erases the about_me' do
+        expect(user.about_me).to be_empty
+      end
+
+      it 'destroys any sign_ins' do
+        expect(user.sign_ins).to be_empty
+      end
+
+      it 'destroys any profile photo' do
+        expect(user.profile_photo).to be_nil
+      end
+    end
+
+    context 'the update is unsuccessful' do
+      let(:user) { FactoryBot.build(:user, :closed, about_me: 'Hi') }
+
+      before do
+        expect(user).to receive(:update!).and_raise(ActiveRecord::RecordInvalid)
+      end
+
+      it 'raises an ActiveRecord::RecordInvalid error' do
+        expect { subject }.to raise_error(ActiveRecord::RecordInvalid)
+      end
+    end
+  end
+
+  describe '#anonymise!' do
+    subject { user.anonymise! }
+
+    let(:user) { FactoryBot.build(:user, name: 'Bob Smith') }
+
+    context 'when the user has info requests' do
+      before { FactoryBot.create(:info_request, user: user) }
+
+      it 'creates a censor rule for user name if the user has info requests' do
+        subject
+        censor_rule = user.censor_rules.last
+        expect(censor_rule.text).to eq(user.name)
+        expect(censor_rule.replacement).to eq ('[Name Removed]')
+        expect(censor_rule.last_edit_editor).to eq('User#anonymise!')
+        expect(censor_rule.last_edit_comment).to eq('User#anonymise!')
+      end
+    end
+
+    context 'when the user has no info requests' do
+      it 'does not create a censor rule' do
+        subject
+        expect(user.censor_rules).to be_empty
+      end
+    end
+  end
+
   describe '.closed' do
 
     it 'should not return users with closed_at timestamp' do