diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index f06311511cce7cc782c1e8748f8a9cc330b7de0a..032c3b182fb44fc20394a2af9a5173c3328de383 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -3,7 +3,7 @@ module Projects 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) @@ -23,7 +23,17 @@ module Projects if project.previous_changes.include?('path') project.rename_repo end + else + restore_attributes + false end end + + private + + def restore_attributes + project.path = project.path_was if project.errors.include?(:path) + project.name = project.name_was if project.errors.include?(:name) + end end end diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml index 921155e970be937d2913fbbfbf2d4c9c5c0c34d6..b282aa52b25d0a6eadcbd7b1ac1d05062f859f5c 100644 --- a/app/views/projects/edit.html.haml +++ b/app/views/projects/edit.html.haml @@ -4,6 +4,7 @@ %h4.prepend-top-0 Project settings .col-lg-9 + .project-edit-errors = form_for [@project.namespace.becomes(Namespace), @project], remote: true, html: { multipart: true, class: "edit-project" }, authenticity_token: true do |f| %fieldset.append-bottom-0 .form-group @@ -190,6 +191,7 @@ %h4.prepend-top-0.warning-title Rename repository .col-lg-9 + = render 'projects/errors' = form_for([@project.namespace.becomes(Namespace), @project]) do |f| .form-group.project_name_holder = f.label :name, class: 'label-light' do diff --git a/app/views/projects/update.js.haml b/app/views/projects/update.js.haml index 7d9bd08385afc67ae89793a40558547530e01598..938d44efe2952c152b4923aad51b4dd75d753dbb 100644 --- a/app/views/projects/update.js.haml +++ b/app/views/projects/update.js.haml @@ -1,4 +1,4 @@ -- if @project.valid? +- if @project.errors.blank? :plain location.href = "#{edit_namespace_project_path(@project.namespace, @project)}"; - else @@ -6,4 +6,4 @@ $(".project-edit-errors").html("#{escape_javascript(render('errors'))}"); $('.save-project-loader').hide(); $('.project-edit-container').show(); - $('.project-edit-content .btn-save').enable(); + $('.edit-project .btn-save').enable(); diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index e8b9e6b923840a4a5ac2406201aefd2d6950e3c0..6337daca9c74888c4727bce1273e77dc05667718 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -139,6 +139,27 @@ describe Projects::UpdateService, services: true do end end + context 'for invalid project path/name' do + let(:user) { create(:user, admin: true) } + let(:project) { create(:empty_project, path: 'gitlab', name: 'sample') } + let(:params) { { path: 'foo&bar', name: 'foo&bar' } } + + it 'resets to previous values to keep project in a valid state' do + update_project(project, user, params) + + expect(project.path).to eq 'gitlab' + expect(project.name).to eq 'sample' + end + + it 'keeps error messages' do + update_project(project, user, params) + + expect(project.errors).not_to be_blank + expect(project.errors[:name]).to include("can contain only letters, digits, '_', '.', dash and space. It must start with letter, digit or '_'.") + expect(project.errors[:path]).to include("can contain only letters, digits, '_', '-' and '.'. Cannot start with '-', end in '.git' or end in '.atom'") + end + end + def update_project(project, user, opts) Projects::UpdateService.new(project, user, opts).execute end