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

Resolve "Do not pre-select previous user(s) when creating protected branches"

Closes #6407

See merge request gitlab-org/gitlab-ee!6112

app/assets/javascripts/protected_branches/protected_branch_create.js

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

ee/app/assets/javascripts/protected_branches/protected_branch_create.js

@@ -6,8 +6,6 @@ import CreateItemDropdown from '~/create_item_dropdown';
 import AccessDropdown from 'ee/projects/settings/access_dropdown';
 import { ACCESS_LEVELS, LEVEL_TYPES } from './constants';
 
-const PB_LOCAL_STORAGE_KEY = 'protected-branches-defaults';
-
 export default class ProtectedBranchCreate {
   constructor() {
     this.$form = $('.js-new-protected-branch');
@@ -52,8 +50,6 @@ export default class ProtectedBranchCreate {
       onSelect: this.onSelectCallback,
       getData: ProtectedBranchCreate.getProtectedBranches,
     });
-
-    this.loadPreviousSelection();
   }
 
   // Enable submit button after selecting an option
@@ -66,7 +62,6 @@ export default class ProtectedBranchCreate {
       $allowedToPush.length
     );
 
-    this.savePreviousSelection($allowedToMerge, $allowedToPush);
     this.$form.find('input[type="submit"]').attr('disabled', toggle);
   }
 
@@ -109,24 +104,6 @@ export default class ProtectedBranchCreate {
     return formData;
   }
 
-  loadPreviousSelection() {
-    if (this.isLocalStorageAvailable) {
-      const savedDefaults = JSON.parse(window.localStorage.getItem(PB_LOCAL_STORAGE_KEY));
-      if (savedDefaults != null) {
-        this[`${ACCESS_LEVELS.MERGE}_dropdown`].setSelectedItems(savedDefaults.merge);
-        let updatedLabel = this[`${ACCESS_LEVELS.MERGE}_dropdown`].toggleLabel();
-        this[`${ACCESS_LEVELS.MERGE}_dropdown`].$dropdown
-          .find('.dropdown-toggle-text')
-          .text(updatedLabel);
-        this[`${ACCESS_LEVELS.PUSH}_dropdown`].setSelectedItems(savedDefaults.push);
-        updatedLabel = this[`${ACCESS_LEVELS.PUSH}_dropdown`].toggleLabel();
-        this[`${ACCESS_LEVELS.PUSH}_dropdown`].$dropdown
-          .find('.dropdown-toggle-text')
-          .text(updatedLabel);
-      }
-    }
-  }
-
   onFormSubmit(e) {
     e.preventDefault();
 
@@ -136,14 +113,4 @@ export default class ProtectedBranchCreate {
       })
       .catch(() => Flash('Failed to protect the branch'));
   }
-
-  savePreviousSelection(mergeSelection, pushSelection) {
-    if (this.isLocalStorageAvailable) {
-      const branchDefaults = {
-        merge: mergeSelection || [],
-        push: pushSelection || [],
-      };
-      window.localStorage.setItem(PB_LOCAL_STORAGE_KEY, JSON.stringify(branchDefaults));
-    }
-  }
 }

ee/changelogs/unreleased/do-not-pre-select-previous-user-s-when-creating-protected-branches.yml

+---
+title: Do not pre-select previous user(s) when creating protected branches
+merge_request: 6112
+author:
+type: fixed

spec/features/protected_branches_spec.rb

@@ -62,33 +62,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

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)