Merge branch 'mo-update-pending-builds-namespace' into 'master'

Update pending builds when project is transferred

See merge request gitlab-org/gitlab!70296

app/services/ci/update_pending_build_service.rb

@@ -2,7 +2,7 @@
 
 module Ci
   class UpdatePendingBuildService
-    VALID_PARAMS = %i[instance_runners_enabled].freeze
+    VALID_PARAMS = %i[instance_runners_enabled namespace_id namespace_traversal_ids].freeze
 
     InvalidParamsError = Class.new(StandardError)
     InvalidModelError = Class.new(StandardError)

app/services/groups/transfer_service.rb

@@ -29,6 +29,7 @@ module Groups
         update_group_attributes
         ensure_ownership
         update_integrations
+        update_pending_builds!
       end
 
       post_update_hooks(@updated_project_ids)
@@ -217,6 +218,15 @@ module Groups
         PropagateIntegrationWorker.perform_async(integration.id)
       end
     end
+
+    def update_pending_builds!
+      update_params = {
+        namespace_traversal_ids: group.traversal_ids,
+        namespace_id: group.id
+      }
+
+      ::Ci::UpdatePendingBuildService.new(group, update_params).execute
+    end
   end
 end
 

app/services/projects/transfer_service.rb

@@ -104,6 +104,8 @@ module Projects
         update_repository_configuration(@new_path)
 
         execute_system_hooks
+
+        update_pending_builds!
       end
 
       post_update_hooks(project)
@@ -253,6 +255,15 @@ module Projects
       project.integrations.with_default_settings.delete_all
       Integration.create_from_active_default_integrations(project, :project_id)
     end
+
+    def update_pending_builds!
+      update_params = {
+        namespace_id: new_namespace.id,
+        namespace_traversal_ids: new_namespace.traversal_ids
+      }
+
+      ::Ci::UpdatePendingBuildService.new(project, update_params).execute
+    end
   end
 end
 

spec/factories/ci/pending_builds.rb

@@ -9,5 +9,6 @@ FactoryBot.define do
     namespace { project.namespace }
     minutes_exceeded { false }
     tag_ids { build.tags_ids }
+    namespace_traversal_ids { project.namespace.traversal_ids }
   end
 end

spec/services/ci/update_pending_build_service_spec.rb

@@ -3,21 +3,23 @@
 require 'spec_helper'
 
 RSpec.describe Ci::UpdatePendingBuildService do
-  describe '#execute' do
-    let_it_be(:group) { create(:group) }
-    let_it_be(:project) { create(:project, namespace: group) }
-    let_it_be(:pending_build_1) { create(:ci_pending_build, project: project, instance_runners_enabled: false) }
-    let_it_be(:pending_build_2) { create(:ci_pending_build, project: project, instance_runners_enabled: true) }
-    let_it_be(:update_params) { { instance_runners_enabled: true } }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:project) { create(:project, namespace: group) }
+  let_it_be_with_reload(:pending_build_1) { create(:ci_pending_build, project: project, instance_runners_enabled: false) }
+  let_it_be_with_reload(:pending_build_2) { create(:ci_pending_build, project: project, instance_runners_enabled: true) }
+  let_it_be(:update_params) { { instance_runners_enabled: true } }
+
+  let(:service) { described_class.new(model, update_params) }
 
-    subject(:service) { described_class.new(model, update_params).execute }
+  describe '#execute' do
+    subject(:update_pending_builds) { service.execute }
 
     context 'validations' do
       context 'when model is invalid' do
         let(:model) { pending_build_1 }
 
         it 'raises an error' do
-          expect { service }.to raise_error(described_class::InvalidModelError)
+          expect { update_pending_builds }.to raise_error(described_class::InvalidModelError)
         end
       end
 
@@ -26,7 +28,7 @@ RSpec.describe Ci::UpdatePendingBuildService do
         let(:update_params) { { minutes_exceeded: true } }
 
         it 'raises an error' do
-          expect { service }.to raise_error(described_class::InvalidParamsError)
+          expect { update_pending_builds }.to raise_error(described_class::InvalidParamsError)
         end
       end
     end
@@ -35,10 +37,10 @@ RSpec.describe Ci::UpdatePendingBuildService do
       let(:model) { group }
 
       it 'updates all pending builds', :aggregate_failures do
-        service
+        update_pending_builds
 
-        expect(pending_build_1.reload.instance_runners_enabled).to be_truthy
-        expect(pending_build_2.reload.instance_runners_enabled).to be_truthy
+        expect(pending_build_1.instance_runners_enabled).to be_truthy
+        expect(pending_build_2.instance_runners_enabled).to be_truthy
       end
 
       context 'when ci_pending_builds_maintain_shared_runners_data is disabled' do
@@ -47,10 +49,10 @@ RSpec.describe Ci::UpdatePendingBuildService do
         end
 
         it 'does not update all pending builds', :aggregate_failures do
-          service
+          update_pending_builds
 
-          expect(pending_build_1.reload.instance_runners_enabled).to be_falsey
-          expect(pending_build_2.reload.instance_runners_enabled).to be_truthy
+          expect(pending_build_1.instance_runners_enabled).to be_falsey
+          expect(pending_build_2.instance_runners_enabled).to be_truthy
         end
       end
     end
@@ -59,10 +61,10 @@ RSpec.describe Ci::UpdatePendingBuildService do
       let(:model) { project }
 
       it 'updates all pending builds', :aggregate_failures do
-        service
+        update_pending_builds
 
-        expect(pending_build_1.reload.instance_runners_enabled).to be_truthy
-        expect(pending_build_2.reload.instance_runners_enabled).to be_truthy
+        expect(pending_build_1.instance_runners_enabled).to be_truthy
+        expect(pending_build_2.instance_runners_enabled).to be_truthy
       end
 
       context 'when ci_pending_builds_maintain_shared_runners_data is disabled' do
@@ -71,10 +73,10 @@ RSpec.describe Ci::UpdatePendingBuildService do
         end
 
         it 'does not update all pending builds', :aggregate_failures do
-          service
+          update_pending_builds
 
-          expect(pending_build_1.reload.instance_runners_enabled).to be_falsey
-          expect(pending_build_2.reload.instance_runners_enabled).to be_truthy
+          expect(pending_build_1.instance_runners_enabled).to be_falsey
+          expect(pending_build_2.instance_runners_enabled).to be_truthy
         end
       end
     end

spec/services/groups/transfer_service_spec.rb

@@ -651,6 +651,30 @@ RSpec.describe Groups::TransferService do
           expect(project1.public?).to be_truthy
         end
       end
+
+      context 'when group has pending builds' do
+        let_it_be(:project) { create(:project, :public, namespace: group.reload) }
+        let_it_be(:other_project) { create(:project) }
+        let_it_be(:pending_build) { create(:ci_pending_build, project: project) }
+        let_it_be(:unrelated_pending_build) { create(:ci_pending_build, project: other_project) }
+
+        before do
+          group.add_owner(user)
+          new_parent_group.add_owner(user)
+        end
+
+        it 'updates pending builds for the group', :aggregate_failures do
+          transfer_service.execute(new_parent_group)
+
+          pending_build.reload
+          unrelated_pending_build.reload
+
+          expect(pending_build.namespace_id).to eq(group.id)
+          expect(pending_build.namespace_traversal_ids).to eq(group.traversal_ids)
+          expect(unrelated_pending_build.namespace_id).to eq(other_project.namespace_id)
+          expect(unrelated_pending_build.namespace_traversal_ids).to eq(other_project.namespace.traversal_ids)
+        end
+      end
     end
 
     context 'when transferring a subgroup into root group' do

spec/services/projects/transfer_service_spec.rb

@@ -143,6 +143,28 @@ RSpec.describe Projects::TransferService do
         end
       end
     end
+
+    context 'when project has pending builds' do
+      let!(:other_project) { create(:project) }
+      let!(:pending_build) { create(:ci_pending_build, project: project.reload) }
+      let!(:unrelated_pending_build) { create(:ci_pending_build, project: other_project) }
+
+      before do
+        group.reload
+      end
+
+      it 'updates pending builds for the project', :aggregate_failures do
+        execute_transfer
+
+        pending_build.reload
+        unrelated_pending_build.reload
+
+        expect(pending_build.namespace_id).to eq(group.id)
+        expect(pending_build.namespace_traversal_ids).to eq(group.traversal_ids)
+        expect(unrelated_pending_build.namespace_id).to eq(other_project.namespace_id)
+        expect(unrelated_pending_build.namespace_traversal_ids).to eq(other_project.namespace.traversal_ids)
+      end
+    end
   end
 
   context 'when transfer fails' do
@@ -203,6 +225,24 @@ RSpec.describe Projects::TransferService do
         shard_name: project.repository_storage
       )
     end
+
+    context 'when project has pending builds' do
+      let!(:other_project) { create(:project) }
+      let!(:pending_build) { create(:ci_pending_build, project: project.reload) }
+      let!(:unrelated_pending_build) { create(:ci_pending_build, project: other_project) }
+
+      it 'does not update pending builds for the project', :aggregate_failures do
+        attempt_project_transfer
+
+        pending_build.reload
+        unrelated_pending_build.reload
+
+        expect(pending_build.namespace_id).to eq(project.namespace_id)
+        expect(pending_build.namespace_traversal_ids).to eq(project.namespace.traversal_ids)
+        expect(unrelated_pending_build.namespace_id).to eq(other_project.namespace_id)
+        expect(unrelated_pending_build.namespace_traversal_ids).to eq(other_project.namespace.traversal_ids)
+      end
+    end
   end
 
   context 'namespace -> no namespace' do