diff options
| -rw-r--r-- | app/controllers/projects_controller.rb | 3 | ||||
| -rw-r--r-- | app/models/project.rb | 4 | ||||
| -rw-r--r-- | app/services/projects/update_service.rb | 45 | ||||
| -rw-r--r-- | spec/controllers/projects_controller_spec.rb | 42 | ||||
| -rw-r--r-- | spec/models/project_spec.rb | 2 | ||||
| -rw-r--r-- | spec/services/projects/update_service_spec.rb | 5 |
6 files changed, 72 insertions, 29 deletions
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 87a69e8e6f9..c769693255c 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -50,10 +50,13 @@ class ProjectsController < Projects::ApplicationController respond_to do |format| if result[:status] == :success flash[:notice] = _("Project '%{project_name}' was successfully updated.") % { project_name: @project.name } + format.html do redirect_to(edit_project_path(@project)) end else + flash[:alert] = result[:message] + format.html { render 'edit' } end diff --git a/app/models/project.rb b/app/models/project.rb index e50818e3dff..eec9037bd07 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -977,8 +977,6 @@ class Project < ActiveRecord::Base Rails.logger.error "Attempting to rename #{old_path_with_namespace} -> #{new_path_with_namespace}" - expire_caches_before_rename(old_path_with_namespace) - if has_container_registry_tags? Rails.logger.error "Project #{old_path_with_namespace} cannot be renamed because container registry tags are present!" @@ -986,6 +984,8 @@ class Project < ActiveRecord::Base raise StandardError.new('Project cannot be renamed, because images are present in its container registry') end + expire_caches_before_rename(old_path_with_namespace) + if gitlab_shell.mv_repository(repository_storage_path, old_path_with_namespace, new_path_with_namespace) # If repository moved successfully we need to send update instructions to users. # However we cannot allow rollback since we moved repository diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index 55d9cb13ae4..ad71fa2af15 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -1,21 +1,15 @@ module Projects class UpdateService < BaseService def execute - # check that user is allowed to set specified visibility_level - new_visibility = params[:visibility_level] - - if new_visibility && new_visibility.to_i != project.visibility_level - unless can?(current_user, :change_visibility_level, project) && - Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility) - - deny_visibility_level(project, new_visibility) - return error('Visibility level unallowed') - end + unless visibility_level_allowed? + return error('New visibility level not allowed!') end - new_branch = params[:default_branch] + if project.has_container_registry_tags? + return error('Cannot rename project because it contains container registry tags!') + end - if project.repository.exists? && new_branch && new_branch != project.default_branch + if changing_default_branch? project.change_head(new_branch) end @@ -28,8 +22,33 @@ module Projects success else - error('Project could not be updated') + error('Project could not be updated!') end end + + private + + def visibility_level_allowed? + # check that user is allowed to set specified visibility_level + new_visibility = params[:visibility_level] + + if new_visibility && new_visibility.to_i != project.visibility_level + unless can?(current_user, :change_visibility_level, project) && + Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility) + + deny_visibility_level(project, new_visibility) + return false + end + end + + true + end + + def changing_default_branch? + new_branch = params[:default_branch] + + project.repository.exists? && + new_branch && new_branch != project.default_branch + end end end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index f96fe7ad5cb..2a3c382b82c 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -211,24 +211,44 @@ describe ProjectsController do let(:admin) { create(:admin) } let(:project) { create(:project, :repository) } - let(:new_path) { 'renamed_path' } - let(:project_params) { { path: new_path } } before do sign_in(admin) end - it "sets the repository to the right path after a rename" do - controller.instance_variable_set(:@project, project) + context 'when only renaming a project path' do + it "sets the repository to the right path after a rename" do + expect { update_project path: 'renamed_path' } + .to change { project.reload.path } - put :update, - namespace_id: project.namespace, - id: project.id, - project: project_params + expect(project.reload.path).to include 'renamed_path' + expect(assigns(:repository).path).to include project.reload.path + expect(response).to have_http_status(302) + end + end - expect(project.repository.path).to include(new_path) - expect(assigns(:repository).path).to eq(project.repository.path) - expect(response).to have_http_status(302) + context 'when project has container repositories with tags' do + before do + stub_container_registry_config(enabled: true) + stub_container_registry_tags(repository: /image/, tags: %w[rc1]) + create(:container_repository, project: project, name: :image) + end + + it 'does not allow to rename the project' do + expect { update_project path: 'renamed_path' } + .not_to change { project.reload.path } + + expect(project.reload.path).to_not include 'renamed_path' + expect(controller).to set_flash[:alert].to(/container registry tags/) + expect(response).to have_http_status(200) + end + end + + def update_project(**parameters) + put :update, + namespace_id: project.namespace.path, + id: project.path, + project: parameters end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index c4bc129dd37..e636250c37d 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1236,7 +1236,7 @@ describe Project, models: true do subject { project.rename_repo } - it { expect{subject}.to raise_error(Exception) } + it { expect{subject}.to raise_error(StandardError) } end end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 05b18fef061..c974d470ae4 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -40,7 +40,7 @@ describe Projects::UpdateService, services: true do it 'does not update the project to public' do result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::PUBLIC) - expect(result).to eq({ status: :error, message: 'Visibility level unallowed' }) + expect(result).to eq({ status: :error, message: 'New visibility level not allowed!' }) expect(project).to be_private end @@ -92,7 +92,8 @@ describe Projects::UpdateService, services: true do it 'returns an error result when record cannot be updated' do result = update_project(project, admin, { name: 'foo&bar' }) - expect(result).to eq({ status: :error, message: 'Project could not be updated' }) + expect(result).to eq({ status: :error, + message: 'Project could not be updated!' }) end def update_project(project, user, opts) |