Commit d6417faa authored by Jacob Schatz's avatar Jacob Schatz

Merge branch '1738-project-settings-approvals' into 'master'

Resolve "Making it easier to configure approvals in project settings"

Closes #1738

See merge request !1527
parents 1ee72757 d55f885d
/* eslint-disable func-names, space-before-function-paren, quotes, object-shorthand, camelcase, no-var, comma-dangle, prefer-arrow-callback, quote-props, no-param-reassign, max-len */
var Api = {
groupsPath: "/api/:version/groups.json",
groupPath: "/api/:version/groups/:id.json",
namespacesPath: "/api/:version/namespaces.json",
groupProjectsPath: "/api/:version/groups/:id/projects.json",
projectsPath: "/api/:version/projects.json?simple=true",
labelsPath: "/:namespace_path/:project_path/labels",
licensePath: "/api/:version/templates/licenses/:key",
gitignorePath: "/api/:version/templates/gitignores/:key",
gitlabCiYmlPath: "/api/:version/templates/gitlab_ci_ymls/:key",
ldapGroupsPath: "/api/:version/ldap/:provider/groups.json",
dockerfilePath: "/api/:version/templates/dockerfiles/:key",
issuableTemplatePath: "/:namespace_path/:project_path/templates/:type/:key",
groupsPath: '/api/:version/groups.json',
groupPath: '/api/:version/groups/:id.json',
namespacesPath: '/api/:version/namespaces.json',
groupProjectsPath: '/api/:version/groups/:id/projects.json',
projectsPath: '/api/:version/projects.json?simple=true',
labelsPath: '/:namespace_path/:project_path/labels',
licensePath: '/api/:version/templates/licenses/:key',
gitignorePath: '/api/:version/templates/gitignores/:key',
gitlabCiYmlPath: '/api/:version/templates/gitlab_ci_ymls/:key',
ldapGroupsPath: '/api/:version/ldap/:provider/groups.json',
dockerfilePath: '/api/:version/templates/dockerfiles/:key',
issuableTemplatePath: '/:namespace_path/:project_path/templates/:type/:key',
group: function(group_id, callback) {
var url = Api.buildUrl(Api.groupPath)
.replace(':id', group_id);
return $.ajax({
url: url,
dataType: "json"
dataType: 'json'
}).done(function(group) {
return callback(group);
});
},
users: function(search, options, callback = $.noop) {
var url = Api.buildUrl('/autocomplete/users.json');
return $.ajax({
url,
data: $.extend({
search,
per_page: 20
}, options),
dataType: 'json'
}).done(callback);
},
// Return groups list. Filtered by query
groups: function(query, options, callback) {
groups: function(query, options, callback = $.noop) {
var url = Api.buildUrl(Api.groupsPath);
return $.ajax({
url: url,
......@@ -32,7 +43,7 @@ var Api = {
search: query,
per_page: 20
}, options),
dataType: "json"
dataType: 'json'
}).done(function(groups) {
return callback(groups);
});
......@@ -46,7 +57,7 @@ var Api = {
search: query,
per_page: 20
},
dataType: "json"
dataType: 'json'
}).done(function(namespaces) {
return callback(namespaces);
});
......@@ -61,7 +72,7 @@ var Api = {
per_page: 20,
membership: true
}, options),
dataType: "json"
dataType: 'json'
}).done(function(projects) {
return callback(projects);
});
......@@ -72,9 +83,9 @@ var Api = {
.replace(':project_path', project_path);
return $.ajax({
url: url,
type: "POST",
type: 'POST',
data: { 'label': data },
dataType: "json"
dataType: 'json'
}).done(function(label) {
return callback(label);
}).error(function(message) {
......@@ -91,7 +102,7 @@ var Api = {
search: query,
per_page: 20
},
dataType: "json"
dataType: 'json'
}).done(function(projects) {
return callback(projects);
});
......@@ -156,7 +167,7 @@ var Api = {
per_page: 20,
active: true
},
dataType: "json"
dataType: 'json'
}).done(function(groups) {
return callback(groups);
});
......
/* global Api */
export default class ApproversSelect {
constructor() {
this.$approverSelect = $('.js-select-user-and-group');
const name = this.$approverSelect.data('name');
this.fieldNames = [`${name}[approver_ids]`, `${name}[approver_group_ids]`];
this.$loadWrapper = $('.load-wrapper');
this.bindEvents();
this.addEvents();
this.initSelect2();
}
bindEvents() {
this.handleSelectChange = this.handleSelectChange.bind(this);
this.fetchGroups = this.fetchGroups.bind(this);
this.fetchUsers = this.fetchUsers.bind(this);
}
addEvents() {
$(document).on('click', '.js-add-approvers', () => this.addApprover());
$(document).on('click', '.js-approver-remove', e => ApproversSelect.removeApprover(e));
}
static getApprovers(fieldName, selector, key) {
const input = $(`[name="${fieldName}"]`);
const existingApprovers = $(selector).map((i, el) =>
parseInt($(el).data('id'), 10),
);
const selectedApprovers = input.val()
.split(',')
.filter(val => val !== '');
const approvers = {
[key]: [...existingApprovers, ...selectedApprovers],
};
return approvers;
}
fetchGroups(term) {
const options = ApproversSelect.getApprovers(this.fieldNames[1], '.js-approver-group', 'skip_groups');
return Api.groups(term, options);
}
fetchUsers(term) {
const options = ApproversSelect.getApprovers(this.fieldNames[0], '.js-approver', 'skip_users');
return Api.users(term, options);
}
handleSelectChange(e) {
const { added, removed } = e;
const userInput = $(`[name="${this.fieldNames[0]}"]`);
const groupInput = $(`[name="${this.fieldNames[1]}"]`);
if (added) {
if (added.full_name) {
groupInput.val(`${groupInput.val()},${added.id}`.replace(/^,/, ''));
} else {
userInput.val(`${userInput.val()},${added.id}`.replace(/^,/, ''));
}
}
if (removed) {
if (removed.full_name) {
groupInput.val(groupInput.val().replace(new RegExp(`,?${removed.id}`), ''));
} else {
userInput.val(userInput.val().replace(new RegExp(`,?${removed.id}`), ''));
}
}
}
initSelect2() {
this.$approverSelect.select2({
placeholder: 'Search for users or groups',
multiple: true,
minimumInputLength: 0,
query: (query) => {
const fetchGroups = this.fetchGroups(query.term);
const fetchUsers = this.fetchUsers(query.term);
return $.when(fetchGroups, fetchUsers).then((groups, users) => {
const data = {
results: groups[0].concat(users[0]),
};
return query.callback(data);
});
},
formatResult: ApproversSelect.formatResult,
formatSelection: ApproversSelect.formatSelection,
dropdownCss() {
const $input = $('.js-select-user-and-group .select2-input');
const offset = $input.offset();
const inputRightPosition = offset.left + $input.outerWidth();
const $dropdown = $('.select2-drop-active');
let left = offset.left;
if ($dropdown.outerWidth() > $input.outerWidth()) {
left = `${inputRightPosition - $dropdown.width()}px`;
}
return {
left,
right: 'auto',
width: 'auto',
};
},
})
.on('change', this.handleSelectChange);
}
static formatSelection(group) {
return group.full_name || group.name;
}
static formatResult({
name,
username,
avatar_url: avatarUrl,
full_name: fullName,
full_path: fullPath,
}) {
if (username) {
const avatar = avatarUrl || gon.default_avatar_url;
return `
<div class="user-result">
<div class="user-image">
<img class="avatar s40" src="${avatar}">
</div>
<div class="user-info">
<div class="user-name">${name}</div>
<div class="user-username">@${username}</div>
</div>
</div>
`;
}
return `
<div class="group-result">
<div class="group-name">${fullName}</div>
<div class="group-path">${fullPath}</div>
</div>
`;
}
addApprover() {
this.fieldNames.forEach(ApproversSelect.saveApprovers);
}
static saveApprovers(fieldName) {
const $input = $(`[name="${fieldName}"]`);
const newValue = $input.val();
const $loadWrapper = $('.load-wrapper');
const $approverSelect = $('.js-select-user-and-group');
if (!newValue) {
return;
}
const $form = $('.js-add-approvers').closest('form');
$loadWrapper.removeClass('hidden');
$.ajax({
url: $form.attr('action'),
type: 'POST',
data: {
_method: 'PATCH',
[fieldName]: newValue,
},
success: ApproversSelect.updateApproverList,
complete() {
$input.val('val', '');
$approverSelect.select2('val', '');
$loadWrapper.addClass('hidden');
},
error() {
window.Flash('Failed to add Approver', 'alert');
},
});
}
static removeApprover(e) {
e.preventDefault();
const target = e.currentTarget;
const $loadWrapper = $('.load-wrapper');
$loadWrapper.removeClass('hidden');
$.ajax({
url: target.getAttribute('href'),
type: 'POST',
data: {
_method: 'DELETE',
},
success: ApproversSelect.updateApproverList,
complete: () => $loadWrapper.addClass('hidden'),
error() {
window.Flash('Failed to remove Approver', 'alert');
},
});
}
static updateApproverList(html) {
$('.js-current-approvers').html($(html).find('.js-current-approvers').html());
}
}
......@@ -43,6 +43,7 @@ import BindInOut from './behaviors/bind_in_out';
import GroupName from './group_name';
import GroupsList from './groups_list';
import ProjectsList from './projects_list';
import ApproversSelect from './approvers_select';
import MiniPipelineGraph from './mini_pipeline_graph_dropdown';
import BlobLinePermalinkUpdater from './blob/blob_line_permalink_updater';
import BlobForkSuggestion from './blob/blob_fork_suggestion';
......@@ -432,6 +433,7 @@ const ShortcutsBlob = require('./shortcuts_blob');
case 'edit':
shortcut_handler = new ShortcutsNavigation();
new ProjectNew();
new ApproversSelect();
break;
case 'new':
new ProjectNew();
......
/* eslint-disable func-names, space-before-function-paren, no-var, prefer-rest-params, wrap-iife, no-unused-vars, one-var, no-underscore-dangle, prefer-template, no-else-return, prefer-arrow-callback, max-len */
(function() {
var bind = function(fn, me) { return function() { return fn.apply(me, arguments); }; };
this.ProjectNew = (function() {
function ProjectNew() {
this.toggleSettings = bind(this.toggleSettings, this);
this.$selects = $('.features select');
this.$repoSelects = this.$selects.filter('.js-repo-select');
this.$enableApprovers = $('.js-require-approvals-toggle');
$('.project-edit-container').on('ajax:before', (function(_this) {
return function() {
......@@ -19,10 +17,15 @@
this.initVisibilitySelect();
this.toggleSettings();
this.toggleSettingsOnclick();
this.bindEvents();
this.toggleRepoVisibility();
}
ProjectNew.prototype.bindEvents = function() {
this.$selects.on('change', () => this.toggleSettings());
$('#require_approvals').on('change', e => this.toggleApproverSettingsVisibility(e));
};
ProjectNew.prototype.initVisibilitySelect = function() {
const visibilityContainer = document.querySelector('.js-visibility-select');
if (!visibilityContainer) return;
......@@ -30,6 +33,15 @@
visibilitySelect.init();
};
ProjectNew.prototype.toggleApproverSettingsVisibility = function(e) {
this.$requiredApprovals = $('#project_approvals_before_merge');
const enabled = $(e.target).prop('checked');
const val = enabled ? 1 : 0;
this.$requiredApprovals.val(val);
this.$requiredApprovals.prop('min', val);
$('.nested-settings').toggleClass('hidden', !enabled);
};
ProjectNew.prototype.toggleSettings = function() {
var self = this;
......@@ -42,10 +54,6 @@
});
};
ProjectNew.prototype.toggleSettingsOnclick = function() {
this.$selects.on('change', this.toggleSettings);
};
ProjectNew.prototype._showOrHide = function(checkElement, container) {
var $container = $(container);
......
......@@ -388,7 +388,7 @@ import Vue from 'vue';
} else {
avatar = gon.default_avatar_url;
}
return "<div class='user-result " + (!user.username ? 'no-username' : void 0) + "'> <div class='user-image'><img class='avatar s24' src='" + avatar + "'></div> <div class='user-name'>" + user.name + "</div> <div class='user-username'>" + (user.username || "") + "</div> </div>";
return "<div class='user-result " + (!user.username ? 'no-username' : void 0) + "'> <div class='user-image'><img class='avatar s40' src='" + avatar + "'></div> <div><div class='user-name'>" + user.name + "</div> <div class='user-username'>" + (user.username || "") + "</div> </div> </div>";
};
UsersSelect.prototype.formatSelection = function(user) {
......
......@@ -47,9 +47,10 @@
}
.select2-drop {
box-shadow: $select2-drop-shadow1 0 0 1px 0, $select2-drop-shadow2 0 2px 18px 0;
border-radius: $border-radius-default;
border: none;
background-color: $white-light;
border: 1px solid $dropdown-border-color;
border-radius: $border-radius-base;
box-shadow: 0 2px 4px $dropdown-shadow-color;
min-width: 175px;
}
......@@ -63,7 +64,7 @@
}
.select2-highlighted {
background: $gl-link-color !important;
background: $dropdown-hover-color !important;
}
.select2-results li.select2-result-with-children > .select2-result-label {
......@@ -124,8 +125,8 @@
&.select2-container-active .select2-choices,
&.select2-dropdown-open .select2-choices {
border-color: $border-white-normal;
box-shadow: $gl-btn-active-gradient;
border-color: $dropdown-input-focus-border;
box-shadow: 0 0 4px $search-input-focus-shadow-color;
}
}
......@@ -237,6 +238,7 @@
.user-result {
min-height: 24px;
display: flex;
.user-image {
float: left;
......
......@@ -46,11 +46,19 @@
}
}
.load-wrapper {
position: absolute;
background: $black-transparent;
width: 100%;
height: 100%;
display: flex;
align-items: center;
justify-content: center;
}
@media (max-width: $screen-xs-max) {
.input-group > div {
margin-bottom: 14px;
&:last-child {
margin-bottom: 0;
}
......
......@@ -35,3 +35,28 @@
margin-left: 5px;
}
}
.nested-settings {
padding-left: 20px;
}
.input-btn-group {
display: flex;
.input-large {
flex: 1;
}
.btn {
margin-left: 10px;
}
}
.settings-flex-row {
display: flex;
align-items: center;
.pull-right {
margin-left: auto;
}
}
......@@ -43,55 +43,75 @@
.hint
Description parsed with #{link_to "GitLab Flavored Markdown", help_page_path('user/markdown'), target: '_blank'}.
.form-group
= form.label :approvals_before_merge, class: 'label-light' do
Approvals required
= form.number_field :approvals_before_merge, class: "form-control", min: 0
.help-block
Number of users to approve a merge request before it can be accepted. 0 - approving is disabled
.form-group.reset-approvals-on-push
.checkbox
= form.label :reset_approvals_on_push do
= form.check_box :reset_approvals_on_push
%span.descr Reset approvals on push
.help-block Approvals are reset when new data is pushed to the merge request
= label_tag :require_approvals do
= check_box_tag :require_approvals, nil, project.approvals_before_merge.nonzero?, class: 'js-require-approvals-toggle'
%strong Activate merge request approvals
= link_to icon('question-circle'), help_page_path("user/project/merge_requests/merge_request_approvals"), target: '_blank'
.descr Merge request approvals allow you to set the number of necessary approvals and predefine a list of approvers that you will need to approve every merge request in a project.
.form-group
.nested-settings{ class: project.approvals_before_merge.nonzero? ? '' : 'hidden' }
.form-group
= form.label :approver_ids, class: 'label-light' do
Approvers
= users_select_tag("project[approver_ids]", multiple: true, class: 'input-large', scope: :all, email_user: true)
.help-block
Add an approver suggestion for each merge request
= form.label :approver_group_ids, class: 'label-light' do
Approver groups
- skip_groups = project.approver_groups.pluck(:group_id)
= groups_select_tag('project[approver_group_ids]', multiple: true, data: { skip_groups: skip_groups, all_available: true }, class: 'input-large')
= hidden_field_tag "project[approver_ids]"
= hidden_field_tag "project[approver_group_ids]"
.input-group.input-btn-group
= hidden_field_tag :approver_user_and_group_ids, '', { class: 'js-select-user-and-group input-large', tabindex: 1, 'data-name': 'project' }
%button.btn.btn-success.js-add-approvers{ type: 'button', title: 'Add approvers(s)' }
Add
.help-block
Add a group as an approver suggestion for each merge request
Add an approver or group suggestion for each merge request
.panel.panel-default.prepend-top-10
.panel.panel-default.prepend-top-10.js-current-approvers
.panel-heading
Approvers
%span.badge
- ids = []
- project.approvers.each do |user|
- ids << user.user_id
- project.approver_groups.each do |group|
- group.users.each do |user|
- unless ids.include?(user.id)
- ids << user.id
= ids.count
%ul.well-list.approver-list
.load-wrapper.hidden
= icon('spinner spin', class: 'approver-list-loader')
- project.approvers.each do |approver|
%li.approver
%li.approver.settings-flex-row.js-approver{ data: { id: approver.user_id } }
= link_to approver.user.name, approver.user
.pull-right
= link_to namespace_project_approver_path(project.namespace, project, approver), data: { confirm: "Are you sure you want to remove approver #{approver.user.name}"}, method: :delete, class: "btn-xs btn btn-remove", title: 'Remove approver' do
= icon("sign-out")
Remove
%button{ href: namespace_project_approver_path(project.namespace, project, approver), data: { confirm: "Are you sure you want to remove approver #{approver.user.name}"}, class: "btn btn-remove js-approver-remove", title: 'Remove approver' }
= icon("trash")
- project.approver_groups.each do |approver_group|
%li.approver-group
%li.approver-group.settings-flex-row.js-approver-group{ data: { id: approver_group.group.id } }
.span
%span.light
Group:
= link_to approver_group.group.name, approver_group.group
%span.badge
= approver_group.group.members.count
.pull-right
= link_to namespace_project_approver_group_path(project.namespace, project, approver_group), data: { confirm: "Are you sure you want to remove group #{approver_group.group.name}" }, method: :delete, class: "btn-xs btn btn-remove", title: 'Remove group' do
= icon("sign-out")
Remove
%button{ href: namespace_project_approver_group_path(project.namespace, project, approver_group), data: { confirm: "Are you sure you want to remove group #{approver_group.group.name}" }, class: "btn btn-remove js-approver-remove", title: 'Remove group' }
= icon("trash")
- if project.approvers.empty? && project.approver_groups.empty?
%li There are no approvers
.form-group
= form.label :approvals_before_merge, class: 'label-light' do
Approvals required
= form.number_field :approvals_before_merge, class: "form-control", min: 0
.help-block
.form-group.reset-approvals-on-push
.checkbox
= form.label :reset_approvals_on_push do
= form.check_box :reset_approvals_on_push
%strong Reset approvals on push
.descr Approvals are reset when new data is pushed to the merge request
:javascript
new UsersSelect();
new GroupsSelect();
......@@ -5,7 +5,7 @@ describe 'Project settings > [EE] Merge Requests', feature: true, js: true do
include WaitForAjax
let(:user) { create(:user) }
let(:project) { create(:empty_project) }
let(:project) { create(:empty_project, approvals_before_merge: 1) }
let(:group) { create(:group) }
let(:approver) { create(:user) }
......@@ -16,20 +16,42 @@ describe 'Project settings > [EE] Merge Requests', feature: true, js: true do
group.add_developer(user)
end
scenario 'adds approver' do
visit edit_project_path(project)
find('#s2id_approver_user_and_group_ids .select2-input').click
wait_for_ajax
expect(find('.select2-results')).to have_content(user.name)
find('.user-result', text: user.name).click
click_button 'Add'
expect(find('.js-current-approvers')).to have_content(user.name)
end
scenario 'adds approver group' do
visit edit_project_path(project)
find('#s2id_project_approver_group_ids .select2-input').click
find('#s2id_approver_user_and_group_ids .select2-input').click
wait_for_ajax
expect(find('.select2-results')).to have_content(group.name)
within('.js-current-approvers') do
expect(find('.panel-heading .badge')).to have_content('0')
end
find('.select2-results').click
expect(find('.select2-results')).to have_content(group.name)
find('.select2-results .group-result').click
click_button 'Add'
click_button 'Save changes'
expect(find('.approver-list-loader')).to be_visible
expect(page).to have_css('.js-current-approvers li.approver-group', count: 1)
expect(page).to have_css('.approver-list li.approver-group', count: 1)
expect(page).to have_css('.js-current-approvers li.approver-group', count: 1)
within('.js-current-approvers') do
expect(find('.panel-heading .badge')).to have_content('2')
end
end
context 'with an approver group' do
......@@ -40,13 +62,13 @@ describe 'Project settings > [EE] Merge Requests', feature: true, js: true do
scenario 'removes approver group' do
visit edit_project_path(project)
expect(find('.approver-list')).to have_content(group.name)
expect(find('.js-current-approvers')).to have_content(group.name)
within('.approver-list') do
within('.js-current-approvers') do
click_on "Remove"
end
expect(find('.approver-list')).not_to have_content(group.name)
expect(find('.js-current-approvers')).not_to have_content(group.name)
end
end
end
......@@ -25,4 +25,13 @@ describe ProjectsController, '(JavaScript fixtures)', type: :controller do
expect(response).to be_success
store_frontend_fixture(response, example.description)
end
it 'projects/edit.html.raw' do |example|
get :edit,
namespace_id: project.namespace.to_param,
id: project
expect(response).to be_success
store_frontend_fixture(response, example.description)
end
end
require('~/project_new');
describe('ApproversSelect', function () {
const projectSettingsTemplate = 'projects/edit.html.raw';
preloadFixtures(projectSettingsTemplate);
beforeEach(() => {
loadFixtures(projectSettingsTemplate);
this.$requireApprovalsToggle = $('.js-require-approvals-toggle');
this.project = new window.ProjectNew();
});
it('shows approver settings if enabled', () => {
expect(this.$requireApprovalsToggle).not.toBeChecked();
expect($('.nested-settings').hasClass('hidden')).toBe(true);
this.$requireApprovalsToggle.click();
expect($('.js-current-approvers').hasClass('hidden')).toBe(false);
});
it('hides approver settings if disabled', () => {
expect('#require_approvals').not.toBeChecked();
expect($('.nested-settings').hasClass('hidden')).toBe(true);
this.$requireApprovalsToggle.click();
this.$requireApprovalsToggle.click();
expect($('.nested-settings').hasClass('hidden')).toBe(true);
});
it('sets required approvers to 0 if approvers disabled', () => {
expect($('[name="project[approvals_before_merge]"]').val()).toBe('0');
});
it('sets required approvers to 1 if approvers enabled', () => {
this.$requireApprovalsToggle.click();
expect($('[name="project[approvals_before_merge]"]').val()).toBe('1');
});
it('sets minimum for approvers field if enabled', () => {
expect($('[name="project[approvals_before_merge]"]').attr('min')).toBe('0');
this.$requireApprovalsToggle.click();
expect($('[name="project[approvals_before_merge]"]').attr('min')).toBe('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