Added special char test to the default beforeInsert callback. I removed the quotes from the milestone displayTpl and added a skip setting for emoji instance25286-customer-label-doesn-t-autocomplete-correctly

Review changes

2 files changed, 69 insertions, 19 deletions

diff --git a/app/assets/javascripts/gfm_auto_complete.js.es6 b/app/assets/javascripts/gfm_auto_complete.js.es6
index 6f9d6283071..2f3da745119 100644
--- a/app/assets/javascripts/gfm_auto_complete.js.es6
+++ b/app/assets/javascripts/gfm_auto_complete.js.es6
@@ -52,6 +52,10 @@
         return $.fn.atwho["default"].callbacks.filter(query, data, searchKey);
       },
       beforeInsert: function(value) {
+        if (value && !this.setting.skipSpecialCharacterTest) {
+          var withoutAt = value.substring(1);
+          if (withoutAt && /[^\w\d]/.test(withoutAt)) value = value.charAt() + '"' + withoutAt + '"';
+        }
         if (!GitLab.GfmAutoComplete.dataLoaded) {
           return this.at;
         } else {
@@ -117,6 +121,7 @@
         insertTpl: ':${name}:',
         data: ['loading'],
         startWithSpace: false,
+        skipSpecialCharacterTest: true,
         callbacks: {
           sorter: this.DefaultOptions.sorter,
           filter: this.DefaultOptions.filter,
@@ -141,6 +146,7 @@
         data: ['loading'],
         startWithSpace: false,
         alwaysHighlightFirst: true,
+        skipSpecialCharacterTest: true,
         callbacks: {
           sorter: this.DefaultOptions.sorter,
           filter: this.DefaultOptions.filter,
@@ -219,12 +225,13 @@
             }
           };
         })(this),
-        insertTpl: '${atwho-at}"${title}"',
+        insertTpl: '${atwho-at}${title}',
         data: ['loading'],
         startWithSpace: false,
         callbacks: {
           matcher: this.DefaultOptions.matcher,
           sorter: this.DefaultOptions.sorter,
+          beforeInsert: this.DefaultOptions.beforeInsert,
           beforeSave: function(milestones) {
             return $.map(milestones, function(m) {
               if (m.title == null) {
@@ -284,18 +291,11 @@
         callbacks: {
           matcher: this.DefaultOptions.matcher,
           sorter: this.DefaultOptions.sorter,
+          beforeInsert: this.DefaultOptions.beforeInsert,
           beforeSave: function(merges) {
-            var sanitizeLabelTitle;
-            sanitizeLabelTitle = function(title) {
-              if (/[\w\?&]+\s+[\w\?&]+/g.test(title)) {
-                return "\"" + (sanitize(title)) + "\"";
-              } else {
-                return sanitize(title);
-              }
-            };
             return $.map(merges, function(m) {
               return {
-                title: sanitizeLabelTitle(m.title),
+                title: sanitize(m.title),
                 color: m.color,
                 search: "" + m.title
               };
@@ -308,6 +308,7 @@
         at: '/',
         alias: 'commands',
         searchKey: 'search',
+        skipSpecialCharacterTest: true,
         displayTpl: function(value) {
           var tpl = '<li>/${name}';
           if (value.aliases.length > 0) {
diff --git a/spec/features/issues/gfm_autocomplete_spec.rb b/spec/features/issues/gfm_autocomplete_spec.rb
index c421da97d76..cd0512a37e6 100644
--- a/spec/features/issues/gfm_autocomplete_spec.rb
+++ b/spec/features/issues/gfm_autocomplete_spec.rb
@@ -2,8 +2,9 @@ require 'rails_helper'
 
 feature 'GFM autocomplete', feature: true, js: true do
   include WaitForAjax
-  let(:user)    { create(:user) }
+  let(:user)    { create(:user, username: 'someone.special') }
   let(:project) { create(:project) }
+  let(:label) { create(:label, project: project, title: 'special+') }
   let(:issue)   { create(:issue, project: project) }
 
   before do
@@ -23,21 +24,69 @@ feature 'GFM autocomplete', feature: true, js: true do
     expect(page).to have_selector('.atwho-container')
   end
 
-  it 'opens autocomplete menu when field is prefixed with non-text character' do
+  it 'doesnt open autocomplete menu character is prefixed with text' do
     page.within '.timeline-content-form' do
-      find('#note_note').native.send_keys('')
+      find('#note_note').native.send_keys('testing')
       find('#note_note').native.send_keys('@')
     end
 
-    expect(page).to have_selector('.atwho-container')
+    expect(page).not_to have_selector('.atwho-view')
   end
 
-  it 'doesnt open autocomplete menu character is prefixed with text' do
-    page.within '.timeline-content-form' do
-      find('#note_note').native.send_keys('testing')
-      find('#note_note').native.send_keys('@')
+  context 'if a selected value has special characters' do
+    it 'wraps the result in double quotes' do
+      note = find('#note_note')
+      page.within '.timeline-content-form' do
+        note.native.send_keys('')
+        note.native.send_keys("~#{label.title[0]}")
+        sleep 1
+        note.click
+      end
+
+      label_item = find('.atwho-view li', text: label.title)
+
+      expect_to_wrap(true, label_item, note, label.title)
     end
 
-    expect(page).not_to have_selector('.atwho-view')
+    it 'doesn\'t wrap for assignee values' do
+      note = find('#note_note')
+      page.within '.timeline-content-form' do
+        note.native.send_keys('')
+        note.native.send_keys("@#{user.username[0]}")
+        sleep 1
+        note.click
+      end
+
+      user_item = find('.atwho-view li', text: user.username)
+
+      expect_to_wrap(false, user_item, note, user.username)
+    end
+
+    it 'doesn\'t wrap for emoji values' do
+      note = find('#note_note')
+      page.within '.timeline-content-form' do
+        note.native.send_keys('')
+        note.native.send_keys(":cartwheel")
+        sleep 1
+        note.click
+      end
+
+      emoji_item = find('.atwho-view li', text: 'cartwheel_tone1')
+
+      expect_to_wrap(false, emoji_item, note, 'cartwheel_tone1')
+    end
+
+    def expect_to_wrap(should_wrap, item, note, value)
+      expect(item).to have_content(value)
+      expect(item).not_to have_content("\"#{value}\"")
+
+      item.click
+
+      if should_wrap
+        expect(note.value).to include("\"#{value}\"")
+      else
+        expect(note.value).not_to include("\"#{value}\"")
+      end
+    end
   end
 end