Fetch latest link in the description for zoom link, add more tests and remove...

Fetch latest link in the description for zoom link, add more tests and remove frontend spec unnecessary tests

app/assets/javascripts/issue_show/components/app.vue

@@ -55,6 +55,11 @@ export default {
       required: false,
       default: true,
     },
+    zoomMeetingUrl: {
+      type: String,
+      required: false,
+      default: null,
+    },
     issuableRef: {
       type: String,
       required: true,
@@ -342,7 +347,7 @@ export default {
         :title-text="state.titleText"
         :show-inline-edit-button="showInlineEditButton"
       />
-      <pinned-links :description-html="state.descriptionHtml" />
+      <pinned-links :zoom-meeting-url="zoomMeetingUrl" />
       <description-component
         v-if="state.descriptionHtml"
         :can-update="canUpdate"

app/assets/javascripts/issue_show/components/pinned_links.vue

@@ -8,40 +8,19 @@ export default {
     GlLink,
   },
   props: {
-    descriptionHtml: {
+    zoomMeetingUrl: {
       type: String,
-      required: true,
-    },
-  },
-  computed: {
-    linksInDescription() {
-      const el = document.createElement('div');
-      el.innerHTML = this.descriptionHtml;
-      return [...el.querySelectorAll('a')].map(a => a.href);
-    },
-    // Detect links matching the following formats:
-    // Zoom Start links: https://zoom.us/s/<meeting-id>
-    // Zoom Join links: https://zoom.us/j/<meeting-id>
-    // Personal Zoom links: https://zoom.us/my/<meeting-id>
-    // Vanity Zoom links: https://gitlab.zoom.us/j/<meeting-id> (also /s and /my)
-    zoomHref() {
-      const zoomRegex = /^https:\/\/([\w\d-]+\.)?zoom\.us\/(s|j|my)\/.+/;
-      return this.linksInDescription.reduce((acc, currentLink) => {
-        let lastLink = acc;
-        if (zoomRegex.test(currentLink)) {
-          lastLink = currentLink;
-        }
-        return lastLink;
-      }, '');
+      required: false,
+      default: null,
     },
   },
 };
 </script>
 
 <template>
-  <div v-if="zoomHref" class="border-bottom mb-3 mt-n2">
+  <div v-if="zoomMeetingUrl" class="border-bottom mb-3 mt-n2">
     <gl-link
-      :href="zoomHref"
+      :href="zoomMeetingUrl"
       target="_blank"
       class="btn btn-inverted btn-secondary btn-sm text-dark mb-3"
     >

app/helpers/issuables_helper.rb

@@ -282,6 +282,10 @@ module IssuablesHelper
 
     data[:hasClosingMergeRequest] = issuable.merge_requests_count(current_user) != 0 if issuable.is_a?(Issue)
 
+    zoom_links = Gitlab::ZoomLinkExtractor.new(issuable.description).links
+
+    data[:zoomMeetingUrl] = zoom_links.last if zoom_links.any?
+
     if parent.is_a?(Group)
       data[:groupPath] = parent.path
     else

changelogs/unreleased/issue-zoom-url.yml

+---
+title: Extract zoom link from issue and pass to frontend
+merge_request: 29910
+author: raju249
+type: added

lib/gitlab/zoom_link_extractor.rb

+# frozen_string_literal: true
+
+# Detect links matching the following formats:
+# Zoom Start links: https://zoom.us/s/<meeting-id>
+# Zoom Join links: https://zoom.us/j/<meeting-id>
+# Personal Zoom links: https://zoom.us/my/<meeting-id>
+# Vanity Zoom links: https://gitlab.zoom.us/j/<meeting-id> (also /s and /my)
+
+module Gitlab
+  class ZoomLinkExtractor
+    ZOOM_REGEXP = %r{https://(?:[\w-]+\.)?zoom\.us/(?:s|j|my)/\S+}.freeze
+
+    def initialize(text)
+      @text = text.to_s
+    end
+
+    def links
+      @text.scan(ZOOM_REGEXP)
+    end
+  end
+end

spec/frontend/issue_show/components/pinned_links_spec.js

@@ -5,10 +5,6 @@ import PinnedLinks from '~/issue_show/components/pinned_links.vue';
 const localVue = createLocalVue();
 
 const plainZoomUrl = 'https://zoom.us/j/123456789';
-const vanityZoomUrl = 'https://gitlab.zoom.us/j/123456789';
-const startZoomUrl = 'https://zoom.us/s/123456789';
-const personalZoomUrl = 'https://zoom.us/my/hunter-zoloman';
-const randomUrl = 'https://zoom.us.com';
 
 describe('PinnedLinks', () => {
   let wrapper;
@@ -27,7 +23,7 @@ describe('PinnedLinks', () => {
       localVue,
       sync: false,
       propsData: {
-        descriptionHtml: '',
+        zoomMeetingUrl: null,
         ...props,
       },
     });
@@ -35,55 +31,15 @@ describe('PinnedLinks', () => {
 
   it('displays Zoom link', () => {
     createComponent({
-      descriptionHtml: `<a href="${plainZoomUrl}">Zoom</a>`,
+      zoomMeetingUrl: `<a href="${plainZoomUrl}">Zoom</a>`,
     });
 
     expect(link.text).toBe('Join Zoom meeting');
   });
 
-  it('detects plain Zoom link', () => {
+  it('does not render if there are no links', () => {
     createComponent({
-      descriptionHtml: `<a href="${plainZoomUrl}">Zoom</a>`,
-    });
-
-    expect(link.href).toBe(plainZoomUrl);
-  });
-
-  it('detects vanity Zoom link', () => {
-    createComponent({
-      descriptionHtml: `<a href="${vanityZoomUrl}">Zoom</a>`,
-    });
-
-    expect(link.href).toBe(vanityZoomUrl);
-  });
-
-  it('detects Zoom start meeting link', () => {
-    createComponent({
-      descriptionHtml: `<a href="${startZoomUrl}">Zoom</a>`,
-    });
-
-    expect(link.href).toBe(startZoomUrl);
-  });
-
-  it('detects personal Zoom room link', () => {
-    createComponent({
-      descriptionHtml: `<a href="${personalZoomUrl}">Zoom</a>`,
-    });
-
-    expect(link.href).toBe(personalZoomUrl);
-  });
-
-  it('only renders final Zoom link in description', () => {
-    createComponent({
-      descriptionHtml: `<a href="${plainZoomUrl}">Zoom</a><a href="${vanityZoomUrl}">Zoom</a>`,
-    });
-
-    expect(link.href).toBe(vanityZoomUrl);
-  });
-
-  it('does not render for other links', () => {
-    createComponent({
-      descriptionHtml: `<a href="${randomUrl}">Some other link</a>`,
+      zoomMeetingUrl: null,
     });
 
     expect(wrapper.find(GlLink).exists()).toBe(false);

spec/helpers/issuables_helper_spec.rb

@@ -202,5 +202,46 @@ describe IssuablesHelper do
       }
       expect(helper.issuable_initial_data(issue)).to match(hash_including(expected_data))
     end
+
+    describe '#zoomMeetingUrl in issue' do
+      let(:issue) { create(:issue, author: user, description: description) }
+
+      before do
+        assign(:project, issue.project)
+      end
+
+      context 'no zoom links in the issue description' do
+        let(:description) { 'issue text' }
+
+        it 'does not set zoomMeetingUrl' do
+          expect(helper.issuable_initial_data(issue))
+              .not_to include(:zoomMeetingUrl)
+        end
+      end
+
+      context 'no zoom links in the issue description if it has link but not a zoom link' do
+        let(:description) { 'issue text https://stackoverflow.com/questions/22' }
+
+        it 'does not set zoomMeetingUrl' do
+          expect(helper.issuable_initial_data(issue))
+              .not_to include(:zoomMeetingUrl)
+        end
+      end
+
+      context 'with two zoom links in description' do
+        let(:description) do
+          <<~TEXT
+            issue text and
+            zoom call on https://zoom.us/j/123456789 this url
+            and new zoom url https://zoom.us/s/lastone and some more text
+          TEXT
+        end
+
+        it 'sets zoomMeetingUrl value to the last url' do
+          expect(helper.issuable_initial_data(issue))
+            .to include(zoomMeetingUrl: 'https://zoom.us/s/lastone')
+        end
+      end
+    end
   end
 end

spec/lib/gitlab/zoom_link_extractor_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::ZoomLinkExtractor do
+  describe "#links" do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:text, :links) do
+      'issue text https://zoom.us/j/123 and https://zoom.us/s/1123433' | %w[https://zoom.us/j/123 https://zoom.us/s/1123433]
+      'https://zoom.us/j/1123433 issue text' | %w[https://zoom.us/j/1123433]
+      'issue https://zoom.us/my/1123433 text' | %w[https://zoom.us/my/1123433]
+      'issue https://gitlab.com and https://gitlab.zoom.us/s/1123433' | %w[https://gitlab.zoom.us/s/1123433]
+      'https://gitlab.zoom.us/j/1123433' | %w[https://gitlab.zoom.us/j/1123433]
+      'https://gitlab.zoom.us/my/1123433' | %w[https://gitlab.zoom.us/my/1123433]
+    end
+
+    with_them do
+      subject { described_class.new(text).links }
+
+      it { is_expected.to eq(links) }
+    end
+  end
+end