Merge branch 'return-303-for-branch-deletion' into 'master'

Return status code 303 after a branch DELETE operation to avoid project deletion

Closes #14994

See merge request !3583

Signed-off-by: Rémy Coutable <remy@rymai.me>

4 files changed, 22 insertions, 2 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 47ea0360d9c..093decb7083 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -2,6 +2,9 @@ Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.7.0 (unreleased)
   - Don't attempt to look up an avatar in repo if repo directory does not exist (Stan hu)
+  - All images in discussions and wikis now link to their source files !3464 (Connor Shea).
+  - Improved Markdown rendering performance !3389 (Yorick Peterse)
+  - Don't attempt to look up an avatar in repo if repo directory does not exist (Stan Hu)
   - Preserve time notes/comments have been updated at when moving issue
   - Make HTTP(s) label consistent on clone bar (Stan Hu)
   - Expose label description in API (Mariusz Jachimowicz)
@@ -20,6 +23,7 @@ v 8.6.5
   - Fix importing from GitHub Enterprise. !3529
   - Check permissions when user attempts to import members from another project. !3535
   - Only update repository language if it is not set to improve performance. !3556
+  - Return status code 303 after a branch DELETE operation to avoid project deletion (Stan Hu). !3583
 
 v 8.6.4
   - Don't attempt to fetch any tags from a forked repo (Stan Hu)
diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb
index 657ee94cfd7..74150ad606b 100644
--- a/app/controllers/projects/application_controller.rb
+++ b/app/controllers/projects/application_controller.rb
@@ -68,7 +68,9 @@ class Projects::ApplicationController < ApplicationController
   end
 
   def require_non_empty_project
-    redirect_to namespace_project_path(@project.namespace, @project) if @project.empty_repo?
+    # Be sure to return status code 303 to avoid a double DELETE:
+    # http://api.rubyonrails.org/classes/ActionController/Redirecting.html
+    redirect_to namespace_project_path(@project.namespace, @project), status: 303 if @project.empty_repo?
   end
 
   def require_branch_head
diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb
index 43ea717cbd2..fb918cc7ddb 100644
--- a/app/controllers/projects/branches_controller.rb
+++ b/app/controllers/projects/branches_controller.rb
@@ -48,7 +48,7 @@ class Projects::BranchesController < Projects::ApplicationController
     respond_to do |format|
       format.html do
         redirect_to namespace_project_branches_path(@project.namespace,
-                                                    @project)
+                                                    @project), status: 303
       end
       format.js { render status: status[:return_code] }
     end
diff --git a/spec/controllers/projects/branches_controller_spec.rb b/spec/controllers/projects/branches_controller_spec.rb
index 98ae424ed7c..8ad73472117 100644
--- a/spec/controllers/projects/branches_controller_spec.rb
+++ b/spec/controllers/projects/branches_controller_spec.rb
@@ -93,6 +93,20 @@ describe Projects::BranchesController do
     end
   end
 
+  describe "POST destroy with HTML format" do
+    render_views
+
+    it 'returns 303' do
+      post :destroy,
+           format: :html,
+           id: 'foo/bar/baz',
+           namespace_id: project.namespace.to_param,
+           project_id: project.to_param
+
+      expect(response.status).to eq(303)
+    end
+  end
+
   describe "POST destroy" do
     render_views