From 6b279507dabe031872541b04e94b352843dd091b Mon Sep 17 00:00:00 2001
From: Jens Kraemer <jk@jkraemer.net>
Date: Mon, 10 Feb 2020 16:11:10 +0800
Subject: [PATCH] removes references to deleted users in custom field values

- also adds a migration to remove already existing custom values
  pointing to previously deleted user records.
---
 app/models/user.rb                            |  4 +++
 ...1351_remove_orphaned_user_custom_values.rb | 12 +++++++
 test/unit/user_test.rb                        | 33 +++++++++++++++++++
 3 files changed, 49 insertions(+)
 create mode 100644 db/migrate/20200211051351_remove_orphaned_user_custom_values.rb

diff --git a/app/models/user.rb b/app/models/user.rb
index 76f479bcd..166706bde 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -916,6 +916,10 @@ class User < Principal
     Watcher.where('user_id = ?', id).delete_all
     WikiContent.where(['author_id = ?', id]).update_all(['author_id = ?', substitute.id])
     WikiContent::Version.where(['author_id = ?', id]).update_all(['author_id = ?', substitute.id])
+    user_custom_field_ids = CustomField.where(field_format: 'user').pluck(:id)
+    if user_custom_field_ids.any?
+      CustomValue.where(custom_field_id: user_custom_field_ids, value: self.id.to_s).delete_all
+    end
   end
 
   # Singleton class method is public
diff --git a/db/migrate/20200211051351_remove_orphaned_user_custom_values.rb b/db/migrate/20200211051351_remove_orphaned_user_custom_values.rb
new file mode 100644
index 000000000..f57bd51af
--- /dev/null
+++ b/db/migrate/20200211051351_remove_orphaned_user_custom_values.rb
@@ -0,0 +1,12 @@
+class RemoveOrphanedUserCustomValues < ActiveRecord::Migration[5.2]
+  def up
+    user_custom_field_ids = CustomField.where(field_format: 'user').pluck(:id)
+    if user_custom_field_ids.any?
+      user_ids = Principal.pluck(:id)
+      CustomValue.
+        where(custom_field_id: user_custom_field_ids).
+        where.not(value: [nil, ''] + user_ids).
+        delete_all
+    end
+  end
+end
diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb
index 4f158904e..2cb3d8018 100644
--- a/test/unit/user_test.rb
+++ b/test/unit/user_test.rb
@@ -1295,6 +1295,39 @@ class UserTest < ActiveSupport::TestCase
     assert_equal [], User.find(2).bookmarked_project_ids
   end
 
+  def test_remove_custom_field_references_upon_destroy
+    cf1 = IssueCustomField.create(field_format: 'user', name: 'user cf', is_for_all: true, tracker_ids: Tracker.pluck(:id))
+    cf2 = IssueCustomField.create(field_format: 'user', name: 'users cf', is_for_all: true, multiple: true, tracker_ids: Tracker.pluck(:id))
+
+    issue = Issue.first
+    issue.init_journal(@admin)
+    assert_difference ->{cf1.custom_values.count} do
+      assert_difference ->{cf2.custom_values.count}, 2 do
+        issue.update_attributes(custom_field_values: {
+          cf1.id => @jsmith.id,
+          cf2.id => [@dlopper.id, @jsmith.id]
+        })
+      end
+    end
+    assert cv1 = cf1.custom_values.where(customized_id: issue.id).last
+    assert_equal @jsmith.id.to_s, cv1.value
+
+    assert cv2 = cf2.custom_values.where(customized_id: issue.id)
+    assert_equal 2, cv2.size
+    assert cv2a = cv2.detect{|cv|cv.value == @dlopper.id.to_s}
+    assert cv2b = cv2.detect{|cv|cv.value == @jsmith.id.to_s}
+
+    assert_difference ->{CustomValue.count}, -2 do
+      @jsmith.destroy
+    end
+
+    assert_raise(ActiveRecord::RecordNotFound){cv1.reload}
+    assert_raise(ActiveRecord::RecordNotFound){cv2b.reload}
+
+    cv2a.reload
+    assert_equal @dlopper.id.to_s, cv2a.value
+  end
+
   if Object.const_defined?(:OpenID)
     def test_setting_identity_url
       normalized_open_id_url = 'http://example.com/'
-- 
2.20.1