Incorporate backend review comments and findings

app/services/suggestions/apply_service.rb

@@ -5,17 +5,18 @@ module Suggestions
     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 }
+      'project_path' => ->(suggestion, user) { suggestion.project.path },
+      'project_name' => ->(suggestion, user) { suggestion.project.name },
+      'file_path' => ->(suggestion, user) { suggestion.file_path },
+      'branch_name' => ->(suggestion, user) { suggestion.branch },
+      'username' => ->(suggestion, user) { user.username },
+      'user_full_name' => ->(suggestion, user) { 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
+    PLACEHOLDERS_REGEX = Regexp.union(PLACEHOLDERS.keys.map { |key| Regexp.new(Regexp.escape(key)) }).freeze
 
     attr_reader :current_user
 
@@ -102,7 +103,7 @@ module Suggestions
       message = suggestion_commit_message(suggestion.project)
 
       Gitlab::StringPlaceholderReplacer.replace_string_placeholders(message, PLACEHOLDERS_REGEX) do |key|
-        PLACEHOLDERS[key].call(suggestion, current_user.name)
+        PLACEHOLDERS[key].call(suggestion, current_user)
       end
     end
   end

doc/user/discussions/index.md

@@ -445,7 +445,7 @@ suggestion.
 
 ### Configure the commit message for applied suggestions
 
-GitLab will use `Apply suggestion to <file-name>` by default as commit messages
+GitLab will use `Apply suggestion to %{file_path}` by default as commit messages
 when applying change suggestions. This commit message can be customized to
 follow any guidelines you might have. To do so, open the **Merge requests** tab
 within your project settings and change the **Merge suggestions** text.
@@ -458,7 +458,8 @@ You can also use following variables besides static text:
 - `%{project_name}`: The human readable name of the project. E.g: *My Project*
 - `%{file_path}`: The full path of the file the suggestion is applied to. E.g: *docs/index.md*
 - `%{branch_name}`: The name of the branch the suggestion is applied on. E.g: *my-feature-branch*
-- `%{user_name}`: The name of the user applying the suggestion. E.g: *User 1*
+- `%{username}`: The username of the user applying the suggestion. E.g: *user_1*
+- `%{user_full_name}`: The full name of the user applying the suggestion. E.g: *User 1*
 
 ## Start a thread by replying to a standard comment
 

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

 - if @project.feature_available?(:issuable_default_templates)
-  %p= s_('ProjectSettings|Choose your merge method, merge options, merge checks, and set up a default description template for merge requests.')
+  %p= s_('ProjectSettings|Choose your merge method, merge options, merge checks, merge suggestions, and set up a default description template for merge requests.')
 - else
-  %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.')

locale/gitlab.pot

@@ -14124,13 +14124,10 @@ msgstr ""
 msgid "ProjectSettings|Build, test, and deploy your changes"
 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."
+msgid "ProjectSettings|Choose your merge method, merge options, merge checks, merge suggestions, and set up a default description template for merge requests."
 msgstr ""
 
 msgid "ProjectSettings|Contact an admin to change this setting."

spec/services/suggestions/apply_service_spec.rb

@@ -65,17 +65,17 @@ describe Suggestions::ApplyService do
       end
 
       context 'is specified' do
-        let(:message) { 'refactor: %{project_path} %{project_name} %{file_path} %{branch_name} %{user_name}' }
+        let(:message) { 'refactor: %{project_path} %{project_name} %{file_path} %{branch_name} %{username} %{user_full_name}' }
 
         it 'sets custom commit message' do
-          expect(project.repository.commit.message).to eq("refactor: project-1 Project_1 files/ruby/popen.rb master Test User")
+          expect(project.repository.commit.message).to eq("refactor: project-1 Project_1 files/ruby/popen.rb master test.user Test User")
         end
       end
     end
   end
 
   let(:project) { create(:project, :repository, path: 'project-1', name: 'Project_1') }
-  let(:user) { create(:user, :commit_email, name: 'Test User') }
+  let(:user) { create(:user, :commit_email, name: 'Test User', username: 'test.user') }
 
   let(:position) { build_position }
 

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

@@ -36,7 +36,8 @@ describe 'projects/edit' do
       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}')
+      expect(rendered).to have_content('%{username}')
+      expect(rendered).to have_content('%{user_full_name}')
     end
 
     it 'displays a placeholder if none is set' do