Merge branch '25286-customer-label-doesn-t-autocomplete-correctly' into 'master'
Correct autocomplete for values with special characters ## What does this MR do? This adds a check for any special chars in any value passed to the `DefaultOptions.beforeInsert` callback function. If special chars are found and `skipSpecialCharTest` option is `false`, it will wrap the value in quotation marks. This fixed autocompleting `~customer+` instead of `~"customer+"`. ## Are there points in the code the reviewer needs to double check? ## Why was this MR needed? ## Screenshots (if relevant) ![2016-12-03_10.37.11](/uploads/59159623638939933d23b447692775b8/2016-12-03_10.37.11.gif) ## Does this MR meet the acceptance criteria? - [ ] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added - [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md) - [ ] API support added - Tests - [ ] Added for this feature/bug - [ ] All builds are passing - [ ] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html) - [ ] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [ ] Branch has no merge conflicts with `master` (if it does - rebase it please) - [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) ## What are the relevant issue numbers? Closes #25286, #24961 See merge request !7910
This commit is contained in:
commit
4db62fef53
|
@ -52,6 +52,10 @@
|
||||||
return $.fn.atwho["default"].callbacks.filter(query, data, searchKey);
|
return $.fn.atwho["default"].callbacks.filter(query, data, searchKey);
|
||||||
},
|
},
|
||||||
beforeInsert: function(value) {
|
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) {
|
if (!GitLab.GfmAutoComplete.dataLoaded) {
|
||||||
return this.at;
|
return this.at;
|
||||||
} else {
|
} else {
|
||||||
|
@ -117,6 +121,7 @@
|
||||||
insertTpl: ':${name}:',
|
insertTpl: ':${name}:',
|
||||||
data: ['loading'],
|
data: ['loading'],
|
||||||
startWithSpace: false,
|
startWithSpace: false,
|
||||||
|
skipSpecialCharacterTest: true,
|
||||||
callbacks: {
|
callbacks: {
|
||||||
sorter: this.DefaultOptions.sorter,
|
sorter: this.DefaultOptions.sorter,
|
||||||
filter: this.DefaultOptions.filter,
|
filter: this.DefaultOptions.filter,
|
||||||
|
@ -141,6 +146,7 @@
|
||||||
data: ['loading'],
|
data: ['loading'],
|
||||||
startWithSpace: false,
|
startWithSpace: false,
|
||||||
alwaysHighlightFirst: true,
|
alwaysHighlightFirst: true,
|
||||||
|
skipSpecialCharacterTest: true,
|
||||||
callbacks: {
|
callbacks: {
|
||||||
sorter: this.DefaultOptions.sorter,
|
sorter: this.DefaultOptions.sorter,
|
||||||
filter: this.DefaultOptions.filter,
|
filter: this.DefaultOptions.filter,
|
||||||
|
@ -219,12 +225,13 @@
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
})(this),
|
})(this),
|
||||||
insertTpl: '${atwho-at}"${title}"',
|
insertTpl: '${atwho-at}${title}',
|
||||||
data: ['loading'],
|
data: ['loading'],
|
||||||
startWithSpace: false,
|
startWithSpace: false,
|
||||||
callbacks: {
|
callbacks: {
|
||||||
matcher: this.DefaultOptions.matcher,
|
matcher: this.DefaultOptions.matcher,
|
||||||
sorter: this.DefaultOptions.sorter,
|
sorter: this.DefaultOptions.sorter,
|
||||||
|
beforeInsert: this.DefaultOptions.beforeInsert,
|
||||||
beforeSave: function(milestones) {
|
beforeSave: function(milestones) {
|
||||||
return $.map(milestones, function(m) {
|
return $.map(milestones, function(m) {
|
||||||
if (m.title == null) {
|
if (m.title == null) {
|
||||||
|
@ -284,18 +291,11 @@
|
||||||
callbacks: {
|
callbacks: {
|
||||||
matcher: this.DefaultOptions.matcher,
|
matcher: this.DefaultOptions.matcher,
|
||||||
sorter: this.DefaultOptions.sorter,
|
sorter: this.DefaultOptions.sorter,
|
||||||
|
beforeInsert: this.DefaultOptions.beforeInsert,
|
||||||
beforeSave: function(merges) {
|
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 $.map(merges, function(m) {
|
||||||
return {
|
return {
|
||||||
title: sanitizeLabelTitle(m.title),
|
title: sanitize(m.title),
|
||||||
color: m.color,
|
color: m.color,
|
||||||
search: "" + m.title
|
search: "" + m.title
|
||||||
};
|
};
|
||||||
|
@ -308,6 +308,7 @@
|
||||||
at: '/',
|
at: '/',
|
||||||
alias: 'commands',
|
alias: 'commands',
|
||||||
searchKey: 'search',
|
searchKey: 'search',
|
||||||
|
skipSpecialCharacterTest: true,
|
||||||
displayTpl: function(value) {
|
displayTpl: function(value) {
|
||||||
var tpl = '<li>/${name}';
|
var tpl = '<li>/${name}';
|
||||||
if (value.aliases.length > 0) {
|
if (value.aliases.length > 0) {
|
||||||
|
|
|
@ -2,8 +2,9 @@ require 'rails_helper'
|
||||||
|
|
||||||
feature 'GFM autocomplete', feature: true, js: true do
|
feature 'GFM autocomplete', feature: true, js: true do
|
||||||
include WaitForAjax
|
include WaitForAjax
|
||||||
let(:user) { create(:user) }
|
let(:user) { create(:user, username: 'someone.special') }
|
||||||
let(:project) { create(:project) }
|
let(:project) { create(:project) }
|
||||||
|
let(:label) { create(:label, project: project, title: 'special+') }
|
||||||
let(:issue) { create(:issue, project: project) }
|
let(:issue) { create(:issue, project: project) }
|
||||||
|
|
||||||
before do
|
before do
|
||||||
|
@ -23,15 +24,6 @@ feature 'GFM autocomplete', feature: true, js: true do
|
||||||
expect(page).to have_selector('.atwho-container')
|
expect(page).to have_selector('.atwho-container')
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'opens autocomplete menu when field is prefixed with non-text character' do
|
|
||||||
page.within '.timeline-content-form' do
|
|
||||||
find('#note_note').native.send_keys('')
|
|
||||||
find('#note_note').native.send_keys('@')
|
|
||||||
end
|
|
||||||
|
|
||||||
expect(page).to have_selector('.atwho-container')
|
|
||||||
end
|
|
||||||
|
|
||||||
it 'doesnt open autocomplete menu character is prefixed with text' do
|
it 'doesnt open autocomplete menu character is prefixed with text' do
|
||||||
page.within '.timeline-content-form' do
|
page.within '.timeline-content-form' do
|
||||||
find('#note_note').native.send_keys('testing')
|
find('#note_note').native.send_keys('testing')
|
||||||
|
@ -40,4 +32,61 @@ feature 'GFM autocomplete', feature: true, js: true do
|
||||||
|
|
||||||
expect(page).not_to have_selector('.atwho-view')
|
expect(page).not_to have_selector('.atwho-view')
|
||||||
end
|
end
|
||||||
|
|
||||||
|
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
|
||||||
|
|
||||||
|
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
|
end
|
||||||
|
|
Loading…
Reference in New Issue