Merge branch '37952-fix-missing-empty-rule-ux-in-mr-widget' into 'master'

Resolve "Add missing popover and remove "none" in MR widget"

Closes #37952

See merge request gitlab-org/gitlab!21095

app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js

@@ -172,6 +172,7 @@ export default class MergeRequestStore {
     this.conflictsDocsPath = data.conflicts_docs_path;
     this.ciEnvironmentsStatusPath = data.ci_environments_status_path;
     this.securityApprovalsHelpPagePath = data.security_approvals_help_page_path;
+    this.eligibleApproversDocsPath = data.eligible_approvers_docs_path;
   }
 
   get isNothingToMergeState() {

app/views/projects/merge_requests/_widget.html.haml

@@ -9,5 +9,6 @@
     window.gl.mrWidgetData.squash_before_merge_help_path = '#{help_page_path("user/project/merge_requests/squash_and_merge")}';
     window.gl.mrWidgetData.troubleshooting_docs_path = '#{help_page_path('user/project/merge_requests/index.md', anchor: 'troubleshooting')}';
     window.gl.mrWidgetData.security_approvals_help_page_path = '#{help_page_path('user/application_security/index.html', anchor: 'security-approvals-in-merge-requests-ultimate')}';
+    window.gl.mrWidgetData.eligible_approvers_docs_path = '#{help_page_path('user/project/merge_requests/merge_request_approvals', anchor: 'eligible-approvers')}';
 
 #js-vue-mr-widget.mr-widget

changelogs/unreleased/37952-fix-missing-empty-rule-ux-in-mr-widget.yml

+---
+title: Resolve Add missing popover and remove none in MR widget
+merge_request: 21095
+author:
+type: other

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

@@ -243,6 +243,7 @@ export default {
       :approval-rules="mr.approvalRules"
       :is-loading-rules="isLoadingRules"
       :security-approvals-help-page-path="mr.securityApprovalsHelpPagePath"
+      :eligible-approvers-docs-path="mr.eligibleApproversDocsPath"
     />
   </mr-widget-container>
 </template>

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

@@ -37,6 +37,11 @@ export default {
       required: false,
       default: '',
     },
+    eligibleApproversDocsPath: {
+      type: String,
+      required: false,
+      default: '',
+    },
   },
   computed: {
     isCollapsed() {
@@ -84,6 +89,7 @@ export default {
       <approvals-list
         :approval-rules="approvalRules"
         :security-approvals-help-page-path="securityApprovalsHelpPagePath"
+        :eligible-approvers-docs-path="eligibleApproversDocsPath"
       />
     </div>
   </div>

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

@@ -3,6 +3,7 @@ import _ from 'underscore';
 import { sprintf, __ } from '~/locale';
 import UserAvatarList from '~/vue_shared/components/user_avatar/user_avatar_list.vue';
 import ApprovalCheckRulePopover from 'ee/approvals/components/approval_check_rule_popover.vue';
+import EmptyRuleName from 'ee/approvals/components/empty_rule_name.vue';
 import { RULE_TYPE_CODE_OWNER, RULE_TYPE_ANY_APPROVER } from 'ee/approvals/constants';
 import ApprovedIcon from './approved_icon.vue';
 
@@ -11,6 +12,7 @@ export default {
     UserAvatarList,
     ApprovedIcon,
     ApprovalCheckRulePopover,
+    EmptyRuleName,
   },
   props: {
     approvalRules: {
@@ -22,6 +24,11 @@ export default {
       required: false,
       default: '',
     },
+    eligibleApproversDocsPath: {
+      type: String,
+      required: false,
+      default: '',
+    },
   },
   computed: {
     sections() {
@@ -70,10 +77,8 @@ export default {
         name: rule.name,
       });
     },
-    ruleName(rule) {
-      return rule.rule_type === RULE_TYPE_ANY_APPROVER ? __('Any eligible user') : rule.name;
-    },
   },
+  ruleTypeAnyApprover: RULE_TYPE_ANY_APPROVER,
 };
 </script>
 
@@ -97,21 +102,30 @@ export default {
       </tr>
       <tr v-for="rule in rules" :key="rule.id">
         <td class="w-0"><approved-icon :is-approved="rule.approved" /></td>
-        <td :colspan="rule.fallback ? 2 : 1">
+        <td :colspan="rule.rule_type === $options.ruleTypeAnyApprover ? 2 : 1">
           <div class="d-none d-sm-block js-name" :class="rule.nameClass">
-            {{ ruleName(rule) }}
+            <empty-rule-name
+              v-if="rule.rule_type === $options.ruleTypeAnyApprover"
+              :eligible-approvers-docs-path="eligibleApproversDocsPath"
+            />
+            <span v-else>{{ rule.name }}</span>
             <approval-check-rule-popover
               :rule="rule"
               :security-approvals-help-page-path="securityApprovalsHelpPagePath"
             />
           </div>
           <div class="d-flex d-sm-none flex-column js-summary">
-            <span>{{ summaryText(rule) }}</span>
+            <empty-rule-name
+              v-if="rule.rule_type === $options.ruleTypeAnyApprover"
+              :eligible-approvers-docs-path="eligibleApproversDocsPath"
+            />
+            <span v-else>{{ summaryText(rule) }}</span>
             <user-avatar-list
               v-if="!rule.fallback"
               class="mt-2"
               :items="rule.approvers"
               :img-size="24"
+              empty-text=""
             />
             <div v-if="rule.approved_by.length" class="mt-2">
               <span>{{ s__('MRApprovals|Approved by') }}</span>
@@ -124,7 +138,7 @@ export default {
           </div>
         </td>
         <td v-if="!rule.fallback" class="d-none d-sm-table-cell js-approvers">
-          <div><user-avatar-list :items="rule.approvers" :img-size="24" /></div>
+          <div><user-avatar-list :items="rule.approvers" :img-size="24" empty-text="" /></div>
         </td>
         <td class="w-0 d-none d-sm-table-cell text-nowrap js-pending">
           {{ pendingApprovalsText(rule) }}

ee/spec/features/merge_request/user_sets_approval_rules_spec.rb

@@ -63,7 +63,7 @@ describe 'Merge request > User sets approval rules', :js do
         td = tr.find(:css, '.js-approvers')
 
         # The approver granted by the private group is not visible
-        expect(td).to have_text('None')
+        expect(td).to have_text('')
       end
     end
   end