Commit f5a1bcec authored by Vijay Hawoldar's avatar Vijay Hawoldar

Remove owner validation in AdditionalPack transfer

When changing the namespace for Ci::Minutes::AdditionalPacks, we do not
need to validate that both the source and target namespaces share an
owner

Changelog: changed
EE: true
parent 95fa62fe
...@@ -16,7 +16,6 @@ module Ci ...@@ -16,7 +16,6 @@ module Ci
authorize_current_user! authorize_current_user!
validate_namespaces! validate_namespaces!
validate_owners!
Ci::Minutes::AdditionalPack.transaction do Ci::Minutes::AdditionalPack.transaction do
update_packs update_packs
...@@ -50,20 +49,10 @@ module Ci ...@@ -50,20 +49,10 @@ module Ci
raise ChangeNamespaceError, 'Namespace and target must be different' if namespace == target raise ChangeNamespaceError, 'Namespace and target must be different' if namespace == target
end 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! def reset_ci_minutes!
::Ci::Minutes::RefreshCachedDataService.new(namespace).execute ::Ci::Minutes::RefreshCachedDataService.new(namespace).execute
::Ci::Minutes::RefreshCachedDataService.new(target).execute ::Ci::Minutes::RefreshCachedDataService.new(target).execute
end end
def owner_ids_for(namespace)
namespace.user_namespace? ? Array(namespace.owner_id) : namespace.owner_ids
end
end end
end end
end end
......
...@@ -116,18 +116,12 @@ RSpec.describe API::Ci::Minutes do ...@@ -116,18 +116,12 @@ RSpec.describe API::Ci::Minutes do
describe 'PATCH /namespaces/:id/minutes/move/:target_id' do describe 'PATCH /namespaces/:id/minutes/move/:target_id' do
let_it_be(:namespace) { create(:group) } let_it_be(:namespace) { create(:group) }
let_it_be(:target) { create(:group, owner: namespace.owner) } 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(:child_namespace) { create(:group, :nested) }
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let(:namespace_id) { namespace.id } let(:namespace_id) { namespace.id }
let(:target_id) { target.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) } subject(:move_packs) { patch api("/namespaces/#{namespace_id}/minutes/move/#{target_id}", user) }
context 'when unauthorized' do context 'when unauthorized' do
...@@ -175,17 +169,6 @@ RSpec.describe API::Ci::Minutes do ...@@ -175,17 +169,6 @@ RSpec.describe API::Ci::Minutes do
end end
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 context 'when the transfer is successful' do
let(:target_id) { target.id } let(:target_id) { target.id }
......
...@@ -79,31 +79,18 @@ RSpec.describe Ci::Minutes::AdditionalPacks::ChangeNamespaceService do ...@@ -79,31 +79,18 @@ RSpec.describe Ci::Minutes::AdditionalPacks::ChangeNamespaceService do
let!(:existing_packs) { create_list(:ci_minutes_additional_pack, 5, namespace: namespace) } let!(:existing_packs) { create_list(:ci_minutes_additional_pack, 5, namespace: namespace) }
context 'when both namespaces are groups' do context 'when both namespaces are groups' do
before do
namespace.add_owner(admin)
target.add_owner(admin)
end
include_examples 'namespace change' include_examples 'namespace change'
end end
context 'when a namespace is a kind of user' do context 'when a namespace is a kind of user' do
let_it_be(:namespace) { admin.namespace } let_it_be(:namespace) { admin.namespace }
before do
target.add_owner(admin)
end
include_examples 'namespace change' include_examples 'namespace change'
end end
context 'when a target is a kind of user' do context 'when a target is a kind of user' do
let(:target) { admin.namespace } let(:target) { admin.namespace }
before do
namespace.add_owner(admin)
end
include_examples 'namespace change' include_examples 'namespace change'
end end
end end
...@@ -144,15 +131,6 @@ RSpec.describe Ci::Minutes::AdditionalPacks::ChangeNamespaceService do ...@@ -144,15 +131,6 @@ RSpec.describe Ci::Minutes::AdditionalPacks::ChangeNamespaceService do
end end
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 context 'when the namespace is the same as the target' do
let(:target) { namespace } 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