Merge branch '46546-do-not-pre-select-previous-user-s-when-creating-protected-branches' into 'master'

CE port gitlab-ee!6112

Closes #46546

See merge request gitlab-org/gitlab-ce!19714

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));
-    }
-  }
 }

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

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

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)