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

@@ -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), "")

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)
 }

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)

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)
 }