Merge branch 'cleaner-merge-commit-messages' into 'master'

Cleaner merge commit messages

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/23462

See merge request !7722

6 files changed, 79 insertions, 12 deletions

diff --git a/app/assets/javascripts/merge_request.js b/app/assets/javascripts/merge_request.js
index 70f9a8d1955..244c2f6746c 100644
--- a/app/assets/javascripts/merge_request.js
+++ b/app/assets/javascripts/merge_request.js
@@ -1,4 +1,4 @@
-/* eslint-disable func-names, space-before-function-paren, no-var, space-before-blocks, prefer-rest-params, wrap-iife, quotes, no-underscore-dangle, one-var, one-var-declaration-per-line, consistent-return, dot-notation, quote-props, comma-dangle, object-shorthand, padded-blocks, max-len */
+/* eslint-disable func-names, space-before-function-paren, no-var, space-before-blocks, prefer-rest-params, wrap-iife, quotes, no-underscore-dangle, one-var, one-var-declaration-per-line, consistent-return, dot-notation, quote-props, comma-dangle, object-shorthand, padded-blocks, max-len, prefer-arrow-callback */
 /* global MergeRequestTabs */
 
 /*= require jquery.waitforimages */
@@ -27,6 +27,7 @@
       // Prevent duplicate event bindings
       this.disableTaskList();
       this.initMRBtnListeners();
+      this.initCommitMessageListeners();
       if ($("a.btn-close").length) {
         this.initTaskList();
       }
@@ -108,6 +109,26 @@
     // note so that we can re-use its form here
     };
 
+    MergeRequest.prototype.initCommitMessageListeners = function() {
+      var textarea = $('textarea.js-commit-message');
+
+      $('a.js-with-description-link').on('click', function(e) {
+        e.preventDefault();
+
+        textarea.val(textarea.data('messageWithDescription'));
+        $('p.js-with-description-hint').hide();
+        $('p.js-without-description-hint').show();
+      });
+
+      $('a.js-without-description-link').on('click', function(e) {
+        e.preventDefault();
+
+        textarea.val(textarea.data('messageWithoutDescription'));
+        $('p.js-with-description-hint').show();
+        $('p.js-without-description-hint').hide();
+      });
+    };
+
     return MergeRequest;
 
   })();
diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb
index a6659ea2fd1..20218775659 100644
--- a/app/helpers/merge_requests_helper.rb
+++ b/app/helpers/merge_requests_helper.rb
@@ -59,6 +59,10 @@ module MergeRequestsHelper
     @mr_closes_issues ||= @merge_request.closes_issues
   end
 
+  def mr_issues_mentioned_but_not_closing
+    @mr_issues_mentioned_but_not_closing ||= @merge_request.issues_mentioned_but_not_closing
+  end
+
   def mr_change_branches_path(merge_request)
     new_namespace_project_merge_request_path(
       @project.namespace, @project,
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index b73d7acefea..b7c775777c7 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -568,6 +568,19 @@ class MergeRequest < ActiveRecord::Base
     end
   end
 
+  def issues_mentioned_but_not_closing(current_user = self.author)
+    return [] unless target_branch == project.default_branch
+
+    ext = Gitlab::ReferenceExtractor.new(project, current_user)
+    ext.analyze(description)
+
+    issues = ext.issues
+    closing_issues = Gitlab::ClosingIssueExtractor.new(project, current_user).
+      closed_by_message(description)
+
+    issues - closing_issues
+  end
+
   def target_project_path
     if target_project
       target_project.path_with_namespace
@@ -612,13 +625,24 @@ class MergeRequest < ActiveRecord::Base
     self.target_project.repository.branch_names.include?(self.target_branch)
   end
 
-  def merge_commit_message
-    message = "Merge branch '#{source_branch}' into '#{target_branch}'\n\n"
-    message << "#{title}\n\n"
-    message << "#{description}\n\n" if description.present?
+  def merge_commit_message(include_description: false)
+    closes_issues_references = closes_issues.map do |issue|
+      issue.to_reference(target_project)
+    end
+
+    message = [
+      "Merge branch '#{source_branch}' into '#{target_branch}'",
+      title
+    ]
+
+    if !include_description && closes_issues_references.present?
+      message << "Closes #{closes_issues_references.to_sentence}"
+    end
+
+    message << "#{description}" if include_description && description.present?
     message << "See merge request #{to_reference}"
 
-    message
+    message.join("\n\n")
   end
 
   def reset_merge_when_build_succeeds
diff --git a/app/views/projects/merge_requests/widget/_open.html.haml b/app/views/projects/merge_requests/widget/_open.html.haml
index eee711dc5af..f4aa1609a1b 100644
--- a/app/views/projects/merge_requests/widget/_open.html.haml
+++ b/app/views/projects/merge_requests/widget/_open.html.haml
@@ -30,11 +30,18 @@
     - elsif @merge_request.can_be_merged? || resolved_conflicts
       = render 'projects/merge_requests/widget/open/accept'
 
-  - if mr_closes_issues.present?
+  - if mr_closes_issues.present? || mr_issues_mentioned_but_not_closing.present?
     .mr-widget-footer
       %span
-        %i.fa.fa-check
-        Accepting this merge request will close #{"issue".pluralize(mr_closes_issues.size)}
-        = succeed '.' do
-          != markdown issues_sentence(mr_closes_issues), pipeline: :gfm, author: @merge_request.author
-        = mr_assign_issues_link
+        = icon('check')
+        - if mr_closes_issues.present?
+          Accepting this merge request will close #{"issue".pluralize(mr_closes_issues.size)}
+          = succeed '.' do
+            != markdown issues_sentence(mr_closes_issues), pipeline: :gfm, author: @merge_request.author
+          = mr_assign_issues_link
+        - if mr_issues_mentioned_but_not_closing.present?
+          #{"Issue".pluralize(mr_issues_mentioned_but_not_closing.size)}
+          != markdown issues_sentence(mr_issues_mentioned_but_not_closing), pipeline: :gfm, author: @merge_request.author
+          #{mr_issues_mentioned_but_not_closing.size > 1 ? 'are' : 'is'} mentioned but will not closed.
+         
+
diff --git a/app/views/projects/merge_requests/widget/open/_accept.html.haml b/app/views/projects/merge_requests/widget/open/_accept.html.haml
index 435fe835fae..d6f7f23533c 100644
--- a/app/views/projects/merge_requests/widget/open/_accept.html.haml
+++ b/app/views/projects/merge_requests/widget/open/_accept.html.haml
@@ -41,6 +41,8 @@
           Modify commit message
     .js-toggle-content.hide.prepend-top-default
       = render 'shared/commit_message_container', params: params,
+          message_with_description: @merge_request.merge_commit_message(include_description: true),
+          message_without_description: @merge_request.merge_commit_message,
           text: @merge_request.merge_commit_message,
           rows: 14, hint: true
 
diff --git a/app/views/shared/_commit_message_container.html.haml b/app/views/shared/_commit_message_container.html.haml
index 0a38327baa2..c196bc06b17 100644
--- a/app/views/shared/_commit_message_container.html.haml
+++ b/app/views/shared/_commit_message_container.html.haml
@@ -1,5 +1,6 @@
 .form-group.commit_message-group
   - nonce = SecureRandom.hex
+  - descriptions = local_assigns.slice(:message_with_description, :message_without_description)
   = label_tag "commit_message-#{nonce}", class: 'control-label' do
     Commit message
   .col-sm-10
@@ -8,9 +9,17 @@
       = text_area_tag 'commit_message',
           (params[:commit_message] || local_assigns[:text] || local_assigns[:placeholder]),
           class: 'form-control js-commit-message', placeholder: local_assigns[:placeholder],
+          data: descriptions,
           required: true, rows: (local_assigns[:rows] || 3),
           id: "commit_message-#{nonce}"
     - if local_assigns[:hint]
       %p.hint
         Try to keep the first line under 52 characters
         and the others under 72.
+    - if descriptions.present?
+      %p.hint.js-with-description-hint
+        = link_to "#", class: "js-with-description-link" do
+          Include description in commit message
+      %p.hint.js-without-description-hint.hide
+        = link_to "#", class: "js-without-description-link" do
+          Don't include description in commit message