Link issues from comments and automatically close them

Any mention of Issues, MergeRequests, or Commits via GitLab-flavored markdown
references in descriptions, titles, or attached Notes creates a back-reference
Note that links to the original referencer. Furthermore, pushing commits with
commit messages that match a (configurable) regexp to a project's default
branch will close any issues mentioned by GFM in the matched closing phrase.
If accepting a merge request would close any Issues in this way, a banner is
appended to the merge request's main panel to indicate this.

14 files changed, 268 insertions, 31 deletions

diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 235247f3e52..55d2c3f04fc 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -3,6 +3,7 @@ require 'gitlab/satellite/satellite'
 class Projects::MergeRequestsController < Projects::ApplicationController
   before_filter :module_enabled
   before_filter :merge_request, only: [:edit, :update, :show, :commits, :diffs, :automerge, :automerge_check, :ci_status]
+  before_filter :closes_issues, only: [:edit, :update, :show, :commits, :diffs]
   before_filter :validates_merge_request, only: [:show, :diffs]
   before_filter :define_show_vars, only: [:show, :diffs]
 
@@ -135,6 +136,10 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     @merge_request ||= @project.merge_requests.find_by_iid!(params[:id])
   end
 
+  def closes_issues
+    @closes_issues ||= @merge_request.closes_issues
+  end
+
   def authorize_modify_merge_request!
     return render_404 unless can?(current_user, :modify_merge_request, @merge_request)
   end
diff --git a/app/models/commit.rb b/app/models/commit.rb
index da80c2940ff..f8ca6a5fe82 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -2,6 +2,9 @@ class Commit
   include ActiveModel::Conversion
   include StaticModel
   extend ActiveModel::Naming
+  include Mentionable
+
+  attr_mentionable :safe_message
 
   # Safe amount of files with diffs in one commit to render
   # Used to prevent 500 error on huge commits by suppressing diff
@@ -65,6 +68,29 @@ class Commit
     end
   end
 
+  # Regular expression that identifies commit message clauses that trigger issue closing.
+  def issue_closing_regex
+    @issue_closing_regex ||= Regexp.new(Gitlab.config.gitlab.issue_closing_pattern)
+  end
+
+  # Discover issues should be closed when this commit is pushed to a project's
+  # default branch.
+  def closes_issues project
+    md = issue_closing_regex.match(safe_message)
+    if md
+      extractor = Gitlab::ReferenceExtractor.new
+      extractor.analyze(md[0])
+      extractor.issues_for(project)
+    else
+      []
+    end
+  end
+
+  # Mentionable override.
+  def gfm_reference
+    "commit #{sha[0..5]}"
+  end
+
   def method_missing(m, *args, &block)
     @raw.send(m, *args, &block)
   end
diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb
index f22070f8504..27e39339ae8 100644
--- a/app/models/concerns/mentionable.rb
+++ b/app/models/concerns/mentionable.rb
@@ -1,12 +1,47 @@
 # == Mentionable concern
 #
-# Contains common functionality shared between Issues and Notes
+# Contains functionality related to objects that can mention Users, Issues, MergeRequests, or Commits by
+# GFM references.
 #
-# Used by Issue, Note
+# Used by Issue, Note, MergeRequest, and Commit.
 #
 module Mentionable
   extend ActiveSupport::Concern
 
+  module ClassMethods
+    # Indicate which attributes of the Mentionable to search for GFM references.
+    def attr_mentionable *attrs
+      mentionable_attrs.concat(attrs.map(&:to_s))
+    end
+
+    # Accessor for attributes marked mentionable.
+    def mentionable_attrs
+      @mentionable_attrs ||= []
+    end
+  end
+
+  # Generate a GFM back-reference that will construct a link back to this Mentionable when rendered. Must
+  # be overridden if this model object can be referenced directly by GFM notation.
+  def gfm_reference
+    raise NotImplementedError.new("#{self.class} does not implement #gfm_reference")
+  end
+
+  # Construct a String that contains possible GFM references.
+  def mentionable_text
+    self.class.mentionable_attrs.map { |attr| send(attr) || '' }.join
+  end
+
+  # The GFM reference to this Mentionable, which shouldn't be included in its #references.
+  def local_reference
+    self
+  end
+
+  # Determine whether or not a cross-reference Note has already been created between this Mentionable and
+  # the specified target.
+  def has_mentioned? target
+    Note.cross_reference_exists?(target, local_reference)
+  end
+
   def mentioned_users
     users = []
     return users if mentionable_text.blank?
@@ -24,14 +59,39 @@ module Mentionable
     users.uniq
   end
 
-  def mentionable_text
-    if self.class == Issue
-      description
-    elsif self.class == Note
-      note
-    else
-      nil
+  # Extract GFM references to other Mentionables from this Mentionable. Always excludes its #local_reference.
+  def references p = project, text = mentionable_text
+    return [] if text.blank?
+    ext = Gitlab::ReferenceExtractor.new
+    ext.analyze(text)
+    (ext.issues_for(p) + ext.merge_requests_for(p) + ext.commits_for(p)).uniq - [local_reference]
+  end
+
+  # Create a cross-reference Note for each GFM reference to another Mentionable found in +mentionable_text+.
+  def create_cross_references! p = project, a = author, without = []
+    refs = references(p) - without
+    refs.each do |ref|
+      Note.create_cross_reference_note(ref, local_reference, a, p)
     end
   end
 
+  # If the mentionable_text field is about to change, locate any *added* references and create cross references for
+  # them. Invoke from an observer's #before_save implementation.
+  def notice_added_references p = project, a = author
+    ch = changed_attributes
+    original, mentionable_changed = "", false
+    self.class.mentionable_attrs.each do |attr|
+      if ch[attr]
+        original << ch[attr]
+        mentionable_changed = true
+      end
+    end
+
+    # Only proceed if the saved changes actually include a chance to an attr_mentionable field.
+    return unless mentionable_changed
+
+    preexisting = references(p, original)
+    create_cross_references!(p, a, preexisting)
+  end
+
 end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 03c1c166137..ffe9681fc83 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -32,6 +32,7 @@ class Issue < ActiveRecord::Base
   attr_accessible :title, :assignee_id, :position, :description,
                   :milestone_id, :label_list, :author_id_of_changes,
                   :state_event
+  attr_mentionable :title, :description
 
   acts_as_taggable_on :labels
 
@@ -56,4 +57,10 @@ class Issue < ActiveRecord::Base
 
   # Both open and reopened issues should be listed as opened
   scope :opened, -> { with_state(:opened, :reopened) }
+
+  # Mentionable overrides.
+
+  def gfm_reference
+    "issue ##{iid}"
+  end
 end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 0bb4b231f62..514fc79f7c5 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -31,7 +31,7 @@ class MergeRequest < ActiveRecord::Base
   belongs_to :source_project, foreign_key: :source_project_id, class_name: "Project"
 
   attr_accessible :title, :assignee_id, :source_project_id, :source_branch, :target_project_id, :target_branch, :milestone_id, :author_id_of_changes, :state_event
-
+  attr_mentionable :title
 
   attr_accessor :should_remove_source_branch
 
@@ -255,6 +255,20 @@ class MergeRequest < ActiveRecord::Base
     target_project
   end
 
+  # Return the set of issues that will be closed if this merge request is accepted.
+  def closes_issues
+    if target_branch == project.default_branch
+      unmerged_commits.map { |c| c.closes_issues(project) }.flatten.uniq.sort_by(&:id)
+    else
+      []
+    end
+  end
+
+  # Mentionable override.
+  def gfm_reference
+    "merge request !#{iid}"
+  end
+
   private
 
   def dump_commits(commits)
diff --git a/app/models/note.rb b/app/models/note.rb
index 7598978ad4d..e819a5516b5 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -24,6 +24,7 @@ class Note < ActiveRecord::Base
 
   attr_accessible :note, :noteable, :noteable_id, :noteable_type, :project_id,
                   :attachment, :line_code, :commit_id
+  attr_mentionable :note
 
   belongs_to :project
   belongs_to :noteable, polymorphic: true
@@ -54,15 +55,36 @@ class Note < ActiveRecord::Base
   serialize :st_diff
   before_create :set_diff, if: ->(n) { n.line_code.present? }
 
-  def self.create_status_change_note(noteable, project, author, status)
+  def self.create_status_change_note(noteable, project, author, status, source)
+    body = "_Status changed to #{status}#{' by ' + source.gfm_reference if source}_"
+
+    create({
+      noteable: noteable,
+      project: project,
+      author: author,
+      note: body,
+      system: true
+    }, without_protection: true)
+  end
+
+  # +noteable+ was referenced from +mentioner+, by including GFM in either +mentioner+'s description or an associated Note.
+  # Create a system Note associated with +noteable+ with a GFM back-reference to +mentioner+.
+  def self.create_cross_reference_note(noteable, mentioner, author, project)
     create({
       noteable: noteable,
+      commit_id: (noteable.sha if noteable.respond_to? :sha),
       project: project,
       author: author,
-      note: "_Status changed to #{status}_"
+      note: "_mentioned in #{mentioner.gfm_reference}_",
+      system: true
     }, without_protection: true)
   end
 
+  # Determine whether or not a cross-reference note already exists.
+  def self.cross_reference_exists?(noteable, mentioner)
+    where(noteable_id: noteable.id, system: true, note: "_mentioned in #{mentioner.gfm_reference}_").any?
+  end
+
   def commit_author
     @commit_author ||=
       project.users.find_by_email(noteable.author_email) ||
@@ -191,6 +213,16 @@ class Note < ActiveRecord::Base
     for_issue? || (for_merge_request? && !for_diff_line?)
   end
 
+  # Mentionable override.
+  def gfm_reference
+    noteable.gfm_reference
+  end
+
+  # Mentionable override.
+  def local_reference
+    noteable
+  end
+
   def noteable_type_name
     if noteable_type.present?
       noteable_type.downcase
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 3d649519d8f..aeec48ee5cc 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -18,6 +18,7 @@ class Repository
   end
 
   def commit(id = nil)
+    return nil unless raw_repository
     commit = Gitlab::Git::Commit.find(raw_repository, id)
     commit = Commit.new(commit) if commit
     commit
diff --git a/app/observers/activity_observer.rb b/app/observers/activity_observer.rb
index 477ebd78aeb..d754ac8185a 100644
--- a/app/observers/activity_observer.rb
+++ b/app/observers/activity_observer.rb
@@ -5,8 +5,8 @@ class ActivityObserver < BaseObserver
     event_author_id = record.author_id
 
     if record.kind_of?(Note)
-      # Skip system status notes like 'status changed to close'
-      return true if record.note.include?("_Status changed to ")
+      # Skip system notes, like status changes and cross-references.
+      return true if record.system?
 
       # Skip wall notes to prevent spamming of dashboard
       return true if record.noteable_type.blank?
diff --git a/app/observers/base_observer.rb b/app/observers/base_observer.rb
index 92b73981d27..f9a0242ce77 100644
--- a/app/observers/base_observer.rb
+++ b/app/observers/base_observer.rb
@@ -10,4 +10,8 @@ class BaseObserver < ActiveRecord::Observer
   def current_user
     Thread.current[:current_user]
   end
+
+  def current_commit
+    Thread.current[:current_commit]
+  end
 end
diff --git a/app/observers/issue_observer.rb b/app/observers/issue_observer.rb
index 50538419776..886d8b776fb 100644
--- a/app/observers/issue_observer.rb
+++ b/app/observers/issue_observer.rb
@@ -1,6 +1,8 @@
 class IssueObserver < BaseObserver
   def after_create(issue)
     notification.new_issue(issue, current_user)
+
+    issue.create_cross_references!(issue.project, current_user)
   end
 
   def after_close(issue, transition)
@@ -17,12 +19,14 @@ class IssueObserver < BaseObserver
     if issue.is_being_reassigned?
       notification.reassigned_issue(issue, current_user)
     end
+
+    issue.notice_added_references(issue.project, current_user)
   end
 
   protected
 
   # Create issue note with service comment like 'Status changed to closed'
   def create_note(issue)
-    Note.create_status_change_note(issue, issue.project, current_user, issue.state)
+    Note.create_status_change_note(issue, issue.project, current_user, issue.state, current_commit)
   end
 end
diff --git a/app/observers/merge_request_observer.rb b/app/observers/merge_request_observer.rb
index 689a25ff4b7..d70da514cd2 100644
--- a/app/observers/merge_request_observer.rb
+++ b/app/observers/merge_request_observer.rb
@@ -7,11 +7,13 @@ class MergeRequestObserver < ActivityObserver
     end
 
     notification.new_merge_request(merge_request, current_user)
+
+    merge_request.create_cross_references!(merge_request.project, current_user)
   end
 
   def after_close(merge_request, transition)
     create_event(merge_request, Event::CLOSED)
-    Note.create_status_change_note(merge_request, merge_request.target_project, current_user, merge_request.state)
+    Note.create_status_change_note(merge_request, merge_request.target_project, current_user, merge_request.state, nil)
 
     notification.close_mr(merge_request, current_user)
   end
@@ -33,11 +35,13 @@ class MergeRequestObserver < ActivityObserver
 
   def after_reopen(merge_request, transition)
     create_event(merge_request, Event::REOPENED)
-    Note.create_status_change_note(merge_request, merge_request.target_project, current_user, merge_request.state)
+    Note.create_status_change_note(merge_request, merge_request.target_project, current_user, merge_request.state, nil)
   end
 
   def after_update(merge_request)
     notification.reassigned_merge_request(merge_request, current_user) if merge_request.is_being_reassigned?
+
+    merge_request.notice_added_references(merge_request.project, current_user)
   end
 
   def create_event(record, status)
diff --git a/app/observers/note_observer.rb b/app/observers/note_observer.rb
index 7b79161cce4..d31b6e8d7ce 100644
--- a/app/observers/note_observer.rb
+++ b/app/observers/note_observer.rb
@@ -1,5 +1,17 @@
 class NoteObserver < BaseObserver
   def after_create(note)
     notification.new_note(note)
+
+    unless note.system?
+      # Create a cross-reference note if this Note contains GFM that names an
+      # issue, merge request, or commit.
+      note.references.each do |mentioned|
+        Note.create_cross_reference_note(mentioned, note.noteable, note.author, note.project)
+      end
+    end
+  end
+
+  def after_update(note)
+    note.notice_added_references(note.project, current_user)
   end
 end
diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb
index e8b32f52ce1..e774b2276e8 100644
--- a/app/services/git_push_service.rb
+++ b/app/services/git_push_service.rb
@@ -1,22 +1,24 @@
 class GitPushService
-  attr_accessor :project, :user, :push_data
+  attr_accessor :project, :user, :push_data, :push_commits
 
   # This method will be called after each git update
   # and only if the provided user and project is present in GitLab.
   #
   # All callbacks for post receive action should be placed here.
   #
-  # Now this method do next:
-  #  1. Ensure project satellite exists
-  #  2. Update merge requests
-  #  3. Execute project web hooks
-  #  4. Execute project services
-  #  5. Create Push Event
+  # Next, this method:
+  #  1. Creates the push event
+  #  2. Ensures that the project satellite exists
+  #  3. Updates merge requests
+  #  4. Recognizes cross-references from commit messages
+  #  5. Executes the project's web hooks
+  #  6. Executes the project's services
   #
   def execute(project, user, oldrev, newrev, ref)
     @project, @user = project, user
 
     # Collect data for this git push
+    @push_commits = project.repository.commits_between(oldrev, newrev)
     @push_data = post_receive_data(oldrev, newrev, ref)
 
     create_push_event
@@ -25,11 +27,27 @@ class GitPushService
     project.discover_default_branch
     project.repository.expire_cache
 
-    if push_to_branch?(ref, oldrev)
+    if push_to_existing_branch?(ref, oldrev)
       project.update_merge_requests(oldrev, newrev, ref, @user)
+      process_commit_messages(ref)
       project.execute_hooks(@push_data.dup)
       project.execute_services(@push_data.dup)
     end
+
+    if push_to_new_branch?(ref, oldrev)
+      # Re-find the pushed commits.
+      if is_default_branch?(ref)
+        # Initial push to the default branch. Take the full history of that branch as "newly pushed".
+        @push_commits = project.repository.commits(newrev)
+      else
+        # Use the pushed commits that aren't reachable by the default branch
+        # as a heuristic. This may include more commits than are actually pushed, but
+        # that shouldn't matter because we check for existing cross-references later.
+        @push_commits = project.repository.commits_between(project.default_branch, newrev)
+      end
+
+      process_commit_messages(ref)
+    end
   end
 
   # This method provide a sample data
@@ -45,7 +63,7 @@ class GitPushService
   protected
 
   def create_push_event
-    Event.create(
+    Event.create!(
       project: project,
       action: Event::PUSHED,
       data: push_data,
@@ -53,6 +71,36 @@ class GitPushService
     )
   end
 
+  # Extract any GFM references from the pushed commit messages. If the configured issue-closing regex is matched,
+  # close the referenced Issue. Create cross-reference Notes corresponding to any other referenced Mentionables.
+  def process_commit_messages ref
+    is_default_branch = is_default_branch?(ref)
+
+    @push_commits.each do |commit|
+      # Close issues if these commits were pushed to the project's default branch and the commit message matches the
+      # closing regex. Exclude any mentioned Issues from cross-referencing even if the commits are being pushed to
+      # a different branch.
+      issues_to_close = commit.closes_issues(project)
+      author = commit_user(commit)
+
+      if !issues_to_close.empty? && is_default_branch
+        Thread.current[:current_user] = author
+        Thread.current[:current_commit] = commit
+
+        issues_to_close.each { |i| i.close && i.save }
+      end
+
+      # Create cross-reference notes for any other references. Omit any issues that were referenced in an
+      # issue-closing phrase, or have already been mentioned from this commit (probably from this commit
+      # being pushed to a different branch).
+      refs = commit.references(project) - issues_to_close
+      refs.reject! { |r| commit.has_mentioned?(r) }
+      refs.each do |r|
+        Note.create_cross_reference_note(r, commit, author, project)
+      end
+    end
+  end
+
   # Produce a hash of post-receive data
   #
   # data = {
@@ -72,8 +120,6 @@ class GitPushService
   # }
   #
   def post_receive_data(oldrev, newrev, ref)
-    push_commits = project.repository.commits_between(oldrev, newrev)
-
     # Total commits count
     push_commits_count = push_commits.size
 
@@ -116,10 +162,26 @@ class GitPushService
     data
   end
 
-  def push_to_branch? ref, oldrev
+  def push_to_existing_branch? ref, oldrev
     ref_parts = ref.split('/')
 
     # Return if this is not a push to a branch (e.g. new commits)
-    !(ref_parts[1] !~ /heads/ || oldrev == "00000000000000000000000000000000")
+    ref_parts[1] =~ /heads/ && oldrev != "0000000000000000000000000000000000000000"
+  end
+
+  def push_to_new_branch? ref, oldrev
+    ref_parts = ref.split('/')
+
+    ref_parts[1] =~ /heads/ && oldrev == "0000000000000000000000000000000000000000"
+  end
+
+  def is_default_branch? ref
+    ref == "refs/heads/#{project.default_branch}"
+  end
+
+  def commit_user commit
+    User.where(email: commit.author_email).first ||
+      User.where(name: commit.author_name).first ||
+      user
   end
 end
diff --git a/app/views/projects/merge_requests/show/_mr_box.html.haml b/app/views/projects/merge_requests/show/_mr_box.html.haml
index 594f4061c23..c4b614b51da 100644
--- a/app/views/projects/merge_requests/show/_mr_box.html.haml
+++ b/app/views/projects/merge_requests/show/_mr_box.html.haml
@@ -33,4 +33,10 @@
         %i.icon-ok
         Merged by #{link_to_member(@project, @merge_request.merge_event.author)}
         #{time_ago_in_words(@merge_request.merge_event.created_at)} ago.
-
+  - if !@closes_issues.empty? && @merge_request.opened?
+    .ui-box-bottom.alert-info
+      %span
+        %i.icon-ok
+        Accepting this merge request will close #{@closes_issues.size == 1 ? 'issue' : 'issues'}
+        = succeed '.' do
+          != gfm(@closes_issues.map { |i| "##{i.iid}" }.to_sentence)