Commit 8007dd2f authored by Kushal Pandya's avatar Kushal Pandya

Merge branch 'ph/283972/humanReadableCollapsedApprovalsRules' into 'master'

Added human readable summary text to approval rules

See merge request gitlab-org/gitlab!49011
parents d7840984 248f26b5
<script> <script>
import { uniqueId } from 'lodash'; import { uniqueId } from 'lodash';
import { GlIcon, GlButton, GlCollapse, GlCollapseToggleDirective } from '@gitlab/ui'; import {
GlIcon,
GlButton,
GlCollapse,
GlCollapseToggleDirective,
GlSafeHtmlDirective,
} from '@gitlab/ui';
import { mapState } from 'vuex';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import { __, n__, sprintf } from '~/locale';
import App from '../app.vue'; import App from '../app.vue';
import MrRules from './mr_rules.vue'; import MrRules from './mr_rules.vue';
import MrRulesHiddenInputs from './mr_rules_hidden_inputs.vue'; import MrRulesHiddenInputs from './mr_rules_hidden_inputs.vue';
...@@ -16,7 +25,9 @@ export default { ...@@ -16,7 +25,9 @@ export default {
}, },
directives: { directives: {
CollapseToggle: GlCollapseToggleDirective, CollapseToggle: GlCollapseToggleDirective,
SafeHtml: GlSafeHtmlDirective,
}, },
mixins: [glFeatureFlagsMixin()],
data() { data() {
return { return {
collapseId: uniqueId('approval-rules-expandable-section-'), collapseId: uniqueId('approval-rules-expandable-section-'),
...@@ -24,11 +35,79 @@ export default { ...@@ -24,11 +35,79 @@ export default {
}; };
}, },
computed: { computed: {
...mapState({
rules: state => state.approvals.rules,
canOverride: state => state.settings.canOverride,
}),
toggleIcon() { toggleIcon() {
return this.isCollapsed ? 'chevron-down' : 'chevron-right'; return this.isCollapsed ? 'chevron-down' : 'chevron-right';
}, },
isCollapseFeatureEnabled() { isCollapseFeatureEnabled() {
return gon.features?.mergeRequestReviewers && gon.features?.mrCollapsedApprovalRules; return this.glFeatures.mergeRequestReviewers && this.glFeatures.mrCollapsedApprovalRules;
},
hasOptionalRules() {
return this.rules.every(r => r.approvalsRequired === 0);
},
requiredRules() {
return this.rules.reduce((acc, rule) => {
if (rule.approvalsRequired > 0) {
acc.push(rule);
}
return acc;
}, []);
},
collapsedSummary() {
const rulesLength = this.requiredRules.length;
const firstRule = this.requiredRules[0];
if (this.hasOptionalRules) {
return __('Approvals are optional.');
} else if (rulesLength === 1 && firstRule.ruleType === 'any_approver') {
return sprintf(
n__(
'%{strong_start}%{count} member%{strong_end} must approve to merge. Anyone with role Developer or higher can approve.',
'%{strong_start}%{count} members%{strong_end} must approve to merge. Anyone with role Developer or higher can approve.',
firstRule.approvalsRequired,
),
{
strong_start: '<strong>',
strong_end: '</strong>',
count: firstRule.approvalsRequired,
},
false,
);
} else if (rulesLength === 1 && firstRule.ruleType !== 'any_approver') {
return sprintf(
n__(
'%{strong_start}%{count} eligible member%{strong_end} must approve to merge.',
'%{strong_start}%{count} eligible members%{strong_end} must approve to merge.',
firstRule.approvalsRequired,
),
{
strong_start: '<strong>',
strong_end: '</strong>',
count: firstRule.approvalsRequired,
},
false,
);
} else if (rulesLength > 1) {
return sprintf(
n__(
'%{strong_start}%{count} approval rule%{strong_end} requires eligible members to approve before merging.',
'%{strong_start}%{count} approval rules%{strong_end} require eligible members to approve before merging.',
rulesLength,
),
{
strong_start: '<strong>',
strong_end: '</strong>',
count: rulesLength,
},
false,
);
}
return null;
}, },
}, },
}; };
...@@ -36,7 +115,17 @@ export default { ...@@ -36,7 +115,17 @@ export default {
<template> <template>
<div v-if="isCollapseFeatureEnabled" class="gl-mt-2"> <div v-if="isCollapseFeatureEnabled" class="gl-mt-2">
<gl-button v-collapse-toggle="collapseId" variant="link" button-text-classes="flex"> <p
v-safe-html="collapsedSummary"
class="gl-mb-0 gl-text-gray-500"
data-testid="collapsedSummaryText"
></p>
<gl-button
v-if="canOverride"
v-collapse-toggle="collapseId"
variant="link"
button-text-classes="flex"
>
<gl-icon :name="toggleIcon" class="mr-1" /> <gl-icon :name="toggleIcon" class="mr-1" />
<span>{{ s__('ApprovalRule|Approval rules') }}</span> <span>{{ s__('ApprovalRule|Approval rules') }}</span>
</gl-button> </gl-button>
......
...@@ -20,9 +20,14 @@ export default { ...@@ -20,9 +20,14 @@ export default {
computed: { computed: {
...mapState(['settings']), ...mapState(['settings']),
}, },
created() {
this.onInputChangeDebounced = debounce(event => {
this.onInputChange(event);
}, 1000);
},
methods: { methods: {
...mapActions(['putRule', 'postRule']), ...mapActions(['putRule', 'postRule']),
onInputChange: debounce(function debounceSearch(event) { onInputChange(event) {
const { value } = event.target; const { value } = event.target;
const approvalsRequired = parseInt(value, 10); const approvalsRequired = parseInt(value, 10);
...@@ -35,7 +40,7 @@ export default { ...@@ -35,7 +40,7 @@ export default {
approvalsRequired, approvalsRequired,
}); });
} }
}, 1000), },
}, },
}; };
</script> </script>
...@@ -48,6 +53,6 @@ export default { ...@@ -48,6 +53,6 @@ export default {
type="number" type="number"
:min="rule.minApprovalsRequired || 0" :min="rule.minApprovalsRequired || 0"
data-qa-selector="approvals_number_field" data-qa-selector="approvals_number_field"
@input="onInputChange" @input="onInputChangeDebounced"
/> />
</template> </template>
...@@ -21,6 +21,7 @@ export default function mountApprovalInput(el) { ...@@ -21,6 +21,7 @@ export default function mountApprovalInput(el) {
prefix: 'mr-edit', prefix: 'mr-edit',
canEdit: parseBoolean(el.dataset.canEdit), canEdit: parseBoolean(el.dataset.canEdit),
allowMultiRule: parseBoolean(el.dataset.allowMultiRule), allowMultiRule: parseBoolean(el.dataset.allowMultiRule),
canOverride: parseBoolean(el.dataset.canOverride),
}); });
store.dispatch('setTargetBranch', targetBranch); store.dispatch('setTargetBranch', targetBranch);
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#js-mr-approvals-input{ data: { 'project_id': @target_project.id, #js-mr-approvals-input{ data: { 'project_id': @target_project.id,
'can_edit': can?(current_user, :update_approvers, issuable).to_s, 'can_edit': can?(current_user, :update_approvers, issuable).to_s,
'allow_multi_rule': @target_project.multiple_approval_rules_available?.to_s, 'allow_multi_rule': @target_project.multiple_approval_rules_available?.to_s,
'can-override': @target_project.can_override_approvers?.to_s,
'mr_id': issuable.iid, 'mr_id': issuable.iid,
'mr_settings_path': presenter.api_approval_settings_path, 'mr_settings_path': presenter.api_approval_settings_path,
'eligible_approvers_docs_path': help_page_path('user/project/merge_requests/merge_request_approvals', anchor: 'eligible-approvers'), 'eligible_approvers_docs_path': help_page_path('user/project/merge_requests/merge_request_approvals', anchor: 'eligible-approvers'),
......
...@@ -16,10 +16,16 @@ describe('EE Approvals MREditApp', () => { ...@@ -16,10 +16,16 @@ describe('EE Approvals MREditApp', () => {
let store; let store;
let axiosMock; let axiosMock;
const factory = () => { const factory = (mergeRequestReviewers = false, mrCollapsedApprovalRules = false) => {
wrapper = mount(MREditApp, { wrapper = mount(MREditApp, {
localVue, localVue,
store: new Vuex.Store(store), store: new Vuex.Store(store),
provide: {
glFeatures: {
mergeRequestReviewers,
mrCollapsedApprovalRules,
},
},
}); });
}; };
...@@ -57,16 +63,19 @@ describe('EE Approvals MREditApp', () => { ...@@ -57,16 +63,19 @@ describe('EE Approvals MREditApp', () => {
}); });
describe('with rules', () => { describe('with rules', () => {
beforeEach(() => { beforeEach(() => {});
store.modules.approvals.state.rules = [{ id: 7, approvers: [] }];
factory();
});
it('renders MR rules', () => { it('renders MR rules', () => {
store.modules.approvals.state.rules = [{ id: 7, approvers: [] }];
factory();
expect(wrapper.find(MRRules).findAll('.js-name')).toHaveLength(1); expect(wrapper.find(MRRules).findAll('.js-name')).toHaveLength(1);
}); });
it('renders hidden inputs', () => { it('renders hidden inputs', () => {
store.modules.approvals.state.rules = [{ id: 7, approvers: [] }];
factory();
expect( expect(
wrapper wrapper
.find('.js-approval-rules') .find('.js-approval-rules')
...@@ -74,5 +83,83 @@ describe('EE Approvals MREditApp', () => { ...@@ -74,5 +83,83 @@ describe('EE Approvals MREditApp', () => {
.exists(), .exists(),
).toBe(true); ).toBe(true);
}); });
describe('summary text', () => {
const findSummaryText = () => wrapper.find('[data-testid="collapsedSummaryText"]');
it('optional approvals', () => {
store.modules.approvals.state.rules = [];
factory(true, true);
expect(findSummaryText().text()).toEqual('Approvals are optional.');
});
it('multiple optional approval rules', () => {
store.modules.approvals.state.rules = [
{ ruleType: 'any_approver', approvalsRequired: 0 },
{ ruleType: 'regular', approvalsRequired: 0, approvers: [] },
];
factory(true, true);
expect(findSummaryText().text()).toEqual('Approvals are optional.');
});
it('anyone can approve', () => {
store.modules.approvals.state.rules = [
{
ruleType: 'any_approver',
approvalsRequired: 1,
},
];
factory(true, true);
expect(findSummaryText().text()).toEqual(
'1 member must approve to merge. Anyone with role Developer or higher can approve.',
);
});
it('2 required approval', () => {
store.modules.approvals.state.rules = [
{
ruleType: 'any_approver',
approvalsRequired: 1,
},
{
ruleType: 'regular',
approvalsRequired: 1,
approvers: [],
},
];
factory(true, true);
expect(findSummaryText().text()).toEqual(
'2 approval rules require eligible members to approve before merging.',
);
});
it('multiple required approval', () => {
store.modules.approvals.state.rules = [
{
ruleType: 'any_approver',
approvalsRequired: 1,
},
{
ruleType: 'regular',
approvalsRequired: 1,
approvers: [],
},
{
ruleType: 'regular',
approvalsRequired: 2,
approvers: [],
},
];
factory(true, true);
expect(findSummaryText().text()).toEqual(
'3 approval rules require eligible members to approve before merging.',
);
});
});
}); });
}); });
...@@ -792,6 +792,21 @@ msgid_plural "%{strong_start}%{commit_count}%{strong_end} Commits" ...@@ -792,6 +792,21 @@ msgid_plural "%{strong_start}%{commit_count}%{strong_end} Commits"
msgstr[0] "" msgstr[0] ""
msgstr[1] "" msgstr[1] ""
msgid "%{strong_start}%{count} approval rule%{strong_end} requires eligible members to approve before merging."
msgid_plural "%{strong_start}%{count} approval rules%{strong_end} require eligible members to approve before merging."
msgstr[0] ""
msgstr[1] ""
msgid "%{strong_start}%{count} eligible member%{strong_end} must approve to merge."
msgid_plural "%{strong_start}%{count} eligible members%{strong_end} must approve to merge."
msgstr[0] ""
msgstr[1] ""
msgid "%{strong_start}%{count} member%{strong_end} must approve to merge. Anyone with role Developer or higher can approve."
msgid_plural "%{strong_start}%{count} members%{strong_end} must approve to merge. Anyone with role Developer or higher can approve."
msgstr[0] ""
msgstr[1] ""
msgid "%{strong_start}%{human_size}%{strong_end} Files" msgid "%{strong_start}%{human_size}%{strong_end} Files"
msgstr "" msgstr ""
...@@ -3602,6 +3617,9 @@ msgstr "" ...@@ -3602,6 +3617,9 @@ msgstr ""
msgid "ApprovalStatusTooltip|Fails to adhere to separation of duties" msgid "ApprovalStatusTooltip|Fails to adhere to separation of duties"
msgstr "" msgstr ""
msgid "Approvals are optional."
msgstr ""
msgid "Approvals|Section: %section" msgid "Approvals|Section: %section"
msgstr "" msgstr ""
......
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