Commit 4d6fefc4 authored by David Fernandez's avatar David Fernandez

Fix a regression bug on NPM packages check

Do not check for NPM packages if there is no incoming update
for the group's path
parent 98d75c2e
...@@ -112,6 +112,7 @@ module EE ...@@ -112,6 +112,7 @@ module EE
def valid_path_change_with_npm_packages? def valid_path_change_with_npm_packages?
return true unless group.packages_feature_available? return true unless group.packages_feature_available?
return true if params[:path].blank?
return true if !group.has_parent? && group.path == params[:path] return true if !group.has_parent? && group.path == params[:path]
npm_packages = Packages::GroupPackagesFinder.new(current_user, group, package_type: :npm).execute npm_packages = Packages::GroupPackagesFinder.new(current_user, group, package_type: :npm).execute
......
...@@ -135,7 +135,7 @@ describe Groups::UpdateService, '#execute' do ...@@ -135,7 +135,7 @@ describe Groups::UpdateService, '#execute' do
context 'with project' do context 'with project' do
let(:project) { create(:project, namespace: group) } let(:project) { create(:project, namespace: group) }
shared_examples 'transfer not allowed' do shared_examples 'with packages' do
before do before do
stub_licensed_features(packages: true) stub_licensed_features(packages: true)
group.add_owner(user) group.add_owner(user)
...@@ -148,10 +148,16 @@ describe Groups::UpdateService, '#execute' do ...@@ -148,10 +148,16 @@ describe Groups::UpdateService, '#execute' do
expect(update_group(group, user, path: 'updated')).to be false expect(update_group(group, user, path: 'updated')).to be false
expect(group.errors[:path]).to include('cannot change when group contains projects with NPM packages') expect(group.errors[:path]).to include('cannot change when group contains projects with NPM packages')
end end
it 'allows name update' do
expect(update_group(group, user, name: 'Updated')).to be true
expect(group.errors).to be_empty
expect(group.name).to eq('Updated')
end
end end
end end
it_behaves_like 'transfer not allowed' it_behaves_like 'with packages'
context 'located in a subgroup' do context 'located in a subgroup' do
let(:subgroup) { create(:group, parent: group) } let(:subgroup) { create(:group, parent: group) }
...@@ -161,7 +167,7 @@ describe Groups::UpdateService, '#execute' do ...@@ -161,7 +167,7 @@ describe Groups::UpdateService, '#execute' do
subgroup.add_owner(user) subgroup.add_owner(user)
end end
it_behaves_like 'transfer not allowed' it_behaves_like 'with packages'
it 'does allow a path update if there is not a root namespace change' do it 'does allow a path update if there is not a root namespace change' do
expect(update_group(subgroup, user, path: 'updated')).to be true expect(update_group(subgroup, user, path: 'updated')).to be true
......
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