diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index eaf14009242a5dcb145113bc160688479f6be49f..cc8321d97ad1cd26baacafb9c4a37349e22edfe9 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -116,7 +116,7 @@ class Projects::IssuesController < Projects::ApplicationController
end
def closed_by_merge_requests
- @closed_by_mr = @issue.closed_by_merge_requests(current_user)
+ @closed_by_merge_requests ||= @issue.closed_by_merge_requests(current_user)
end
protected
diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb
index 1adbf3a79b1603ee144e7b62fd3f0dec9cba7315..fda18e7b316d7c130338ac6e85a47eaf0d17d3bc 100644
--- a/app/helpers/issues_helper.rb
+++ b/app/helpers/issues_helper.rb
@@ -84,7 +84,7 @@ module IssuesHelper
end
def merge_requests_sentence(merge_requests)
- merge_requests.map(&:to_reference).to_sentence
+ merge_requests.map(&:to_reference).to_sentence(last_word_connector: ', or ')
end
# Required for Gitlab::Markdown::IssueReferenceFilter
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 0e8bcc1a4ece30b363b26d6c05db5396ff6d5026..efa6a2699929096733f043a2d2927a76aa45cb8a 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -86,6 +86,10 @@ module Issuable
assignee_id_changed?
end
+ def open?
+ opened? || reopened?
+ end
+
#
# Votes
#
diff --git a/app/models/issue.rb b/app/models/issue.rb
index c24a329847ce2a2b6f06187861cadc4276dbfa5f..721831080334cefe53755d5df5d130af734a8931 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -98,11 +98,11 @@ class Issue < ActiveRecord::Base
# From all notes on this issue, we'll select the system notes about linked
# merge requests. Of those, the MRs closing `self` are returned.
- def closed_by_merge_requests(current_user)
+ def closed_by_merge_requests(current_user = nil)
+ return [] unless open?
+
notes.system.flat_map do |note|
- ext = Gitlab::ReferenceExtractor.new(self.project, current_user)
- ext.analyze(note.note)
- ext.merge_requests
- end.uniq.select { |mr| mr.closes_issue?(self) }
+ note.all_references(current_user).merge_requests
+ end.uniq.select { |mr| mr.open? && mr.closes_issue?(self) }
end
end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 3ae74ceac685b2949c6993d73509f16ac49f07c5..0042b95c4f109ef22aa9146c4bbefb1c9c8dca26 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -222,10 +222,6 @@ class MergeRequest < ActiveRecord::Base
self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last
end
- def open?
- opened? || reopened?
- end
-
def work_in_progress?
!!(title =~ /\A\[?WIP\]?:? /i)
end
diff --git a/app/views/projects/issues/_closed_by_box.html.haml b/app/views/projects/issues/_closed_by_box.html.haml
index fe886b6d7d783a4d3a7853f48bdce1bab60bd364..aef352029d06d5425d2bee39b108af321c49dfb5 100644
--- a/app/views/projects/issues/_closed_by_box.html.haml
+++ b/app/views/projects/issues/_closed_by_box.html.haml
@@ -1,6 +1,3 @@
-.issue-closed-by-widget
- %i.fa.fa-check
- - if @closed_by_mr.count == 1
- This issue will be closed when #{gfm(@closed_by_mr.first.to_reference)} is accepted.
- -else
- This issue will be closed when any of merge requests #{gfm(merge_requests_sentence(@closed_by_mr.sort))} is accepted.
+.issue-closed-by-widget
+ = icon('check')
+ This issue will be closed automatically when merge request #{gfm(merge_requests_sentence(@closed_by_merge_requests.sort))} is accepted.
diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml
index 309f276882d84eff136f00f4883aa1bc25fa6129..f01bf2505dad68e7e8b91463af86244f1b5a5bba 100644
--- a/app/views/projects/issues/show.html.haml
+++ b/app/views/projects/issues/show.html.haml
@@ -46,7 +46,7 @@
= markdown(@issue.description)
%textarea.hidden.js-task-list-field
= @issue.description
- - if @closed_by_mr.present?
+ - if @closed_by_merge_requests.present?
= render 'projects/issues/closed_by_box'
.issue-discussion
= render 'projects/issues/discussion'
diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb
index c08ddb4cae1bd6ba25dd6a110b8cb9cead4a67c6..78a6b631eb25cb059abd0a8dfe21fc9e8a885560 100644
--- a/spec/helpers/issues_helper_spec.rb
+++ b/spec/helpers/issues_helper_spec.rb
@@ -117,4 +117,14 @@ describe IssuesHelper do
end
end
+ describe "#merge_requests_sentence" do
+ subject { merge_requests_sentence(merge_requests)}
+ let(:merge_requests) do
+ [ build(:merge_request, iid: 1), build(:merge_request, iid: 2),
+ build(:merge_request, iid: 3)]
+ end
+
+ it { is_expected.to eq("!1, !2, or !3") }
+ end
+
end
diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb
index 8f706f8934b419eaca5e269d16712fbcb7f85100..0f13c4410cdc482943c7d9e17c25b804e81acb57 100644
--- a/spec/models/concerns/issuable_spec.rb
+++ b/spec/models/concerns/issuable_spec.rb
@@ -68,7 +68,6 @@ describe Issue, "Issuable" do
end
end
-
describe "#to_hook_data" do
let(:hook_data) { issue.to_hook_data(user) }
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index 623332cd2f95c16784f6958ad5443194cf4c78bf..c9aa1b063c660cd04f8bd9ab6b0984c1df4052fb 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -68,6 +68,43 @@ describe Issue do
end
end
+ describe '#closed_by_merge_requests' do
+ let(:project) { create(:project) }
+ let(:issue) { create(:issue, project: project, state: "opened")}
+ let(:closed_issue) { build(:issue, project: project, state: "closed")}
+
+ let(:mr) do
+ opts = {
+ title: 'Awesome merge_request',
+ description: "Fixes #{issue.to_reference}",
+ source_branch: 'feature',
+ target_branch: 'master'
+ }
+ MergeRequests::CreateService.new(project, project.owner, opts).execute
+ end
+
+ let(:closed_mr) do
+ opts = {
+ title: 'Awesome merge_request 2',
+ description: "Fixes #{issue.to_reference}",
+ source_branch: 'feature',
+ target_branch: 'master',
+ state: 'closed'
+ }
+ MergeRequests::CreateService.new(project, project.owner, opts).execute
+ end
+
+ it 'returns the merge request to close this issue' do
+ allow(mr).to receive(:closes_issue?).with(issue).and_return(true)
+
+ expect(issue.closed_by_merge_requests).to eq([mr])
+ end
+
+ it "returns an empty array when the current issue is closed already" do
+ expect(closed_issue.closed_by_merge_requests).to eq([])
+ end
+ end
+
it_behaves_like 'an editable mentionable' do
subject { create(:issue) }