From 32979803f232c43c7b87e30bec275b79182b06ad Mon Sep 17 00:00:00 2001 From: Luke Bennett Date: Mon, 12 Sep 2016 21:02:09 +0100 Subject: Replace single quotes with escaped single quotes and write spec to test multiple lable selection over a page load Added spec for removing label with space and single quote Fixed removing label with single quote --- app/assets/javascripts/gl_dropdown.js | 13 ++++----- app/assets/javascripts/labels_select.js | 2 +- .../shared/issuable/_label_dropdown.html.haml | 2 +- spec/features/issues/filter_issues_spec.rb | 31 ++++++++++++++++++++-- 4 files changed, 38 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/gl_dropdown.js b/app/assets/javascripts/gl_dropdown.js index bea141bae51..b95b7f29d1e 100644 --- a/app/assets/javascripts/gl_dropdown.js +++ b/app/assets/javascripts/gl_dropdown.js @@ -600,14 +600,15 @@ selectedObject = this.renderedData[selectedIndex]; } } + field = []; fieldName = typeof this.options.fieldName === 'function' ? this.options.fieldName(selectedObject) : this.options.fieldName; value = this.options.id ? this.options.id(selectedObject, el) : selectedObject.id; if (isInput) { field = $(this.el); - } else { - field = this.dropdown.parent().find("input[name='" + fieldName + "'][value='" + escape(value) + "']"); + } else if(value) { + field = this.dropdown.parent().find("input[name='" + fieldName + "'][value='" + value.toString().replace(/'/g, '\\\'') + "']"); } - if (el.hasClass(ACTIVE_CLASS)) { + if (field.length && el.hasClass(ACTIVE_CLASS)) { el.removeClass(ACTIVE_CLASS); if (isInput) { field.val(''); @@ -617,7 +618,7 @@ } else if (el.hasClass(INDETERMINATE_CLASS)) { el.addClass(ACTIVE_CLASS); el.removeClass(INDETERMINATE_CLASS); - if (value == null) { + if (field.length && value == null) { field.remove(); } if (!field.length && fieldName) { @@ -630,7 +631,7 @@ this.dropdown.parent().find("input[name='" + fieldName + "']").remove(); } } - if (value == null) { + if (field.length && value == null) { field.remove(); } // Toggle active class for the tick mark @@ -638,7 +639,7 @@ if (value != null) { if (!field.length && fieldName) { this.addInput(fieldName, value, selectedObject); - } else { + } else if (field.length) { field.val(value).trigger('change'); } } diff --git a/app/assets/javascripts/labels_select.js b/app/assets/javascripts/labels_select.js index 29a967a35a0..3f15a117ca8 100644 --- a/app/assets/javascripts/labels_select.js +++ b/app/assets/javascripts/labels_select.js @@ -166,7 +166,7 @@ instance.addInput(this.fieldName, label.id); } } - if ($form.find("input[type='hidden'][name='" + ($dropdown.data('fieldName')) + "'][value='" + escape(this.id(label)) + "']").length) { + if (this.id(label) && $form.find("input[type='hidden'][name='" + ($dropdown.data('fieldName')) + "'][value='" + this.id(label).toString().replace(/'/g, '\\\'') + "']").length) { selectedClass.push('is-active'); } if ($dropdown.hasClass('js-multiselect') && removesAll) { diff --git a/app/views/shared/issuable/_label_dropdown.html.haml b/app/views/shared/issuable/_label_dropdown.html.haml index 24a1a616919..d34d28f6736 100644 --- a/app/views/shared/issuable/_label_dropdown.html.haml +++ b/app/views/shared/issuable/_label_dropdown.html.haml @@ -12,7 +12,7 @@ - if params[:label_name].present? - if params[:label_name].respond_to?('any?') - params[:label_name].each do |label| - = hidden_field_tag "label_name[]", u(label), id: nil + = hidden_field_tag "label_name[]", label, id: nil .dropdown %button.dropdown-menu-toggle.js-label-select.js-multiselect{class: classes.join(' '), type: "button", data: dropdown_data} %span.dropdown-toggle-text diff --git a/spec/features/issues/filter_issues_spec.rb b/spec/features/issues/filter_issues_spec.rb index 0e9f814044e..69fda27cc61 100644 --- a/spec/features/issues/filter_issues_spec.rb +++ b/spec/features/issues/filter_issues_spec.rb @@ -101,7 +101,7 @@ describe 'Filter issues', feature: true do expect(find('.js-label-select .dropdown-toggle-text')).to have_content('No Label') end - it 'filters by no label' do + it 'filters by a label' do find('.dropdown-menu-labels a', text: label.title).click page.within '.labels-filter' do expect(page).to have_content label.title @@ -109,7 +109,7 @@ describe 'Filter issues', feature: true do expect(find('.js-label-select .dropdown-toggle-text')).to have_content(label.title) end - it 'filters by wont fix labels' do + it "filters by `won't fix` and another label" do find('.dropdown-menu-labels a', text: label.title).click page.within '.labels-filter' do expect(page).to have_content wontfix.title @@ -117,6 +117,33 @@ describe 'Filter issues', feature: true do end expect(find('.js-label-select .dropdown-toggle-text')).to have_content(wontfix.title) end + + it "filters by `won't fix` label followed by another label after page load" do + find('.dropdown-menu-labels a', text: wontfix.title).click + # Close label dropdown to load + find('body').click + expect(find('.filtered-labels')).to have_content(wontfix.title) + + find('.js-label-select').click + wait_for_ajax + find('.dropdown-menu-labels a', text: label.title).click + # Close label dropdown to load + find('body').click + expect(find('.filtered-labels')).to have_content(label.title) + + find('.js-label-select').click + wait_for_ajax + expect(find('.dropdown-menu-labels li', text: wontfix.title)).to have_css('.is-active') + expect(find('.dropdown-menu-labels li', text: label.title)).to have_css('.is-active') + end + + it "selects and unselects `won't fix`" do + find('.dropdown-menu-labels a', text: wontfix.title).click + find('.dropdown-menu-labels a', text: wontfix.title).click + # Close label dropdown to load + find('body').click + expect(page).not_to have_css('.filtered-labels') + end end describe 'Filter issues for assignee and label from issues#index' do -- cgit v1.2.1