From e38e502e209e2fb826b997d807361551762e24ae Mon Sep 17 00:00:00 2001
From: David Schneiderbauer <daviian@users.noreply.github.com>
Date: Mon, 2 Oct 2017 22:23:41 +0200
Subject: [PATCH] Fix deletion of unprotected branches (#2630)

* fix deletion of unprotected branches

* fmt fix

* changed internal protected branch api

* fix lint error

Signed-off-by: David Schneiderbauer <dschneiderbauer@gmail.com>
---
 cmd/hook.go                   |  2 +-
 integrations/internal_test.go | 10 +++++-----
 models/branches.go            |  5 +++++
 routers/private/branch.go     |  7 +------
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/cmd/hook.go b/cmd/hook.go
index 6adf353ec..9a6fc6778 100644
--- a/cmd/hook.go
+++ b/cmd/hook.go
@@ -126,7 +126,7 @@ func runHookPreReceive(c *cli.Context) error {
 			log.GitLogger.Fatal(2, "retrieve protected branches information failed")
 		}
 
-		if protectBranch != nil {
+		if protectBranch != nil && protectBranch.IsProtected() {
 			// check and deletion
 			if newCommitID == git.EmptySHA {
 				fail(fmt.Sprintf("branch %s is protected from deletion", branchName), "")
diff --git a/integrations/internal_test.go b/integrations/internal_test.go
index c22e951bc..dd203d89f 100644
--- a/integrations/internal_test.go
+++ b/integrations/internal_test.go
@@ -17,7 +17,7 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func assertProtectedBranch(t *testing.T, repoID int64, branchName string, isErr, canPush bool) {
+func assertProtectedBranch(t *testing.T, repoID int64, branchName string, isErr, isProtected bool) {
 	reqURL := fmt.Sprintf("/api/internal/branch/%d/%s", repoID, url.QueryEscape(branchName))
 	req := NewRequest(t, "GET", reqURL)
 	t.Log(reqURL)
@@ -31,14 +31,14 @@ func assertProtectedBranch(t *testing.T, repoID int64, branchName string, isErr,
 		var branch models.ProtectedBranch
 		t.Log(string(resp.Body))
 		assert.NoError(t, json.Unmarshal(resp.Body, &branch))
-		assert.Equal(t, canPush, branch.CanPush)
+		assert.Equal(t, isProtected, branch.IsProtected())
 	}
 }
 
 func TestInternal_GetProtectedBranch(t *testing.T) {
 	prepareTestEnv(t)
 
-	assertProtectedBranch(t, 1, "master", false, true)
-	assertProtectedBranch(t, 1, "dev", false, true)
-	assertProtectedBranch(t, 1, "lunny/dev", false, true)
+	assertProtectedBranch(t, 1, "master", false, false)
+	assertProtectedBranch(t, 1, "dev", false, false)
+	assertProtectedBranch(t, 1, "lunny/dev", false, false)
 }
diff --git a/models/branches.go b/models/branches.go
index 4e8ed0522..383aa8226 100644
--- a/models/branches.go
+++ b/models/branches.go
@@ -38,6 +38,11 @@ func (protectBranch *ProtectedBranch) BeforeUpdate() {
 	protectBranch.UpdatedUnix = time.Now().Unix()
 }
 
+// IsProtected returns if the branch is protected
+func (protectBranch *ProtectedBranch) IsProtected() bool {
+	return protectBranch.ID > 0
+}
+
 // GetProtectedBranchByRepoID getting protected branch by repo ID
 func GetProtectedBranchByRepoID(RepoID int64) ([]*ProtectedBranch, error) {
 	protectedBranches := make([]*ProtectedBranch, 0)
diff --git a/routers/private/branch.go b/routers/private/branch.go
index 8e42f7303..e3c6ba7fd 100644
--- a/routers/private/branch.go
+++ b/routers/private/branch.go
@@ -20,11 +20,6 @@ func GetProtectedBranchBy(ctx *macaron.Context) {
 			"err": err.Error(),
 		})
 		return
-	} else if protectBranch != nil {
-		ctx.JSON(200, protectBranch)
-	} else {
-		ctx.JSON(200, &models.ProtectedBranch{
-			CanPush: true,
-		})
 	}
+	ctx.JSON(200, protectBranch)
 }