Merge branch 'issue_19262' into 'master'

Disallow system notes for closed issuables

Closes #19262

See merge request !8848

3 files changed, 44 insertions, 0 deletions

diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb
index 110072e3a16..87ba72cf991 100644
--- a/app/services/system_note_service.rb
+++ b/app/services/system_note_service.rb
@@ -385,6 +385,7 @@ module SystemNoteService
   # Returns Boolean
   def cross_reference_disallowed?(noteable, mentioner)
     return true if noteable.is_a?(ExternalIssue) && !noteable.project.jira_tracker_active?
+    return true if noteable.is_a?(Issuable) && (noteable.try(:closed?) || noteable.try(:merged?))
     return false unless mentioner.is_a?(MergeRequest)
     return false unless noteable.is_a?(Commit)
 
diff --git a/changelogs/unreleased/issue_19262.yml b/changelogs/unreleased/issue_19262.yml
new file mode 100644
index 00000000000..5dea1493f23
--- /dev/null
+++ b/changelogs/unreleased/issue_19262.yml
@@ -0,0 +1,4 @@
+---
+title: Disallow system notes for closed issuables
+merge_request:
+author:
diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index 7913a180f9b..7f027ae02a2 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -418,6 +418,45 @@ describe SystemNoteService, services: true do
           to be_truthy
       end
     end
+
+    context 'when noteable is an Issue' do
+      let(:issue) { create(:issue, project: project) }
+
+      it 'is truthy when issue is closed' do
+        issue.close
+
+        expect(described_class.cross_reference_disallowed?(issue, project.commit)).
+          to be_truthy
+      end
+
+      it 'is falsey when issue is open' do
+        expect(described_class.cross_reference_disallowed?(issue, project.commit)).
+          to be_falsy
+      end
+    end
+
+    context 'when noteable is a Merge Request' do
+      let(:merge_request) { create(:merge_request, :simple, source_project: project) }
+
+      it 'is truthy when merge request is closed' do
+        allow(merge_request).to receive(:closed?).and_return(:true)
+
+        expect(described_class.cross_reference_disallowed?(merge_request, project.commit)).
+          to be_truthy
+      end
+
+      it 'is truthy when merge request is merged' do
+        allow(merge_request).to receive(:closed?).and_return(:true)
+
+        expect(described_class.cross_reference_disallowed?(merge_request, project.commit)).
+          to be_truthy
+      end
+
+      it 'is falsey when merge request is open' do
+        expect(described_class.cross_reference_disallowed?(merge_request, project.commit)).
+          to be_falsy
+      end
+    end
   end
 
   describe '.cross_reference_exists?' do