Allow custom apply suggestion commit messages

This commit adds the capability to define a custom
commit message, which is used when a change
suggestion on a merge request is applied. The
configuration is done on a project settings level and
supports free text with GitLab variables. The old
hardcoded commit message is used as a fallback
option.

app/controllers/projects_controller.rb

@@ -387,6 +387,7 @@ class ProjectsController < Projects::ApplicationController
       :merge_method,
       :initialize_with_readme,
       :autoclose_referenced_issues,
+      :suggestion_commit_message,
 
       project_feature_attributes: %i[
         builds_access_level

app/services/suggestions/apply_service.rb

@@ -2,6 +2,23 @@
 
 module Suggestions
   class ApplyService < ::BaseService
+    DEFAULT_SUGGESTION_COMMIT_MESSAGE = 'Apply suggestion to %{file_path}'
+
+    PLACEHOLDERS = {
+      'project_path' => ->(suggestion, user_name) { suggestion.project.path },
+      'project_name' => ->(suggestion, user_name) { suggestion.project.name },
+      'file_path' => ->(suggestion, user_name) { suggestion.file_path },
+      'branch_name' => ->(suggestion, user_name) { suggestion.branch },
+      'user_name' => ->(suggestion, user_name) { user_name }
+    }.freeze
+
+    # This regex is built dynamically using the keys from the PLACEHOLDER struct.
+    # So, we can easily add new placeholder just by modifying the PLACEHOLDER hash.
+    # This regex will build the new PLACEHOLDER_REGEX with the new information
+    PLACEHOLDERS_REGEX = /(#{PLACEHOLDERS.keys.join('|')})/.freeze
+
+    attr_reader :current_user
+
     def initialize(current_user)
       @current_user = current_user
     end
@@ -22,7 +39,7 @@ module Suggestions
       end
 
       params = file_update_params(suggestion, diff_file)
-      result = ::Files::UpdateService.new(suggestion.project, @current_user, params).execute
+      result = ::Files::UpdateService.new(suggestion.project, current_user, params).execute
 
       if result[:status] == :success
         suggestion.update(commit_id: result[:result], applied: true)
@@ -46,13 +63,14 @@ module Suggestions
 
     def file_update_params(suggestion, diff_file)
       blob = diff_file.new_blob
+      project = suggestion.project
       file_path = suggestion.file_path
       branch_name = suggestion.branch
       file_content = new_file_content(suggestion, blob)
-      commit_message = "Apply suggestion to #{file_path}"
+      commit_message = processed_suggestion_commit_message(suggestion)
 
       file_last_commit =
-        Gitlab::Git::Commit.last_for_path(suggestion.project.repository,
+        Gitlab::Git::Commit.last_for_path(project.repository,
                                           blob.commit_id,
                                           blob.path)
 
@@ -75,5 +93,17 @@ module Suggestions
 
       content.join
     end
+
+    def suggestion_commit_message(project)
+      project.suggestion_commit_message || DEFAULT_SUGGESTION_COMMIT_MESSAGE
+    end
+
+    def processed_suggestion_commit_message(suggestion)
+      message = suggestion_commit_message(suggestion.project)
+
+      Gitlab::StringPlaceholderReplacer.replace_string_placeholders(message, PLACEHOLDERS_REGEX) do |key|
+        PLACEHOLDERS[key].call(suggestion, current_user.name)
+      end
+    end
   end
 end

app/views/projects/_merge_request_merge_suggestions_settings.html.haml

+- form = local_assigns.fetch(:form)
+
+.form-group
+  %b= s_('ProjectSettings|Merge suggestions')
+  %p.text-secondary
+    = s_('ProjectSettings|The commit message used to apply merge request suggestions')
+    = link_to icon('question-circle'),
+      help_page_path('user/discussions/index.md',
+      anchor: 'configure-the-commit-message-for-applied-suggestions'),
+      target: '_blank'
+  .mb-2
+    = form.text_field :suggestion_commit_message, class: 'form-control mb-2', placeholder: Suggestions::ApplyService::DEFAULT_SUGGESTION_COMMIT_MESSAGE
+    %p.form-text.text-muted
+      = s_('ProjectSettings|The variables GitLab supports:')
+      - Suggestions::ApplyService::PLACEHOLDERS.keys.each do |placeholder|
+        %code
+          = "%{#{placeholder}}".html_safe

app/views/projects/_merge_request_settings.html.haml

@@ -5,3 +5,5 @@
 = render 'projects/merge_request_merge_options_settings', project: @project, form: form
 
 = render 'projects/merge_request_merge_checks_settings', project: @project, form: form
+
+= render 'projects/merge_request_merge_suggestions_settings', project: @project, form: form

app/views/projects/_merge_request_settings_description_text.html.haml

-%p= s_('ProjectSettings|Choose your merge method, merge options, and merge checks.')
+%p= s_('ProjectSettings|Choose your merge method, merge options, merge checks, and merge suggestions.')

db/migrate/20191208110214_add_suggestion_commit_message_to_projects.rb

+# frozen_string_literal: true
+
+class AddSuggestionCommitMessageToProjects < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def change
+    add_column :projects, :suggestion_commit_message, :string, limit: 255
+  end
+end

db/schema.rb

@@ -3347,6 +3347,7 @@ ActiveRecord::Schema.define(version: 2020_01_08_233040) do
     t.date "marked_for_deletion_at"
     t.integer "marked_for_deletion_by_user_id"
     t.boolean "autoclose_referenced_issues"
+    t.string "suggestion_commit_message", limit: 255
     t.index "lower((name)::text)", name: "index_projects_on_lower_name"
     t.index ["created_at", "id"], name: "index_projects_on_created_at_and_id"
     t.index ["creator_id"], name: "index_projects_on_creator_id"

ee/app/views/projects/_merge_request_settings.html.haml

@@ -6,6 +6,8 @@
 
 = render_ce 'projects/merge_request_merge_checks_settings', project: @project, form: form
 
+= render 'projects/merge_request_merge_suggestions_settings', project: @project, form: form
+
 - if @project.feature_available?(:issuable_default_templates)
   .form-group
     = form.label :merge_requests_template, class: 'label-bold' do

locale/gitlab.pot

@@ -14127,6 +14127,9 @@ msgstr ""
 msgid "ProjectSettings|Choose your merge method, merge options, and merge checks."
 msgstr ""
 
+msgid "ProjectSettings|Choose your merge method, merge options, merge checks, and merge suggestions."
+msgstr ""
+
 msgid "ProjectSettings|Choose your merge method, merge options, merge checks, and set up a default description template for merge requests."
 msgstr ""
 
@@ -14211,6 +14214,9 @@ msgstr ""
 msgid "ProjectSettings|Merge requests"
 msgstr ""
 
+msgid "ProjectSettings|Merge suggestions"
+msgstr ""
+
 msgid "ProjectSettings|No merge commits are created"
 msgstr ""
 
@@ -14262,6 +14268,12 @@ msgstr ""
 msgid "ProjectSettings|Submit changes to be merged upstream"
 msgstr ""
 
+msgid "ProjectSettings|The commit message used to apply merge request suggestions"
+msgstr ""
+
+msgid "ProjectSettings|The variables GitLab supports:"
+msgstr ""
+
 msgid "ProjectSettings|These checks must pass before merge requests can be merged"
 msgstr ""
 

spec/lib/gitlab/import_export/safe_model_attributes.yml

@@ -535,6 +535,7 @@ Project:
 - merge_requests_disable_committers_approval
 - require_password_to_approve
 - autoclose_referenced_issues
+- suggestion_commit_message
 ProjectTracingSetting:
 - external_url
 Author:

spec/services/suggestions/apply_service_spec.rb

@@ -48,10 +48,34 @@ describe Suggestions::ApplyService do
       expect(commit.committer_email).to eq(user.commit_email)
       expect(commit.author_name).to eq(user.name)
     end
+
+    context 'without a custom suggestion commit messge' do
+      before do
+        project.update!(suggestion_commit_message: nil)
+      end
+
+      it 'sets default commit message' do
+        apply(suggestion)
+
+        expect(project.repository.commit.message).to eq("Apply suggestion to files/ruby/popen.rb")
+      end
+    end
+
+    context 'with a custom suggestion commit message' do
+      before do
+        project.update!(suggestion_commit_message: 'refactor: %{project_path} %{project_name} %{file_path} %{branch_name} %{user_name}')
+      end
+
+      it 'sets custom commit message' do
+        apply(suggestion)
+
+        expect(project.repository.commit.message).to eq("refactor: project-1 Project_1 files/ruby/popen.rb master Test User")
+      end
+    end
   end
 
-  let(:project) { create(:project, :repository) }
-  let(:user) { create(:user, :commit_email) }
+  let(:project) { create(:project, :repository, path: 'project-1', name: 'Project_1') }
+  let(:user) { create(:user, :commit_email, name: 'Test User') }
 
   let(:position) { build_position }
 
@@ -113,7 +137,8 @@ describe Suggestions::ApplyService do
     context 'non-fork project' do
       let(:merge_request) do
         create(:merge_request, source_project: project,
-                               target_project: project)
+                               target_project: project,
+                               source_branch: 'master')
       end
 
       before do

spec/views/projects/edit.html.haml_spec.rb

@@ -28,6 +28,32 @@ describe 'projects/edit' do
     end
   end
 
+  context 'merge suggestions settings' do
+    it 'displays all possible variables' do
+      render
+
+      expect(rendered).to have_content('%{project_path}')
+      expect(rendered).to have_content('%{project_name}')
+      expect(rendered).to have_content('%{file_path}')
+      expect(rendered).to have_content('%{branch_name}')
+      expect(rendered).to have_content('%{user_name}')
+    end
+
+    it 'displays a placeholder if none is set' do
+      render
+
+      expect(rendered).to have_field('project[suggestion_commit_message]', placeholder: 'Apply suggestion to %{file_path}')
+    end
+
+    it 'displays the user entered value' do
+      project.update!(suggestion_commit_message: 'refactor: changed %{file_path}')
+
+      render
+
+      expect(rendered).to have_field('project[suggestion_commit_message]', with: 'refactor: changed %{file_path}')
+    end
+  end
+
   context 'forking' do
     before do
       assign(:project, project)