Merge branch 'remove-approvals-commented-by-ff' into 'master'

Remove the approvals_commented_by feature flag

See merge request gitlab-org/gitlab!65319

app/controllers/projects/merge_requests_controller.rb

@@ -30,7 +30,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
   before_action :check_user_can_push_to_source_branch!, only: [:rebase]
   before_action only: [:show] do
     push_frontend_feature_flag(:file_identifier_hash)
-    push_frontend_feature_flag(:approvals_commented_by, @project, default_enabled: true)
     push_frontend_feature_flag(:merge_request_widget_graphql, @project, default_enabled: :yaml)
     push_frontend_feature_flag(:default_merge_ref_for_diffs, @project, default_enabled: :yaml)
     push_frontend_feature_flag(:core_security_mr_widget_counts, @project)

config/feature_flags/development/approvals_commented_by.yml

----
-name: approvals_commented_by
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38136
-rollout_issue_url: 
-milestone: '13.3'
-type: development
-group: group::source code
-default_enabled: true

ee/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals_list.vue

@@ -94,15 +94,12 @@ export default {
 <template>
   <table class="table m-0">
     <thead class="thead-white text-nowrap">
-      <tr
-        :class="glFeatures.approvalsCommentedBy ? 'd-md-table-row' : 'd-sm-table-row'"
-        class="d-none"
-      >
+      <tr class="d-md-table-row d-none">
         <th class="w-0"></th>
         <th class="w-25 gl-pl-0!">{{ s__('MRApprovals|Approvers') }}</th>
         <th class="w-50"></th>
         <th>{{ s__('MRApprovals|Approvals') }}</th>
-        <th v-if="glFeatures.approvalsCommentedBy">{{ s__('MRApprovals|Commented by') }}</th>
+        <th>{{ s__('MRApprovals|Commented by') }}</th>
         <th>{{ s__('MRApprovals|Approved by') }}</th>
       </tr>
     </thead>
@@ -116,10 +113,7 @@ export default {
       <tr v-for="rule in rules" :key="rule.id">
         <td class="w-0 gl-pr-4!"><approved-icon class="gl-pl-2" :is-approved="rule.approved" /></td>
         <td :colspan="rule.rule_type === $options.ruleTypeAnyApprover ? 2 : 1" class="gl-pl-0!">
-          <div
-            :class="glFeatures.approvalsCommentedBy ? 'd-md-flex' : 'd-sm-flex'"
-            class="d-none js-name align-items-center"
-          >
+          <div class="d-md-flex d-none js-name align-items-center">
             <empty-rule-name
               v-if="rule.rule_type === $options.ruleTypeAnyApprover"
               :eligible-approvers-docs-path="eligibleApproversDocsPath"
@@ -140,10 +134,7 @@ export default {
               :security-approvals-help-page-path="securityApprovalsHelpPagePath"
             />
           </div>
-          <div
-            :class="glFeatures.approvalsCommentedBy ? 'd-md-none' : 'd-sm-none'"
-            class="d-flex flex-column js-summary"
-          >
+          <div class="d-md-none d-flex flex-column js-summary">
             <empty-rule-name
               v-if="rule.rule_type === $options.ruleTypeAnyApprover"
               :eligible-approvers-docs-path="eligibleApproversDocsPath"
@@ -156,10 +147,7 @@ export default {
               :img-size="24"
               empty-text=""
             />
-            <div
-              v-if="glFeatures.approvalsCommentedBy && rule.commented_by.length > 0"
-              class="mt-2"
-            >
+            <div v-if="rule.commented_by.length > 0" class="mt-2">
               <span>{{ s__('MRApprovals|Commented by') }}</span>
               <user-avatar-list
                 class="d-inline-block align-middle"
@@ -179,22 +167,14 @@ export default {
         </td>
         <td
           v-if="rule.rule_type !== $options.ruleTypeAnyApprover"
-          :class="glFeatures.approvalsCommentedBy ? 'd-md-table-cell' : 'd-sm-table-cell'"
-          class="d-none js-approvers"
+          class="d-md-table-cell d-none js-approvers"
         >
           <div><user-avatar-list :items="rule.approvers" :img-size="24" empty-text="" /></div>
         </td>
-        <td
-          :class="glFeatures.approvalsCommentedBy ? 'd-md-table-cell' : 'd-sm-table-cell'"
-          class="w-0 d-none text-nowrap js-pending"
-        >
+        <td class="d-md-table-cell w-0 d-none text-nowrap js-pending">
           {{ pendingApprovalsText(rule) }}
         </td>
-        <td
-          v-if="glFeatures.approvalsCommentedBy"
-          :class="glFeatures.approvalsCommentedBy ? 'd-md-table-cell' : 'd-sm-table-cell'"
-          class="d-none js-commented-by"
-        >
+        <td class="d-md-table-cell d-none js-commented-by">
           <user-avatar-list :items="rule.commented_by" :img-size="24" empty-text="" />
         </td>
         <td class="d-none d-md-table-cell js-approved-by">

ee/spec/frontend/vue_mr_widget/components/approvals/approvals_list_spec.js

@@ -59,16 +59,9 @@ const testRules = () => [testRuleApproved(), testRuleUnapproved(), testRuleOptio
 describe('EE MRWidget approvals list', () => {
   let wrapper;
 
-  const createComponent = (props = {}, options = {}) => {
-    const { approvalsCommentedBy = true } = options;
-
+  const createComponent = (props = {}) => {
     wrapper = shallowMount(ApprovalsList, {
       propsData: props,
-      provide: {
-        glFeatures: {
-          approvalsCommentedBy,
-        },
-      },
     });
   };
 
@@ -361,35 +354,4 @@ describe('EE MRWidget approvals list', () => {
       expect(ruleSection.at(1).text()).toEqual(rule.section);
     });
   });
-
-  describe('approvalsCommentedBy feature flag is off', () => {
-    let row;
-
-    beforeEach(() => {
-      const rule = testRuleApproved();
-      createComponent(
-        {
-          approvalRules: [rule],
-        },
-        { approvalsCommentedBy: false },
-      );
-      row = findRows().at(0);
-    });
-
-    it('does not render commented by in table head', () => {
-      expect(wrapper.find('thead').text()).not.toContain('Commented by');
-    });
-
-    it('does not render commented by in summary text', () => {
-      const summary = findRowElement(row, 'summary');
-
-      expect(summary.findAll(UserAvatarList).length).toBe(2);
-    });
-
-    it('does not render commented by in user avatar list', () => {
-      const commentedBy = findRowElement(row, 'commented-by');
-
-      expect(commentedBy.exists()).toBe(false);
-    });
-  });
 });