Merge branch 'sy-change-issue-type' into 'master'

Allow issue type changes via internal API

See merge request gitlab-org/gitlab!60173

app/controllers/projects/issues_controller.rb

@@ -108,10 +108,10 @@ class Projects::IssuesController < Projects::ApplicationController
     params[:issue] ||= ActionController::Parameters.new(
       assignee_ids: ""
     )
-    build_params = issue_create_params.merge(
+    build_params = issue_params.merge(
       merge_request_to_resolve_discussions_of: params[:merge_request_to_resolve_discussions_of],
       discussion_to_resolve: params[:discussion_to_resolve],
-      confidential: !!Gitlab::Utils.to_boolean(issue_create_params[:confidential])
+      confidential: !!Gitlab::Utils.to_boolean(issue_params[:confidential])
     )
     service = ::Issues::BuildService.new(project, current_user, build_params)
 
@@ -128,7 +128,7 @@ class Projects::IssuesController < Projects::ApplicationController
   end
 
   def create
-    create_params = issue_create_params.merge(spammable_params).merge(
+    create_params = issue_params.merge(spammable_params).merge(
       merge_request_to_resolve_discussions_of: params[:merge_request_to_resolve_discussions_of],
       discussion_to_resolve: params[:discussion_to_resolve]
     )
@@ -314,17 +314,8 @@ class Projects::IssuesController < Projects::ApplicationController
       task_num
       lock_version
       discussion_locked
-    ] + [{ label_ids: [], assignee_ids: [], update_task: [:index, :checked, :line_number, :line_source] }]
-  end
-
-  def issue_create_params
-    create_params = %i[
       issue_type
-    ]
-
-    params.require(:issue).permit(
-      *create_params
-    ).merge(issue_params)
+    ] + [{ label_ids: [], assignee_ids: [], update_task: [:index, :checked, :line_number, :line_source] }]
   end
 
   def reorder_params

app/services/issues/update_service.rb

@@ -24,10 +24,11 @@ module Issues
     def filter_params(issue)
       super
 
-      # filter confidential in `Issues::UpdateService` and not in `IssuableBaseService#filtr_params`
+      # filter confidential in `Issues::UpdateService` and not in `IssuableBaseService#filter_params`
       # because we do allow users that cannot admin issues to set confidential flag when creating an issue
       unless can_admin_issuable?(issue)
         params.delete(:confidential)
+        params.delete(:issue_type)
       end
     end
 

changelogs/unreleased/sy-change-issue-type.yml

+---
+title: Add internal API support for updating issue types on issues
+merge_request: 60173
+author:
+type: added

ee/app/services/ee/issues/update_service.rb

@@ -36,6 +36,7 @@ module EE
         super
 
         handle_iteration_change(issue)
+        handle_issue_type_change(issue)
       end
 
       private
@@ -54,6 +55,12 @@ module EE
         end
       end
 
+      def handle_issue_type_change(issue)
+        return unless issue.previous_changes.include?('issue_type')
+
+        ::IncidentManagement::Incidents::CreateSlaService.new(issue, current_user).execute
+      end
+
       def handle_promotion(issue)
         return unless params.delete(:promote_to_epic)
 

ee/app/services/incident_management/incidents/create_sla_service.rb

@@ -12,6 +12,7 @@ module IncidentManagement
       def execute
         return not_enabled_success unless issuable.sla_available?
         return not_enabled_success unless incident_setting&.sla_timer?
+        return success(sla: issuable.issuable_sla) if issuable.issuable_sla
 
         sla = issuable.build_issuable_sla(
           due_at: issuable.created_at + incident_setting.sla_timer_minutes.minutes

ee/spec/services/ee/issues/update_service_spec.rb

@@ -169,6 +169,41 @@ RSpec.describe Issues::UpdateService do
       end
     end
 
+    context 'changing issue_type' do
+      let!(:sla_setting) { create(:project_incident_management_setting, :sla_enabled, project: project) }
+
+      before do
+        stub_licensed_features(incident_sla: true)
+      end
+
+      context 'from issue to incident' do
+        it 'creates an SLA' do
+          expect { update_issue(issue_type: 'incident') }.to change(IssuableSla, :count).by(1)
+          expect(issue.reload.issuable_sla).to be_present
+        end
+      end
+
+      context 'from incident to issue' do
+        let(:issue) { create(:incident, project: project) }
+        let!(:sla) { create(:issuable_sla, issue: issue) }
+
+        it 'does not remove the SLA or create a new one' do
+          expect { update_issue(issue_type: 'issue') }.not_to change(IssuableSla, :count)
+          expect(issue.reload.issuable_sla).to be_present
+        end
+      end
+
+      # Not an expected scenario, but covers an SLA-agnostic hypothetical
+      context 'from test_case to issue' do
+        let(:issue) { create(:quality_test_case, project: project) }
+
+        it 'does nothing' do
+          expect { update_issue(issue_type: 'issue') }.not_to change(IssuableSla, :count)
+          expect(issue.reload.issuable_sla).to be_nil
+        end
+      end
+    end
+
     context 'assigning epic' do
       before do
         stub_licensed_features(epics: true)

ee/spec/services/incident_management/incidents/create_sla_service_spec.rb

@@ -54,6 +54,17 @@ RSpec.describe IncidentManagement::Incidents::CreateSlaService do
         it_behaves_like 'no issuable sla created'
       end
 
+      context 'issuable sla already exists' do
+        let!(:issuable_sla) { create(:issuable_sla, issue: incident) }
+
+        it 'returns a success with the sla', :aggregate_failures do
+          expect { subject }.not_to change(IssuableSla, :count)
+
+          expect(create_issuable_sla_response.success?).to eq(true)
+          expect(response_payload_sla).to be_a(IssuableSla)
+        end
+      end
+
       it 'creates the issuable sla with the given offset', :aggregate_failures do
         expect { subject }.to change(IssuableSla, :count)
 

spec/controllers/projects/issues_controller_spec.rb

@@ -586,13 +586,15 @@ RSpec.describe Projects::IssuesController do
   end
 
   describe 'PUT #update' do
+    let(:issue_params) { { title: 'New title' } }
+
     subject do
       put :update,
         params: {
           namespace_id: project.namespace,
           project_id: project,
           id: issue.to_param,
-          issue: { title: 'New title' }
+          issue: issue_params
         },
         format: :json
     end
@@ -614,6 +616,17 @@ RSpec.describe Projects::IssuesController do
         expect(issue.reload.title).to eq('New title')
       end
 
+      context 'with issue_type param' do
+        let(:issue_params) { { issue_type: 'incident' } }
+
+        it 'permits the parameter' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(issue.reload.issue_type).to eql('incident')
+        end
+      end
+
       context 'when the SpamVerdictService disallows' do
         before do
           stub_application_setting(recaptcha_enabled: true)

spec/services/issues/update_service_spec.rb

@@ -282,7 +282,14 @@ RSpec.describe Issues::UpdateService, :mailer do
         end
 
         it 'filters out params that cannot be set without the :admin_issue permission' do
-          described_class.new(project, guest, opts.merge(confidential: true)).execute(issue)
+          described_class.new(
+            project,
+            guest,
+            opts.merge(
+              confidential: true,
+              issue_type: 'test_case'
+            )
+          ).execute(issue)
 
           expect(issue).to be_valid
           expect(issue.title).to eq 'New title'
@@ -293,6 +300,7 @@ RSpec.describe Issues::UpdateService, :mailer do
           expect(issue.due_date).to be_nil
           expect(issue.discussion_locked).to be_falsey
           expect(issue.confidential).to be_falsey
+          expect(issue.issue_type).to eql('issue')
         end
       end