Merge branch 'vij-change-ci-minutes-pack-namespace' into 'master'

Allow changing AdditionalPack namespace

See merge request gitlab-org/gitlab!64212

ee/app/services/ci/minutes/additional_packs/base_service.rb

+# frozen_string_literal: true
+
+module Ci
+  module Minutes
+    module AdditionalPacks
+      class BaseService
+        include BaseServiceUtility
+
+        private
+
+        # rubocop: disable Cop/UserAdmin
+        def authorize_current_user!
+          # Using #admin? is discouraged as it will bypass admin mode authorisation checks,
+          # however those checks are not in place in our REST API yet, and this service is only
+          # going to be used by the API for admin-only actions
+          raise Gitlab::Access::AccessDeniedError unless current_user&.admin?
+        end
+        # rubocop: enable Cop/UserAdmin
+      end
+    end
+  end
+end

ee/app/services/ci/minutes/additional_packs/change_namespace_service.rb

+# frozen_string_literal: true
+
+module Ci
+  module Minutes
+    module AdditionalPacks
+      class ChangeNamespaceService < ::Ci::Minutes::AdditionalPacks::BaseService
+        ChangeNamespaceError = Class.new(StandardError)
+
+        def initialize(current_user, namespace, target)
+          @current_user = current_user
+          @namespace = namespace
+          @target = target
+        end
+
+        def execute
+          authorize_current_user!
+
+          validate_namespaces!
+          validate_owners!
+
+          Ci::Minutes::AdditionalPack.transaction do
+            update_packs
+
+            success
+          end
+        rescue ChangeNamespaceError => e
+          error(e.message)
+        end
+
+        private
+
+        attr_reader :current_user, :namespace, :target
+
+        def additional_packs
+          @additional_packs ||= namespace.ci_minutes_additional_packs
+        end
+
+        def update_packs
+          return unless additional_packs.any?
+
+          additional_packs.update_all(namespace_id: target.id)
+        end
+
+        def validate_namespaces!
+          raise ChangeNamespaceError, 'Namespace must be provided' unless namespace.present?
+          raise ChangeNamespaceError, 'Target namespace must be provided' unless target.present?
+          raise ChangeNamespaceError, 'Namespace must be a top-level namespace' unless namespace.root?
+          raise ChangeNamespaceError, 'Target namespace must be a top-level namespace' unless target.root?
+        end
+
+        def validate_owners!
+          shared_ids = namespace.owner_ids & target.owner_ids
+
+          raise ChangeNamespaceError, 'Both namespaces must share the same owner' unless shared_ids.any?
+        end
+      end
+    end
+  end
+end

ee/app/services/ci/minutes/additional_packs/create_service.rb

@@ -3,9 +3,7 @@
 module Ci
   module Minutes
     module AdditionalPacks
-      class CreateService
-        include BaseServiceUtility
-
+      class CreateService < ::Ci::Minutes::AdditionalPacks::BaseService
         def initialize(current_user, namespace, params = {})
           @current_user = current_user
           @namespace = namespace

ee/lib/api/ci/minutes.rb

@@ -29,6 +29,27 @@ module API
             bad_request!(result[:message])
           end
         end
+
+        desc 'Transfer purchased CI minutes packs to another namespace'
+        params do
+          requires :id, type: String, desc: 'The ID of the namespace to transfer from'
+          requires :target_id, type: String, desc: 'The ID of the namespace for the packs to transfer to'
+        end
+        patch ':id/minutes/move/:target_id' do
+          namespace = find_namespace(params[:id])
+          target_namespace = find_namespace(params[:target_id])
+
+          not_found!('Namespace') unless namespace
+          not_found!('Target namespace') unless target_namespace
+
+          result = ::Ci::Minutes::AdditionalPacks::ChangeNamespaceService.new(current_user, namespace, target_namespace).execute
+
+          if result[:status] == :success
+            accepted!
+          else
+            bad_request!(result[:message])
+          end
+        end
       end
     end
   end

ee/spec/requests/api/ci/minutes_spec.rb

@@ -3,6 +3,14 @@
 require 'spec_helper'
 
 RSpec.describe API::Ci::Minutes do
+  shared_examples 'not found error' do
+    it 'returns an error' do
+      subject
+
+      expect(response).to have_gitlab_http_status(:not_found)
+    end
+  end
+
   describe 'POST /namespaces/:id/minutes' do
     let_it_be(:namespace) { create(:namespace) }
     let_it_be(:user) { create(:user) }
@@ -32,11 +40,7 @@ RSpec.describe API::Ci::Minutes do
       context 'when the namespace cannot be found' do
         let(:namespace_id) { non_existing_record_id }
 
-        it 'returns an error' do
-          post_minutes
-
-          expect(response).to have_gitlab_http_status(:not_found)
-        end
+        it_behaves_like 'not found error'
       end
 
       context 'when the additional pack does not exist' do
@@ -78,4 +82,93 @@ RSpec.describe API::Ci::Minutes do
       end
     end
   end
+
+  describe 'PATCH /namespaces/:id/minutes/move/:target_id' do
+    let_it_be(:namespace) { create(:group) }
+    let_it_be(:target) { create(:group, owner: namespace.owner) }
+    let_it_be(:unrelated_target) { create(:group) }
+    let_it_be(:child_namespace) { create(:group, :nested) }
+    let_it_be(:user) { create(:user) }
+
+    let(:namespace_id) { namespace.id }
+    let(:target_id) { target.id }
+
+    before do
+      namespace.add_owner(user)
+      target.add_owner(user)
+    end
+
+    subject(:move_packs) { patch api("/namespaces/#{namespace_id}/minutes/move/#{target_id}", user) }
+
+    context 'when unauthorized' do
+      it 'returns an error' do
+        move_packs
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+    end
+
+    context 'when authorized' do
+      let_it_be(:user) { create(:admin) }
+
+      context 'when the namespace cannot be found' do
+        let(:namespace_id) { non_existing_record_id }
+
+        it_behaves_like 'not found error'
+      end
+
+      context 'when the target namespace cannot be found' do
+        let(:target_id) { non_existing_record_id }
+
+        it_behaves_like 'not found error'
+      end
+
+      context 'when the namespace is not a top-level namespace' do
+        let(:namespace_id) { child_namespace.id }
+
+        it 'returns an error' do
+          move_packs
+
+          expect(response).to have_gitlab_http_status(:bad_request)
+          expect(response.body).to include('Namespace must be a top-level namespace')
+        end
+      end
+
+      context 'when the target namespace is not a top-level namespace' do
+        let(:target_id) { child_namespace.id }
+
+        it 'returns an error' do
+          move_packs
+
+          expect(response).to have_gitlab_http_status(:bad_request)
+          expect(response.body).to include('Target namespace must be a top-level namespace')
+        end
+      end
+
+      context 'when the target namespace is not owned by the same user' do
+        let(:target_id) { unrelated_target.id }
+
+        it 'returns an error' do
+          move_packs
+
+          expect(response).to have_gitlab_http_status(:bad_request)
+          expect(response.body).to include('Both namespaces must share the same owner')
+        end
+      end
+
+      context 'when the transfer is successful' do
+        let(:target_id) { target.id }
+
+        before do
+          create(:ci_minutes_additional_pack, namespace: namespace)
+        end
+
+        it 'moves the packs and returns an accepted response' do
+          expect { move_packs }.to change(target.ci_minutes_additional_packs, :count).by(1)
+
+          expect(response).to have_gitlab_http_status(:accepted)
+        end
+      end
+    end
+  end
 end

ee/spec/services/ci/minutes/additional_packs/change_namespace_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::Minutes::AdditionalPacks::ChangeNamespaceService do
+  describe '#execute' do
+    let_it_be(:namespace) { create(:group) }
+    let_it_be(:target) { create(:group) }
+    let_it_be(:subgroup) { build(:group, :nested) }
+    let_it_be(:existing_packs) { create_list(:ci_minutes_additional_pack, 5, namespace: namespace) }
+    let_it_be(:admin) { create(:user, :admin) }
+    let_it_be(:non_admin) { build(:user) }
+
+    subject(:change_namespace) { described_class.new(user, namespace, target).execute }
+
+    context 'with a non-admin user' do
+      let(:user) { non_admin }
+
+      it 'raises an error' do
+        expect { change_namespace }.to raise_error(Gitlab::Access::AccessDeniedError)
+      end
+    end
+
+    context 'with an admin user' do
+      let(:user) { admin }
+
+      context 'with valid namespace and target namespace' do
+        before do
+          namespace.add_owner(admin)
+          target.add_owner(admin)
+        end
+
+        it 'moves all existing packs to the target namespace', :aggregate_failures do
+          expect(target.ci_minutes_additional_packs).to be_empty
+
+          change_namespace
+
+          expect(target.ci_minutes_additional_packs).to match_array(existing_packs)
+          expect(existing_packs.first.reload.namespace).to eq target
+          expect(change_namespace[:status]).to eq :success
+        end
+
+        context 'when updating packs fails' do
+          before do
+            allow_next_instance_of(described_class) do |instance|
+              allow(instance).to receive(:success).and_raise(StandardError)
+            end
+          end
+
+          it 'rolls back updates for all packs', :aggregate_failures do
+            expect { change_namespace }.to raise_error(StandardError)
+            expect(namespace.ci_minutes_additional_packs.count).to eq 5
+            expect(target.ci_minutes_additional_packs.count).to eq 0
+          end
+        end
+
+        context 'when the namespace has no additional packs to move' do
+          before do
+            allow_next_instance_of(described_class) do |instance|
+              allow(instance).to receive(:additional_packs).and_return([])
+            end
+          end
+
+          it 'returns success' do
+            expect(change_namespace[:status]).to eq :success
+          end
+        end
+      end
+
+      context 'when the namespace is not provided' do
+        let(:namespace) { nil }
+
+        it 'returns an error' do
+          expect(change_namespace[:status]).to eq :error
+          expect(change_namespace[:message]).to eq 'Namespace must be provided'
+        end
+      end
+
+      context 'when the target namespace is not provided' do
+        let(:target) { nil }
+
+        it 'returns an error' do
+          expect(change_namespace[:status]).to eq :error
+          expect(change_namespace[:message]).to eq 'Target namespace must be provided'
+        end
+      end
+
+      context 'when the namespace is not a top-level namespace' do
+        let(:namespace) { subgroup }
+
+        it 'returns an error' do
+          expect(change_namespace[:status]).to eq :error
+          expect(change_namespace[:message]).to eq 'Namespace must be a top-level namespace'
+        end
+      end
+
+      context 'when the target namespace is not a top-level namespace' do
+        let(:target) { subgroup }
+
+        it 'returns an error' do
+          expect(change_namespace[:status]).to eq :error
+          expect(change_namespace[:message]).to eq 'Target namespace must be a top-level namespace'
+        end
+      end
+
+      context 'when the namespaces do not share an owner' do
+        let(:namespace) { create(:group) }
+
+        it 'returns an error' do
+          expect(change_namespace[:status]).to eq :error
+          expect(change_namespace[:message]).to eq 'Both namespaces must share the same owner'
+        end
+      end
+    end
+  end
+end