Commit 211ce494 authored by Luke Bennett's avatar Luke Bennett Committed by Phil Hughes

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

parent 0af09f7b
import $ from 'jquery'; import $ from 'jquery';
import _ from 'underscore';
import ProtectedBranchAccessDropdown from './protected_branch_access_dropdown'; import ProtectedBranchAccessDropdown from './protected_branch_access_dropdown';
import CreateItemDropdown from '../create_item_dropdown'; import CreateItemDropdown from '../create_item_dropdown';
import AccessorUtilities from '../lib/utils/accessor'; import AccessorUtilities from '../lib/utils/accessor';
const PB_LOCAL_STORAGE_KEY = 'protected-branches-defaults';
export default class ProtectedBranchCreate { export default class ProtectedBranchCreate {
constructor() { constructor() {
this.$form = $('.js-new-protected-branch'); this.$form = $('.js-new-protected-branch');
...@@ -43,8 +40,6 @@ export default class ProtectedBranchCreate { ...@@ -43,8 +40,6 @@ export default class ProtectedBranchCreate {
onSelect: this.onSelectCallback, onSelect: this.onSelectCallback,
getData: ProtectedBranchCreate.getProtectedBranches, getData: ProtectedBranchCreate.getProtectedBranches,
}); });
this.loadPreviousSelection($allowedToMergeDropdown.data('glDropdown'), $allowedToPushDropdown.data('glDropdown'));
} }
// This will run after clicked callback // This will run after clicked callback
...@@ -59,39 +54,10 @@ export default class ProtectedBranchCreate { ...@@ -59,39 +54,10 @@ export default class ProtectedBranchCreate {
$allowedToPushInput.length $allowedToPushInput.length
); );
this.savePreviousSelection($allowedToMergeInput.val(), $allowedToPushInput.val());
this.$form.find('input[type="submit"]').prop('disabled', completedForm); this.$form.find('input[type="submit"]').prop('disabled', completedForm);
} }
static getProtectedBranches(term, callback) { static getProtectedBranches(term, callback) {
callback(gon.open_branches); 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));
}
}
} }
...@@ -6,8 +6,6 @@ import CreateItemDropdown from '~/create_item_dropdown'; ...@@ -6,8 +6,6 @@ import CreateItemDropdown from '~/create_item_dropdown';
import AccessDropdown from 'ee/projects/settings/access_dropdown'; import AccessDropdown from 'ee/projects/settings/access_dropdown';
import { ACCESS_LEVELS, LEVEL_TYPES } from './constants'; import { ACCESS_LEVELS, LEVEL_TYPES } from './constants';
const PB_LOCAL_STORAGE_KEY = 'protected-branches-defaults';
export default class ProtectedBranchCreate { export default class ProtectedBranchCreate {
constructor() { constructor() {
this.$form = $('.js-new-protected-branch'); this.$form = $('.js-new-protected-branch');
...@@ -52,8 +50,6 @@ export default class ProtectedBranchCreate { ...@@ -52,8 +50,6 @@ export default class ProtectedBranchCreate {
onSelect: this.onSelectCallback, onSelect: this.onSelectCallback,
getData: ProtectedBranchCreate.getProtectedBranches, getData: ProtectedBranchCreate.getProtectedBranches,
}); });
this.loadPreviousSelection();
} }
// Enable submit button after selecting an option // Enable submit button after selecting an option
...@@ -66,7 +62,6 @@ export default class ProtectedBranchCreate { ...@@ -66,7 +62,6 @@ export default class ProtectedBranchCreate {
$allowedToPush.length $allowedToPush.length
); );
this.savePreviousSelection($allowedToMerge, $allowedToPush);
this.$form.find('input[type="submit"]').attr('disabled', toggle); this.$form.find('input[type="submit"]').attr('disabled', toggle);
} }
...@@ -109,24 +104,6 @@ export default class ProtectedBranchCreate { ...@@ -109,24 +104,6 @@ export default class ProtectedBranchCreate {
return formData; 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) { onFormSubmit(e) {
e.preventDefault(); e.preventDefault();
...@@ -136,14 +113,4 @@ export default class ProtectedBranchCreate { ...@@ -136,14 +113,4 @@ export default class ProtectedBranchCreate {
}) })
.catch(() => Flash('Failed to protect the branch')); .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));
}
}
} }
---
title: Do not pre-select previous user(s) when creating protected branches
merge_request: 6112
author:
type: fixed
...@@ -62,33 +62,6 @@ feature 'Protected Branches', :js do ...@@ -62,33 +62,6 @@ feature 'Protected Branches', :js do
expect(page).to have_content('No branches to show') expect(page).to have_content('No branches to show')
end end
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 end
context 'logged in as admin' do context 'logged in as admin' do
......
...@@ -5,6 +5,12 @@ shared_examples "protected branches > access control > CE" do ...@@ -5,6 +5,12 @@ shared_examples "protected branches > access control > CE" do
set_protected_branch_name('master') 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 within('.js-new-protected-branch') do
allowed_to_push_button = find(".js-allowed-to-push") allowed_to_push_button = find(".js-allowed-to-push")
...@@ -25,6 +31,18 @@ shared_examples "protected branches > access control > CE" do ...@@ -25,6 +31,18 @@ shared_examples "protected branches > access control > CE" do
set_protected_branch_name('master') 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" click_on "Protect"
expect(ProtectedBranch.count).to eq(1) expect(ProtectedBranch.count).to eq(1)
...@@ -59,6 +77,12 @@ shared_examples "protected branches > access control > CE" do ...@@ -59,6 +77,12 @@ shared_examples "protected branches > access control > CE" do
end end
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" click_on "Protect"
expect(ProtectedBranch.count).to eq(1) expect(ProtectedBranch.count).to eq(1)
...@@ -70,6 +94,18 @@ shared_examples "protected branches > access control > CE" do ...@@ -70,6 +94,18 @@ shared_examples "protected branches > access control > CE" do
set_protected_branch_name('master') 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" click_on "Protect"
expect(ProtectedBranch.count).to eq(1) expect(ProtectedBranch.count).to eq(1)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment