Remove diaspora_handle from AccountDeletions and add unique index

app/models/account_deletion.rb

@@ -5,23 +5,15 @@
 class AccountDeletion < ApplicationRecord
   include Diaspora::Federated::Base
 
-  scope :uncompleted, -> { where('completed_at is null') }
+  scope :uncompleted, -> { where("completed_at is null") }
 
   belongs_to :person
-  after_commit :queue_delete_account, :on => :create
+  after_commit :queue_delete_account, on: :create
 
-  def person=(person)
-    self[:diaspora_handle] = person.diaspora_handle
-    self[:person_id] = person.id
-  end
-
-  def diaspora_handle=(diaspora_handle)
-    self[:diaspora_handle] = diaspora_handle
-    self[:person_id] ||= Person.find_by_diaspora_handle(diaspora_handle).id
-  end
+  delegate :diaspora_handle, to: :person
 
   def queue_delete_account
-    Workers::DeleteAccount.perform_async(self.id)
+    Workers::DeleteAccount.perform_async(id)
   end
 
   def perform!

app/models/user.rb

@@ -507,7 +507,7 @@ class User < ApplicationRecord
   def close_account!
     self.person.lock_access!
     self.lock_access!
-    AccountDeletion.create(:person => self.person)
+    AccountDeletion.create(person: person)
   end
 
   def closed_account?

db/migrate/20170813141631_cleanup_account_deletions_and_add_unique_index.rb

+class CleanupAccountDeletionsAndAddUniqueIndex < ActiveRecord::Migration[5.1]
+  def up
+    remove_column :account_deletions, :diaspora_handle
+
+    duplicate_query = "WHERE a1.person_id = a2.person_id AND a1.id > a2.id"
+    if AppConfig.postgres?
+      execute("DELETE FROM account_deletions AS a1 USING account_deletions AS a2 #{duplicate_query}")
+    else
+      execute("DELETE a1 FROM account_deletions a1, account_deletions a2 #{duplicate_query}")
+    end
+
+    add_index :account_deletions, :person_id, name: :index_account_deletions_on_person_id, unique: true
+  end
+
+  def down
+    remove_index :account_deletions, name: :index_account_deletions_on_person_id
+    add_column :account_deletions, :diaspora_handle, :string
+  end
+end

lib/account_deleter.rb

@@ -106,6 +106,6 @@ class AccountDeleter
   end
 
   def mark_account_deletion_complete
-    AccountDeletion.where(:diaspora_handle => self.person.diaspora_handle).where(:person_id => self.person.id).update_all(["completed_at = ?", Time.now])
+    AccountDeletion.find_by(person: person)&.update_attributes(completed_at: Time.now.utc)
   end
 end

lib/diaspora/federation/receive.rb

@@ -8,7 +8,7 @@ module Diaspora
       end
 
       def self.account_deletion(entity)
-        AccountDeletion.create!(person: author_of(entity), diaspora_handle: entity.author)
+        AccountDeletion.create!(person: author_of(entity))
       end
 
       def self.comment(entity)

spec/factories.rb

@@ -51,9 +51,6 @@ FactoryGirl.define do
 
   factory :account_deletion do
     association :person
-    after(:build) do |delete|
-      delete.diaspora_handle = delete.person.diaspora_handle
-    end
   end
 
   factory :like do

spec/integration/federation/receive_federation_messages_spec.rb

@@ -14,18 +14,19 @@ describe "Receive federation messages feature" do
   context "with public receive" do
     let(:recipient) { nil }
 
-    it "receives account deletion correctly" do
-      post_message(generate_payload(DiasporaFederation::Entities::AccountDeletion.new(diaspora_id: sender_id), sender))
+    context "account deletion" do
+      it "receives account deletion correctly" do
+        post_message(generate_payload(DiasporaFederation::Entities::AccountDeletion.new(author: sender_id), sender))
 
-      expect(AccountDeletion.exists?(diaspora_handle: sender_id)).to be_truthy
-    end
-
-    it "rejects account deletion with wrong diaspora_id" do
-      delete_id = Fabricate.sequence(:diaspora_id)
-      post_message(generate_payload(DiasporaFederation::Entities::AccountDeletion.new(diaspora_id: delete_id), sender))
+        expect(AccountDeletion.exists?(person: sender.person)).to be_truthy
+      end
 
-      expect(AccountDeletion.exists?(diaspora_handle: delete_id)).to be_falsey
-      expect(AccountDeletion.exists?(diaspora_handle: sender_id)).to be_falsey
+      it "rejects account deletion with wrong author" do
+        delete_id = Fabricate.sequence(:diaspora_id)
+        expect {
+          post_message(generate_payload(DiasporaFederation::Entities::AccountDeletion.new(author: delete_id), sender))
+        }.not_to change(AccountDeletion, :count)
+      end
     end
 
     context "reshare" do

spec/lib/diaspora/federation/receive_spec.rb

@@ -8,9 +8,7 @@ describe Diaspora::Federation::Receive do
     it "saves the account deletion" do
       Diaspora::Federation::Receive.account_deletion(account_deletion_entity)
 
-      account_deletion = AccountDeletion.find_by!(diaspora_handle: sender.diaspora_handle)
-
-      expect(account_deletion.person).to eq(sender)
+      expect(AccountDeletion.exists?(person: sender)).to be_truthy
     end
   end