Commit 817cb54a authored by Etienne Baqué's avatar Etienne Baqué

Merge branch 'vij-modify-minutes-validation' into 'master'

Remove owner validation in AdditionalPack transfer

See merge request gitlab-org/gitlab!70790
parents 1370dc78 f5a1bcec
......@@ -16,7 +16,6 @@ module Ci
authorize_current_user!
validate_namespaces!
validate_owners!
Ci::Minutes::AdditionalPack.transaction do
update_packs
......@@ -50,20 +49,10 @@ module Ci
raise ChangeNamespaceError, 'Namespace and target must be different' if namespace == target
end
def validate_owners!
shared_ids = owner_ids_for(namespace) & owner_ids_for(target)
raise ChangeNamespaceError, 'Both namespaces must share the same owner' unless shared_ids.any?
end
def reset_ci_minutes!
::Ci::Minutes::RefreshCachedDataService.new(namespace).execute
::Ci::Minutes::RefreshCachedDataService.new(target).execute
end
def owner_ids_for(namespace)
namespace.user_namespace? ? Array(namespace.owner_id) : namespace.owner_ids
end
end
end
end
......
......@@ -116,18 +116,12 @@ RSpec.describe API::Ci::Minutes do
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
......@@ -175,17 +169,6 @@ RSpec.describe API::Ci::Minutes do
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 }
......
......@@ -79,31 +79,18 @@ RSpec.describe Ci::Minutes::AdditionalPacks::ChangeNamespaceService do
let!(:existing_packs) { create_list(:ci_minutes_additional_pack, 5, namespace: namespace) }
context 'when both namespaces are groups' do
before do
namespace.add_owner(admin)
target.add_owner(admin)
end
include_examples 'namespace change'
end
context 'when a namespace is a kind of user' do
let_it_be(:namespace) { admin.namespace }
before do
target.add_owner(admin)
end
include_examples 'namespace change'
end
context 'when a target is a kind of user' do
let(:target) { admin.namespace }
before do
namespace.add_owner(admin)
end
include_examples 'namespace change'
end
end
......@@ -144,15 +131,6 @@ RSpec.describe Ci::Minutes::AdditionalPacks::ChangeNamespaceService do
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
context 'when the namespace is the same as the target' do
let(:target) { namespace }
......
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