Add tests for the new zoom link service interface

app/helpers/issuables_helper.rb

@@ -281,7 +281,7 @@ module IssuablesHelper
     }
 
     data[:hasClosingMergeRequest] = issuable.merge_requests_count(current_user) != 0 if issuable.is_a?(Issue)
-    data[:zoomMeetingUrl] = ZoomMeeting.canonical_meeting_url(issuable)
+    data[:zoomMeetingUrl] = ZoomMeeting.canonical_meeting_url(issuable) if issuable.is_a?(Issue)
 
     if parent.is_a?(Group)
       data[:groupPath] = parent.path

app/models/concerns/issuable.rb

@@ -66,7 +66,6 @@ module Issuable
     has_many :label_links, as: :target, dependent: :destroy, inverse_of: :target # rubocop:disable Cop/ActiveRecordDependent
     has_many :labels, through: :label_links
     has_many :todos, as: :target, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
-    has_many :zoom_meetings
 
     has_one :metrics
 

app/models/zoom_meeting.rb

@@ -14,14 +14,13 @@ class ZoomMeeting < ApplicationRecord
 
   scope :added_to_issue, -> { where(issue_status: :added) }
   scope :removed_from_issue, -> { where(issue_status: :removed) }
+  scope :canonical_meetings, -> (issue) { where(issue: issue)&.added_to_issue }
 
-  class << self
-    def canonical_meeting_url(issue)
-      canonical_meeting(issue)&.url
-    end
+  def self.canonical_meeting(issue)
+    canonical_meetings(issue)&.first
+  end
 
-    def canonical_meeting(issue)
-      issue&.zoom_meetings&.added_to_issue&.first
-    end
+  def self.canonical_meeting_url(issue)
+    canonical_meeting(issue)&.url
   end
 end

app/services/issues/zoom_link_service.rb

@@ -30,7 +30,7 @@ module Issues
     end
 
     def can_remove_link?
-      can? && @added_meeting
+      can? && !!@added_meeting
     end
 
     def parse_link(link)
@@ -42,11 +42,11 @@ module Issues
     attr_reader :issue
 
     def fetch_added_meeting
-      ZoomMeeting.where(issue: @issue).added_to_issue.first
+      ZoomMeeting.canonical_meeting(@issue)
     end
 
     def create_zoom_meeting(link)
-      meeting = ZoomMeeting.create(
+      ZoomMeeting.create(
         issue: @issue,
         project: @issue.project,
         issue_status: :added,
@@ -72,7 +72,6 @@ module Issues
       ServiceResponse.error(message: message)
     end
 
-
     def can?
       current_user.can?(:update_issue, project)
     end

spec/helpers/issuables_helper_spec.rb

@@ -203,44 +203,54 @@ describe IssuablesHelper do
     end
 
     describe '#zoomMeetingUrl in issue' do
-      let(:issue) { create(:issue, author: user, description: description) }
+      let(:issue) { create(:issue, author: user) }
 
       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)
+      shared_examples 'sets zoomMeetingUrl to nil' do
+        specify do
+          expect(helper.issuable_initial_data(issue)[:zoomMeetingUrl])
+            .to be_nil
         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' }
+      context 'with no "added" zoom mettings' do
+        it_behaves_like 'sets zoomMeetingUrl to nil'
+
+        context 'with multiple removed meetings' do
+          before do
+            create(:zoom_meeting, issue: issue, issue_status: :removed)
+            create(:zoom_meeting, issue: issue, issue_status: :removed)
+          end
 
-        it 'does not set zoomMeetingUrl' do
-          expect(helper.issuable_initial_data(issue))
-              .not_to include(:zoomMeetingUrl)
+          it_behaves_like 'sets zoomMeetingUrl to nil'
         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
+      context 'with "added" zoom meeting' do
+        before { create(:zoom_meeting, issue: issue) }
+
+        shared_examples 'sets zoomMeetingUrl to canonical meeting url' do
+          specify do
+            expect(helper.issuable_initial_data(issue))
+              .to include(zoomMeetingUrl: 'https://zoom.us/j/123456789')
+          end
         end
 
-        it 'sets zoomMeetingUrl value to the last url' do
-          expect(helper.issuable_initial_data(issue))
-            .to include(zoomMeetingUrl: 'https://zoom.us/s/lastone')
+        it_behaves_like 'sets zoomMeetingUrl to canonical meeting url'
+
+        context 'with muliple "removed" zoom meetings' do
+          before do
+            create(:zoom_meeting, issue: issue, issue_status: :removed)
+            create(:zoom_meeting, issue: issue, issue_status: :removed)
+          end
+
+          it_behaves_like 'sets zoomMeetingUrl to canonical meeting url'
         end
       end
+
     end
   end
 

spec/services/issues/zoom_link_service_spec.rb

@@ -14,27 +14,16 @@ describe Issues::ZoomLinkService do
     project.add_reporter(user)
   end
 
-  shared_context 'with Zoom link' do
+  shared_context '"added" Zoom meeting' do
     before do
-      issue.update!(description: "Description\n\n#{zoom_link}")
+      create(:zoom_meeting, issue: issue)
     end
   end
 
-  shared_context 'with Zoom link not at the end' do
+  shared_context '"removed" zoom meetings' do
     before do
-      issue.update!(description: "Description with #{zoom_link} some where")
-    end
-  end
-
-  shared_context 'without Zoom link' do
-    before do
-      issue.update!(description: "Description\n\nhttp://example.com")
-    end
-  end
-
-  shared_context 'without issue description' do
-    before do
-      issue.update!(description: nil)
+      create(:zoom_meeting, issue: issue, issue_status: :removed)
+      create(:zoom_meeting, issue: issue, issue_status: :removed)
     end
   end
 
@@ -45,16 +34,16 @@ describe Issues::ZoomLinkService do
   end
 
   describe '#add_link' do
-    shared_examples 'can add link' do
+    shared_examples 'can add meeting' do
       it 'appends the link to issue description' do
         expect(result).to be_success
-        expect(result.payload[:description])
-          .to eq("#{issue.description}\n\n#{zoom_link}")
+        expect(result.payload[:zoom_meetings].map(&:url))
+          .to include(zoom_link)
       end
     end
 
-    shared_examples 'cannot add link' do
-      it 'cannot add the link' do
+    shared_examples 'cannot add meeting' do
+      it 'cannot add the meeting' do
         expect(result).to be_error
         expect(result.message).to eq('Failed to add a Zoom meeting')
       end
@@ -62,43 +51,31 @@ describe Issues::ZoomLinkService do
 
     subject(:result) { service.add_link(zoom_link) }
 
-    context 'without Zoom link in the issue description' do
-      include_context 'without Zoom link'
-      include_examples 'can add link'
+    context 'without existing Zoom meeting' do
+      include_examples 'can add meeting'
 
-      context 'with invalid Zoom link' do
+      context 'with invalid Zoom meeting' do
         let(:zoom_link) { 'https://not-zoom.link' }
 
-        include_examples 'cannot add link'
+        include_examples 'cannot add meeting'
       end
 
       context 'with insufficient permissions' do
         include_context 'insufficient permissions'
-        include_examples 'cannot add link'
+        include_examples 'cannot add meeting'
       end
     end
 
-    context 'with Zoom link in the issue description' do
-      include_context 'with Zoom link'
-      include_examples 'cannot add link'
-
-      context 'but not at the end' do
-        include_context 'with Zoom link not at the end'
-        include_examples 'can add link'
-      end
-    end
-
-    context 'without issue description' do
-      include_context 'without issue description'
-      include_examples 'can add link'
+    context 'with "added" Zoom meeting' do
+      include_context '"added" Zoom meeting'
+      include_examples 'cannot add meeting'
     end
   end
 
   describe '#can_add_link?' do
     subject { service.can_add_link? }
 
-    context 'without Zoom link in the issue description' do
-      include_context 'without Zoom link'
+    context 'without Zoom link' do
 
       it { is_expected.to eq(true) }
 
@@ -109,74 +86,76 @@ describe Issues::ZoomLinkService do
       end
     end
 
-    context 'with Zoom link in the issue description' do
-      include_context 'with Zoom link'
+    context 'with Zoom meeting in the issue description' do
+      include_context  '"added" Zoom meeting'
 
       it { is_expected.to eq(false) }
     end
   end
 
   describe '#remove_link' do
-    shared_examples 'cannot remove link' do
-      it 'cannot remove the link' do
+    shared_examples 'cannot remove meeting' do
+      it 'cannot remove the meeting' do
         expect(result).to be_error
         expect(result.message).to eq('Failed to remove a Zoom meeting')
       end
     end
 
+    shared_examples 'can remove meeting' do
+      it 'can remove the meeting' do
+        expect(result).to be_success
+        expect(result.payload[:zoom_meetings].filter { |z| z.issue_status == 1 })
+        .to be_empty
+      end
+    end
+
     subject(:result) { service.remove_link }
 
-    context 'with Zoom link in the issue description' do
-      include_context 'with Zoom link'
+    context 'with Zoom meeting' do
+      include_context '"added" Zoom meeting'
 
-      it 'removes the link from the issue description' do
-        expect(result).to be_success
-        expect(result.payload[:description])
-          .to eq(issue.description.delete_suffix("\n\n#{zoom_link}"))
+      context 'removes the link' do
+        include_examples 'can remove meeting'
       end
 
       context 'with insufficient permissions' do
         include_context 'insufficient permissions'
-        include_examples 'cannot remove link'
+        include_examples 'cannot remove meeting'
       end
-
-      context 'but not at the end' do
-        include_context 'with Zoom link not at the end'
-        include_examples 'cannot remove link'
-      end
-    end
-
-    context 'without Zoom link in the issue description' do
-      include_context 'without Zoom link'
-      include_examples 'cannot remove link'
     end
 
-    context 'without issue description' do
-      include_context 'without issue description'
-      include_examples 'cannot remove link'
+    context 'without "added" Zoom meeting' do
+      include_context '"removed" zoom meetings'
+      include_examples 'cannot remove meeting'
     end
   end
 
   describe '#can_remove_link?' do
     subject { service.can_remove_link? }
 
-    context 'with Zoom link in the issue description' do
-      include_context 'with Zoom link'
+    context 'without Zoom meeting' do
+      it { is_expected.to eq(false) }
+    end
+
+    context 'with only "removed" zoom meetings' do
+      include_context '"removed" zoom meetings'
+      it { is_expected.to eq(false) }
+    end
 
+    context 'with "added" Zoom meeting' do
+      include_context '"added" Zoom meeting'
       it { is_expected.to eq(true) }
 
+      context 'with "removed" zoom meetings' do
+        include_context '"removed" zoom meetings'
+        it { is_expected.to eq(true) }
+      end
+
       context 'with insufficient permissions' do
         include_context 'insufficient permissions'
-
         it { is_expected.to eq(false) }
       end
     end
-
-    context 'without Zoom link in the issue description' do
-      include_context 'without Zoom link'
-
-      it { is_expected.to eq(false) }
-    end
   end
 
   describe '#parse_link' do