Commit 56bc99c6 authored by Tim Zallmann's avatar Tim Zallmann

Merge branch '2452-zero-approvals-mr-widget' into 'master'

Add approval widget support for zero approvals.

See merge request gitlab-org/gitlab-ee!6032
parents 541f2f42 698ac8f7
......@@ -28,10 +28,6 @@
border-top: solid 1px $border-color;
}
.mr-widget-border-top {
border-top: 1px solid $border-color;
}
.mr-widget-footer {
padding: 0;
}
......@@ -709,64 +705,6 @@
}
}
#merge-request-widget-app .loading {
padding-top: 5px;
border-top: 1px solid $well-inner-border;
}
.approvals-footer {
display: flex;
.approvers-prefix {
display: flex;
align-items: center;
flex-wrap: wrap;
}
.approvers-list {
display: flex;
align-items: center;
.link-to-member-avatar:not(:first-child) {
img {
margin-left: 0;
}
}
}
.unapprove-btn {
border: 0;
background: transparent;
cursor: pointer;
&:hover {
color: $gl-text-color-secondary;
text-decoration: none;
}
&:focus {
outline: none;
}
}
.approver-avatar {
position: relative;
}
}
.link-to-member-avatar {
.disabled {
pointer-events: none;
cursor: default;
}
.avatar {
margin-bottom: 0;
margin-left: 7px;
display: block;
}
}
.mr-memory-usage {
width: 100%;
......
......@@ -22,16 +22,6 @@
window.gl.mrWidgetData.squash_before_merge_help_path = '#{help_page_path("user/project/merge_requests/squash_and_merge")}';
// Append static, server-generated data not included in merge request entity (EE-Only)
// Object.assign would be useful here, but it blows up Phantom.js in tests
window.gl.mrWidgetData.is_geo_secondary_node = '#{Gitlab::Geo.secondary?}' === 'true';
window.gl.mrWidgetData.geo_secondary_help_path = '#{help_page_path("/gitlab-geo/configuration.md")}';
window.gl.mrWidgetData.sast_help_path = '#{help_page_path("user/project/merge_requests/sast")}';
window.gl.mrWidgetData.sast_container_help_path = '#{help_page_path("user/project/merge_requests/container_scanning")}';
window.gl.mrWidgetData.dast_help_path = '#{help_page_path("user/project/merge_requests/dast")}';
window.gl.mrWidgetData.dependency_scanning_help_path = '#{help_page_path("user/project/merge_requests/dependency_scanning")}';
window.gl.mrWidgetData.vulnerability_feedback_help_path = '#{help_page_path("user/project/merge_requests/index", anchor: "interacting-with-security-reports-ultimate")}';
#js-vue-mr-widget.mr-widget
.content-block.content-block-small.emoji-list-container.js-noteable-awards
......
import $ from 'jquery';
export default function initApproversCheckbox() {
$('#require_approvals').on('change', e => {
const $requiredApprovals = $('#project_approvals_before_merge');
const enabled = $(e.target).prop('checked');
const val = enabled ? 1 : 0;
$requiredApprovals.val(val);
$requiredApprovals.prop('min', val);
$('.nested-settings').toggleClass('hidden', !enabled);
});
}
......@@ -5,7 +5,6 @@ import UsersSelect from '~/users_select';
import UserCallout from '~/user_callout';
import groupsSelect from '~/groups_select';
import ApproversSelect from 'ee/approvers_select';
import initApproversCheckbox from 'ee/approvers_checkbox';
import initServiceDesk from 'ee/projects/settings_service_desk';
document.addEventListener('DOMContentLoaded', () => {
......@@ -15,6 +14,5 @@ document.addEventListener('DOMContentLoaded', () => {
new UserCallout({ className: 'js-service-desk-callout' });
new UserCallout({ className: 'js-mr-approval-callout' });
new ApproversSelect();
initApproversCheckbox();
initServiceDesk();
});
<script>
import { n__, s__, sprintf } from '~/locale';
import Flash from '~/flash';
import Icon from '~/vue_shared/components/icon.vue';
import MrWidgetAuthor from '~/vue_merge_request_widget/components/mr_widget_author.vue';
import tooltip from '~/vue_shared/directives/tooltip';
import eventHub from '~/vue_merge_request_widget/event_hub';
export default {
name: 'ApprovalsBody',
components: {
MrWidgetAuthor,
Icon,
},
directives: {
tooltip,
},
props: {
mr: {
......@@ -23,6 +29,11 @@ export default {
required: false,
default: () => [],
},
approvalsOptional: {
type: Boolean,
required: false,
default: false,
},
approvalsLeft: {
type: Number,
required: false,
......@@ -51,6 +62,14 @@ export default {
},
computed: {
approvalsRequiredStringified() {
if (this.approvalsOptional) {
if (this.userCanApprove) {
return s__('mrWidget|No Approval required; you can still approve');
}
return s__('mrWidget|No Approval required');
}
if (this.approvalsLeft === 0) {
return s__('mrWidget|Approved');
}
......@@ -85,11 +104,14 @@ export default {
'btn-inverted': this.showApproveButton && this.approvalsLeft <= 0,
};
},
showApprovalDocLink() {
return this.approvalsOptional && this.showApproveButton;
},
showApproveButton() {
return this.userCanApprove && !this.userHasApproved && this.mr.isOpen;
},
showSuggestedApprovers() {
return this.suggestedApprovers && this.suggestedApprovers.length;
return this.approvalsLeft > 0 && this.suggestedApprovers && this.suggestedApprovers.length;
},
},
methods: {
......@@ -131,8 +153,25 @@ export default {
{{ approveButtonText }}
</button>
</span>
<span class="approvals-required-text bold">
<span
class="approvals-required-text"
:class="approvalsOptional ? 'text-muted' : 'bold'"
>
{{ approvalsRequiredStringified }}
<a
v-if="showApprovalDocLink"
:href="mr.approvalsHelpPath"
:data-title="__('About this feature')"
data-placement="bottom"
target="_blank"
rel="noopener noreferrer nofollow"
data-container="body"
v-tooltip
>
<icon
name="question-o"
/>
</a>
<span v-if="showSuggestedApprovers">
<mr-widget-author
v-for="approver in suggestedApprovers"
......
......@@ -35,6 +35,13 @@ export default {
}
return 'success';
},
approvalsOptional() {
return (
!this.fetchingApprovals &&
this.mr.approvals.approvals_required === 0 &&
this.mr.approvals.approved_by.length === 0
);
},
},
created() {
const flashErrorMessage = s__(
......@@ -54,17 +61,11 @@ export default {
<template>
<div
v-if="mr.approvalsRequired"
class="mr-widget-approvals-container mr-widget-body mr-widget-section media"
class="mr-widget-approvals-container mr-widget-section media"
>
<div
v-if="fetchingApprovals"
class="mr-widget-icon"
>
<i class="fa fa-spinner fa-spin"></i>
</div>
<status-icon
v-else
:status="status"
:class="approvalsOptional ? 'zero-approvals' : ''"
:status="fetchingApprovals ? 'loading' : status"
/>
<div
v-show="fetchingApprovals"
......@@ -85,6 +86,7 @@ export default {
:user-has-approved="mr.approvals.user_has_approved"
:approved-by="mr.approvals.approved_by"
:approvals-left="mr.approvals.approvals_left"
:approvals-optional="approvalsOptional"
:suggested-approvers="mr.approvals.suggested_approvers"
/>
<approvals-footer
......
......@@ -18,6 +18,7 @@ export default class MergeRequestStore extends CEMergeRequestStore {
this.dependencyScanningHelp = data.dependency_scanning_help_path;
this.vulnerabilityFeedbackPath = data.vulnerability_feedback_path;
this.vulnerabilityFeedbackHelpPath = data.vulnerability_feedback_help_path;
this.approvalsHelpPath = data.approvals_help_path;
this.securityReportsPipelineId = data.pipeline_id;
this.initCodeclimate(data);
......@@ -41,8 +42,7 @@ export default class MergeRequestStore extends CEMergeRequestStore {
this.isApproved = this.isApproved || false;
this.approvals = this.approvals || null;
this.approvalsPath = data.approvals_path || this.approvalsPath;
this.approvalsRequired =
data.approvalsRequired || (Boolean(this.approvalsPath) && data.approvals_before_merge > 0);
this.approvalsRequired = data.approvalsRequired || Boolean(this.approvalsPath);
}
setApprovals(data) {
......
.mr-widget-border-top {
border-top: 1px solid $border-color;
}
.link-to-member-avatar {
.disabled {
pointer-events: none;
cursor: default;
}
.avatar {
margin-bottom: 0;
margin-left: 7px;
display: block;
}
}
.mr-widget-approvals-container {
.zero-approvals .ci-status-icon-success svg {
fill: $gray-darkest;
}
}
.approvals-body {
.approve-btn {
vertical-align: baseline;
}
}
.approvals-footer {
display: flex;
.approvers-prefix {
display: flex;
align-items: center;
flex-wrap: wrap;
}
.approvers-list {
display: flex;
align-items: center;
.link-to-member-avatar:not(:first-child) {
img {
margin-left: 0;
}
}
}
.unapprove-btn {
border: 0;
background: transparent;
cursor: pointer;
&:hover {
color: $gl-text-color-secondary;
text-decoration: none;
}
&:focus {
outline: none;
}
}
.approver-avatar {
position: relative;
}
}
- return unless project.feature_available?(:merge_request_approvers)
- can_override_approvers = project.can_override_approvers?
.form-group.reset-approvals-on-push
.form-check
= label_tag :require_approvals do
= check_box_tag :require_approvals, nil, project.approvals_before_merge.nonzero?, class: 'js-require-approvals-toggle'
%strong Merge request approvals
= link_to icon('question-circle'), help_page_path("user/project/merge_requests/merge_request_approvals"), target: '_blank'
.form-group
%strong= _('Merge request approvals')
= link_to icon('question-circle'), help_page_path("user/project/merge_requests/merge_request_approvals"), target: '_blank'
.nested-settings{ class: project.approvals_before_merge.nonzero? ? '' : 'hidden' }
.nested-settings
.form-group
= form.label :approver_ids, class: 'label-light' do
Approvers
= 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' }
= hidden_field_tag :approver_user_and_group_ids, '', { class: 'js-select-user-and-group input-large', tabindex: 1, 'data-name': 'project', :style => "max-width: unset;" }
%button.btn.btn-success.js-add-approvers{ type: 'button', title: 'Add approvers(s)' }
Add
.form-text.text-muted
Add users or groups who are allowed to approve every merge request
.card.prepend-top-10.js-current-approvers
.load-wrapper.hidden
= icon('spinner spin', class: 'approver-list-loader')
.card-header
Approvers
%span.badge
%span.badge.badge-pill
- ids = []
- project.approvers.each do |user|
- ids << user.user_id
......@@ -34,8 +33,6 @@
- ids << user.id
= ids.count
%ul.content-list.approver-list
.load-wrapper.hidden
= icon('spinner spin', class: 'approver-list-loader')
- project.approvers.each do |approver|
%li.approver.settings-flex-row.js-approver{ data: { id: approver.user_id } }
= link_to approver.user.name, approver.user
......@@ -44,11 +41,11 @@
= icon("trash")
- project.approver_groups.each do |approver_group|
%li.approver-group.settings-flex-row.js-approver-group{ data: { id: approver_group.group.id } }
.span
%span
%span.light
Group:
= link_to approver_group.group.name, approver_group.group
%span.badge
%span.badge.badge-pill
= approver_group.group.members.count
.float-right
%button{ href: project_approver_group_path(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' }
......
= render :file => "/app/views/projects/merge_requests/show"
-# haml-lint:disable InlineJavaScript
:javascript
// Append static, server-generated data not included in merge request entity (EE-Only)
// Object.assign would be useful here, but it blows up Phantom.js in tests
window.gl.mrWidgetData.is_geo_secondary_node = '#{Gitlab::Geo.secondary?}' === 'true';
window.gl.mrWidgetData.geo_secondary_help_path = '#{help_page_path("/gitlab-geo/configuration.md")}';
window.gl.mrWidgetData.sast_help_path = '#{help_page_path("user/project/merge_requests/sast")}';
window.gl.mrWidgetData.sast_container_help_path = '#{help_page_path("user/project/merge_requests/container_scanning")}';
window.gl.mrWidgetData.dast_help_path = '#{help_page_path("user/project/merge_requests/dast")}';
window.gl.mrWidgetData.dependency_scanning_help_path = '#{help_page_path("user/project/merge_requests/dependency_scanning")}';
window.gl.mrWidgetData.vulnerability_feedback_help_path = '#{help_page_path("user/project/merge_requests/index", anchor: "interacting-with-security-reports-ultimate")}';
window.gl.mrWidgetData.approvals_help_path = '#{help_page_path("user/project/merge_requests/merge_request_approvals")}';
......@@ -8,8 +8,8 @@ msgid ""
msgstr ""
"Project-Id-Version: gitlab 1.0.0\n"
"Report-Msgid-Bugs-To: \n"
"POT-Creation-Date: 2018-06-01 01:47-0500\n"
"PO-Revision-Date: 2018-06-01 01:47-0500\n"
"POT-Creation-Date: 2018-06-07 18:08+0200\n"
"PO-Revision-Date: 2018-06-07 18:08+0200\n"
"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
"Language-Team: LANGUAGE <LL@li.org>\n"
"Language: \n"
......@@ -66,6 +66,11 @@ msgid_plural "%d metrics"
msgstr[0] ""
msgstr[1] ""
msgid "%d new license"
msgid_plural "%d new licenses"
msgstr[0] ""
msgstr[1] ""
msgid "%d staged change"
msgid_plural "%d staged changes"
msgstr[0] ""
......@@ -237,6 +242,9 @@ msgstr ""
msgid "About auto deploy"
msgstr ""
msgid "About this feature"
msgstr ""
msgid "Abuse Reports"
msgstr ""
......@@ -252,6 +260,9 @@ msgstr ""
msgid "Access Tokens"
msgstr ""
msgid "Access to '%{classification_label}' not allowed"
msgstr ""
msgid "Access to failing storages has been temporarily disabled to allow the mount to recover. Reset storage information after the issue has been resolved to allow access again."
msgstr ""
......@@ -1148,6 +1159,9 @@ msgstr ""
msgid "CircuitBreakerApiLink|circuitbreaker api"
msgstr ""
msgid "ClassificationLabelUnavailable|is unavailable: %{reason}"
msgstr ""
msgid "Clear search input"
msgstr ""
......@@ -1235,6 +1249,9 @@ msgstr ""
msgid "ClusterIntegration|Copy Ingress IP Address to clipboard"
msgstr ""
msgid "ClusterIntegration|Copy Jupyter Hostname to clipboard"
msgstr ""
msgid "ClusterIntegration|Copy Kubernetes cluster name"
msgstr ""
......@@ -1319,6 +1336,12 @@ msgstr ""
msgid "ClusterIntegration|Integration status"
msgstr ""
msgid "ClusterIntegration|Jupyter Hostname"
msgstr ""
msgid "ClusterIntegration|JupyterHub"
msgstr ""
msgid "ClusterIntegration|Kubernetes cluster"
msgstr ""
......@@ -1734,6 +1757,9 @@ msgstr ""
msgid "ContainerRegistry|You can also use a %{deploy_token} for read-only access to the registry images."
msgstr ""
msgid "Continue"
msgstr ""
msgid "Continuous Integration and Deployment"
msgstr ""
......@@ -2258,9 +2284,15 @@ msgstr ""
msgid "Environments|No deployments yet"
msgstr ""
msgid "Environments|No pod name has been specified"
msgstr ""
msgid "Environments|Open"
msgstr ""
msgid "Environments|Pod logs from %{podName}"
msgstr ""
msgid "Environments|Re-deploy"
msgstr ""
......@@ -2327,6 +2359,9 @@ msgstr ""
msgid "Error loading last commit."
msgstr ""
msgid "Error loading merge requests."
msgstr ""
msgid "Error loading project data. Please try again."
msgstr ""
......@@ -2485,6 +2520,9 @@ msgstr ""
msgid "Format"
msgstr ""
msgid "Found errors in your .gitlab-ci.yml:"
msgstr ""
msgid "From %{provider_title}"
msgstr ""
......@@ -2521,6 +2559,9 @@ msgstr ""
msgid "GeoNodes|Checksummed"
msgstr ""
msgid "GeoNodes|Data is out of date from %{timeago}"
msgstr ""
msgid "GeoNodes|Data replication lag"
msgstr ""
......@@ -3250,6 +3291,9 @@ msgstr ""
msgid "Merge request"
msgstr ""
msgid "Merge request approvals"
msgstr ""
msgid "Merge requests"
msgstr ""
......@@ -3636,6 +3680,9 @@ msgstr ""
msgid "Open"
msgstr ""
msgid "Open in Xcode"
msgstr ""
msgid "Opened"
msgstr ""
......@@ -3894,6 +3941,9 @@ msgstr ""
msgid "Preferences"
msgstr ""
msgid "Preferences|Navigation theme"
msgstr ""
msgid "Primary"
msgstr ""
......@@ -4023,10 +4073,10 @@ msgstr ""
msgid "ProjectCreationLevel|Default project creation protection"
msgstr ""
msgid "ProjectCreationLevel|Developers + Masters"
msgid "ProjectCreationLevel|Developers + Maintainers"
msgstr ""
msgid "ProjectCreationLevel|Masters"
msgid "ProjectCreationLevel|Maintainers"
msgstr ""
msgid "ProjectCreationLevel|No one"
......@@ -4239,6 +4289,9 @@ msgstr ""
msgid "Reference:"
msgstr ""
msgid "Refresh"
msgstr ""
msgid "Register / Sign In"
msgstr ""
......@@ -4430,6 +4483,12 @@ msgstr ""
msgid "Scoped issue boards"
msgstr ""
msgid "Scroll to bottom"
msgstr ""
msgid "Scroll to top"
msgstr ""
msgid "Search"
msgstr ""
......@@ -4636,9 +4695,6 @@ msgstr ""
msgid "Something went wrong while fetching group member contributions"
msgstr ""
msgid "Something went wrong while fetching the latest pipeline status."
msgstr ""
msgid "Something went wrong while fetching the projects."
msgstr ""
......@@ -5785,6 +5841,9 @@ msgstr ""
msgid "You can also star a label to make it a priority label."
msgstr ""
msgid "You can also test your .gitlab-ci.yml in the %{linkStart}Lint%{linkEnd}"
msgstr ""
msgid "You can easily install a Runner on a Kubernetes cluster. %{link_to_help_page}"
msgstr ""
......@@ -5925,6 +5984,15 @@ msgstr ""
msgid "ciReport|%{namespace} is affected by %{vulnerability}."
msgstr ""
msgid "ciReport|%{packagesString} and "
msgstr ""
msgid "ciReport|%{packagesString} and %{lastPackage}"
msgstr ""
msgid "ciReport|%{remainingPackagesCount} more"
msgstr ""
msgid "ciReport|%{reportName} is loading"
msgstr ""
......@@ -6006,6 +6074,12 @@ msgstr ""
msgid "ciReport|Learn more about whitelisting"
msgstr ""
msgid "ciReport|License management detected %{licenseInfo}"
msgstr ""
msgid "ciReport|License management detected no new licenses"
msgstr ""
msgid "ciReport|Links"
msgstr ""
......@@ -6248,6 +6322,12 @@ msgstr ""
msgid "mrWidget|Merged by"
msgstr ""
msgid "mrWidget|No Approval required"
msgstr ""
msgid "mrWidget|No Approval required; you can still approve"
msgstr ""
msgid "mrWidget|Plain diff"
msgstr ""
......@@ -6390,3 +6470,8 @@ msgstr ""
msgid "with %{additions} additions, %{deletions} deletions."
msgstr ""
msgid "within %d minute "
msgid_plural "within %d minutes "
msgstr[0] ""
msgstr[1] ""
......@@ -13,6 +13,7 @@ describe('Approvals Body Component', () => {
userHasApproved: true,
approvedBy: [],
approvalsLeft: 1,
approvalsOptional: false,
pendingAvatarSvg: '<svg></svg>',
checkmarkSvg: '<svg></svg>',
};
......@@ -48,11 +49,35 @@ describe('Approvals Body Component', () => {
});
it('should display the correct string for 2 possible approvers', done => {
const correctText = 'Requires 2 more approvals by';
vm.approvalsLeft = 2;
vm.suggestedApprovers.push({ name: 'Approver 2' });
Vue.nextTick(() => {
const correctText = 'Requires 2 more approvals by';
expect(vm.approvalsRequiredStringified).toBe(correctText);
done();
});
});
it('should display the correct string for 0 approvals required', done => {
const correctText = 'No Approval required';
vm.approvalsOptional = true;
Vue.nextTick(() => {
expect(vm.approvalsRequiredStringified).toBe(correctText);
done();
});
});
it('should display the correct string for 0 approvals required and if the user is able to approve', done => {
const correctText = 'No Approval required; you can still approve';
vm.approvalsOptional = true;
vm.userCanApprove = true;
Vue.nextTick(() => {
expect(vm.approvalsRequiredStringified).toBe(correctText);
done();
});
......@@ -76,6 +101,18 @@ describe('Approvals Body Component', () => {
done();
});
});
it('shows the "Requires 2 more approvals" without by when no suggested approvals are available', done => {
const correctText = 'Requires 2 more approvals';
vm.approvalsLeft = 2;
vm.suggestedApprovers = [];
Vue.nextTick(() => {
expect(vm.approvalsRequiredStringified).toBe(correctText);
done();
});
});
});
describe('showApproveButton', () => {
......
import $ from 'jquery';
import initApproversCheckbox from 'ee/approvers_checkbox';
describe('ApproversSelect', function () {
const projectSettingsTemplate = 'projects/edit.html.raw';
preloadFixtures(projectSettingsTemplate);
beforeEach(() => {
loadFixtures(projectSettingsTemplate);
this.$requireApprovalsToggle = $('.js-require-approvals-toggle');
initApproversCheckbox();
});
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