diff --git a/models/user/user.go b/models/user/user.go index 925be83713..1797d3eefc 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -1461,3 +1461,15 @@ func GetUserOrOrgIDByName(ctx context.Context, name string) (int64, error) { } return id, nil } + +// GetUserOrOrgByName returns the user or org by name +func GetUserOrOrgByName(ctx context.Context, name string) (*User, error) { + var u User + has, err := db.GetEngine(ctx).Where("lower_name = ?", strings.ToLower(name)).Get(&u) + if err != nil { + return nil, err + } else if !has { + return nil, ErrUserNotExist{Name: name} + } + return &u, nil +} diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index b740a400a4..b5bf0031ac 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1863,6 +1863,7 @@ pulls.desc = Enable pull requests and code reviews. pulls.new = New Pull Request pulls.new.blocked_user = Cannot create pull request because you are blocked by the repository owner. pulls.new.must_collaborator = You must be a collaborator to create pull request. +pulls.new.already_existed = A pull request between these branches already exists pulls.edit.already_changed = Unable to save changes to the pull request. It appears the content has already been changed by another user. Please refresh the page and try editing again to avoid overwriting their changes. pulls.view = View Pull Request pulls.compare_changes = New Pull Request diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 073c784242..b422c36d29 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -30,6 +30,7 @@ import ( "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers/api/v1/utils" + "code.gitea.io/gitea/routers/common" asymkey_service "code.gitea.io/gitea/services/asymkey" "code.gitea.io/gitea/services/automerge" "code.gitea.io/gitea/services/context" @@ -1082,7 +1083,7 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) } else if len(headInfos) == 2 { // There is a head repository (the head repository could also be the same base repo) headRefToGuess = headInfos[1] - headUser, err = user_model.GetUserByName(ctx, headInfos[0]) + headUser, err = user_model.GetUserOrOrgByName(ctx, headInfos[0]) if err != nil { if user_model.IsErrUserNotExist(err) { ctx.APIErrorNotFound("GetUserByName") @@ -1098,28 +1099,23 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) isSameRepo := ctx.Repo.Owner.ID == headUser.ID - // Check if current user has fork of repository or in the same repository. - headRepo := repo_model.GetForkedRepo(ctx, headUser.ID, baseRepo.ID) - if headRepo == nil && !isSameRepo { - err = baseRepo.GetBaseRepo(ctx) + var headRepo *repo_model.Repository + if isSameRepo { + headRepo = baseRepo + } else { + headRepo, err = common.FindHeadRepo(ctx, baseRepo, headUser.ID) if err != nil { ctx.APIErrorInternal(err) return nil, nil } - - // Check if baseRepo's base repository is the same as headUser's repository. - if baseRepo.BaseRepo == nil || baseRepo.BaseRepo.OwnerID != headUser.ID { - log.Trace("parseCompareInfo[%d]: does not have fork or in same repository", baseRepo.ID) - ctx.APIErrorNotFound("GetBaseRepo") + if headRepo == nil { + ctx.APIErrorNotFound("head repository not found") return nil, nil } - // Assign headRepo so it can be used below. - headRepo = baseRepo.BaseRepo } var headGitRepo *git.Repository if isSameRepo { - headRepo = ctx.Repo.Repository headGitRepo = ctx.Repo.GitRepo closer = func() {} // no need to close the head repo because it shares the base repo } else { @@ -1143,7 +1139,7 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) return nil, nil } - if !permBase.CanReadIssuesOrPulls(true) || !permBase.CanRead(unit.TypeCode) { + if !permBase.CanRead(unit.TypeCode) { log.Trace("Permission Denied: User %-v cannot create/read pull requests or cannot read code in Repo %-v\nUser in baseRepo has Permissions: %-+v", ctx.Doer, baseRepo, permBase) ctx.APIErrorNotFound("Can't read pulls or can't read UnitTypeCode") return nil, nil diff --git a/routers/common/compare.go b/routers/common/compare.go index fda31a07ba..be689bbdb5 100644 --- a/routers/common/compare.go +++ b/routers/common/compare.go @@ -4,6 +4,8 @@ package common import ( + "context" + repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" @@ -20,3 +22,54 @@ type CompareInfo struct { HeadBranch string DirectComparison bool } + +// maxForkTraverseLevel defines the maximum levels to traverse when searching for the head repository. +const maxForkTraverseLevel = 10 + +// FindHeadRepo tries to find the head repository based on the base repository and head user ID. +func FindHeadRepo(ctx context.Context, baseRepo *repo_model.Repository, headUserID int64) (*repo_model.Repository, error) { + if baseRepo.IsFork { + curRepo := baseRepo + for curRepo.OwnerID != headUserID { // We assume the fork deepth is not too deep. + if err := curRepo.GetBaseRepo(ctx); err != nil { + return nil, err + } + if curRepo.BaseRepo == nil { + return findHeadRepoFromRootBase(ctx, curRepo, headUserID, maxForkTraverseLevel) + } + curRepo = curRepo.BaseRepo + } + return curRepo, nil + } + + return findHeadRepoFromRootBase(ctx, baseRepo, headUserID, maxForkTraverseLevel) +} + +func findHeadRepoFromRootBase(ctx context.Context, baseRepo *repo_model.Repository, headUserID int64, traverseLevel int) (*repo_model.Repository, error) { + if traverseLevel == 0 { + return nil, nil + } + // test if we are lucky + repo, err := repo_model.GetUserFork(ctx, baseRepo.ID, headUserID) + if err != nil { + return nil, err + } + if repo != nil { + return repo, nil + } + + firstLevelForkedRepos, err := repo_model.GetRepositoriesByForkID(ctx, baseRepo.ID) + if err != nil { + return nil, err + } + for _, repo := range firstLevelForkedRepos { + forked, err := findHeadRepoFromRootBase(ctx, repo, headUserID, traverseLevel-1) + if err != nil { + return nil, err + } + if forked != nil { + return forked, nil + } + } + return nil, nil +} diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 7750278a8d..f66dabbf87 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -259,7 +259,7 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { } else if len(headInfos) == 2 { headInfosSplit := strings.Split(headInfos[0], "/") if len(headInfosSplit) == 1 { - ci.HeadUser, err = user_model.GetUserByName(ctx, headInfos[0]) + ci.HeadUser, err = user_model.GetUserOrOrgByName(ctx, headInfos[0]) if err != nil { if user_model.IsErrUserNotExist(err) { ctx.NotFound(nil) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 4353e00840..488389e204 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1340,6 +1340,17 @@ func CompareAndPullRequestPost(ctx *context.Context) { return } + // Check if a pull request already exists with the same head and base branch. + pr, err := issues_model.GetUnmergedPullRequest(ctx, ci.HeadRepo.ID, repo.ID, ci.HeadBranch, ci.BaseBranch, issues_model.PullRequestFlowGithub) + if err != nil && !issues_model.IsErrPullRequestNotExist(err) { + ctx.ServerError("GetUnmergedPullRequest", err) + return + } + if pr != nil { + ctx.JSONError(ctx.Tr("repo.pulls.new.already_existed")) + return + } + content := form.Content if filename := ctx.Req.Form.Get("template-file"); filename != "" { if template, err := issue_template.UnmarshalFromRepo(ctx.Repo.GitRepo, ctx.Repo.Repository.DefaultBranch, filename); err == nil { diff --git a/tests/integration/pull_create_test.go b/tests/integration/pull_create_test.go index ddafdf33b8..ff60b70cf9 100644 --- a/tests/integration/pull_create_test.go +++ b/tests/integration/pull_create_test.go @@ -4,6 +4,7 @@ package integration import ( + "encoding/base64" "fmt" "net/http" "net/http/httptest" @@ -17,7 +18,9 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/git/gitcmd" + api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" @@ -153,8 +156,16 @@ func TestPullCreate(t *testing.T) { url := test.RedirectURL(resp) assert.Regexp(t, "^/user2/repo1/pulls/[0-9]*$", url) + // test create the pull request again and it should fail now + link := "/user2/repo1/compare/master...user1/repo1:master" + req := NewRequestWithValues(t, "POST", link, map[string]string{ + "_csrf": GetUserCSRFToken(t, session), + "title": "This is a pull title", + }) + session.MakeRequest(t, req, http.StatusBadRequest) + // check .diff can be accessed and matches performed change - req := NewRequest(t, "GET", url+".diff") + req = NewRequest(t, "GET", url+".diff") resp = session.MakeRequest(t, req, http.StatusOK) assert.Regexp(t, `\+Hello, World \(Edited\)`, resp.Body) assert.Regexp(t, "^diff", resp.Body) @@ -295,6 +306,95 @@ func TestPullCreatePrFromBaseToFork(t *testing.T) { }) } +func TestCreatePullRequestFromNestedOrgForks(t *testing.T) { + onGiteaRun(t, func(t *testing.T, _ *url.URL) { + session := loginUser(t, "user1") + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteOrganization) + + const ( + baseOrg = "test-fork-org1" + midForkOrg = "test-fork-org2" + leafForkOrg = "test-fork-org3" + repoName = "test-fork-repo" + patchBranch = "teabot-patch-1" + ) + + createOrg := func(name string) { + req := NewRequestWithJSON(t, "POST", "/api/v1/orgs", &api.CreateOrgOption{ + UserName: name, + Visibility: "public", + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusCreated) + } + + createOrg(baseOrg) + createOrg(midForkOrg) + createOrg(leafForkOrg) + + req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/orgs/%s/repos", baseOrg), &api.CreateRepoOption{ + Name: repoName, + AutoInit: true, + DefaultBranch: "main", + Private: false, + Readme: "Default", + }).AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusCreated) + var baseRepo api.Repository + DecodeJSON(t, resp, &baseRepo) + assert.Equal(t, "main", baseRepo.DefaultBranch) + + forkIntoOrg := func(srcOrg, dstOrg string) api.Repository { + req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/forks", srcOrg, repoName), &api.CreateForkOption{ + Organization: util.ToPointer(dstOrg), + }).AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusAccepted) + var forkRepo api.Repository + DecodeJSON(t, resp, &forkRepo) + assert.NotNil(t, forkRepo.Owner) + if forkRepo.Owner != nil { + assert.Equal(t, dstOrg, forkRepo.Owner.UserName) + } + return forkRepo + } + + forkIntoOrg(baseOrg, midForkOrg) + forkIntoOrg(midForkOrg, leafForkOrg) + + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s", leafForkOrg, repoName, "patch-from-org3.txt"), &api.CreateFileOptions{ + FileOptions: api.FileOptions{ + BranchName: "main", + NewBranchName: patchBranch, + Message: "create patch from org3", + }, + ContentBase64: base64.StdEncoding.EncodeToString([]byte("patch content")), + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusCreated) + + prPayload := map[string]string{ + "head": fmt.Sprintf("%s:%s", leafForkOrg, patchBranch), + "base": "main", + "title": "test creating pull from test-fork-org3 to test-fork-org1", + } + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/pulls", baseOrg, repoName), prPayload).AddTokenAuth(token) + resp = MakeRequest(t, req, http.StatusCreated) + var pr api.PullRequest + DecodeJSON(t, resp, &pr) + assert.Equal(t, prPayload["title"], pr.Title) + if assert.NotNil(t, pr.Head) { + assert.Equal(t, patchBranch, pr.Head.Ref) + if assert.NotNil(t, pr.Head.Repository) { + assert.Equal(t, fmt.Sprintf("%s/%s", leafForkOrg, repoName), pr.Head.Repository.FullName) + } + } + if assert.NotNil(t, pr.Base) { + assert.Equal(t, "main", pr.Base.Ref) + if assert.NotNil(t, pr.Base.Repository) { + assert.Equal(t, fmt.Sprintf("%s/%s", baseOrg, repoName), pr.Base.Repository.FullName) + } + } + }) +} + func TestPullCreateParallel(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { sessionFork := loginUser(t, "user1")