Merge branch '372362-add-approval-rules-section' into 'master'

Add approvals section

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/100270



Merged-by: Kushal Pandya <kushalspandya@gmail.com>
Approved-by: Tomas Bulva <tbulva@gitlab.com>
Approved-by: Kushal Pandya <kushalspandya@gmail.com>
Co-authored-by: Jacques <jerasmus@gitlab.com>

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';

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) -->

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>

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>

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);

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') } }

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 ""

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,
+    });
+  });
 });

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 = [

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);
+  });
 });

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,
+    });
+  });
 });