From 9f9f0c35ecd9f7a5a057030253791d051f832f6d Mon Sep 17 00:00:00 2001
From: Zeger-Jan van de Weg <mail@zjvandeweg.nl>
Date: Mon, 12 Oct 2015 12:04:20 +0200
Subject: Show merge requests which close current issue

---
 CHANGELOG                                             |  1 +
 app/assets/stylesheets/pages/issues.scss              |  5 +++++
 app/controllers/projects/issues_controller.rb         |  7 +++++++
 app/controllers/projects/merge_requests_controller.rb |  2 +-
 app/helpers/issues_helper.rb                          |  4 ++++
 app/models/issue.rb                                   | 10 ++++++++++
 app/models/merge_request.rb                           |  4 ++++
 app/views/projects/issues/_closed_by_box.html.haml    |  6 ++++++
 app/views/projects/issues/show.html.haml              |  3 ++-
 9 files changed, 40 insertions(+), 2 deletions(-)
 create mode 100644 app/views/projects/issues/_closed_by_box.html.haml

diff --git a/CHANGELOG b/CHANGELOG
index 39926692147..e15fc9cb24a 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -3,6 +3,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 v 8.1.0 (unreleased)
   - Fix error preventing displaying of commit data for a directory with a leading dot (Stan Hu)
   - Speed up load times of issue detail pages by roughly 1.5x
+  - If a merge request is to close an issue, show this on the issue page (Zeger-Jan van de Weg)
   - Make diff file view easier to use on mobile screens (Stan Hu)
   - Improved performance of finding users by username or Email address
   - Fix bug where merge request comments created by API would not trigger notifications (Stan Hu)
diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss
index 4bf58cb4a59..41c069f0ad3 100644
--- a/app/assets/stylesheets/pages/issues.scss
+++ b/app/assets/stylesheets/pages/issues.scss
@@ -132,6 +132,11 @@ form.edit-issue {
   }
 }
 
+.issue-closed-by-widget {
+  padding: 16px 0;
+  margin: 0px;
+}
+
 .issue-form .select2-container {
   width: 250px !important;
 }
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 97485c101fb..eaf14009242 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -14,6 +14,9 @@ class Projects::IssuesController < Projects::ApplicationController
   # Allow issues bulk update
   before_action :authorize_admin_issues!, only: [:bulk_update]
 
+  # Cross-reference merge requests
+  before_action :closed_by_merge_requests, only: [:show]
+
   respond_to :html
 
   def index
@@ -112,6 +115,10 @@ class Projects::IssuesController < Projects::ApplicationController
     render nothing: true
   end
 
+  def closed_by_merge_requests
+    @closed_by_mr = @issue.closed_by_merge_requests(current_user)
+  end
+
   protected
 
   def issue
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 98df6984bf7..0d9c5461959 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -259,7 +259,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     @commits = @merge_request.commits
 
     @merge_request_diff = @merge_request.merge_request_diff
-    
+
     if @merge_request.locked_long_ago?
       @merge_request.unlock_mr
       @merge_request.close
diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb
index 6ddb37cd0dc..1adbf3a79b1 100644
--- a/app/helpers/issues_helper.rb
+++ b/app/helpers/issues_helper.rb
@@ -83,6 +83,10 @@ module IssuesHelper
     end
   end
 
+  def merge_requests_sentence(merge_requests)
+    merge_requests.map(&:to_reference).to_sentence
+  end
+
   # Required for Gitlab::Markdown::IssueReferenceFilter
   module_function :url_for_issue
 end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index fc7e9abe29e..c24a329847c 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -95,4 +95,14 @@ class Issue < ActiveRecord::Base
   def source_project
     project
   end
+
+  # 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)
+    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) }
+  end
 end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index c83b15c7d39..3ae74ceac68 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -294,6 +294,10 @@ class MergeRequest < ActiveRecord::Base
     target_project
   end
 
+  def closes_issue?(issue)
+    closes_issues.include?(issue)
+  end
+
   # Return the set of issues that will be closed if this merge request is accepted.
   def closes_issues(current_user = self.author)
     if target_branch == project.default_branch
diff --git a/app/views/projects/issues/_closed_by_box.html.haml b/app/views/projects/issues/_closed_by_box.html.haml
new file mode 100644
index 00000000000..fe886b6d7d7
--- /dev/null
+++ b/app/views/projects/issues/_closed_by_box.html.haml
@@ -0,0 +1,6 @@
+.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.
diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml
index 5cb814c9ea8..309f276882d 100644
--- a/app/views/projects/issues/show.html.haml
+++ b/app/views/projects/issues/show.html.haml
@@ -46,6 +46,7 @@
                 = markdown(@issue.description)
             %textarea.hidden.js-task-list-field
               = @issue.description
-
+  - if @closed_by_mr.present?
+    = render 'projects/issues/closed_by_box'
   .issue-discussion
     = render 'projects/issues/discussion'
-- 
cgit v1.2.1


From 94a788f66dfcc13ad02855b05c38826f958038af Mon Sep 17 00:00:00 2001
From: Zeger-Jan van de Weg <mail@zjvandeweg.nl>
Date: Tue, 13 Oct 2015 09:41:46 +0200
Subject: Only accept open issues and merge requests

---
 app/controllers/projects/issues_controller.rb      |  2 +-
 app/helpers/issues_helper.rb                       |  2 +-
 app/models/concerns/issuable.rb                    |  4 +++
 app/models/issue.rb                                | 10 +++---
 app/models/merge_request.rb                        |  4 ---
 app/views/projects/issues/_closed_by_box.html.haml |  9 ++----
 app/views/projects/issues/show.html.haml           |  2 +-
 spec/helpers/issues_helper_spec.rb                 | 10 ++++++
 spec/models/concerns/issuable_spec.rb              |  1 -
 spec/models/issue_spec.rb                          | 37 ++++++++++++++++++++++
 10 files changed, 62 insertions(+), 19 deletions(-)

diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index eaf14009242..cc8321d97ad 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 1adbf3a79b1..fda18e7b316 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 0e8bcc1a4ec..efa6a269992 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 c24a329847c..72183108033 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 3ae74ceac68..0042b95c4f1 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 fe886b6d7d7..aef352029d0 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 309f276882d..f01bf2505da 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 c08ddb4cae1..78a6b631eb2 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 8f706f8934b..0f13c4410cd 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 623332cd2f9..c9aa1b063c6 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) }
 
-- 
cgit v1.2.1