diff --git a/models/user/user.go b/models/user/user.go index 6143992a2537b..b4bd8694bf78b 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -1444,3 +1444,15 @@ func DisabledFeaturesWithLoginType(user *User) *container.Set[string] { } return &setting.Admin.UserDisabledFeatures } + +// 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 3c43b862805ec..08114f0d5faeb 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1858,6 +1858,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 5500870c0faad..6ed0a51fb1323 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -29,6 +29,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" @@ -1076,7 +1077,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") @@ -1092,28 +1093,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 { @@ -1137,7 +1133,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 fda31a07ba736..be689bbdb54a9 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 2e56f6934a05e..a841f45ac6c4f 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -258,7 +258,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 a9dd95f2d4a62..f55f0b60078b1 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1305,6 +1305,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 17265c836a238..581fe0a3fddbf 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")