From 4878ef2fa7176d7d82da66eef0c6ce185f91441a Mon Sep 17 00:00:00 2001 From: Luke Bennett Date: Wed, 27 Jun 2018 11:41:16 +0000 Subject: [PATCH] CE port gitlab-ee!6112 --- .../protected_branch_create.js | 34 -------------- ...ser-s-when-creating-protected-branches.yml | 5 ++ spec/features/protected_branches_spec.rb | 47 ++++++++----------- .../protected_branches_access_control_ce.rb | 36 ++++++++++++++ 4 files changed, 61 insertions(+), 61 deletions(-) create mode 100644 changelogs/unreleased/46546-do-not-pre-select-previous-user-s-when-creating-protected-branches.yml diff --git a/app/assets/javascripts/protected_branches/protected_branch_create.js b/app/assets/javascripts/protected_branches/protected_branch_create.js index 7c61c070a35..b601b19e7be 100644 --- a/app/assets/javascripts/protected_branches/protected_branch_create.js +++ b/app/assets/javascripts/protected_branches/protected_branch_create.js @@ -1,11 +1,8 @@ import $ from 'jquery'; -import _ from 'underscore'; import ProtectedBranchAccessDropdown from './protected_branch_access_dropdown'; import CreateItemDropdown from '../create_item_dropdown'; import AccessorUtilities from '../lib/utils/accessor'; -const PB_LOCAL_STORAGE_KEY = 'protected-branches-defaults'; - export default class ProtectedBranchCreate { constructor() { this.$form = $('.js-new-protected-branch'); @@ -43,8 +40,6 @@ export default class ProtectedBranchCreate { onSelect: this.onSelectCallback, getData: ProtectedBranchCreate.getProtectedBranches, }); - - this.loadPreviousSelection($allowedToMergeDropdown.data('glDropdown'), $allowedToPushDropdown.data('glDropdown')); } // This will run after clicked callback @@ -59,39 +54,10 @@ export default class ProtectedBranchCreate { $allowedToPushInput.length ); - this.savePreviousSelection($allowedToMergeInput.val(), $allowedToPushInput.val()); this.$form.find('input[type="submit"]').prop('disabled', completedForm); } static getProtectedBranches(term, callback) { callback(gon.open_branches); } - - loadPreviousSelection(mergeDropdown, pushDropdown) { - let mergeIndex = 0; - let pushIndex = 0; - if (this.isLocalStorageAvailable) { - const savedDefaults = JSON.parse(window.localStorage.getItem(PB_LOCAL_STORAGE_KEY)); - if (savedDefaults != null) { - mergeIndex = _.findLastIndex(mergeDropdown.fullData.roles, { - id: parseInt(savedDefaults.mergeSelection, 0), - }); - pushIndex = _.findLastIndex(pushDropdown.fullData.roles, { - id: parseInt(savedDefaults.pushSelection, 0), - }); - } - } - mergeDropdown.selectRowAtIndex(mergeIndex); - pushDropdown.selectRowAtIndex(pushIndex); - } - - savePreviousSelection(mergeSelection, pushSelection) { - if (this.isLocalStorageAvailable) { - const branchDefaults = { - mergeSelection, - pushSelection, - }; - window.localStorage.setItem(PB_LOCAL_STORAGE_KEY, JSON.stringify(branchDefaults)); - } - } } diff --git a/changelogs/unreleased/46546-do-not-pre-select-previous-user-s-when-creating-protected-branches.yml b/changelogs/unreleased/46546-do-not-pre-select-previous-user-s-when-creating-protected-branches.yml new file mode 100644 index 00000000000..7d42d971022 --- /dev/null +++ b/changelogs/unreleased/46546-do-not-pre-select-previous-user-s-when-creating-protected-branches.yml @@ -0,0 +1,5 @@ +--- +title: CE port gitlab-ee!6112 +merge_request: 19714 +author: +type: other diff --git a/spec/features/protected_branches_spec.rb b/spec/features/protected_branches_spec.rb index 4c0f9971425..39bd4af6cd0 100644 --- a/spec/features/protected_branches_spec.rb +++ b/spec/features/protected_branches_spec.rb @@ -60,33 +60,6 @@ feature 'Protected Branches', :js do expect(page).to have_content('No branches to show') end end - - describe "Saved defaults" do - it "keeps the allowed to merge and push dropdowns defaults based on the previous selection" do - visit project_protected_branches_path(project) - form = '.js-new-protected-branch' - - within form do - find(".js-allowed-to-merge").click - wait_for_requests - click_link 'No one' - find(".js-allowed-to-push").click - wait_for_requests - click_link 'Developers + Maintainers' - end - - visit project_protected_branches_path(project) - - within form do - page.within(".js-allowed-to-merge") do - expect(page.find(".dropdown-toggle-text")).to have_content("No one") - end - page.within(".js-allowed-to-push") do - expect(page.find(".dropdown-toggle-text")).to have_content("Developers + Maintainers") - end - end - end - end end context 'logged in as admin' do @@ -97,6 +70,7 @@ feature 'Protected Branches', :js do describe "explicit protected branches" do it "allows creating explicit protected branches" do visit project_protected_branches_path(project) + set_defaults set_protected_branch_name('some-branch') click_on "Protect" @@ -110,6 +84,7 @@ feature 'Protected Branches', :js do project.repository.add_branch(admin, 'some-branch', commit.id) visit project_protected_branches_path(project) + set_defaults set_protected_branch_name('some-branch') click_on "Protect" @@ -118,6 +93,7 @@ feature 'Protected Branches', :js do it "displays an error message if the named branch does not exist" do visit project_protected_branches_path(project) + set_defaults set_protected_branch_name('some-branch') click_on "Protect" @@ -128,6 +104,7 @@ feature 'Protected Branches', :js do describe "wildcard protected branches" do it "allows creating protected branches with a wildcard" do visit project_protected_branches_path(project) + set_defaults set_protected_branch_name('*-stable') click_on "Protect" @@ -141,6 +118,7 @@ feature 'Protected Branches', :js do project.repository.add_branch(admin, 'staging-stable', 'master') visit project_protected_branches_path(project) + set_defaults set_protected_branch_name('*-stable') click_on "Protect" @@ -157,6 +135,7 @@ feature 'Protected Branches', :js do visit project_protected_branches_path(project) set_protected_branch_name('*-stable') + set_defaults click_on "Protect" visit project_protected_branches_path(project) @@ -180,4 +159,18 @@ feature 'Protected Branches', :js do find(".dropdown-input-field").set(branch_name) click_on("Create wildcard #{branch_name}") end + + def set_defaults + find(".js-allowed-to-merge").click + within('.qa-allowed-to-merge-dropdown') do + expect(first("li")).to have_content("Roles") + find(:link, 'No one').click + end + + find(".js-allowed-to-push").click + within('.qa-allowed-to-push-dropdown') do + expect(first("li")).to have_content("Roles") + find(:link, 'No one').click + end + end end diff --git a/spec/support/shared_examples/features/protected_branches_access_control_ce.rb b/spec/support/shared_examples/features/protected_branches_access_control_ce.rb index 5241c0fa6f1..a8f2c2e7a5a 100644 --- a/spec/support/shared_examples/features/protected_branches_access_control_ce.rb +++ b/spec/support/shared_examples/features/protected_branches_access_control_ce.rb @@ -5,6 +5,12 @@ shared_examples "protected branches > access control > CE" do set_protected_branch_name('master') + find(".js-allowed-to-merge").click + within('.qa-allowed-to-merge-dropdown') do + expect(first("li")).to have_content("Roles") + find(:link, 'No one').click + end + within('.js-new-protected-branch') do allowed_to_push_button = find(".js-allowed-to-push") @@ -25,6 +31,18 @@ shared_examples "protected branches > access control > CE" do set_protected_branch_name('master') + find(".js-allowed-to-merge").click + within('.qa-allowed-to-merge-dropdown') do + expect(first("li")).to have_content("Roles") + find(:link, 'No one').click + end + + find(".js-allowed-to-push").click + within('.qa-allowed-to-push-dropdown') do + expect(first("li")).to have_content("Roles") + find(:link, 'No one').click + end + click_on "Protect" expect(ProtectedBranch.count).to eq(1) @@ -59,6 +77,12 @@ shared_examples "protected branches > access control > CE" do end end + find(".js-allowed-to-push").click + within('.qa-allowed-to-push-dropdown') do + expect(first("li")).to have_content("Roles") + find(:link, 'No one').click + end + click_on "Protect" expect(ProtectedBranch.count).to eq(1) @@ -70,6 +94,18 @@ shared_examples "protected branches > access control > CE" do set_protected_branch_name('master') + find(".js-allowed-to-merge").click + within('.qa-allowed-to-merge-dropdown') do + expect(first("li")).to have_content("Roles") + find(:link, 'No one').click + end + + find(".js-allowed-to-push").click + within('.qa-allowed-to-push-dropdown') do + expect(first("li")).to have_content("Roles") + find(:link, 'No one').click + end + click_on "Protect" expect(ProtectedBranch.count).to eq(1)