[BUG] Detect protected branch on branch rename
- If a branch cannot be renamed due to a protected branch rule, show this error in the UI instead of throwing an internal server error. - Add integration test (also simplify the existing one). - Resolves #2751
This commit is contained in:
		
							parent
							
								
									5194bd15ef
								
							
						
					
					
						commit
						0f79122053
					
				
					 3 changed files with 61 additions and 23 deletions
				
			
		| 
						 | 
				
			
			@ -2513,6 +2513,7 @@ settings.lfs_pointers.inRepo=In Repo
 | 
			
		|||
settings.lfs_pointers.exists=Exists in store
 | 
			
		||||
settings.lfs_pointers.accessible=Accessible to User
 | 
			
		||||
settings.lfs_pointers.associateAccessible=Associate accessible %d OIDs
 | 
			
		||||
settings.rename_branch_failed_protected=Cannot rename branch %s because it is a protected branch.
 | 
			
		||||
settings.rename_branch_failed_exist=Cannot rename branch because target branch %s exists.
 | 
			
		||||
settings.rename_branch_failed_not_exist=Cannot rename branch %s because it does not exist.
 | 
			
		||||
settings.rename_branch_success =Branch %s was successfully renamed to %s.
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -4,6 +4,7 @@
 | 
			
		|||
package setting
 | 
			
		||||
 | 
			
		||||
import (
 | 
			
		||||
	"errors"
 | 
			
		||||
	"fmt"
 | 
			
		||||
	"net/http"
 | 
			
		||||
	"net/url"
 | 
			
		||||
| 
						 | 
				
			
			@ -316,7 +317,12 @@ func RenameBranchPost(ctx *context.Context) {
 | 
			
		|||
 | 
			
		||||
	msg, err := repository.RenameBranch(ctx, ctx.Repo.Repository, ctx.Doer, ctx.Repo.GitRepo, form.From, form.To)
 | 
			
		||||
	if err != nil {
 | 
			
		||||
		ctx.ServerError("RenameBranch", err)
 | 
			
		||||
		if errors.Is(err, git_model.ErrBranchIsProtected) {
 | 
			
		||||
			ctx.Flash.Error(ctx.Tr("repo.settings.rename_branch_failed_protected", form.To))
 | 
			
		||||
			ctx.Redirect(fmt.Sprintf("%s/branches", ctx.Repo.RepoLink))
 | 
			
		||||
		} else {
 | 
			
		||||
			ctx.ServerError("RenameBranch", err)
 | 
			
		||||
		}
 | 
			
		||||
		return
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -10,6 +10,7 @@ import (
 | 
			
		|||
	git_model "code.gitea.io/gitea/models/git"
 | 
			
		||||
	repo_model "code.gitea.io/gitea/models/repo"
 | 
			
		||||
	"code.gitea.io/gitea/models/unittest"
 | 
			
		||||
	gitea_context "code.gitea.io/gitea/services/context"
 | 
			
		||||
	"code.gitea.io/gitea/tests"
 | 
			
		||||
 | 
			
		||||
	"github.com/stretchr/testify/assert"
 | 
			
		||||
| 
						 | 
				
			
			@ -18,33 +19,63 @@ import (
 | 
			
		|||
func TestRenameBranch(t *testing.T) {
 | 
			
		||||
	defer tests.PrepareTestEnv(t)()
 | 
			
		||||
 | 
			
		||||
	unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: 1, Name: "master"})
 | 
			
		||||
	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
 | 
			
		||||
	unittest.AssertExistsAndLoadBean(t, &git_model.Branch{RepoID: repo.ID, Name: "master"})
 | 
			
		||||
 | 
			
		||||
	// get branch setting page
 | 
			
		||||
	session := loginUser(t, "user2")
 | 
			
		||||
	req := NewRequest(t, "GET", "/user2/repo1/settings/branches")
 | 
			
		||||
	resp := session.MakeRequest(t, req, http.StatusOK)
 | 
			
		||||
	htmlDoc := NewHTMLParser(t, resp.Body)
 | 
			
		||||
	t.Run("Normal", func(t *testing.T) {
 | 
			
		||||
		defer tests.PrintCurrentTest(t)()
 | 
			
		||||
 | 
			
		||||
	postData := map[string]string{
 | 
			
		||||
		"_csrf": htmlDoc.GetCSRF(),
 | 
			
		||||
		"from":  "master",
 | 
			
		||||
		"to":    "main",
 | 
			
		||||
	}
 | 
			
		||||
	req = NewRequestWithValues(t, "POST", "/user2/repo1/settings/rename_branch", postData)
 | 
			
		||||
	session.MakeRequest(t, req, http.StatusSeeOther)
 | 
			
		||||
		req := NewRequestWithValues(t, "POST", "/user2/repo1/settings/rename_branch", map[string]string{
 | 
			
		||||
			"_csrf": GetCSRF(t, session, "/user2/repo1/settings/branches"),
 | 
			
		||||
			"from":  "master",
 | 
			
		||||
			"to":    "main",
 | 
			
		||||
		})
 | 
			
		||||
		session.MakeRequest(t, req, http.StatusSeeOther)
 | 
			
		||||
 | 
			
		||||
	// check new branch link
 | 
			
		||||
	req = NewRequestWithValues(t, "GET", "/user2/repo1/src/branch/main/README.md", postData)
 | 
			
		||||
	session.MakeRequest(t, req, http.StatusOK)
 | 
			
		||||
		// check new branch link
 | 
			
		||||
		req = NewRequest(t, "GET", "/user2/repo1/src/branch/main/README.md")
 | 
			
		||||
		session.MakeRequest(t, req, http.StatusOK)
 | 
			
		||||
 | 
			
		||||
	// check old branch link
 | 
			
		||||
	req = NewRequestWithValues(t, "GET", "/user2/repo1/src/branch/master/README.md", postData)
 | 
			
		||||
	resp = session.MakeRequest(t, req, http.StatusSeeOther)
 | 
			
		||||
	location := resp.Header().Get("Location")
 | 
			
		||||
	assert.Equal(t, "/user2/repo1/src/branch/main/README.md", location)
 | 
			
		||||
		// check old branch link
 | 
			
		||||
		req = NewRequest(t, "GET", "/user2/repo1/src/branch/master/README.md")
 | 
			
		||||
		resp := session.MakeRequest(t, req, http.StatusSeeOther)
 | 
			
		||||
		location := resp.Header().Get("Location")
 | 
			
		||||
		assert.Equal(t, "/user2/repo1/src/branch/main/README.md", location)
 | 
			
		||||
 | 
			
		||||
	// check db
 | 
			
		||||
	repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
 | 
			
		||||
	assert.Equal(t, "main", repo1.DefaultBranch)
 | 
			
		||||
		// check db
 | 
			
		||||
		repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
 | 
			
		||||
		assert.Equal(t, "main", repo1.DefaultBranch)
 | 
			
		||||
	})
 | 
			
		||||
 | 
			
		||||
	t.Run("Protected branch", func(t *testing.T) {
 | 
			
		||||
		defer tests.PrintCurrentTest(t)()
 | 
			
		||||
 | 
			
		||||
		// Add protected branch
 | 
			
		||||
		req := NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/edit", map[string]string{
 | 
			
		||||
			"_csrf":       GetCSRF(t, session, "/user2/repo1/settings/branches/edit"),
 | 
			
		||||
			"rule_name":   "*",
 | 
			
		||||
			"enable_push": "true",
 | 
			
		||||
		})
 | 
			
		||||
		session.MakeRequest(t, req, http.StatusSeeOther)
 | 
			
		||||
 | 
			
		||||
		// Verify it was added.
 | 
			
		||||
		unittest.AssertExistsIf(t, true, &git_model.ProtectedBranch{RuleName: "*", RepoID: repo.ID})
 | 
			
		||||
 | 
			
		||||
		req = NewRequestWithValues(t, "POST", "/user2/repo1/settings/rename_branch", map[string]string{
 | 
			
		||||
			"_csrf": GetCSRF(t, session, "/user2/repo1/settings/branches"),
 | 
			
		||||
			"from":  "main",
 | 
			
		||||
			"to":    "main2",
 | 
			
		||||
		})
 | 
			
		||||
		session.MakeRequest(t, req, http.StatusSeeOther)
 | 
			
		||||
 | 
			
		||||
		flashCookie := session.GetCookie(gitea_context.CookieNameFlash)
 | 
			
		||||
		assert.NotNil(t, flashCookie)
 | 
			
		||||
		assert.EqualValues(t, "error%3DCannot%2Brename%2Bbranch%2Bmain2%2Bbecause%2Bit%2Bis%2Ba%2Bprotected%2Bbranch.", flashCookie.Value)
 | 
			
		||||
 | 
			
		||||
		// Verify it didn't change.
 | 
			
		||||
		repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
 | 
			
		||||
		assert.Equal(t, "main", repo1.DefaultBranch)
 | 
			
		||||
	})
 | 
			
		||||
}
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue