Commit d8d3b217 authored by Vijay Hawoldar's avatar Vijay Hawoldar

Allow changing AdditionalPack namespace

To support a customer changing a the namespace for their
purchased additional Ci minute packs, this commit will add
a new REST API endpoint and service

Changelog: added
EE: true
parent bf7b31d8
# 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
# 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
......@@ -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
......
......@@ -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
......
......@@ -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
# 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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment