Merge branch '207222-batch-destroy-notes-when-removing-project' into 'master'

Fix timeouts when destroying a project with many notes

See merge request gitlab-org/gitlab!62389

app/models/note.rb

@@ -96,7 +96,9 @@ class Note < ApplicationRecord
 
   validate :does_not_exceed_notes_limit?, on: :create, unless: [:system?, :importing?]
 
-  # @deprecated attachments are handler by the MarkdownUploader
+  # @deprecated attachments are handled by the Upload model.
+  #
+  # https://gitlab.com/gitlab-org/gitlab/-/issues/20830
   mount_uploader :attachment, AttachmentUploader
 
   # Scopes

app/models/project.rb

@@ -234,7 +234,15 @@ class Project < ApplicationRecord
   has_many :events
   has_many :milestones
   has_many :iterations
-  has_many :notes
+
+  # Projects with a very large number of notes may time out destroying them
+  # through the foreign key. Additionally, the deprecated attachment uploader
+  # for notes requires us to use dependent: :destroy to avoid orphaning uploaded
+  # files.
+  #
+  # https://gitlab.com/gitlab-org/gitlab/-/issues/207222
+  has_many :notes, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
+
   has_many :snippets, class_name: 'ProjectSnippet'
   has_many :hooks, class_name: 'ProjectHook'
   has_many :protected_branches

spec/models/note_spec.rb

@@ -1522,4 +1522,16 @@ RSpec.describe Note do
       it { is_expected.to be_truthy }
     end
   end
+
+  describe '#attachment' do
+    it 'is cleaned up correctly when project is destroyed' do
+      note = create(:note_on_issue, :with_attachment)
+
+      attachment = note.attachment
+
+      note.project.destroy!
+
+      expect(attachment).not_to be_exist
+    end
+  end
 end

spec/models/project_spec.rb

@@ -28,7 +28,7 @@ RSpec.describe Project, factory_default: :keep do
     it { is_expected.to have_many(:project_members).dependent(:delete_all) }
     it { is_expected.to have_many(:users).through(:project_members) }
     it { is_expected.to have_many(:requesters).dependent(:delete_all) }
-    it { is_expected.to have_many(:notes) }
+    it { is_expected.to have_many(:notes).dependent(:destroy) }
     it { is_expected.to have_many(:snippets).class_name('ProjectSnippet') }
     it { is_expected.to have_many(:deploy_keys_projects) }
     it { is_expected.to have_many(:deploy_keys) }