Fix renaming repository when name contains invalid chars under settings

4 files changed, 36 insertions, 3 deletions

diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb
index f06311511cc..032c3b182fb 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 921155e970b..b282aa52b25 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 7d9bd08385a..938d44efe29 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 e8b9e6b9238..6337daca9c7 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