diff --git a/app/assets/javascripts/gl_dropdown.js b/app/assets/javascripts/gl_dropdown.js index f85aa6dd7b7..24abea0d30d 100644 --- a/app/assets/javascripts/gl_dropdown.js +++ b/app/assets/javascripts/gl_dropdown.js @@ -477,7 +477,8 @@ } else { if (!selected) { value = this.options.id ? this.options.id(data) : data.id; - fieldName = this.options.fieldName; + fieldName = typeof this.options.fieldName === 'function' ? this.options.fieldName() : this.options.fieldName; + field = this.dropdown.parent().find("input[name='" + fieldName + "'][value='" + value + "']"); if (field.length) { selected = true; @@ -534,7 +535,6 @@ GitLabDropdown.prototype.rowClicked = function(el) { var field, fieldName, groupName, isInput, selectedIndex, selectedObject, value; - fieldName = this.options.fieldName; isInput = $(this.el).is('input'); if (this.renderedData) { groupName = el.data('group'); @@ -546,6 +546,7 @@ selectedObject = this.renderedData[selectedIndex]; } } + 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); @@ -560,10 +561,9 @@ field.remove(); } if (this.options.toggleLabel) { - return this.updateLabel(selectedObject, el, this); - } else { - return selectedObject; + this.updateLabel(selectedObject, el, this); } + return selectedObject; } else if (el.hasClass(INDETERMINATE_CLASS)) { el.addClass(ACTIVE_CLASS); el.removeClass(INDETERMINATE_CLASS); @@ -571,7 +571,7 @@ field.remove(); } if (!field.length && fieldName) { - this.addInput(fieldName, value); + this.addInput(fieldName, value, selectedObject); } return selectedObject; } else { @@ -590,7 +590,7 @@ } if (value != null) { if (!field.length && fieldName) { - this.addInput(fieldName, value); + this.addInput(fieldName, value, selectedObject); } else { field.val(value).trigger('change'); } @@ -599,12 +599,15 @@ } }; - GitLabDropdown.prototype.addInput = function(fieldName, value) { + GitLabDropdown.prototype.addInput = function(fieldName, value, selectedObject) { var $input; $input = $('').attr('type', 'hidden').attr('name', fieldName).val(value); if (this.options.inputId != null) { $input.attr('id', this.options.inputId); } + if (selectedObject && selectedObject.type) { + $input.attr('data-type', selectedObject.type); + } return this.dropdown.before($input); }; diff --git a/app/views/projects/protected_branches/_create_protected_branch.html.haml b/app/views/projects/protected_branches/_create_protected_branch.html.haml index d4c6fa24768..95d8743f546 100644 --- a/app/views/projects/protected_branches/_create_protected_branch.html.haml +++ b/app/views/projects/protected_branches/_create_protected_branch.html.haml @@ -22,16 +22,18 @@ %label.col-md-2.text-right{ for: 'merge_access_levels_attributes' } Allowed to merge: .col-md-10 - = dropdown_tag('Select', - options: { toggle_class: 'js-allowed-to-merge wide', - data: { field_name: 'protected_branch[merge_access_levels_attributes][0][access_level]', input_id: 'merge_access_levels_attributes' }}) + .js-allowed-to-merge-container + = dropdown_tag('Select', + options: { toggle_class: 'js-allowed-to-merge wide', + data: { field_name: 'protected_branch[merge_access_levels_attributes][0][access_level]', input_id: 'merge_access_levels_attributes' }}) .form-group %label.col-md-2.text-right{ for: 'push_access_levels_attributes' } Allowed to push: .col-md-10 - = dropdown_tag('Select', - options: { toggle_class: 'js-allowed-to-push wide', - data: { field_name: 'protected_branch[push_access_levels_attributes][0][access_level]', input_id: 'push_access_levels_attributes' }}) + .js-allowed-to-push-container + = dropdown_tag('Select', + options: { toggle_class: 'js-allowed-to-push wide', + data: { field_name: 'protected_branch[push_access_levels_attributes][0][access_level]', input_id: 'push_access_levels_attributes' }}) .panel-footer = f.submit 'Protect', class: 'btn-create btn', disabled: true diff --git a/spec/features/protected_branches/access_control_ce_spec.rb b/spec/features/protected_branches/access_control_ce_spec.rb new file mode 100644 index 00000000000..395c61a4743 --- /dev/null +++ b/spec/features/protected_branches/access_control_ce_spec.rb @@ -0,0 +1,71 @@ +RSpec.shared_examples "protected branches > access control > CE" do + ProtectedBranch::PushAccessLevel.human_access_levels.each do |(access_type_id, access_type_name)| + it "allows creating protected branches that #{access_type_name} can push to" do + visit namespace_project_protected_branches_path(project.namespace, project) + set_protected_branch_name('master') + within('.new_protected_branch') do + allowed_to_push_button = find(".js-allowed-to-push") + + unless allowed_to_push_button.text == access_type_name + allowed_to_push_button.click + within(".dropdown.open .dropdown-menu") { click_on access_type_name } + end + end + click_on "Protect" + + expect(ProtectedBranch.count).to eq(1) + expect(ProtectedBranch.last.push_access_levels.map(&:access_level)).to eq([access_type_id]) + end + + it "allows updating protected branches so that #{access_type_name} can push to them" do + visit namespace_project_protected_branches_path(project.namespace, project) + set_protected_branch_name('master') + click_on "Protect" + + expect(ProtectedBranch.count).to eq(1) + + within(".protected-branches-list") do + find(".js-allowed-to-push").click + within('.js-allowed-to-push-container') { click_on access_type_name } + end + + wait_for_ajax + expect(ProtectedBranch.last.push_access_levels.map(&:access_level)).to include(access_type_id) + end + end + + ProtectedBranch::MergeAccessLevel.human_access_levels.each do |(access_type_id, access_type_name)| + it "allows creating protected branches that #{access_type_name} can merge to" do + visit namespace_project_protected_branches_path(project.namespace, project) + set_protected_branch_name('master') + within('.new_protected_branch') do + allowed_to_merge_button = find(".js-allowed-to-merge") + + unless allowed_to_merge_button.text == access_type_name + allowed_to_merge_button.click + within(".dropdown.open .dropdown-menu") { click_on access_type_name } + end + end + click_on "Protect" + + expect(ProtectedBranch.count).to eq(1) + expect(ProtectedBranch.last.merge_access_levels.map(&:access_level)).to eq([access_type_id]) + end + + it "allows updating protected branches so that #{access_type_name} can merge to them" do + visit namespace_project_protected_branches_path(project.namespace, project) + set_protected_branch_name('master') + click_on "Protect" + + expect(ProtectedBranch.count).to eq(1) + + within(".protected-branches-list") do + find(".js-allowed-to-merge").click + within('.js-allowed-to-merge-container') { click_on access_type_name } + end + + wait_for_ajax + expect(ProtectedBranch.last.merge_access_levels.map(&:access_level)).to include(access_type_id) + end + end +end diff --git a/spec/features/protected_branches_spec.rb b/spec/features/protected_branches_spec.rb index a0ee6cab7ec..1a3f7b970f6 100644 --- a/spec/features/protected_branches_spec.rb +++ b/spec/features/protected_branches_spec.rb @@ -1,4 +1,5 @@ require 'spec_helper' +Dir["./spec/features/protected_branches/*.rb"].sort.each { |f| require f } feature 'Projected Branches', feature: true, js: true do include WaitForAjax @@ -88,74 +89,6 @@ feature 'Projected Branches', feature: true, js: true do end describe "access control" do - ProtectedBranch::PushAccessLevel.human_access_levels.each do |(access_type_id, access_type_name)| - it "allows creating protected branches that #{access_type_name} can push to" do - visit namespace_project_protected_branches_path(project.namespace, project) - set_protected_branch_name('master') - within('.new_protected_branch') do - allowed_to_push_button = find(".js-allowed-to-push") - - unless allowed_to_push_button.text == access_type_name - allowed_to_push_button.click - within(".dropdown.open .dropdown-menu") { click_on access_type_name } - end - end - click_on "Protect" - - expect(ProtectedBranch.count).to eq(1) - expect(ProtectedBranch.last.push_access_levels.map(&:access_level)).to eq([access_type_id]) - end - - it "allows updating protected branches so that #{access_type_name} can push to them" do - visit namespace_project_protected_branches_path(project.namespace, project) - set_protected_branch_name('master') - click_on "Protect" - - expect(ProtectedBranch.count).to eq(1) - - within(".protected-branches-list") do - find(".js-allowed-to-push").click - within('.js-allowed-to-push-container') { click_on access_type_name } - end - - wait_for_ajax - expect(ProtectedBranch.last.push_access_levels.map(&:access_level)).to include(access_type_id) - end - end - - ProtectedBranch::MergeAccessLevel.human_access_levels.each do |(access_type_id, access_type_name)| - it "allows creating protected branches that #{access_type_name} can merge to" do - visit namespace_project_protected_branches_path(project.namespace, project) - set_protected_branch_name('master') - within('.new_protected_branch') do - allowed_to_merge_button = find(".js-allowed-to-merge") - - unless allowed_to_merge_button.text == access_type_name - allowed_to_merge_button.click - within(".dropdown.open .dropdown-menu") { click_on access_type_name } - end - end - click_on "Protect" - - expect(ProtectedBranch.count).to eq(1) - expect(ProtectedBranch.last.merge_access_levels.map(&:access_level)).to eq([access_type_id]) - end - - it "allows updating protected branches so that #{access_type_name} can merge to them" do - visit namespace_project_protected_branches_path(project.namespace, project) - set_protected_branch_name('master') - click_on "Protect" - - expect(ProtectedBranch.count).to eq(1) - - within(".protected-branches-list") do - find(".js-allowed-to-merge").click - within('.js-allowed-to-merge-container') { click_on access_type_name } - end - - wait_for_ajax - expect(ProtectedBranch.last.merge_access_levels.map(&:access_level)).to include(access_type_id) - end - end + include_examples "protected branches > access control > CE" end end