Merge branch '38234-zoom-quick-action-on-a-new-issue-causes-error' into 'master'

Resolve "Zoom quick action on a new issue causes error"

Closes #38234

See merge request gitlab-org/gitlab!21262

app/services/issues/zoom_link_service.rb

@@ -13,30 +13,29 @@ module Issues
       if can_add_link? && (link = parse_link(link))
         begin
           add_zoom_meeting(link)
-          success(_('Zoom meeting added'))
         rescue ActiveRecord::RecordNotUnique
-          error(_('Failed to add a Zoom meeting'))
+          error(message: _('Failed to add a Zoom meeting'))
         end
       else
-        error(_('Failed to add a Zoom meeting'))
+        error(message: _('Failed to add a Zoom meeting'))
       end
     end
 
     def remove_link
       if can_remove_link?
         remove_zoom_meeting
-        success(_('Zoom meeting removed'))
+        success(message: _('Zoom meeting removed'))
       else
-        error(_('Failed to remove a Zoom meeting'))
+        error(message: _('Failed to remove a Zoom meeting'))
       end
     end
 
     def can_add_link?
-      can_update_issue? && !@added_meeting
+      can_change_link? && !@added_meeting
     end
 
     def can_remove_link?
-      can_update_issue? && !!@added_meeting
+      can_change_link? && @issue.persisted? && !!@added_meeting
     end
 
     def parse_link(link)
@@ -56,14 +55,29 @@ module Issues
     end
 
     def add_zoom_meeting(link)
-      ZoomMeeting.create(
+      zoom_meeting = new_zoom_meeting(link)
+      response =
+        if @issue.persisted?
+          # Save the meeting directly since we only want to update one meeting, not all
+          zoom_meeting.save
+          success(message: _('Zoom meeting added'))
+        else
+          success(message: _('Zoom meeting added'), payload: { zoom_meetings: [zoom_meeting] })
+        end
+
+      track_meeting_added_event
+      SystemNoteService.zoom_link_added(@issue, @project, current_user)
+
+      response
+    end
+
+    def new_zoom_meeting(link)
+      ZoomMeeting.new(
         issue: @issue,
-        project: @issue.project,
+        project: @project,
         issue_status: :added,
         url: link
       )
-      track_meeting_added_event
-      SystemNoteService.zoom_link_added(@issue, @project, current_user)
     end
 
     def remove_zoom_meeting
@@ -72,16 +86,20 @@ module Issues
       SystemNoteService.zoom_link_removed(@issue, @project, current_user)
     end
 
-    def success(message)
-      ServiceResponse.success(message: message)
+    def success(message:, payload: nil)
+      ServiceResponse.success(message: message, payload: payload)
     end
 
-    def error(message)
+    def error(message:)
       ServiceResponse.error(message: message)
     end
 
-    def can_update_issue?
-      can?(current_user, :update_issue, project)
+    def can_change_link?
+      if @issue.persisted?
+        can?(current_user, :update_issue, @project)
+      else
+        can?(current_user, :create_issue, @project)
+      end
     end
   end
 end

changelogs/unreleased/38234-zoom-quick-action-on-a-new-issue-causes-error.yml

+---
+title: Fix Zoom Quick Action server error when creating a GitLab Issue
+merge_request: 21262
+author:
+type: fixed

lib/gitlab/quick_actions/issue_actions.rb

@@ -183,6 +183,7 @@ module Gitlab
         command :zoom do |link|
           result = @zoom_service.add_link(link)
           @execution_message[:zoom] = result.message
+          @updates.merge!(result.payload) if result.payload
         end
 
         desc _('Remove Zoom meeting')

spec/services/issues/zoom_link_service_spec.rb

@@ -27,12 +27,18 @@ describe Issues::ZoomLinkService do
     end
   end
 
-  shared_context 'insufficient permissions' do
+  shared_context 'insufficient issue update permissions' do
     before do
       project.add_guest(user)
     end
   end
 
+  shared_context 'insufficient issue create permissions' do
+    before do
+      expect(service).to receive(:can?).with(user, :create_issue, project).and_return(false)
+    end
+  end
+
   describe '#add_link' do
     shared_examples 'can add meeting' do
       it 'appends the new meeting to zoom_meetings' do
@@ -69,16 +75,38 @@ describe Issues::ZoomLinkService do
     subject(:result) { service.add_link(zoom_link) }
 
     context 'without existing Zoom meeting' do
+      context 'when updating an issue' do
+        before do
+          allow(issue).to receive(:persisted?).and_return(true)
+        end
+
         include_examples 'can add meeting'
 
-      context 'with invalid Zoom url' do
-        let(:zoom_link) { 'https://not-zoom.link' }
+        context 'with insufficient issue update permissions' do
+          include_context 'insufficient issue update permissions'
+          include_examples 'cannot add meeting'
+        end
+      end
 
+      context 'when creating an issue' do
+        before do
+          allow(issue).to receive(:persisted?).and_return(false)
+        end
+
+        it 'creates a new zoom meeting' do
+          expect(result).to be_success
+          expect(result.payload[:zoom_meetings][0].url).to eq(zoom_link)
+        end
+
+        context 'with insufficient issue create permissions' do
+          include_context 'insufficient issue create permissions'
           include_examples 'cannot add meeting'
         end
+      end
+
+      context 'with invalid Zoom url' do
+        let(:zoom_link) { 'https://not-zoom.link' }
 
-      context 'with insufficient permissions' do
-        include_context 'insufficient permissions'
         include_examples 'cannot add meeting'
       end
     end
@@ -92,6 +120,7 @@ describe Issues::ZoomLinkService do
       include_context '"added" Zoom meeting'
       before do
         allow(service).to receive(:can_add_link?).and_return(true)
+        allow(issue).to receive(:persisted?).and_return(true)
       end
 
       include_examples 'cannot add meeting'
@@ -104,8 +133,8 @@ describe Issues::ZoomLinkService do
     context 'without "added" zoom meeting' do
       it { is_expected.to eq(true) }
 
-      context 'with insufficient permissions' do
-        include_context 'insufficient permissions'
+      context 'with insufficient issue update permissions' do
+        include_context 'insufficient issue update permissions'
 
         it { is_expected.to eq(false) }
       end
@@ -156,12 +185,24 @@ describe Issues::ZoomLinkService do
     context 'with Zoom meeting' do
       include_context '"added" Zoom meeting'
 
-      context 'removes the link' do
+      context 'with existing issue' do
+        before do
+          allow(issue).to receive(:persisted?).and_return(true)
+        end
+
         include_examples 'can remove meeting'
       end
 
-      context 'with insufficient permissions' do
-        include_context 'insufficient permissions'
+      context 'without existing issue' do
+        before do
+          allow(issue).to receive(:persisted?).and_return(false)
+        end
+
+        include_examples 'cannot remove meeting'
+      end
+
+      context 'with insufficient issue update permissions' do
+        include_context 'insufficient issue update permissions'
         include_examples 'cannot remove meeting'
       end
     end
@@ -193,8 +234,8 @@ describe Issues::ZoomLinkService do
         it { is_expected.to eq(true) }
       end
 
-      context 'with insufficient permissions' do
-        include_context 'insufficient permissions'
+      context 'with insufficient issue update permissions' do
+        include_context 'insufficient issue update permissions'
         it { is_expected.to eq(false) }
       end
     end