Add latest changes from gitlab-org/gitlab@master

2 files changed, 85 insertions, 91 deletions

diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb
index 1eec3b97ea1..21e308e6636 100644
--- a/spec/services/issues/update_service_spec.rb
+++ b/spec/services/issues/update_service_spec.rb
@@ -187,7 +187,6 @@ describe Issues::UpdateService, :mailer do
         it 'creates system note about issue reassign' do
           note = find_note('assigned to')
 
-          expect(note).not_to be_nil
           expect(note.note).to include "assigned to #{user2.to_reference}"
         end
 
@@ -202,14 +201,12 @@ describe Issues::UpdateService, :mailer do
         it 'creates system note about title change' do
           note = find_note('changed title')
 
-          expect(note).not_to be_nil
           expect(note.note).to eq 'changed title from **{-Old-} title** to **{+New+} title**'
         end
 
         it 'creates system note about discussion lock' do
           note = find_note('locked this issue')
 
-          expect(note).not_to be_nil
           expect(note.note).to eq 'locked this issue'
         end
       end
@@ -221,20 +218,10 @@ describe Issues::UpdateService, :mailer do
 
         note = find_note('changed the description')
 
-        expect(note).not_to be_nil
         expect(note.note).to eq('changed the description')
       end
     end
 
-    it 'creates zoom_link_added system note when a zoom link is added to the description' do
-      update_issue(description: 'Changed description https://zoom.us/j/5873603787')
-
-      note = find_note('added a Zoom call')
-
-      expect(note).not_to be_nil
-      expect(note.note).to eq('added a Zoom call to this issue')
-    end
-
     context 'when issue turns confidential' do
       let(:opts) do
         {
@@ -252,7 +239,6 @@ describe Issues::UpdateService, :mailer do
 
         note = find_note('made the issue confidential')
 
-        expect(note).not_to be_nil
         expect(note.note).to eq 'made the issue confidential'
       end
 
diff --git a/spec/services/issues/zoom_link_service_spec.rb b/spec/services/issues/zoom_link_service_spec.rb
index ba3f007c917..ecca9467965 100644
--- a/spec/services/issues/zoom_link_service_spec.rb
+++ b/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,11 +34,10 @@ describe Issues::ZoomLinkService do
   end
 
   describe '#add_link' do
-    shared_examples 'can add link' do
-      it 'appends the link to issue description' do
+    shared_examples 'can add meeting' do
+      it 'appends the new meeting to zoom_meetings' do
         expect(result).to be_success
-        expect(result.payload[:description])
-          .to eq("#{issue.description}\n\n#{zoom_link}")
+        expect(ZoomMeeting.canonical_meeting_url(issue)).to eq(zoom_link)
       end
 
       it 'tracks the add event' do
@@ -57,55 +45,63 @@ describe Issues::ZoomLinkService do
           .with('IncidentManagement::ZoomIntegration', 'add_zoom_meeting', label: 'Issue ID', value: issue.id)
         result
       end
+
+      it 'creates a zoom_link_added notification' do
+        expect(SystemNoteService).to receive(:zoom_link_added).with(issue, project, user)
+        expect(SystemNoteService).not_to receive(:zoom_link_removed)
+        result
+      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
+
+      it 'creates no notification' do
+        expect(SystemNoteService).not_to receive(:zoom_link_added)
+        expect(SystemNoteService).not_to receive(:zoom_link_removed)
+        result
+      end
     end
 
     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 url' 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 'with "added" Zoom meeting' do
+      include_context '"added" Zoom meeting'
+      include_examples 'cannot add meeting'
+    end
 
-      context 'but not at the end' do
-        include_context 'with Zoom link not at the end'
-        include_examples 'can add link'
+    context 'with "added" Zoom meeting and race condition' do
+      include_context '"added" Zoom meeting'
+      before do
+        allow(service).to receive(:can_add_link?).and_return(true)
       end
-    end
 
-    context 'without issue description' do
-      include_context 'without issue description'
-      include_examples 'can add link'
+      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 "added" zoom meeting' do
       it { is_expected.to eq(true) }
 
       context 'with insufficient permissions' do
@@ -115,81 +111,93 @@ 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
 
-    subject(:result) { service.remove_link }
+      it 'creates no notification' do
+        expect(SystemNoteService).not_to receive(:zoom_link_added)
+        expect(SystemNoteService).not_to receive(:zoom_link_removed)
+        result
+      end
+    end
 
-    context 'with Zoom link in the issue description' do
-      include_context 'with Zoom link'
+    shared_examples 'can remove meeting' do
+      it 'creates no notification' do
+        expect(SystemNoteService).not_to receive(:zoom_link_added).with(issue, project, user)
+        expect(SystemNoteService).to receive(:zoom_link_removed)
+        result
+      end
 
-      it 'removes the link from the issue description' do
+      it 'can remove the meeting' do
         expect(result).to be_success
-        expect(result.payload[:description])
-          .to eq(issue.description.delete_suffix("\n\n#{zoom_link}"))
+        expect(ZoomMeeting.canonical_meeting_url(issue)).to eq(nil)
       end
 
       it 'tracks the remove event' do
         expect(Gitlab::Tracking).to receive(:event)
-          .with('IncidentManagement::ZoomIntegration', 'remove_zoom_meeting', label: 'Issue ID', value: issue.id)
-
+        .with('IncidentManagement::ZoomIntegration', 'remove_zoom_meeting', label: 'Issue ID', value: issue.id)
         result
       end
+    end
 
-      context 'with insufficient permissions' do
-        include_context 'insufficient permissions'
-        include_examples 'cannot remove link'
-      end
+    subject(:result) { service.remove_link }
 
-      context 'but not at the end' do
-        include_context 'with Zoom link not at the end'
-        include_examples 'cannot remove link'
+    context 'with Zoom meeting' do
+      include_context '"added" Zoom meeting'
+
+      context 'removes the link' do
+        include_examples 'can remove meeting'
       end
-    end
 
-    context 'without Zoom link in the issue description' do
-      include_context 'without Zoom link'
-      include_examples 'cannot remove link'
+      context 'with insufficient permissions' do
+        include_context 'insufficient permissions'
+        include_examples 'cannot remove meeting'
+      end
     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