Merge branch 'djadmin-refactor-assign-related-issues' into 'master'

Refactor MR Widget related links component

See merge request gitlab-org/gitlab!82288

app/assets/javascripts/vue_merge_request_widget/components/mr_widget_related_links.vue

 <script>
-import { GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui';
+import { GlSafeHtmlDirective as SafeHtml, GlLink } from '@gitlab/ui';
 import { s__, n__ } from '~/locale';
 import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 
@@ -8,6 +8,9 @@ export default {
   directives: {
     SafeHtml,
   },
+  components: {
+    GlLink,
+  },
   mixins: [glFeatureFlagMixin()],
   props: {
     relatedLinks: {
@@ -37,6 +40,17 @@ export default {
 
       return n__('mrWidget|Closes issue', 'mrWidget|Closes issues', this.relatedLinks.closingCount);
     },
+    assignIssueText() {
+      if (this.relatedLinks.unassignedCount > 1) {
+        return s__('mrWidget|Assign yourself to these issues');
+      }
+      return s__('mrWidget|Assign yourself to this issue');
+    },
+    shouldShowAssignToMeLink() {
+      return (
+        this.relatedLinks.unassignedCount && this.relatedLinks.assignToMe && this.showAssignToMe
+      );
+    },
   },
 };
 </script>
@@ -57,10 +71,14 @@ export default {
       <span v-safe-html="relatedLinks.mentioned"></span>
     </p>
     <p
-      v-if="relatedLinks.assignToMe && showAssignToMe"
+      v-if="shouldShowAssignToMeLink"
       :class="{ 'gl-display-line gl-m-0': glFeatures.restructuredMrWidget }"
     >
-      <span v-html="relatedLinks.assignToMe /* eslint-disable-line vue/no-v-html */"></span>
+      <span>
+        <gl-link rel="nofollow" data-method="post" :href="relatedLinks.assignToMe">{{
+          assignIssueText
+        }}</gl-link>
+      </span>
     </p>
   </section>
 </template>

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

@@ -82,14 +82,16 @@ export default class MergeRequestStore {
       const { closing } = links;
       const mentioned = links.mentioned_but_not_closing;
       const assignToMe = links.assign_to_closing;
+      const unassignedCount = links.assign_to_closing_count;
 
-      if (closing || mentioned || assignToMe) {
+      if (closing || mentioned || unassignedCount) {
         this.relatedLinks = {
           closing,
           mentioned,
           assignToMe,
           closingCount: links.closing_count,
           mentionedCount: links.mentioned_count,
+          unassignedCount: links.assign_to_closing_count,
         };
       }
     }

app/presenters/merge_request_presenter.rb

@@ -149,7 +149,11 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
     )
   end
 
-  def assign_to_closing_issues_link
+  def assign_to_closing_issues_path
+    assign_related_issues_project_merge_request_path(project, merge_request)
+  end
+
+  def assign_to_closing_issues_count
     # rubocop: disable CodeReuse/ServiceClass
     issues = MergeRequests::AssignIssuesService.new(project: project,
                                                     current_user: current_user,
@@ -157,14 +161,7 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
                                                       merge_request: merge_request,
                                                       closes_issues: closing_issues
                                                     }).assignable_issues
-    path = assign_related_issues_project_merge_request_path(project, merge_request)
-    if issues.present?
-      if issues.count > 1
-        link_to _('Assign yourself to these issues'), path, method: :post
-      else
-        link_to _('Assign yourself to this issue'), path, method: :post
-      end
-    end
+    issues.count
     # rubocop: enable CodeReuse/ServiceClass
   end
 

app/serializers/merge_request_widget_entity.rb

@@ -104,7 +104,11 @@ class MergeRequestWidgetEntity < Grape::Entity
   # include them if they are explicitly requested on first load.
   expose :issues_links, if: -> (_, opts) { opts[:issues_links] } do
     expose :assign_to_closing do |merge_request|
-      presenter(merge_request).assign_to_closing_issues_link
+      presenter(merge_request).assign_to_closing_issues_path
+    end
+
+    expose :assign_to_closing_count do |merge_request|
+      presenter(merge_request).assign_to_closing_issues_count
     end
 
     expose :closing do |merge_request|

locale/gitlab.pot

@@ -4942,12 +4942,6 @@ msgstr ""
 msgid "Assign to me"
 msgstr ""
 
-msgid "Assign yourself to these issues"
-msgstr ""
-
-msgid "Assign yourself to this issue"
-msgstr ""
-
 msgid "Assigned %{assignee_users_sentence}."
 msgstr ""
 
@@ -44014,6 +44008,12 @@ msgstr ""
 msgid "mrWidget|Approved by you and others"
 msgstr ""
 
+msgid "mrWidget|Assign yourself to these issues"
+msgstr ""
+
+msgid "mrWidget|Assign yourself to this issue"
+msgstr ""
+
 msgid "mrWidget|Cancel auto-merge"
 msgstr ""
 

spec/frontend/vue_mr_widget/components/mr_widget_related_links_spec.js

+import { GlLink } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
 import RelatedLinks from '~/vue_merge_request_widget/components/mr_widget_related_links.vue';
 
@@ -85,13 +86,29 @@ describe('MRWidgetRelatedLinks', () => {
     expect(content).toContain('Mentions issues #23 and #42');
   });
 
-  it('should have assing issues link', () => {
+  describe('should have correct assign issues link', () => {
+    it.each([
+      [1, 'Assign yourself to this issue'],
+      [2, 'Assign yourself to these issues'],
+    ])('when issue count is %s, link displays correct text', (unassignedCount, text) => {
+      const assignToMe = '/assign';
+
       createComponent({
-      relatedLinks: {
-        assignToMe: '<a href="#">Assign yourself to these issues</a>',
-      },
+        relatedLinks: { assignToMe, unassignedCount },
+      });
+
+      const glLinkWrapper = wrapper.findComponent(GlLink);
+
+      expect(glLinkWrapper.attributes('href')).toBe(assignToMe);
+      expect(glLinkWrapper.text()).toBe(text);
     });
 
-    expect(wrapper.text().trim()).toContain('Assign yourself to these issues');
+    it('when no link is present', () => {
+      createComponent({
+        relatedLinks: { assignToMe: '#', unassignedCount: 0 },
+      });
+
+      expect(wrapper.findComponent(GlLink).exists()).toBe(false);
+    });
   });
 });

spec/presenters/merge_request_presenter_spec.rb

@@ -162,10 +162,19 @@ RSpec.describe MergeRequestPresenter do
       end
     end
 
-    describe '#assign_to_closing_issues_link' do
+    describe '#assign_to_closing_issues_path' do
       subject do
         described_class.new(resource, current_user: user)
-          .assign_to_closing_issues_link
+          .assign_to_closing_issues_path
+      end
+
+      it { is_expected.to match("#{project.full_path}/-/merge_requests/#{resource.iid}/assign_related_issues") }
+    end
+
+    describe '#assign_to_closing_issues_count' do
+      subject do
+        described_class.new(resource, current_user: user)
+          .assign_to_closing_issues_count
       end
 
       before do
@@ -178,33 +187,28 @@ RSpec.describe MergeRequestPresenter do
         let(:issue) { create(:issue) }
         let(:assignable_issues) { [issue] }
 
-        it 'returns correct link with correct text' do
+        it 'returns correct count' do
           is_expected
-            .to match("#{project.full_path}/-/merge_requests/#{resource.iid}/assign_related_issues")
-
-          is_expected
-            .to match("Assign yourself to this issue")
+            .to match(1)
         end
       end
 
       context 'multiple closing issues' do
-        let(:issues) { create_list(:issue, 2) }
+        let(:issues) { build_list(:issue, 2) }
         let(:assignable_issues) { issues }
 
-        it 'returns correct link with correct text' do
-          is_expected
-            .to match("#{project.full_path}/-/merge_requests/#{resource.iid}/assign_related_issues")
-
+        it 'returns correct count' do
           is_expected
-            .to match("Assign yourself to these issues")
+            .to match(2)
         end
       end
 
       context 'no closing issue' do
         let(:assignable_issues) { [] }
 
-        it 'returns correct link with correct text' do
-          is_expected.to be_nil
+        it 'returns correct count' do
+          is_expected
+            .to match(0)
         end
       end
     end

spec/serializers/merge_request_widget_entity_spec.rb

@@ -59,7 +59,7 @@ RSpec.describe MergeRequestWidgetEntity do
       data = described_class.new(resource, request: request, issues_links: true).as_json
 
       expect(data).to include(:issues_links)
-      expect(data[:issues_links]).to include(:assign_to_closing, :closing, :mentioned_but_not_closing, :closing_count, :mentioned_count)
+      expect(data[:issues_links]).to include(:assign_to_closing, :assign_to_closing_count, :closing, :mentioned_but_not_closing, :closing_count, :mentioned_count)
     end
 
     it 'omits issue links by default' do