Fix project settings not giving updates upon failuresh-fix-project-settings-not-showing-error

When project settings cannot be updated, the AJAX request would returned
a snippet of JavaScript that was never displayed. To fix this, we force
the controller to redirect to the edit page by disabling remote fields.
It turns out in this section we don't use remote fields in any case.

3 files changed, 46 insertions, 14 deletions

diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 12db493978b..6b3b767e155 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -77,13 +77,19 @@ class ProjectsController < Projects::ApplicationController
         format.html do
           redirect_to(edit_project_path(@project, anchor: 'js-general-project-settings'))
         end
+
+        format.js
       else
-        flash.now[:alert] = result[:message]
+        flash[:alert] = result[:message]
 
-        format.html { render 'edit' }
-      end
+        format.html do
+          redirect_to(edit_project_path(@project, anchor: 'js-general-project-settings'))
+        end
 
-      format.js
+        format.js do
+          render status: :unprocessable_entity
+        end
+      end
     end
   end
 
diff --git a/app/views/projects/settings/_general.html.haml b/app/views/projects/settings/_general.html.haml
index 520f342f567..544e5018a95 100644
--- a/app/views/projects/settings/_general.html.haml
+++ b/app/views/projects/settings/_general.html.haml
@@ -1,4 +1,4 @@
-= form_for [@project.namespace.becomes(Namespace), @project], remote: true, html: { multipart: true, class: "edit-project js-general-settings-form" }, authenticity_token: true do |f|
+= form_for [@project.namespace.becomes(Namespace), @project], html: { multipart: true, class: "edit-project js-general-settings-form" }, authenticity_token: true do |f|
   %input{ name: 'update_section', type: 'hidden', value: 'js-general-settings' }
   = form_errors(@project)
 
diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
index 7f1bbebd128..c061f0d00c4 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -315,6 +315,41 @@ describe ProjectsController do
       sign_in(admin)
     end
 
+    def update_project(**parameters)
+      put :update,
+          params: {
+            format: format,
+            namespace_id: project.namespace.path,
+            id: project.path,
+            project: parameters
+          }
+    end
+
+    it 'displays error messages' do
+      project.update(import_url: 'http://www.google.com')
+
+      expect { update_project import_url: 'bogus-url' }.not_to change { project.reload.import_url }
+
+      expect(controller).to set_flash[:alert]
+      expect(response).to have_gitlab_http_status(302)
+    end
+
+    it 'renders JSON error message' do
+      project.update(import_url: 'http://www.google.com')
+
+      expect do
+        put :update,
+            params: {
+              format: :js,
+              namespace_id: project.namespace.path,
+              id: project.path,
+              project: { import_url: 'bogus-url' }
+            }
+      end.not_to change { project.reload.import_url }
+
+      expect(response).to have_gitlab_http_status(422)
+    end
+
     shared_examples_for 'updating a project' do
       context 'when only renaming a project path' do
         it "sets the repository to the right path after a rename" do
@@ -392,15 +427,6 @@ describe ProjectsController do
             }
         end.not_to change { project.namespace.reload }
       end
-
-      def update_project(**parameters)
-        put :update,
-            params: {
-              namespace_id: project.namespace.path,
-              id: project.path,
-              project: parameters
-            }
-      end
     end
 
     context 'hashed storage' do