Merge branch '32163-protected-branch-form-should-have-sane-defaults-for-dropdowns' into 'master'

Resolve "Protected branch form should have sane defaults for dropdowns"

Closes #32163

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

app/assets/javascripts/gl_dropdown.js

@@ -738,7 +738,7 @@ GitLabDropdown = (function() {
       : selectedObject.id;
     if (isInput) {
       field = $(this.el);
-    } else if (value) {
+    } else if (value != null) {
       field = this.dropdown.parent().find("input[name='" + fieldName + "'][value='" + value.toString().replace(/'/g, '\\\'') + "']");
     }
 
@@ -746,7 +746,7 @@ GitLabDropdown = (function() {
       return;
     }
 
-    if (el.hasClass(ACTIVE_CLASS)) {
+    if (el.hasClass(ACTIVE_CLASS) && value !== 0) {
       isMarking = false;
       el.removeClass(ACTIVE_CLASS);
       if (field && field.length) {
@@ -852,7 +852,7 @@ GitLabDropdown = (function() {
       if (href && href !== '#') {
         gl.utils.visitUrl(href);
       } else {
-        $el.first().trigger('click');
+        $el.trigger('click');
       }
     }
   };

app/assets/javascripts/protected_branches/protected_branch_create.js

+import _ from 'underscore';
 import ProtectedBranchAccessDropdown from './protected_branch_access_dropdown';
 import ProtectedBranchDropdown from './protected_branch_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');
+    this.isLocalStorageAvailable = AccessorUtilities.isLocalStorageAccessSafe();
+    this.currentProjectUserDefaults = {};
     this.buildDropdowns();
   }
 
   buildDropdowns() {
     const $allowedToMergeDropdown = this.$form.find('.js-allowed-to-merge');
     const $allowedToPushDropdown = this.$form.find('.js-allowed-to-push');
+    const $protectedBranchDropdown = this.$form.find('.js-protected-branch-select');
 
     // Cache callback
     this.onSelectCallback = this.onSelect.bind(this);
@@ -28,15 +35,13 @@ export default class ProtectedBranchCreate {
       onSelect: this.onSelectCallback,
     });
 
-    // Select default
-    $allowedToPushDropdown.data('glDropdown').selectRowAtIndex(0);
-    $allowedToMergeDropdown.data('glDropdown').selectRowAtIndex(0);
-
     // Protected branch dropdown
     this.protectedBranchDropdown = new ProtectedBranchDropdown({
-      $dropdown: this.$form.find('.js-protected-branch-select'),
+      $dropdown: $protectedBranchDropdown,
       onSelect: this.onSelectCallback,
     });
+
+    this.loadPreviousSelection($allowedToMergeDropdown.data('glDropdown'), $allowedToPushDropdown.data('glDropdown'));
   }
 
   // This will run after clicked callback
@@ -45,7 +50,41 @@ export default class ProtectedBranchCreate {
     const $branchInput = this.$form.find('input[name="protected_branch[name]"]');
     const $allowedToMergeInput = this.$form.find('input[name="protected_branch[merge_access_levels_attributes][0][access_level]"]');
     const $allowedToPushInput = this.$form.find('input[name="protected_branch[push_access_levels_attributes][0][access_level]"]');
+    const completedForm = !(
+      $branchInput.val() &&
+      $allowedToMergeInput.length &&
+      $allowedToPushInput.length
+    );
+
+    this.savePreviousSelection($allowedToMergeInput.val(), $allowedToPushInput.val());
+    this.$form.find('input[type="submit"]').attr('disabled', completedForm);
+  }
+
+  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);
+  }
 
-    this.$form.find('input[type="submit"]').attr('disabled', !($branchInput.val() && $allowedToMergeInput.length && $allowedToPushInput.length));
+  savePreviousSelection(mergeSelection, pushSelection) {
+    if (this.isLocalStorageAvailable) {
+      const branchDefaults = {
+        mergeSelection,
+        pushSelection,
+      };
+      window.localStorage.setItem(PB_LOCAL_STORAGE_KEY, JSON.stringify(branchDefaults));
+    }
   }
 }

changelogs/unreleased/32163-protected-branch-form-should-have-sane-defaults-for-dropdowns.yml

+---
+title: Added defaults for protected branches dropdowns on the repository settings
+merge_request: 14278
+author:
+type: changed

spec/features/protected_branches_spec.rb

@@ -150,4 +150,22 @@ feature 'Protected Branches', :js do
     find(".dropdown-input-field").set(branch_name)
     click_on("Create wildcard #{branch_name}")
   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)
+      set_protected_branch_name('some-branch')
+      find(".js-allowed-to-merge").trigger('click')
+      click_link 'No one'
+      find(".js-allowed-to-push").trigger('click')
+      click_link 'Developers + Masters'
+      visit project_protected_branches_path(project)
+      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 + Masters")
+      end
+    end
+  end
 end