Make branch deletion URL more like GitHub's, fixes #1397 (#1994)

* Make branch deletion URL more like GitHub's, fixes #1397

* Add PR branch deletion integration test

* Do not allow deleting protected branch

* Change http error code to 403 if user has no write rights to repository

* Add check to not panic if forked repository has alrady been deleted

3 files changed, 103 insertions, 0 deletions

diff --git a/integrations/editor_test.go b/integrations/editor_test.go
index 24cb8f1967..32eb07fa91 100644
--- a/integrations/editor_test.go
+++ b/integrations/editor_test.go
@@ -126,8 +126,51 @@ func testEditFile(t *testing.T, session *TestSession, user, repo, branch, filePa
 	return resp
 }
 
+func testEditFileToNewBranch(t *testing.T, session *TestSession, user, repo, branch, targetBranch, filePath string) *TestResponse {
+
+	newContent := "Hello, World (Edited)\n"
+
+	// Get to the 'edit this file' page
+	req := NewRequest(t, "GET", path.Join(user, repo, "_edit", branch, filePath))
+	resp := session.MakeRequest(t, req)
+	assert.EqualValues(t, http.StatusOK, resp.HeaderCode)
+
+	htmlDoc := NewHTMLParser(t, resp.Body)
+	lastCommit := htmlDoc.GetInputValueByName("last_commit")
+	assert.NotEmpty(t, lastCommit)
+
+	// Submit the edits
+	req = NewRequestWithValues(t, "POST", path.Join(user, repo, "_edit", branch, filePath),
+		map[string]string{
+			"_csrf":           htmlDoc.GetCSRF(),
+			"last_commit":     lastCommit,
+			"tree_path":       filePath,
+			"content":         newContent,
+			"commit_choice":   "commit-to-new-branch",
+			"new_branch_name": targetBranch,
+		},
+	)
+	req.Header.Add("Content-Type", "application/x-www-form-urlencoded")
+	resp = session.MakeRequest(t, req)
+	assert.EqualValues(t, http.StatusFound, resp.HeaderCode)
+
+	// Verify the change
+	req = NewRequest(t, "GET", path.Join(user, repo, "raw", targetBranch, filePath))
+	resp = session.MakeRequest(t, req)
+	assert.EqualValues(t, http.StatusOK, resp.HeaderCode)
+	assert.EqualValues(t, newContent, string(resp.Body))
+
+	return resp
+}
+
 func TestEditFile(t *testing.T) {
 	prepareTestEnv(t)
 	session := loginUser(t, "user2")
 	testEditFile(t, session, "user2", "repo1", "master", "README.md")
 }
+
+func TestEditFileToNewBranch(t *testing.T) {
+	prepareTestEnv(t)
+	session := loginUser(t, "user2")
+	testEditFileToNewBranch(t, session, "user2", "repo1", "master", "feature/test", "README.md")
+}
diff --git a/integrations/pull_create_test.go b/integrations/pull_create_test.go
index 91f682fbda..9e9ba9eb3a 100644
--- a/integrations/pull_create_test.go
+++ b/integrations/pull_create_test.go
@@ -7,6 +7,7 @@ package integrations
 import (
 	"net/http"
 	"path"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -21,6 +22,9 @@ func testPullCreate(t *testing.T, session *TestSession, user, repo, branch strin
 	htmlDoc := NewHTMLParser(t, resp.Body)
 	link, exists := htmlDoc.doc.Find("button.ui.green.small.button").Parent().Attr("href")
 	assert.True(t, exists, "The template has changed")
+	if branch != "master" {
+		link = strings.Replace(link, ":master", ":"+branch, 1)
+	}
 
 	req = NewRequest(t, "GET", link)
 	resp = session.MakeRequest(t, req)
diff --git a/integrations/pull_merge_test.go b/integrations/pull_merge_test.go
index acb48bd781..da3103bd33 100644
--- a/integrations/pull_merge_test.go
+++ b/integrations/pull_merge_test.go
@@ -32,6 +32,25 @@ func testPullMerge(t *testing.T, session *TestSession, user, repo, pullnum strin
 	return resp
 }
 
+func testPullCleanUp(t *testing.T, session *TestSession, user, repo, pullnum string) *TestResponse {
+	req := NewRequest(t, "GET", path.Join(user, repo, "pulls", pullnum))
+	resp := session.MakeRequest(t, req)
+	assert.EqualValues(t, http.StatusOK, resp.HeaderCode)
+
+	// Click the little green button to craete a pull
+	htmlDoc := NewHTMLParser(t, resp.Body)
+	link, exists := htmlDoc.doc.Find(".comments .merge .delete-button").Attr("data-url")
+	assert.True(t, exists, "The template has changed")
+	req = NewRequestWithValues(t, "POST", link, map[string]string{
+		"_csrf": htmlDoc.GetCSRF(),
+	})
+	req.Header.Add("Content-Type", "application/x-www-form-urlencoded")
+	resp = session.MakeRequest(t, req)
+	assert.EqualValues(t, http.StatusOK, resp.HeaderCode)
+
+	return resp
+}
+
 func TestPullMerge(t *testing.T) {
 	prepareTestEnv(t)
 	session := loginUser(t, "user1")
@@ -46,3 +65,40 @@ func TestPullMerge(t *testing.T) {
 	assert.EqualValues(t, "pulls", elem[3])
 	testPullMerge(t, session, elem[1], elem[2], elem[4])
 }
+
+func TestPullCleanUpAfterMerge(t *testing.T) {
+	prepareTestEnv(t)
+	session := loginUser(t, "user1")
+	testRepoFork(t, session)
+	testEditFileToNewBranch(t, session, "user1", "repo1", "master", "feature/test", "README.md")
+
+	resp := testPullCreate(t, session, "user1", "repo1", "feature/test")
+	redirectedURL := resp.Headers["Location"]
+	assert.NotEmpty(t, redirectedURL, "Redirected URL is not found")
+
+	elem := strings.Split(redirectedURL[0], "/")
+	assert.EqualValues(t, "pulls", elem[3])
+	testPullMerge(t, session, elem[1], elem[2], elem[4])
+
+	// Check PR branch deletion
+	resp = testPullCleanUp(t, session, elem[1], elem[2], elem[4])
+	respJSON := struct {
+		Redirect string
+	}{}
+	DecodeJSON(t, resp, &respJSON)
+
+	assert.NotEmpty(t, respJSON.Redirect, "Redirected URL is not found")
+
+	elem = strings.Split(respJSON.Redirect, "/")
+	assert.EqualValues(t, "pulls", elem[3])
+
+	// Check branch deletion result
+	req := NewRequest(t, "GET", respJSON.Redirect)
+	resp = session.MakeRequest(t, req)
+	assert.EqualValues(t, http.StatusOK, resp.HeaderCode)
+
+	htmlDoc := NewHTMLParser(t, resp.Body)
+	resultMsg := htmlDoc.doc.Find(".ui.message>p").Text()
+
+	assert.EqualValues(t, "user1/feature/test has been deleted.", resultMsg)
+}