From 0771774480ce1f46b27b629d3721917d6c1f1267 Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Fri, 3 Feb 2017 12:06:59 -0200 Subject: [PATCH] Disallow system notes for closed issuables --- app/services/system_note_service.rb | 1 + changelogs/unreleased/issue_19262.yml | 4 +++ spec/services/system_note_service_spec.rb | 39 +++++++++++++++++++++++ 3 files changed, 44 insertions(+) create mode 100644 changelogs/unreleased/issue_19262.yml 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