diff --git a/app/assets/javascripts/projects/settings/branch_rules/components/view/constants.js b/app/assets/javascripts/projects/settings/branch_rules/components/view/constants.js index 1fd87b9897c2b18d627b5f5b152dee2c6e78e065..264c26294333b7d969cda77d348bd44ad77f8606 100644 --- a/app/assets/javascripts/projects/settings/branch_rules/components/view/constants.js +++ b/app/assets/javascripts/projects/settings/branch_rules/components/view/constants.js @@ -19,9 +19,14 @@ export const I18N = { ), disallowForcePushDescription: s__('BranchRules|Force push is not allowed.'), approvalsTitle: s__('BranchRules|Approvals'), + manageApprovalsLinkTitle: s__('BranchRules|Manage in Merge Request Approvals'), + approvalsDescription: s__( + 'BranchRules|Approvals to ensure separation of duties for new merge requests. %{linkStart}Lean more.%{linkEnd}', + ), statusChecksTitle: s__('BranchRules|Status checks'), allowedToPushHeader: s__('BranchRules|Allowed to push (%{total})'), allowedToMergeHeader: s__('BranchRules|Allowed to merge (%{total})'), + approvalsHeader: s__('BranchRules|Required approvals (%{total})'), noData: s__('BranchRules|No data to display'), }; @@ -33,3 +38,5 @@ export const WILDCARDS_HELP_PATH = 'user/project/protected_branches#configure-multiple-protected-branches-by-using-a-wildcard'; export const PROTECTED_BRANCHES_HELP_PATH = 'user/project/protected_branches'; + +export const APPROVALS_HELP_PATH = 'user/project/merge_requests/approvals/index.md'; diff --git a/app/assets/javascripts/projects/settings/branch_rules/components/view/index.vue b/app/assets/javascripts/projects/settings/branch_rules/components/view/index.vue index 6534ff883a6a20313847c263e2366adb62e73066..318940478a82d04ef95b4208a5fa337a0fefc7a1 100644 --- a/app/assets/javascripts/projects/settings/branch_rules/components/view/index.vue +++ b/app/assets/javascripts/projects/settings/branch_rules/components/view/index.vue @@ -11,16 +11,19 @@ import { BRANCH_PARAM_NAME, WILDCARDS_HELP_PATH, PROTECTED_BRANCHES_HELP_PATH, + APPROVALS_HELP_PATH, } from './constants'; const wildcardsHelpDocLink = helpPagePath(WILDCARDS_HELP_PATH); const protectedBranchesHelpDocLink = helpPagePath(PROTECTED_BRANCHES_HELP_PATH); +const approvalsHelpDocLink = helpPagePath(APPROVALS_HELP_PATH); export default { name: 'RuleView', i18n: I18N, wildcardsHelpDocLink, protectedBranchesHelpDocLink, + approvalsHelpDocLink, components: { Protection, GlSprintf, GlLink, GlLoadingIcon }, inject: { projectPath: { @@ -29,6 +32,9 @@ export default { protectedBranchesPath: { default: '', }, + approvalRulesPath: { + default: '', + }, }, apollo: { project: { @@ -48,7 +54,9 @@ export default { data() { return { branch: getParameterByName(BRANCH_PARAM_NAME), - branchProtection: {}, + branchProtection: { + approvalRules: {}, + }, }; }, computed: { @@ -75,6 +83,15 @@ export default { total: this.pushAccessLevels.total, }); }, + approvalsHeader() { + const total = this.approvals.reduce( + (sum, { approvalsRequired }) => sum + approvalsRequired, + 0, + ); + return sprintf(this.$options.i18n.approvalsHeader, { + total, + }); + }, allBranches() { return this.branch === ALL_BRANCHES_WILDCARD; }, @@ -86,6 +103,9 @@ export default { ? this.$options.i18n.targetBranch : this.$options.i18n.branchNameOrPattern; }, + approvals() { + return this.branchProtection?.approvalRules?.nodes || []; + }, }, methods: { getAccessLevels(accessLevels = {}) { @@ -164,7 +184,22 @@ export default { /> <!-- Approvals --> - <!-- Follow-up: add approval section (https://gitlab.com/gitlab-org/gitlab/-/issues/372362) --> + <h4 class="gl-mb-1 gl-mt-5">{{ $options.i18n.approvalsTitle }}</h4> + <gl-sprintf :message="$options.i18n.approvalsDescription"> + <template #link="{ content }"> + <gl-link :href="$options.approvalsHelpDocLink"> + {{ content }} + </gl-link> + </template> + </gl-sprintf> + + <protection + class="gl-mt-3" + :header="approvalsHeader" + :header-link-title="$options.i18n.manageApprovalsLinkTitle" + :header-link-href="approvalRulesPath" + :approvals="approvals" + /> <!-- Status checks --> <!-- Follow-up: add status checks section (https://gitlab.com/gitlab-org/gitlab/-/issues/372362) --> diff --git a/app/assets/javascripts/projects/settings/branch_rules/components/view/protection.vue b/app/assets/javascripts/projects/settings/branch_rules/components/view/protection.vue index 8434b7bfce59fa03cafa0e9327c2140f07890904..cfe2df0dbda1facc5d8ef742c165a4428b5e5a7f 100644 --- a/app/assets/javascripts/projects/settings/branch_rules/components/view/protection.vue +++ b/app/assets/javascripts/projects/settings/branch_rules/components/view/protection.vue @@ -41,6 +41,11 @@ export default { required: false, default: () => [], }, + approvals: { + type: Array, + required: false, + default: () => [], + }, }, computed: { showUsersDivider() { @@ -80,5 +85,15 @@ export default { :title="$options.i18n.groupsTitle" :access-levels="groups" /> + + <!-- Approvals --> + <protection-row + v-for="(approval, index) in approvals" + :key="approval.name" + :show-divider="index !== 0" + :title="approval.name" + :users="approval.eligibleApprovers.nodes" + :approvals-required="approval.approvalsRequired" + /> </gl-card> </template> diff --git a/app/assets/javascripts/projects/settings/branch_rules/components/view/protection_row.vue b/app/assets/javascripts/projects/settings/branch_rules/components/view/protection_row.vue index 2509c2538b20b5acaef8f83c334d77b38e244240..28a1c09fa821232fa32bb948206ed5663323e806 100644 --- a/app/assets/javascripts/projects/settings/branch_rules/components/view/protection_row.vue +++ b/app/assets/javascripts/projects/settings/branch_rules/components/view/protection_row.vue @@ -36,6 +36,11 @@ export default { required: false, default: () => [], }, + approvalsRequired: { + type: Number, + required: false, + default: 0, + }, }, computed: { avatarBadgeSrOnlyText() { @@ -48,6 +53,11 @@ export default { commaSeparateList() { return this.accessLevels.length > 1; }, + approvalsRequiredTitle() { + return this.approvalsRequired + ? n__('%d approval required', '%d approvals required', this.approvalsRequired) + : null; + }, }, }; </script> @@ -57,34 +67,44 @@ export default { class="gl-display-flex gl-align-items-center gl-border-gray-100 gl-mb-4 gl-pt-4" :class="{ 'gl-border-t-solid': showDivider }" > - <div class="gl-mr-7">{{ title }}</div> + <div class="gl-display-flex gl-w-half gl-justify-content-space-between"> + <div class="gl-mr-7 gl-w-quarter">{{ title }}</div> + + <gl-avatars-inline + v-if="users.length" + class="gl-w-quarter!" + :avatars="users" + :collapsed="true" + :max-visible="$options.MAX_VISIBLE_AVATARS" + :avatar-size="$options.AVATAR_SIZE" + badge-tooltip-prop="name" + :badge-tooltip-max-chars="$options.AVATAR_TOOLTIP_MAX_CHARS" + :badge-sr-only-text="avatarBadgeSrOnlyText" + > + <template #avatar="{ avatar }"> + <gl-avatar-link + :key="avatar.username" + v-gl-tooltip + target="_blank" + :href="avatar.webUrl" + :title="avatar.name" + > + <gl-avatar :src="avatar.avatarUrl" :label="avatar.name" :size="$options.AVATAR_SIZE" /> + </gl-avatar-link> + </template> + </gl-avatars-inline> - <gl-avatars-inline - v-if="users.length" - :avatars="users" - :collapsed="true" - :max-visible="$options.MAX_VISIBLE_AVATARS" - :avatar-size="$options.AVATAR_SIZE" - badge-tooltip-prop="name" - :badge-tooltip-max-chars="$options.AVATAR_TOOLTIP_MAX_CHARS" - :badge-sr-only-text="avatarBadgeSrOnlyText" - > - <template #avatar="{ avatar }"> - <gl-avatar-link - :key="avatar.username" - v-gl-tooltip - target="_blank" - :href="avatar.webUrl" - :title="avatar.name" - > - <gl-avatar :src="avatar.avatarUrl" :label="avatar.name" :size="$options.AVATAR_SIZE" /> - </gl-avatar-link> - </template> - </gl-avatars-inline> + <div + v-for="(item, index) in accessLevels" + :key="index" + data-testid="access-level" + class="gl-w-quarter" + > + <span v-if="commaSeparateList && index > 0" data-testid="comma-separator">,</span> + {{ item.accessLevelDescription }} + </div> - <div v-for="(item, index) in accessLevels" :key="index" data-testid="access-level"> - <span v-if="commaSeparateList && index > 0" data-testid="comma-separator">,</span> - {{ item.accessLevelDescription }} + <div class="gl-ml-7 gl-w-quarter">{{ approvalsRequiredTitle }}</div> </div> </div> </template> diff --git a/app/assets/javascripts/projects/settings/branch_rules/mount_branch_rules.js b/app/assets/javascripts/projects/settings/branch_rules/mount_branch_rules.js index 39164063d05ee97dd79979656e28716cbfe78b3c..07fd0a7080f3b35172ed8ef02d91336a1c2dff44 100644 --- a/app/assets/javascripts/projects/settings/branch_rules/mount_branch_rules.js +++ b/app/assets/javascripts/projects/settings/branch_rules/mount_branch_rules.js @@ -14,7 +14,7 @@ export default function mountBranchRules(el) { defaultClient: createDefaultClient(), }); - const { projectPath, protectedBranchesPath } = el.dataset; + const { projectPath, protectedBranchesPath, approvalRulesPath } = el.dataset; return new Vue({ el, @@ -22,6 +22,7 @@ export default function mountBranchRules(el) { provide: { projectPath, protectedBranchesPath, + approvalRulesPath, }, render(h) { return h(View); diff --git a/app/views/projects/settings/branch_rules/index.html.haml b/app/views/projects/settings/branch_rules/index.html.haml index ab692a23e4468eb185a5483536f06e31d766c173..a7e80101a88407bc1e8e26c3922b27890eecf7b3 100644 --- a/app/views/projects/settings/branch_rules/index.html.haml +++ b/app/views/projects/settings/branch_rules/index.html.haml @@ -3,4 +3,4 @@ %h3.gl-mb-5= s_('BranchRules|Branch rules details') -#js-branch-rules{ data: { project_path: @project.full_path, protected_branches_path: project_settings_repository_path(@project, anchor: 'js-protected-branches-settings') } } +#js-branch-rules{ data: { project_path: @project.full_path, protected_branches_path: project_settings_repository_path(@project, anchor: 'js-protected-branches-settings'), approval_rules_path: project_settings_merge_requests_path(@project, anchor: 'js-merge-request-approval-settings') } } diff --git a/locale/gitlab.pot b/locale/gitlab.pot index cf1a3becf8efd5ce259ace241c5b4dc5780d9340..34cdc9c4553615f2382f94f9b497d94fdafb3077 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -143,6 +143,11 @@ msgid_plural "%d additional users" msgstr[0] "" msgstr[1] "" +msgid "%d approval required" +msgid_plural "%d approvals required" +msgstr[0] "" +msgstr[1] "" + msgid "%d approver" msgid_plural "%d approvers" msgstr[0] "" @@ -6826,6 +6831,9 @@ msgstr "" msgid "BranchRules|Approvals" msgstr "" +msgid "BranchRules|Approvals to ensure separation of duties for new merge requests. %{linkStart}Lean more.%{linkEnd}" +msgstr "" + msgid "BranchRules|Branch" msgstr "" @@ -6853,6 +6861,9 @@ msgstr "" msgid "BranchRules|Keep stable branches secure and force developers to use merge requests. %{linkStart}What are protected branches?%{linkEnd}" msgstr "" +msgid "BranchRules|Manage in Merge Request Approvals" +msgstr "" + msgid "BranchRules|Manage in Protected Branches" msgstr "" @@ -6874,6 +6885,9 @@ msgstr "" msgid "BranchRules|Require approval from code owners." msgstr "" +msgid "BranchRules|Required approvals (%{total})" +msgstr "" + msgid "BranchRules|Roles" msgstr "" diff --git a/spec/frontend/projects/settings/branch_rules/components/view/index_spec.js b/spec/frontend/projects/settings/branch_rules/components/view/index_spec.js index 3176b28d5474a356eaf89baa11a2f85bdc2c6ab1..bf4026b65db59ed350f6408ed1c81b9c39a28b5f 100644 --- a/spec/frontend/projects/settings/branch_rules/components/view/index_spec.js +++ b/spec/frontend/projects/settings/branch_rules/components/view/index_spec.js @@ -33,6 +33,7 @@ describe('View branch rules', () => { let fakeApollo; const projectPath = 'test/testing'; const protectedBranchesPath = 'protected/branches'; + const approvalRulesPath = 'approval/rules'; const branchProtectionsMockRequestHandler = jest .fn() .mockResolvedValue(branchProtectionsMockResponse); @@ -42,7 +43,7 @@ describe('View branch rules', () => { wrapper = shallowMountExtended(RuleView, { apolloProvider: fakeApollo, - provide: { projectPath, protectedBranchesPath }, + provide: { projectPath, protectedBranchesPath, approvalRulesPath }, }); await waitForPromises(); @@ -57,6 +58,7 @@ describe('View branch rules', () => { const findBranchProtectionTitle = () => wrapper.findByText(I18N.protectBranchTitle); const findBranchProtections = () => wrapper.findAllComponents(Protection); const findForcePushTitle = () => wrapper.findByText(I18N.allowForcePushDescription); + const findApprovalsTitle = () => wrapper.findByText(I18N.approvalsTitle); it('gets the branch param from url and renders it in the view', () => { expect(util.getParameterByName).toHaveBeenCalledWith('branch'); @@ -98,4 +100,14 @@ describe('View branch rules', () => { ...protectionMockProps, }); }); + + it('renders a branch protection component for approvals', () => { + expect(findApprovalsTitle().exists()).toBe(true); + + expect(findBranchProtections().at(2).props()).toMatchObject({ + header: sprintf(I18N.approvalsHeader, { total: 0 }), + headerLinkHref: approvalRulesPath, + headerLinkTitle: I18N.manageApprovalsLinkTitle, + }); + }); }); diff --git a/spec/frontend/projects/settings/branch_rules/components/view/mock_data.js b/spec/frontend/projects/settings/branch_rules/components/view/mock_data.js index c5774977205f6276d7944ee4e04ed6674bff79cf..c3f573061da7c2e71c5ff52ed9c6be899ba206bf 100644 --- a/spec/frontend/projects/settings/branch_rules/components/view/mock_data.js +++ b/spec/frontend/projects/settings/branch_rules/components/view/mock_data.js @@ -36,6 +36,8 @@ const accessLevelsMock = [ { accessLevelDescription: 'Maintainer' }, ]; +const approvalsRequired = 3; + const groupsMock = [{ name: 'test_group_1' }, { name: 'test_group_2' }]; export const protectionPropsMock = { @@ -45,12 +47,20 @@ export const protectionPropsMock = { roles: accessLevelsMock, users: usersMock, groups: groupsMock, + approvals: [ + { + name: 'test', + eligibleApprovers: { nodes: usersMock }, + approvalsRequired, + }, + ], }; export const protectionRowPropsMock = { title: 'Test title', users: usersMock, accessLevels: accessLevelsMock, + approvalsRequired, }; export const accessLevelsMockResponse = [ diff --git a/spec/frontend/projects/settings/branch_rules/components/view/protection_row_spec.js b/spec/frontend/projects/settings/branch_rules/components/view/protection_row_spec.js index 7770e1fb2aa7a597ad71482d51a3177644b57d83..b0a69bedd3e0a691f87891d7ed8a01ba5a37ca85 100644 --- a/spec/frontend/projects/settings/branch_rules/components/view/protection_row_spec.js +++ b/spec/frontend/projects/settings/branch_rules/components/view/protection_row_spec.js @@ -25,6 +25,8 @@ describe('Branch rule protection row', () => { const findAvatarLinks = () => wrapper.findAllComponents(GlAvatarLink); const findAvatars = () => wrapper.findAllComponents(GlAvatar); const findAccessLevels = () => wrapper.findAllByTestId('access-level'); + const findApprovalsRequired = () => + wrapper.findByText(`${protectionRowPropsMock.approvalsRequired} approvals required`); it('renders a title', () => { expect(findTitle().exists()).toBe(true); @@ -62,4 +64,8 @@ describe('Branch rule protection row', () => { protectionRowPropsMock.accessLevels[1].accessLevelDescription, ); }); + + it('renders the number of approvals required', () => { + expect(findApprovalsRequired().exists()).toBe(true); + }); }); diff --git a/spec/frontend/projects/settings/branch_rules/components/view/protection_spec.js b/spec/frontend/projects/settings/branch_rules/components/view/protection_spec.js index 91d16fd86a6e1ee2d34d3cefc675b6b0dcd7379e..e2fbb4f5bbb05d00364936a5f8b2813e59fafc45 100644 --- a/spec/frontend/projects/settings/branch_rules/components/view/protection_spec.js +++ b/spec/frontend/projects/settings/branch_rules/components/view/protection_spec.js @@ -56,4 +56,13 @@ describe('Branch rule protection', () => { title: i18n.groupsTitle, }); }); + + it('renders a protection row for approvals', () => { + const approval = protectionPropsMock.approvals[0]; + expect(findProtectionRows().at(3).props()).toMatchObject({ + title: approval.name, + users: approval.eligibleApprovers.nodes, + approvalsRequired: approval.approvalsRequired, + }); + }); });