Fix missing deletion of custom field attachments upon issue deletion (#38966).

Patch by Takenori TAKAKI (user:takenory).


git-svn-id: https://svn.redmine.org/redmine/trunk@23161 e93f8b46-1217-0410-a6f0-8f06a7374b81

3 files changed, 46 insertions, 0 deletions

diff --git a/db/migrate/20241022095140_remove_orphaned_custom_value_attachments.rb b/db/migrate/20241022095140_remove_orphaned_custom_value_attachments.rb
new file mode 100644
index 000000000..a598b3595
--- /dev/null
+++ b/db/migrate/20241022095140_remove_orphaned_custom_value_attachments.rb
@@ -0,0 +1,11 @@
+class RemoveOrphanedCustomValueAttachments < ActiveRecord::Migration[7.2]
+  def up
+    Attachment.where(container_type: 'CustomValue')
+              .where('NOT EXISTS (SELECT 1 FROM custom_values WHERE custom_values.id = attachments.container_id)')
+              .destroy_all
+  end
+
+  def down
+    # no-op
+  end
+end
diff --git a/lib/plugins/acts_as_customizable/lib/acts_as_customizable.rb b/lib/plugins/acts_as_customizable/lib/acts_as_customizable.rb
index a63af9d2a..146fa95c2 100644
--- a/lib/plugins/acts_as_customizable/lib/acts_as_customizable.rb
+++ b/lib/plugins/acts_as_customizable/lib/acts_as_customizable.rb
@@ -29,6 +29,7 @@ module Redmine
           return if self.included_modules.include?(Redmine::Acts::Customizable::InstanceMethods)
           cattr_accessor :customizable_options
           self.customizable_options = options
+          before_destroy :store_attachment_custom_value_ids
           has_many :custom_values, lambda {includes(:custom_field)},
                                    :as => :customized,
                                    :inverse_of => :customized,
@@ -38,6 +39,7 @@ module Redmine
           send :include, Redmine::Acts::Customizable::InstanceMethods
           validate :validate_custom_field_values
           after_save :save_custom_field_values
+          after_destroy :destroy_custom_value_attachments
         end
       end
 
@@ -170,6 +172,17 @@ module Redmine
           super
         end
 
+        def store_attachment_custom_value_ids
+          @attachment_custom_value_ids =
+            custom_values.select {|cv| cv.custom_field.field_format == 'attachment'}
+                         .map(&:id)
+        end
+
+        def destroy_custom_value_attachments
+          Attachment.where(:container_id => @attachment_custom_value_ids, :container_type => 'CustomValue')
+                    .destroy_all
+        end
+
         module ClassMethods
         end
       end
diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb
index 1ec7ef79c..5e2e702b1 100644
--- a/test/unit/issue_test.rb
+++ b/test/unit/issue_test.rb
@@ -2190,6 +2190,28 @@ class IssueTest < ActiveSupport::TestCase
     end
   end
 
+  def test_destroy_should_delete_attachments_on_custom_values
+    cf = IssueCustomField.create!(:name => 'Attachable field', :field_format => 'attachment', :is_for_all => true, :tracker_ids => [1])
+    user = User.find(2)
+    issue = Issue.new(:project_id => 1, :tracker_id => 1, :subject => 'test', :author_id => user.id)
+    attachment = Attachment.create!(:container => issue, :file => uploaded_test_file('testfile.txt', 'text/plain'), :author_id => user.id)
+    issue.send(
+      :safe_attributes=,
+      {
+        'custom_fields' =>
+          [
+            {'id' => cf.id.to_s, 'value' => attachment.id.to_s},
+          ]
+      }, user
+    )
+
+    assert_difference 'CustomValue.where(:customized_type => "Issue").count', -(issue.custom_values.count) do
+      assert_difference 'Attachment.count', -1 do
+        issue.destroy
+      end
+    end
+  end
+
   def test_destroying_a_deleted_issue_should_not_raise_an_error
     issue = Issue.find(1)
     Issue.find(1).destroy