From c6cf96d31d80ab79d370a6192fd761b4443daec2 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 21 May 2024 23:23:22 +0800 Subject: [PATCH 1/5] Fix automerge will not work because of some events haven't been triggered (#30780) Replace #25741 Close #24445 Close #30658 Close #20646 ~Depends on #30805~ Since #25741 has been rewritten totally, to make the contribution easier, I will continue the work in this PR. Thanks @6543 --------- Co-authored-by: 6543 <6543@obermui.de> Co-authored-by: wxiaoguang --- models/issues/review.go | 6 +- services/automerge/automerge.go | 108 ++++++---- services/automerge/notify.go | 46 ++++ .../repository/commitstatus/commitstatus.go | 2 +- tests/integration/editor_test.go | 35 ++-- tests/integration/pull_merge_test.go | 198 ++++++++++++++++++ tests/integration/pull_review_test.go | 12 +- 7 files changed, 344 insertions(+), 63 deletions(-) create mode 100644 services/automerge/notify.go diff --git a/models/issues/review.go b/models/issues/review.go index 3c6934b060afc..ca6fd6035b130 100644 --- a/models/issues/review.go +++ b/models/issues/review.go @@ -155,14 +155,14 @@ func (r *Review) LoadCodeComments(ctx context.Context) (err error) { if r.CodeComments != nil { return err } - if err = r.loadIssue(ctx); err != nil { + if err = r.LoadIssue(ctx); err != nil { return err } r.CodeComments, err = fetchCodeCommentsByReview(ctx, r.Issue, nil, r, false) return err } -func (r *Review) loadIssue(ctx context.Context) (err error) { +func (r *Review) LoadIssue(ctx context.Context) (err error) { if r.Issue != nil { return err } @@ -199,7 +199,7 @@ func (r *Review) LoadReviewerTeam(ctx context.Context) (err error) { // LoadAttributes loads all attributes except CodeComments func (r *Review) LoadAttributes(ctx context.Context) (err error) { - if err = r.loadIssue(ctx); err != nil { + if err = r.LoadIssue(ctx); err != nil { return err } if err = r.LoadCodeComments(ctx); err != nil { diff --git a/services/automerge/automerge.go b/services/automerge/automerge.go index bd1317c7f48b7..10f3c28d56be4 100644 --- a/services/automerge/automerge.go +++ b/services/automerge/automerge.go @@ -22,6 +22,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/process" "code.gitea.io/gitea/modules/queue" + notify_service "code.gitea.io/gitea/services/notify" pull_service "code.gitea.io/gitea/services/pull" ) @@ -30,6 +31,8 @@ var prAutoMergeQueue *queue.WorkerPoolQueue[string] // Init runs the task queue to that handles auto merges func Init() error { + notify_service.RegisterNotifier(NewNotifier()) + prAutoMergeQueue = queue.CreateUniqueQueue(graceful.GetManager().ShutdownContext(), "pr_auto_merge", handler) if prAutoMergeQueue == nil { return fmt.Errorf("unable to create pr_auto_merge queue") @@ -47,7 +50,7 @@ func handler(items ...string) []string { log.Error("could not parse data from pr_auto_merge queue (%v): %v", s, err) continue } - handlePull(id, sha) + handlePullRequestAutoMerge(id, sha) } return nil } @@ -62,16 +65,6 @@ func addToQueue(pr *issues_model.PullRequest, sha string) { // ScheduleAutoMerge if schedule is false and no error, pull can be merged directly func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pull *issues_model.PullRequest, style repo_model.MergeStyle, message string) (scheduled bool, err error) { err = db.WithTx(ctx, func(ctx context.Context) error { - lastCommitStatus, err := pull_service.GetPullRequestCommitStatusState(ctx, pull) - if err != nil { - return err - } - - // we don't need to schedule - if lastCommitStatus.IsSuccess() { - return nil - } - if err := pull_model.ScheduleAutoMerge(ctx, doer, pull.ID, style, message); err != nil { return err } @@ -95,8 +88,8 @@ func RemoveScheduledAutoMerge(ctx context.Context, doer *user_model.User, pull * }) } -// MergeScheduledPullRequest merges a previously scheduled pull request when all checks succeeded -func MergeScheduledPullRequest(ctx context.Context, sha string, repo *repo_model.Repository) error { +// StartPRCheckAndAutoMergeBySHA start an automerge check and auto merge task for all pull requests of repository and SHA +func StartPRCheckAndAutoMergeBySHA(ctx context.Context, sha string, repo *repo_model.Repository) error { pulls, err := getPullRequestsByHeadSHA(ctx, sha, repo, func(pr *issues_model.PullRequest) bool { return !pr.HasMerged && pr.CanAutoMerge() }) @@ -111,6 +104,32 @@ func MergeScheduledPullRequest(ctx context.Context, sha string, repo *repo_model return nil } +// StartPRCheckAndAutoMerge start an automerge check and auto merge task for a pull request +func StartPRCheckAndAutoMerge(ctx context.Context, pull *issues_model.PullRequest) { + if pull == nil || pull.HasMerged || !pull.CanAutoMerge() { + return + } + + if err := pull.LoadBaseRepo(ctx); err != nil { + log.Error("LoadBaseRepo: %v", err) + return + } + + gitRepo, err := gitrepo.OpenRepository(ctx, pull.BaseRepo) + if err != nil { + log.Error("OpenRepository: %v", err) + return + } + defer gitRepo.Close() + commitID, err := gitRepo.GetRefCommitID(pull.GetGitRefName()) + if err != nil { + log.Error("GetRefCommitID: %v", err) + return + } + + addToQueue(pull, commitID) +} + func getPullRequestsByHeadSHA(ctx context.Context, sha string, repo *repo_model.Repository, filter func(*issues_model.PullRequest) bool) (map[int64]*issues_model.PullRequest, error) { gitRepo, err := gitrepo.OpenRepository(ctx, repo) if err != nil { @@ -161,7 +180,8 @@ func getPullRequestsByHeadSHA(ctx context.Context, sha string, repo *repo_model. return pulls, nil } -func handlePull(pullID int64, sha string) { +// handlePullRequestAutoMerge merge the pull request if all checks are successful +func handlePullRequestAutoMerge(pullID int64, sha string) { ctx, _, finished := process.GetManager().AddContext(graceful.GetManager().HammerContext(), fmt.Sprintf("Handle AutoMerge of PR[%d] with sha[%s]", pullID, sha)) defer finished() @@ -182,24 +202,50 @@ func handlePull(pullID int64, sha string) { return } + if err = pr.LoadBaseRepo(ctx); err != nil { + log.Error("%-v LoadBaseRepo: %v", pr, err) + return + } + + // check the sha is the same as pull request head commit id + baseGitRepo, err := gitrepo.OpenRepository(ctx, pr.BaseRepo) + if err != nil { + log.Error("OpenRepository: %v", err) + return + } + defer baseGitRepo.Close() + + headCommitID, err := baseGitRepo.GetRefCommitID(pr.GetGitRefName()) + if err != nil { + log.Error("GetRefCommitID: %v", err) + return + } + if headCommitID != sha { + log.Warn("Head commit id of auto merge %-v does not match sha [%s], it may means the head branch has been updated. Just ignore this request because a new request expected in the queue", pr, sha) + return + } + // Get all checks for this pr // We get the latest sha commit hash again to handle the case where the check of a previous push // did not succeed or was not finished yet. - if err = pr.LoadHeadRepo(ctx); err != nil { log.Error("%-v LoadHeadRepo: %v", pr, err) return } - headGitRepo, err := gitrepo.OpenRepository(ctx, pr.HeadRepo) - if err != nil { - log.Error("OpenRepository %-v: %v", pr.HeadRepo, err) - return + var headGitRepo *git.Repository + if pr.BaseRepoID == pr.HeadRepoID { + headGitRepo = baseGitRepo + } else { + headGitRepo, err = gitrepo.OpenRepository(ctx, pr.HeadRepo) + if err != nil { + log.Error("OpenRepository %-v: %v", pr.HeadRepo, err) + return + } + defer headGitRepo.Close() } - defer headGitRepo.Close() headBranchExist := headGitRepo.IsBranchExist(pr.HeadBranch) - if pr.HeadRepo == nil || !headBranchExist { log.Warn("Head branch of auto merge %-v does not exist [HeadRepoID: %d, Branch: %s]", pr, pr.HeadRepoID, pr.HeadBranch) return @@ -238,25 +284,11 @@ func handlePull(pullID int64, sha string) { return } - var baseGitRepo *git.Repository - if pr.BaseRepoID == pr.HeadRepoID { - baseGitRepo = headGitRepo - } else { - if err = pr.LoadBaseRepo(ctx); err != nil { - log.Error("%-v LoadBaseRepo: %v", pr, err) - return - } - - baseGitRepo, err = gitrepo.OpenRepository(ctx, pr.BaseRepo) - if err != nil { - log.Error("OpenRepository %-v: %v", pr.BaseRepo, err) - return - } - defer baseGitRepo.Close() - } - if err := pull_service.Merge(ctx, pr, doer, baseGitRepo, scheduledPRM.MergeStyle, "", scheduledPRM.Message, true); err != nil { log.Error("pull_service.Merge: %v", err) + // FIXME: if merge failed, we should display some error message to the pull request page. + // The resolution is add a new column on automerge table named `error_message` to store the error message and displayed + // on the pull request page. But this should not be finished in a bug fix PR which will be backport to release branch. return } } diff --git a/services/automerge/notify.go b/services/automerge/notify.go new file mode 100644 index 0000000000000..cb078214f63b7 --- /dev/null +++ b/services/automerge/notify.go @@ -0,0 +1,46 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package automerge + +import ( + "context" + + issues_model "code.gitea.io/gitea/models/issues" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/log" + notify_service "code.gitea.io/gitea/services/notify" +) + +type automergeNotifier struct { + notify_service.NullNotifier +} + +var _ notify_service.Notifier = &automergeNotifier{} + +// NewNotifier create a new automergeNotifier notifier +func NewNotifier() notify_service.Notifier { + return &automergeNotifier{} +} + +func (n *automergeNotifier) PullRequestReview(ctx context.Context, pr *issues_model.PullRequest, review *issues_model.Review, comment *issues_model.Comment, mentions []*user_model.User) { + // as a missing / blocking reviews could have blocked a pending automerge let's recheck + if review.Type == issues_model.ReviewTypeApprove { + if err := StartPRCheckAndAutoMergeBySHA(ctx, review.CommitID, pr.BaseRepo); err != nil { + log.Error("StartPullRequestAutoMergeCheckBySHA: %v", err) + } + } +} + +func (n *automergeNotifier) PullReviewDismiss(ctx context.Context, doer *user_model.User, review *issues_model.Review, comment *issues_model.Comment) { + if err := review.LoadIssue(ctx); err != nil { + log.Error("LoadIssue: %v", err) + return + } + if err := review.Issue.LoadPullRequest(ctx); err != nil { + log.Error("LoadPullRequest: %v", err) + return + } + // as reviews could have blocked a pending automerge let's recheck + StartPRCheckAndAutoMerge(ctx, review.Issue.PullRequest) +} diff --git a/services/repository/commitstatus/commitstatus.go b/services/repository/commitstatus/commitstatus.go index 444ae04d0c228..adc59abed8f47 100644 --- a/services/repository/commitstatus/commitstatus.go +++ b/services/repository/commitstatus/commitstatus.go @@ -115,7 +115,7 @@ func CreateCommitStatus(ctx context.Context, repo *repo_model.Repository, creato } if status.State.IsSuccess() { - if err := automerge.MergeScheduledPullRequest(ctx, sha, repo); err != nil { + if err := automerge.StartPRCheckAndAutoMergeBySHA(ctx, sha, repo); err != nil { return fmt.Errorf("MergeScheduledPullRequest[repo_id: %d, user_id: %d, sha: %s]: %w", repo.ID, creator.ID, sha, err) } } diff --git a/tests/integration/editor_test.go b/tests/integration/editor_test.go index 045567ce77c4d..f510c79bc6b01 100644 --- a/tests/integration/editor_test.go +++ b/tests/integration/editor_test.go @@ -4,6 +4,7 @@ package integration import ( + "fmt" "net/http" "net/http/httptest" "net/url" @@ -19,25 +20,29 @@ import ( func TestCreateFile(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { session := loginUser(t, "user2") + testCreateFile(t, session, "user2", "repo1", "master", "test.txt", "Content") + }) +} - // Request editor page - req := NewRequest(t, "GET", "/user2/repo1/_new/master/") - resp := session.MakeRequest(t, req, http.StatusOK) +func testCreateFile(t *testing.T, session *TestSession, user, repo, branch, filePath, content string) *httptest.ResponseRecorder { + // Request editor page + newURL := fmt.Sprintf("/%s/%s/_new/%s/", user, repo, branch) + req := NewRequest(t, "GET", newURL) + resp := session.MakeRequest(t, req, http.StatusOK) - doc := NewHTMLParser(t, resp.Body) - lastCommit := doc.GetInputValueByName("last_commit") - assert.NotEmpty(t, lastCommit) + doc := NewHTMLParser(t, resp.Body) + lastCommit := doc.GetInputValueByName("last_commit") + assert.NotEmpty(t, lastCommit) - // Save new file to master branch - req = NewRequestWithValues(t, "POST", "/user2/repo1/_new/master/", map[string]string{ - "_csrf": doc.GetCSRF(), - "last_commit": lastCommit, - "tree_path": "test.txt", - "content": "Content", - "commit_choice": "direct", - }) - session.MakeRequest(t, req, http.StatusSeeOther) + // Save new file to master branch + req = NewRequestWithValues(t, "POST", newURL, map[string]string{ + "_csrf": doc.GetCSRF(), + "last_commit": lastCommit, + "tree_path": filePath, + "content": content, + "commit_choice": "direct", }) + return session.MakeRequest(t, req, http.StatusSeeOther) } func TestCreateFileOnProtectedBranch(t *testing.T) { diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index 826568caf2b4f..979c408388760 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -12,6 +12,8 @@ import ( "net/url" "os" "path" + "path/filepath" + "strconv" "strings" "testing" "time" @@ -19,7 +21,9 @@ import ( "code.gitea.io/gitea/models" auth_model "code.gitea.io/gitea/models/auth" "code.gitea.io/gitea/models/db" + git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" + pull_model "code.gitea.io/gitea/models/pull" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" @@ -30,8 +34,10 @@ import ( api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/translation" + "code.gitea.io/gitea/services/automerge" "code.gitea.io/gitea/services/pull" repo_service "code.gitea.io/gitea/services/repository" + commitstatus_service "code.gitea.io/gitea/services/repository/commitstatus" files_service "code.gitea.io/gitea/services/repository/files" "github.com/stretchr/testify/assert" @@ -648,3 +654,195 @@ func TestPullMergeIndexerNotifier(t *testing.T) { } }) } + +func testResetRepo(t *testing.T, repoPath, branch, commitID string) { + f, err := os.OpenFile(filepath.Join(repoPath, "refs", "heads", branch), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o644) + assert.NoError(t, err) + _, err = f.WriteString(commitID + "\n") + assert.NoError(t, err) + f.Close() + + repo, err := git.OpenRepository(context.Background(), repoPath) + assert.NoError(t, err) + defer repo.Close() + id, err := repo.GetBranchCommitID(branch) + assert.NoError(t, err) + assert.EqualValues(t, commitID, id) +} + +func TestPullAutoMergeAfterCommitStatusSucceed(t *testing.T) { + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + // create a pull request + session := loginUser(t, "user1") + user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + forkedName := "repo1-1" + testRepoFork(t, session, "user2", "repo1", "user1", forkedName) + defer func() { + testDeleteRepository(t, session, "user1", forkedName) + }() + testEditFile(t, session, "user1", forkedName, "master", "README.md", "Hello, World (Edited)\n") + testPullCreate(t, session, "user1", forkedName, false, "master", "master", "Indexer notifier test pull") + + baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"}) + forkedRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user1", Name: forkedName}) + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ + BaseRepoID: baseRepo.ID, + BaseBranch: "master", + HeadRepoID: forkedRepo.ID, + HeadBranch: "master", + }) + + // add protected branch for commit status + csrf := GetCSRF(t, session, "/user2/repo1/settings/branches") + // Change master branch to protected + req := NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/edit", map[string]string{ + "_csrf": csrf, + "rule_name": "master", + "enable_push": "true", + "enable_status_check": "true", + "status_check_contexts": "gitea/actions", + }) + session.MakeRequest(t, req, http.StatusSeeOther) + + // first time insert automerge record, return true + scheduled, err := automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test") + assert.NoError(t, err) + assert.True(t, scheduled) + + // second time insert automerge record, return false because it does exist + scheduled, err = automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test") + assert.Error(t, err) + assert.False(t, scheduled) + + // reload pr again + pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID}) + assert.False(t, pr.HasMerged) + assert.Empty(t, pr.MergedCommitID) + + // update commit status to success, then it should be merged automatically + baseGitRepo, err := gitrepo.OpenRepository(db.DefaultContext, baseRepo) + assert.NoError(t, err) + sha, err := baseGitRepo.GetRefCommitID(pr.GetGitRefName()) + assert.NoError(t, err) + masterCommitID, err := baseGitRepo.GetBranchCommitID("master") + assert.NoError(t, err) + + branches, _, err := baseGitRepo.GetBranchNames(0, 100) + assert.NoError(t, err) + assert.ElementsMatch(t, []string{"sub-home-md-img-check", "home-md-img-check", "pr-to-update", "branch2", "DefaultBranch", "develop", "feature/1", "master"}, branches) + baseGitRepo.Close() + defer func() { + testResetRepo(t, baseRepo.RepoPath(), "master", masterCommitID) + }() + + err = commitstatus_service.CreateCommitStatus(db.DefaultContext, baseRepo, user1, sha, &git_model.CommitStatus{ + State: api.CommitStatusSuccess, + TargetURL: "https://gitea.com", + Context: "gitea/actions", + }) + assert.NoError(t, err) + + time.Sleep(2 * time.Second) + + // realod pr again + pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID}) + assert.True(t, pr.HasMerged) + assert.NotEmpty(t, pr.MergedCommitID) + + unittest.AssertNotExistsBean(t, &pull_model.AutoMerge{PullID: pr.ID}) + }) +} + +func TestPullAutoMergeAfterCommitStatusSucceedAndApproval(t *testing.T) { + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + // create a pull request + session := loginUser(t, "user1") + user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + forkedName := "repo1-2" + testRepoFork(t, session, "user2", "repo1", "user1", forkedName) + defer func() { + testDeleteRepository(t, session, "user1", forkedName) + }() + testEditFile(t, session, "user1", forkedName, "master", "README.md", "Hello, World (Edited)\n") + testPullCreate(t, session, "user1", forkedName, false, "master", "master", "Indexer notifier test pull") + + baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"}) + forkedRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user1", Name: forkedName}) + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ + BaseRepoID: baseRepo.ID, + BaseBranch: "master", + HeadRepoID: forkedRepo.ID, + HeadBranch: "master", + }) + + // add protected branch for commit status + csrf := GetCSRF(t, session, "/user2/repo1/settings/branches") + // Change master branch to protected + req := NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/edit", map[string]string{ + "_csrf": csrf, + "rule_name": "master", + "enable_push": "true", + "enable_status_check": "true", + "status_check_contexts": "gitea/actions", + "required_approvals": "1", + }) + session.MakeRequest(t, req, http.StatusSeeOther) + + // first time insert automerge record, return true + scheduled, err := automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test") + assert.NoError(t, err) + assert.True(t, scheduled) + + // second time insert automerge record, return false because it does exist + scheduled, err = automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test") + assert.Error(t, err) + assert.False(t, scheduled) + + // reload pr again + pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID}) + assert.False(t, pr.HasMerged) + assert.Empty(t, pr.MergedCommitID) + + // update commit status to success, then it should be merged automatically + baseGitRepo, err := gitrepo.OpenRepository(db.DefaultContext, baseRepo) + assert.NoError(t, err) + sha, err := baseGitRepo.GetRefCommitID(pr.GetGitRefName()) + assert.NoError(t, err) + masterCommitID, err := baseGitRepo.GetBranchCommitID("master") + assert.NoError(t, err) + baseGitRepo.Close() + defer func() { + testResetRepo(t, baseRepo.RepoPath(), "master", masterCommitID) + }() + + err = commitstatus_service.CreateCommitStatus(db.DefaultContext, baseRepo, user1, sha, &git_model.CommitStatus{ + State: api.CommitStatusSuccess, + TargetURL: "https://gitea.com", + Context: "gitea/actions", + }) + assert.NoError(t, err) + + time.Sleep(2 * time.Second) + + // reload pr again + pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID}) + assert.False(t, pr.HasMerged) + assert.Empty(t, pr.MergedCommitID) + + // approve the PR from non-author + approveSession := loginUser(t, "user2") + req = NewRequest(t, "GET", fmt.Sprintf("/user2/repo1/pulls/%d", pr.Index)) + resp := approveSession.MakeRequest(t, req, http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + testSubmitReview(t, approveSession, htmlDoc.GetCSRF(), "user2", "repo1", strconv.Itoa(int(pr.Index)), sha, "approve", http.StatusOK) + + time.Sleep(2 * time.Second) + + // realod pr again + pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID}) + assert.True(t, pr.HasMerged) + assert.NotEmpty(t, pr.MergedCommitID) + + unittest.AssertNotExistsBean(t, &pull_model.AutoMerge{PullID: pr.ID}) + }) +} diff --git a/tests/integration/pull_review_test.go b/tests/integration/pull_review_test.go index 273332a36b416..df5d7b38ea49a 100644 --- a/tests/integration/pull_review_test.go +++ b/tests/integration/pull_review_test.go @@ -202,10 +202,10 @@ func TestPullView_GivenApproveOrRejectReviewOnClosedPR(t *testing.T) { htmlDoc := NewHTMLParser(t, resp.Body) // Submit an approve review on the PR. - testSubmitReview(t, user2Session, htmlDoc.GetCSRF(), "user2", "repo1", elem[4], "approve", http.StatusUnprocessableEntity) + testSubmitReview(t, user2Session, htmlDoc.GetCSRF(), "user2", "repo1", elem[4], "", "approve", http.StatusUnprocessableEntity) // Submit a reject review on the PR. - testSubmitReview(t, user2Session, htmlDoc.GetCSRF(), "user2", "repo1", elem[4], "reject", http.StatusUnprocessableEntity) + testSubmitReview(t, user2Session, htmlDoc.GetCSRF(), "user2", "repo1", elem[4], "", "reject", http.StatusUnprocessableEntity) }) t.Run("Submit approve/reject review on closed PR", func(t *testing.T) { @@ -222,18 +222,18 @@ func TestPullView_GivenApproveOrRejectReviewOnClosedPR(t *testing.T) { htmlDoc := NewHTMLParser(t, resp.Body) // Submit an approve review on the PR. - testSubmitReview(t, user2Session, htmlDoc.GetCSRF(), "user2", "repo1", elem[4], "approve", http.StatusUnprocessableEntity) + testSubmitReview(t, user2Session, htmlDoc.GetCSRF(), "user2", "repo1", elem[4], "", "approve", http.StatusUnprocessableEntity) // Submit a reject review on the PR. - testSubmitReview(t, user2Session, htmlDoc.GetCSRF(), "user2", "repo1", elem[4], "reject", http.StatusUnprocessableEntity) + testSubmitReview(t, user2Session, htmlDoc.GetCSRF(), "user2", "repo1", elem[4], "", "reject", http.StatusUnprocessableEntity) }) }) } -func testSubmitReview(t *testing.T, session *TestSession, csrf, owner, repo, pullNumber, reviewType string, expectedSubmitStatus int) *httptest.ResponseRecorder { +func testSubmitReview(t *testing.T, session *TestSession, csrf, owner, repo, pullNumber, commitID, reviewType string, expectedSubmitStatus int) *httptest.ResponseRecorder { options := map[string]string{ "_csrf": csrf, - "commit_id": "", + "commit_id": commitID, "content": "test", "type": reviewType, } From 9c8c9ff6d10b35de8d2d7eae0fc2646ad9bbe94a Mon Sep 17 00:00:00 2001 From: Denys Konovalov Date: Tue, 21 May 2024 18:23:49 +0200 Subject: [PATCH 2/5] use existing oauth grant for public client (#31015) Do not try to create a new authorization grant when one exists already, thus preventing a DB-related authorization issue. Fix https://github.com/go-gitea/gitea/pull/30790#issuecomment-2118812426 --------- Co-authored-by: Lunny Xiao --- routers/web/auth/oauth.go | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index 84fa4730441f1..b337b6b156959 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -556,15 +556,30 @@ func GrantApplicationOAuth(ctx *context.Context) { ctx.ServerError("GetOAuth2ApplicationByClientID", err) return } - grant, err := app.CreateGrant(ctx, ctx.Doer.ID, form.Scope) + grant, err := app.GetGrantByUserID(ctx, ctx.Doer.ID) if err != nil { + handleServerError(ctx, form.State, form.RedirectURI) + return + } + if grant == nil { + grant, err = app.CreateGrant(ctx, ctx.Doer.ID, form.Scope) + if err != nil { + handleAuthorizeError(ctx, AuthorizeError{ + State: form.State, + ErrorDescription: "cannot create grant for user", + ErrorCode: ErrorCodeServerError, + }, form.RedirectURI) + return + } + } else if grant.Scope != form.Scope { handleAuthorizeError(ctx, AuthorizeError{ State: form.State, - ErrorDescription: "cannot create grant for user", + ErrorDescription: "a grant exists with different scope", ErrorCode: ErrorCodeServerError, }, form.RedirectURI) return } + if len(form.Nonce) > 0 { err := grant.SetNonce(ctx, form.Nonce) if err != nil { From daf2a4c047c88083d8820bdee9074357d5c5d7b7 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Wed, 22 May 2024 02:00:35 +0900 Subject: [PATCH 3/5] Fix wrong display of recently pushed notification (#25812) There's a bug in #25715: If user pushed a commit into another repo with same branch name, the no-related repo will display the recently pushed notification incorrectly. It is simple to fix this, we should match the repo id in the sql query. ![image](https://github.com/go-gitea/gitea/assets/18380374/9411a926-16f1-419e-a1b5-e953af38bab1) The latest commit is 2 weeks ago. ![image](https://github.com/go-gitea/gitea/assets/18380374/52f9ab22-4999-43ac-a86f-6d36fb1e0411) The notification comes from another repo with same branch name: ![image](https://github.com/go-gitea/gitea/assets/18380374/a26bc335-8e5b-4b9c-a965-c3dc3fa6f252) After: In forked repo: ![image](https://github.com/go-gitea/gitea/assets/18380374/ce6ffc35-deb7-4be7-8b09-184207392f32) New PR Link will redirect to the original repo: ![image](https://github.com/go-gitea/gitea/assets/18380374/7b98e76f-0c75-494c-9462-80cf9f98e786) In the original repo: ![image](https://github.com/go-gitea/gitea/assets/18380374/5f6a821b-e51a-4bbd-9980-d9eb94a3c847) New PR Link: ![image](https://github.com/go-gitea/gitea/assets/18380374/1ce8c879-9f11-4312-8c32-695d7d9af0df) In the same repo: ![image](https://github.com/go-gitea/gitea/assets/18380374/64b56073-4d0e-40c4-b8a0-80be7a775f69) New PR Link: ![image](https://github.com/go-gitea/gitea/assets/18380374/96e1b6a3-fb98-40ee-b2ee-648039fb0dcf) 08/15 Update: Follow #26257, added permission check and logic fix mentioned in https://github.com/go-gitea/gitea/pull/26257#discussion_r1294085203 2024/04/25 Update: Fix #30611 --------- Co-authored-by: silverwind Co-authored-by: Lunny Xiao Co-authored-by: wxiaoguang --- models/fixtures/branch.yml | 36 +++++ models/fixtures/issue_index.yml | 8 + models/fixtures/org_user.yml | 12 ++ models/fixtures/repository.yml | 2 +- models/fixtures/team.yml | 22 +++ models/fixtures/team_unit.yml | 18 +++ models/fixtures/team_user.yml | 12 ++ models/fixtures/user.yml | 8 +- models/git/branch.go | 142 ++++++++++++++--- models/git/branch_list.go | 19 +++ models/organization/org_user_test.go | 6 +- models/repo/repo_list.go | 6 + routers/web/repo/view.go | 26 ++- .../code/recently_pushed_new_branches.tmpl | 4 +- tests/integration/api_user_orgs_test.go | 26 +++ tests/integration/compare_test.go | 2 +- tests/integration/empty_repo_test.go | 13 ++ tests/integration/integration_test.go | 9 ++ tests/integration/pull_compare_test.go | 2 +- tests/integration/pull_create_test.go | 6 +- tests/integration/pull_merge_test.go | 30 ++-- tests/integration/pull_review_test.go | 2 +- tests/integration/pull_status_test.go | 6 +- tests/integration/repo_activity_test.go | 2 +- tests/integration/repo_branch_test.go | 148 +++++++++++++++++- tests/integration/repo_fork_test.go | 13 +- 26 files changed, 508 insertions(+), 72 deletions(-) diff --git a/models/fixtures/branch.yml b/models/fixtures/branch.yml index 93003049c67fb..c7bdff7733995 100644 --- a/models/fixtures/branch.yml +++ b/models/fixtures/branch.yml @@ -45,3 +45,39 @@ is_deleted: false deleted_by_id: 0 deleted_unix: 0 + +- + id: 5 + repo_id: 10 + name: 'master' + commit_id: '65f1bf27bc3bf70f64657658635e66094edbcb4d' + commit_message: 'Initial commit' + commit_time: 1489927679 + pusher_id: 12 + is_deleted: false + deleted_by_id: 0 + deleted_unix: 0 + +- + id: 6 + repo_id: 10 + name: 'outdated-new-branch' + commit_id: 'cb24c347e328d83c1e0c3c908a6b2c0a2fcb8a3d' + commit_message: 'add' + commit_time: 1489927679 + pusher_id: 12 + is_deleted: false + deleted_by_id: 0 + deleted_unix: 0 + +- + id: 14 + repo_id: 11 + name: 'master' + commit_id: '65f1bf27bc3bf70f64657658635e66094edbcb4d' + commit_message: 'Initial commit' + commit_time: 1489927679 + pusher_id: 13 + is_deleted: false + deleted_by_id: 0 + deleted_unix: 0 diff --git a/models/fixtures/issue_index.yml b/models/fixtures/issue_index.yml index de6e955804ab6..5aabc08e388c5 100644 --- a/models/fixtures/issue_index.yml +++ b/models/fixtures/issue_index.yml @@ -1,27 +1,35 @@ - group_id: 1 max_index: 5 + - group_id: 2 max_index: 2 + - group_id: 3 max_index: 2 + - group_id: 10 max_index: 1 + - group_id: 32 max_index: 2 + - group_id: 48 max_index: 1 + - group_id: 42 max_index: 1 + - group_id: 50 max_index: 1 + - group_id: 51 max_index: 1 diff --git a/models/fixtures/org_user.yml b/models/fixtures/org_user.yml index a7fbcb2c5a317..cf21b84aa9fc5 100644 --- a/models/fixtures/org_user.yml +++ b/models/fixtures/org_user.yml @@ -117,3 +117,15 @@ uid: 40 org_id: 41 is_public: true + +- + id: 21 + uid: 12 + org_id: 25 + is_public: true + +- + id: 22 + uid: 2 + org_id: 35 + is_public: true diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml index e5c6224c96ff3..e1f1dd73679b5 100644 --- a/models/fixtures/repository.yml +++ b/models/fixtures/repository.yml @@ -327,7 +327,7 @@ is_archived: false is_mirror: false status: 0 - is_fork: false + is_fork: true fork_id: 10 is_template: false template_id: 0 diff --git a/models/fixtures/team.yml b/models/fixtures/team.yml index 149fe90888b29..b549d0589bc9e 100644 --- a/models/fixtures/team.yml +++ b/models/fixtures/team.yml @@ -239,3 +239,25 @@ num_members: 2 includes_all_repositories: false can_create_org_repo: false + +- + id: 23 + org_id: 25 + lower_name: owners + name: Owners + authorize: 4 # owner + num_repos: 0 + num_members: 1 + includes_all_repositories: false + can_create_org_repo: true + +- + id: 24 + org_id: 35 + lower_name: team24 + name: team24 + authorize: 2 # write + num_repos: 0 + num_members: 1 + includes_all_repositories: true + can_create_org_repo: false diff --git a/models/fixtures/team_unit.yml b/models/fixtures/team_unit.yml index de0e8d738bbf8..110019eee30cf 100644 --- a/models/fixtures/team_unit.yml +++ b/models/fixtures/team_unit.yml @@ -322,3 +322,21 @@ team_id: 22 type: 3 access_mode: 1 + +- + id: 55 + team_id: 18 + type: 1 # code + access_mode: 4 + +- + id: 56 + team_id: 23 + type: 1 # code + access_mode: 4 + +- + id: 57 + team_id: 24 + type: 1 # code + access_mode: 2 diff --git a/models/fixtures/team_user.yml b/models/fixtures/team_user.yml index 02d57ae644fd0..6b2d153278ac5 100644 --- a/models/fixtures/team_user.yml +++ b/models/fixtures/team_user.yml @@ -147,3 +147,15 @@ org_id: 41 team_id: 22 uid: 39 + +- + id: 26 + org_id: 25 + team_id: 23 + uid: 12 + +- + id: 27 + org_id: 35 + team_id: 24 + uid: 2 diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index a3de535508b39..8504d88ce5995 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -918,8 +918,8 @@ num_following: 0 num_stars: 0 num_repos: 0 - num_teams: 1 - num_members: 1 + num_teams: 2 + num_members: 2 visibility: 0 repo_admin_change_team_access: false theme: "" @@ -1289,8 +1289,8 @@ num_following: 0 num_stars: 0 num_repos: 0 - num_teams: 1 - num_members: 1 + num_teams: 2 + num_members: 2 visibility: 2 repo_admin_change_team_access: false theme: "" diff --git a/models/git/branch.go b/models/git/branch.go index 2979dff3d211e..c315d921ffb88 100644 --- a/models/git/branch.go +++ b/models/git/branch.go @@ -10,9 +10,11 @@ import ( "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" @@ -102,8 +104,9 @@ func (err ErrBranchesEqual) Unwrap() error { // for pagination, keyword search and filtering type Branch struct { ID int64 - RepoID int64 `xorm:"UNIQUE(s)"` - Name string `xorm:"UNIQUE(s) NOT NULL"` // git's ref-name is case-sensitive internally, however, in some databases (mssql, mysql, by default), it's case-insensitive at the moment + RepoID int64 `xorm:"UNIQUE(s)"` + Repo *repo_model.Repository `xorm:"-"` + Name string `xorm:"UNIQUE(s) NOT NULL"` // git's ref-name is case-sensitive internally, however, in some databases (mssql, mysql, by default), it's case-insensitive at the moment CommitID string CommitMessage string `xorm:"TEXT"` // it only stores the message summary (the first line) PusherID int64 @@ -139,6 +142,14 @@ func (b *Branch) LoadPusher(ctx context.Context) (err error) { return err } +func (b *Branch) LoadRepo(ctx context.Context) (err error) { + if b.Repo != nil || b.RepoID == 0 { + return nil + } + b.Repo, err = repo_model.GetRepositoryByID(ctx, b.RepoID) + return err +} + func init() { db.RegisterModel(new(Branch)) db.RegisterModel(new(RenamedBranch)) @@ -400,24 +411,111 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to str return committer.Commit() } -// FindRecentlyPushedNewBranches return at most 2 new branches pushed by the user in 6 hours which has no opened PRs created -// except the indicate branch -func FindRecentlyPushedNewBranches(ctx context.Context, repoID, userID int64, excludeBranchName string) (BranchList, error) { - branches := make(BranchList, 0, 2) - subQuery := builder.Select("head_branch").From("pull_request"). - InnerJoin("issue", "issue.id = pull_request.issue_id"). - Where(builder.Eq{ - "pull_request.head_repo_id": repoID, - "issue.is_closed": false, - }) - err := db.GetEngine(ctx). - Where("pusher_id=? AND is_deleted=?", userID, false). - And("name <> ?", excludeBranchName). - And("repo_id = ?", repoID). - And("commit_time >= ?", time.Now().Add(-time.Hour*6).Unix()). - NotIn("name", subQuery). - OrderBy("branch.commit_time DESC"). - Limit(2). - Find(&branches) - return branches, err +type FindRecentlyPushedNewBranchesOptions struct { + Repo *repo_model.Repository + BaseRepo *repo_model.Repository + CommitAfterUnix int64 + MaxCount int +} + +type RecentlyPushedNewBranch struct { + BranchDisplayName string + BranchLink string + BranchCompareURL string + CommitTime timeutil.TimeStamp +} + +// FindRecentlyPushedNewBranches return at most 2 new branches pushed by the user in 2 hours which has no opened PRs created +// if opts.CommitAfterUnix is 0, we will find the branches that were committed to in the last 2 hours +// if opts.ListOptions is not set, we will only display top 2 latest branch +func FindRecentlyPushedNewBranches(ctx context.Context, doer *user_model.User, opts *FindRecentlyPushedNewBranchesOptions) ([]*RecentlyPushedNewBranch, error) { + if doer == nil { + return []*RecentlyPushedNewBranch{}, nil + } + + // find all related repo ids + repoOpts := repo_model.SearchRepoOptions{ + Actor: doer, + Private: true, + AllPublic: false, // Include also all public repositories of users and public organisations + AllLimited: false, // Include also all public repositories of limited organisations + Fork: optional.Some(true), + ForkFrom: opts.BaseRepo.ID, + Archived: optional.Some(false), + } + repoCond := repo_model.SearchRepositoryCondition(&repoOpts).And(repo_model.AccessibleRepositoryCondition(doer, unit.TypeCode)) + if opts.Repo.ID == opts.BaseRepo.ID { + // should also include the base repo's branches + repoCond = repoCond.Or(builder.Eq{"id": opts.BaseRepo.ID}) + } else { + // in fork repo, we only detect the fork repo's branch + repoCond = repoCond.And(builder.Eq{"id": opts.Repo.ID}) + } + repoIDs := builder.Select("id").From("repository").Where(repoCond) + + if opts.CommitAfterUnix == 0 { + opts.CommitAfterUnix = time.Now().Add(-time.Hour * 2).Unix() + } + + baseBranch, err := GetBranch(ctx, opts.BaseRepo.ID, opts.BaseRepo.DefaultBranch) + if err != nil { + return nil, err + } + + // find all related branches, these branches may already created PRs, we will check later + var branches []*Branch + if err := db.GetEngine(ctx). + Where(builder.And( + builder.Eq{ + "pusher_id": doer.ID, + "is_deleted": false, + }, + builder.Gte{"commit_time": opts.CommitAfterUnix}, + builder.In("repo_id", repoIDs), + // newly created branch have no changes, so skip them + builder.Neq{"commit_id": baseBranch.CommitID}, + )). + OrderBy(db.SearchOrderByRecentUpdated.String()). + Find(&branches); err != nil { + return nil, err + } + + newBranches := make([]*RecentlyPushedNewBranch, 0, len(branches)) + if opts.MaxCount == 0 { + // by default we display 2 recently pushed new branch + opts.MaxCount = 2 + } + for _, branch := range branches { + // whether branch have already created PR + count, err := db.GetEngine(ctx).Table("pull_request"). + // we should not only use branch name here, because if there are branches with same name in other repos, + // we can not detect them correctly + Where(builder.Eq{"head_repo_id": branch.RepoID, "head_branch": branch.Name}).Count() + if err != nil { + return nil, err + } + + // if no PR, we add to the result + if count == 0 { + if err := branch.LoadRepo(ctx); err != nil { + return nil, err + } + + branchDisplayName := branch.Name + if branch.Repo.ID != opts.BaseRepo.ID && branch.Repo.ID != opts.Repo.ID { + branchDisplayName = fmt.Sprintf("%s:%s", branch.Repo.FullName(), branchDisplayName) + } + newBranches = append(newBranches, &RecentlyPushedNewBranch{ + BranchDisplayName: branchDisplayName, + BranchLink: fmt.Sprintf("%s/src/branch/%s", branch.Repo.Link(), util.PathEscapeSegments(branch.Name)), + BranchCompareURL: branch.Repo.ComposeBranchCompareURL(opts.BaseRepo, branch.Name), + CommitTime: branch.CommitTime, + }) + } + if len(newBranches) == opts.MaxCount { + break + } + } + + return newBranches, nil } diff --git a/models/git/branch_list.go b/models/git/branch_list.go index 980bd7b4c9df8..5c887461d5a07 100644 --- a/models/git/branch_list.go +++ b/models/git/branch_list.go @@ -7,6 +7,7 @@ import ( "context" "code.gitea.io/gitea/models/db" + repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/optional" @@ -59,6 +60,24 @@ func (branches BranchList) LoadPusher(ctx context.Context) error { return nil } +func (branches BranchList) LoadRepo(ctx context.Context) error { + ids := container.FilterSlice(branches, func(branch *Branch) (int64, bool) { + return branch.RepoID, branch.RepoID > 0 && branch.Repo == nil + }) + + reposMap := make(map[int64]*repo_model.Repository, len(ids)) + if err := db.GetEngine(ctx).In("id", ids).Find(&reposMap); err != nil { + return err + } + for _, branch := range branches { + if branch.RepoID <= 0 || branch.Repo != nil { + continue + } + branch.Repo = reposMap[branch.RepoID] + } + return nil +} + type FindBranchOptions struct { db.ListOptions RepoID int64 diff --git a/models/organization/org_user_test.go b/models/organization/org_user_test.go index 7924517f31209..cf7acdf83ba79 100644 --- a/models/organization/org_user_test.go +++ b/models/organization/org_user_test.go @@ -81,7 +81,7 @@ func TestUserListIsPublicMember(t *testing.T) { {3, map[int64]bool{2: true, 4: false, 28: true}}, {6, map[int64]bool{5: true, 28: true}}, {7, map[int64]bool{5: false}}, - {25, map[int64]bool{24: true}}, + {25, map[int64]bool{12: true, 24: true}}, {22, map[int64]bool{}}, } for _, v := range tt { @@ -108,8 +108,8 @@ func TestUserListIsUserOrgOwner(t *testing.T) { {3, map[int64]bool{2: true, 4: false, 28: false}}, {6, map[int64]bool{5: true, 28: false}}, {7, map[int64]bool{5: true}}, - {25, map[int64]bool{24: false}}, // ErrTeamNotExist - {22, map[int64]bool{}}, // No member + {25, map[int64]bool{12: true, 24: false}}, // ErrTeamNotExist + {22, map[int64]bool{}}, // No member } for _, v := range tt { t.Run(fmt.Sprintf("IsUserOrgOwnerOfOrgId%d", v.orgid), func(t *testing.T) { diff --git a/models/repo/repo_list.go b/models/repo/repo_list.go index 987c7df9b0eb0..eacc98e2225d1 100644 --- a/models/repo/repo_list.go +++ b/models/repo/repo_list.go @@ -175,6 +175,8 @@ type SearchRepoOptions struct { // True -> include just forks // False -> include just non-forks Fork optional.Option[bool] + // If Fork option is True, you can use this option to limit the forks of a special repo by repo id. + ForkFrom int64 // None -> include templates AND non-templates // True -> include just templates // False -> include just non-templates @@ -514,6 +516,10 @@ func SearchRepositoryCondition(opts *SearchRepoOptions) builder.Cond { cond = cond.And(builder.Eq{"is_fork": false}) } else { cond = cond.And(builder.Eq{"is_fork": opts.Fork.Value()}) + + if opts.ForkFrom > 0 && opts.Fork.Value() { + cond = cond.And(builder.Eq{"fork_id": opts.ForkFrom}) + } } } diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index e4e6201c24abd..e1498c0d581e0 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -29,6 +29,7 @@ import ( "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" issue_model "code.gitea.io/gitea/models/issues" + access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" unit_model "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" @@ -1027,15 +1028,26 @@ func renderHomeCode(ctx *context.Context) { return } - showRecentlyPushedNewBranches := true - if ctx.Repo.Repository.IsMirror || - !ctx.Repo.Repository.UnitEnabled(ctx, unit_model.TypePullRequests) { - showRecentlyPushedNewBranches = false + opts := &git_model.FindRecentlyPushedNewBranchesOptions{ + Repo: ctx.Repo.Repository, + BaseRepo: ctx.Repo.Repository, } - if showRecentlyPushedNewBranches { - ctx.Data["RecentlyPushedNewBranches"], err = git_model.FindRecentlyPushedNewBranches(ctx, ctx.Repo.Repository.ID, ctx.Doer.ID, ctx.Repo.Repository.DefaultBranch) + if ctx.Repo.Repository.IsFork { + opts.BaseRepo = ctx.Repo.Repository.BaseRepo + } + + baseRepoPerm, err := access_model.GetUserRepoPermission(ctx, opts.BaseRepo, ctx.Doer) + if err != nil { + ctx.ServerError("GetUserRepoPermission", err) + return + } + + if !opts.Repo.IsMirror && !opts.BaseRepo.IsMirror && + opts.BaseRepo.UnitEnabled(ctx, unit_model.TypePullRequests) && + baseRepoPerm.CanRead(unit_model.TypePullRequests) { + ctx.Data["RecentlyPushedNewBranches"], err = git_model.FindRecentlyPushedNewBranches(ctx, ctx.Doer, opts) if err != nil { - ctx.ServerError("GetRecentlyPushedBranches", err) + ctx.ServerError("FindRecentlyPushedNewBranches", err) return } } diff --git a/templates/repo/code/recently_pushed_new_branches.tmpl b/templates/repo/code/recently_pushed_new_branches.tmpl index b808f413d352f..7f613fcba7eae 100644 --- a/templates/repo/code/recently_pushed_new_branches.tmpl +++ b/templates/repo/code/recently_pushed_new_branches.tmpl @@ -2,10 +2,10 @@
{{$timeSince := TimeSince .CommitTime.AsTime ctx.Locale}} - {{$branchLink := HTMLFormat `%s` $.RepoLink (PathEscapeSegments .Name) .Name}} + {{$branchLink := HTMLFormat `%s` .BranchLink .BranchDisplayName}} {{ctx.Locale.Tr "repo.pulls.recently_pushed_new_branches" $branchLink $timeSince}}
- + {{ctx.Locale.Tr "repo.pulls.compare_changes"}}
diff --git a/tests/integration/api_user_orgs_test.go b/tests/integration/api_user_orgs_test.go index b6b4b6f2b2d2e..c656ded5ae963 100644 --- a/tests/integration/api_user_orgs_test.go +++ b/tests/integration/api_user_orgs_test.go @@ -29,6 +29,7 @@ func TestUserOrgs(t *testing.T) { org3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "org3"}) org17 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "org17"}) + org35 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "private_org35"}) assert.Equal(t, []*api.Organization{ { @@ -55,6 +56,18 @@ func TestUserOrgs(t *testing.T) { Location: "", Visibility: "public", }, + { + ID: 35, + Name: org35.Name, + UserName: org35.Name, + FullName: org35.FullName, + Email: org35.Email, + AvatarURL: org35.AvatarLink(db.DefaultContext), + Description: "", + Website: "", + Location: "", + Visibility: "private", + }, }, orgs) // user itself should get it's org's he is a member of @@ -102,6 +115,7 @@ func TestMyOrgs(t *testing.T) { DecodeJSON(t, resp, &orgs) org3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "org3"}) org17 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "org17"}) + org35 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "private_org35"}) assert.Equal(t, []*api.Organization{ { @@ -128,5 +142,17 @@ func TestMyOrgs(t *testing.T) { Location: "", Visibility: "public", }, + { + ID: 35, + Name: org35.Name, + UserName: org35.Name, + FullName: org35.FullName, + Email: org35.Email, + AvatarURL: org35.AvatarLink(db.DefaultContext), + Description: "", + Website: "", + Location: "", + Visibility: "private", + }, }, orgs) } diff --git a/tests/integration/compare_test.go b/tests/integration/compare_test.go index 7fb8dbc3327ab..9f73ac80e2f9a 100644 --- a/tests/integration/compare_test.go +++ b/tests/integration/compare_test.go @@ -140,7 +140,7 @@ func TestCompareCodeExpand(t *testing.T) { user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) session = loginUser(t, user2.Name) - testRepoFork(t, session, user1.Name, repo.Name, user2.Name, "test_blob_excerpt-fork") + testRepoFork(t, session, user1.Name, repo.Name, user2.Name, "test_blob_excerpt-fork", "") testCreateBranch(t, session, user2.Name, "test_blob_excerpt-fork", "branch/main", "forked-branch", http.StatusSeeOther) testEditFile(t, session, user2.Name, "test_blob_excerpt-fork", "forked-branch", "README.md", strings.Repeat("a\n", 15)+"CHANGED\n"+strings.Repeat("a\n", 15)) diff --git a/tests/integration/empty_repo_test.go b/tests/integration/empty_repo_test.go index ea393a606167a..002aa5600e08b 100644 --- a/tests/integration/empty_repo_test.go +++ b/tests/integration/empty_repo_test.go @@ -6,9 +6,11 @@ package integration import ( "bytes" "encoding/base64" + "fmt" "io" "mime/multipart" "net/http" + "net/http/httptest" "testing" auth_model "code.gitea.io/gitea/models/auth" @@ -24,6 +26,17 @@ import ( "github.com/stretchr/testify/assert" ) +func testAPINewFile(t *testing.T, session *TestSession, user, repo, branch, treePath, content string) *httptest.ResponseRecorder { + url := fmt.Sprintf("/%s/%s/_new/%s", user, repo, branch) + req := NewRequestWithValues(t, "POST", url, map[string]string{ + "_csrf": GetCSRF(t, session, "/user/settings"), + "commit_choice": "direct", + "tree_path": treePath, + "content": content, + }) + return session.MakeRequest(t, req, http.StatusSeeOther) +} + func TestEmptyRepo(t *testing.T) { defer tests.PrepareTestEnv(t)() subPaths := []string{ diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index f9bd352b6221c..18f415083c9e6 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -485,6 +485,7 @@ func VerifyJSONSchema(t testing.TB, resp *httptest.ResponseRecorder, schemaFile assert.True(t, result.Valid()) } +// GetCSRF returns CSRF token from body func GetCSRF(t testing.TB, session *TestSession, urlStr string) string { t.Helper() req := NewRequest(t, "GET", urlStr) @@ -492,3 +493,11 @@ func GetCSRF(t testing.TB, session *TestSession, urlStr string) string { doc := NewHTMLParser(t, resp.Body) return doc.GetCSRF() } + +// GetCSRFFrom returns CSRF token from body +func GetCSRFFromCookie(t testing.TB, session *TestSession, urlStr string) string { + t.Helper() + req := NewRequest(t, "GET", urlStr) + session.MakeRequest(t, req, http.StatusOK) + return session.GetCookie("_csrf").Value +} diff --git a/tests/integration/pull_compare_test.go b/tests/integration/pull_compare_test.go index 39d9103dfd9f7..aed699fd20018 100644 --- a/tests/integration/pull_compare_test.go +++ b/tests/integration/pull_compare_test.go @@ -45,7 +45,7 @@ func TestPullCompare(t *testing.T) { defer tests.PrepareTestEnv(t)() session := loginUser(t, "user1") - testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testCreateBranch(t, session, "user1", "repo1", "branch/master", "master1", http.StatusSeeOther) testEditFile(t, session, "user1", "repo1", "master1", "README.md", "Hello, World (Edited)\n") testPullCreate(t, session, "user1", "repo1", false, "master", "master1", "This is a pull title") diff --git a/tests/integration/pull_create_test.go b/tests/integration/pull_create_test.go index 7add8e1db6516..5a06a7817f661 100644 --- a/tests/integration/pull_create_test.go +++ b/tests/integration/pull_create_test.go @@ -85,7 +85,7 @@ func testPullCreateDirectly(t *testing.T, session *TestSession, baseRepoOwner, b func TestPullCreate(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { session := loginUser(t, "user1") - testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") resp := testPullCreate(t, session, "user1", "repo1", false, "master", "master", "This is a pull title") @@ -113,7 +113,7 @@ func TestPullCreate(t *testing.T) { func TestPullCreate_TitleEscape(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { session := loginUser(t, "user1") - testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") resp := testPullCreate(t, session, "user1", "repo1", false, "master", "master", "XSS PR") @@ -177,7 +177,7 @@ func TestPullBranchDelete(t *testing.T) { defer tests.PrepareTestEnv(t)() session := loginUser(t, "user1") - testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testCreateBranch(t, session, "user1", "repo1", "branch/master", "master1", http.StatusSeeOther) testEditFile(t, session, "user1", "repo1", "master1", "README.md", "Hello, World (Edited)\n") resp := testPullCreate(t, session, "user1", "repo1", false, "master", "master1", "This is a pull title") diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index 979c408388760..3e7054c7e83d2 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -95,7 +95,7 @@ func TestPullMerge(t *testing.T) { hookTasksLenBefore := len(hookTasks) session := loginUser(t, "user1") - testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") resp := testPullCreate(t, session, "user1", "repo1", false, "master", "master", "This is a pull title") @@ -117,7 +117,7 @@ func TestPullRebase(t *testing.T) { hookTasksLenBefore := len(hookTasks) session := loginUser(t, "user1") - testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") resp := testPullCreate(t, session, "user1", "repo1", false, "master", "master", "This is a pull title") @@ -139,7 +139,7 @@ func TestPullRebaseMerge(t *testing.T) { hookTasksLenBefore := len(hookTasks) session := loginUser(t, "user1") - testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") resp := testPullCreate(t, session, "user1", "repo1", false, "master", "master", "This is a pull title") @@ -161,7 +161,7 @@ func TestPullSquash(t *testing.T) { hookTasksLenBefore := len(hookTasks) session := loginUser(t, "user1") - testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited!)\n") @@ -180,7 +180,7 @@ func TestPullSquash(t *testing.T) { func TestPullCleanUpAfterMerge(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { session := loginUser(t, "user1") - testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testEditFileToNewBranch(t, session, "user1", "repo1", "master", "feature/test", "README.md", "Hello, World (Edited - TestPullCleanUpAfterMerge)\n") resp := testPullCreate(t, session, "user1", "repo1", false, "master", "feature/test", "This is a pull title") @@ -215,7 +215,7 @@ func TestPullCleanUpAfterMerge(t *testing.T) { func TestCantMergeWorkInProgress(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { session := loginUser(t, "user1") - testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") resp := testPullCreate(t, session, "user1", "repo1", false, "master", "master", "[wip] This is a pull title") @@ -234,7 +234,7 @@ func TestCantMergeWorkInProgress(t *testing.T) { func TestCantMergeConflict(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { session := loginUser(t, "user1") - testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testEditFileToNewBranch(t, session, "user1", "repo1", "master", "conflict", "README.md", "Hello, World (Edited Once)\n") testEditFileToNewBranch(t, session, "user1", "repo1", "master", "base", "README.md", "Hello, World (Edited Twice)\n") @@ -280,7 +280,7 @@ func TestCantMergeConflict(t *testing.T) { func TestCantMergeUnrelated(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { session := loginUser(t, "user1") - testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testEditFileToNewBranch(t, session, "user1", "repo1", "master", "base", "README.md", "Hello, World (Edited Twice)\n") // Now we want to create a commit on a branch that is totally unrelated to our current head @@ -375,7 +375,7 @@ func TestCantMergeUnrelated(t *testing.T) { func TestFastForwardOnlyMerge(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { session := loginUser(t, "user1") - testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testEditFileToNewBranch(t, session, "user1", "repo1", "master", "update", "README.md", "Hello, World 2\n") // Use API to create a pr from update to master @@ -416,7 +416,7 @@ func TestFastForwardOnlyMerge(t *testing.T) { func TestCantFastForwardOnlyMergeDiverging(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { session := loginUser(t, "user1") - testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testEditFileToNewBranch(t, session, "user1", "repo1", "master", "diverging", "README.md", "Hello, World diverged\n") testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World 2\n") @@ -539,7 +539,7 @@ func TestPullRetargetChildOnBranchDelete(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { session := loginUser(t, "user1") testEditFileToNewBranch(t, session, "user2", "repo1", "master", "base-pr", "README.md", "Hello, World\n(Edited - TestPullRetargetOnCleanup - base PR)\n") - testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testEditFileToNewBranch(t, session, "user1", "repo1", "base-pr", "child-pr", "README.md", "Hello, World\n(Edited - TestPullRetargetOnCleanup - base PR)\n(Edited - TestPullRetargetOnCleanup - child PR)") respBasePR := testPullCreate(t, session, "user2", "repo1", true, "master", "base-pr", "Base Pull Request") @@ -568,7 +568,7 @@ func TestPullRetargetChildOnBranchDelete(t *testing.T) { func TestPullDontRetargetChildOnWrongRepo(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { session := loginUser(t, "user1") - testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testEditFileToNewBranch(t, session, "user1", "repo1", "master", "base-pr", "README.md", "Hello, World\n(Edited - TestPullDontRetargetChildOnWrongRepo - base PR)\n") testEditFileToNewBranch(t, session, "user1", "repo1", "base-pr", "child-pr", "README.md", "Hello, World\n(Edited - TestPullDontRetargetChildOnWrongRepo - base PR)\n(Edited - TestPullDontRetargetChildOnWrongRepo - child PR)") @@ -599,7 +599,7 @@ func TestPullMergeIndexerNotifier(t *testing.T) { onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { // create a pull request session := loginUser(t, "user1") - testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") createPullResp := testPullCreate(t, session, "user1", "repo1", false, "master", "master", "Indexer notifier test pull") @@ -676,7 +676,7 @@ func TestPullAutoMergeAfterCommitStatusSucceed(t *testing.T) { session := loginUser(t, "user1") user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) forkedName := "repo1-1" - testRepoFork(t, session, "user2", "repo1", "user1", forkedName) + testRepoFork(t, session, "user2", "repo1", "user1", forkedName, "") defer func() { testDeleteRepository(t, session, "user1", forkedName) }() @@ -759,7 +759,7 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApproval(t *testing.T) { session := loginUser(t, "user1") user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) forkedName := "repo1-2" - testRepoFork(t, session, "user2", "repo1", "user1", forkedName) + testRepoFork(t, session, "user2", "repo1", "user1", forkedName, "") defer func() { testDeleteRepository(t, session, "user1", forkedName) }() diff --git a/tests/integration/pull_review_test.go b/tests/integration/pull_review_test.go index df5d7b38ea49a..5ecf3ef46918b 100644 --- a/tests/integration/pull_review_test.go +++ b/tests/integration/pull_review_test.go @@ -186,7 +186,7 @@ func TestPullView_GivenApproveOrRejectReviewOnClosedPR(t *testing.T) { user2Session := loginUser(t, "user2") // Have user1 create a fork of repo1. - testRepoFork(t, user1Session, "user2", "repo1", "user1", "repo1") + testRepoFork(t, user1Session, "user2", "repo1", "user1", "repo1", "") t.Run("Submit approve/reject review on merged PR", func(t *testing.T) { // Create a merged PR (made by user1) in the upstream repo1. diff --git a/tests/integration/pull_status_test.go b/tests/integration/pull_status_test.go index 80eea34513f32..26e1baeb11305 100644 --- a/tests/integration/pull_status_test.go +++ b/tests/integration/pull_status_test.go @@ -23,7 +23,7 @@ import ( func TestPullCreate_CommitStatus(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { session := loginUser(t, "user1") - testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testEditFileToNewBranch(t, session, "user1", "repo1", "master", "status1", "README.md", "status1") url := path.Join("user1", "repo1", "compare", "master...status1") @@ -122,7 +122,7 @@ func TestPullCreate_EmptyChangesWithDifferentCommits(t *testing.T) { // so we need to have this meta commit also in develop branch. onGiteaRun(t, func(t *testing.T, u *url.URL) { session := loginUser(t, "user1") - testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testEditFileToNewBranch(t, session, "user1", "repo1", "master", "status1", "README.md", "status1") testEditFileToNewBranch(t, session, "user1", "repo1", "status1", "status1", "README.md", "# repo1\n\nDescription for repo1") @@ -147,7 +147,7 @@ func TestPullCreate_EmptyChangesWithDifferentCommits(t *testing.T) { func TestPullCreate_EmptyChangesWithSameCommits(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { session := loginUser(t, "user1") - testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testCreateBranch(t, session, "user1", "repo1", "branch/master", "status1", http.StatusSeeOther) url := path.Join("user1", "repo1", "compare", "master...status1") req := NewRequestWithValues(t, "POST", url, diff --git a/tests/integration/repo_activity_test.go b/tests/integration/repo_activity_test.go index 792554db4bc93..b04560379d2cd 100644 --- a/tests/integration/repo_activity_test.go +++ b/tests/integration/repo_activity_test.go @@ -20,7 +20,7 @@ func TestRepoActivity(t *testing.T) { session := loginUser(t, "user1") // Create PRs (1 merged & 2 proposed) - testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") resp := testPullCreate(t, session, "user1", "repo1", false, "master", "master", "This is a pull title") elem := strings.Split(test.RedirectURL(resp), "/") diff --git a/tests/integration/repo_branch_test.go b/tests/integration/repo_branch_test.go index baa8da4b75956..d1bc9198c32f4 100644 --- a/tests/integration/repo_branch_test.go +++ b/tests/integration/repo_branch_test.go @@ -4,26 +4,37 @@ package integration import ( + "fmt" "net/http" "net/url" "path" "strings" "testing" + auth_model "code.gitea.io/gitea/models/auth" + org_model "code.gitea.io/gitea/models/organization" + "code.gitea.io/gitea/models/perm" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unit" + "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/setting" + api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/translation" "code.gitea.io/gitea/tests" + "github.com/PuerkitoBio/goquery" "github.com/stretchr/testify/assert" ) func testCreateBranch(t testing.TB, session *TestSession, user, repo, oldRefSubURL, newBranchName string, expectedStatus int) string { var csrf string if expectedStatus == http.StatusNotFound { - csrf = GetCSRF(t, session, path.Join(user, repo, "src/branch/master")) + // src/branch/branch_name may not container "_csrf" input, + // so we need to get it from cookies not from body + csrf = GetCSRFFromCookie(t, session, path.Join(user, repo, "src/branch/master")) } else { - csrf = GetCSRF(t, session, path.Join(user, repo, "src", oldRefSubURL)) + csrf = GetCSRFFromCookie(t, session, path.Join(user, repo, "src", oldRefSubURL)) } req := NewRequestWithValues(t, "POST", path.Join(user, repo, "branches/_new", oldRefSubURL), map[string]string{ "_csrf": csrf, @@ -145,3 +156,136 @@ func TestCreateBranchInvalidCSRF(t *testing.T) { strings.TrimSpace(htmlDoc.doc.Find(".ui.message").Text()), ) } + +func prepareBranch(t *testing.T, session *TestSession, repo *repo_model.Repository) { + baseRefSubURL := fmt.Sprintf("branch/%s", repo.DefaultBranch) + + // create branch with no new commit + testCreateBranch(t, session, repo.OwnerName, repo.Name, baseRefSubURL, "no-commit", http.StatusSeeOther) + + // create branch with commit + testCreateBranch(t, session, repo.OwnerName, repo.Name, baseRefSubURL, "new-commit", http.StatusSeeOther) + testAPINewFile(t, session, repo.OwnerName, repo.Name, "new-commit", "new-commit.txt", "new-commit") + + // create deleted branch + testCreateBranch(t, session, repo.OwnerName, repo.Name, "branch/new-commit", "deleted-branch", http.StatusSeeOther) + testUIDeleteBranch(t, session, repo.OwnerName, repo.Name, "deleted-branch") +} + +func testCreatePullToDefaultBranch(t *testing.T, session *TestSession, baseRepo, headRepo *repo_model.Repository, headBranch, title string) string { + srcRef := headBranch + if baseRepo.ID != headRepo.ID { + srcRef = fmt.Sprintf("%s/%s:%s", headRepo.OwnerName, headRepo.Name, headBranch) + } + resp := testPullCreate(t, session, baseRepo.OwnerName, baseRepo.Name, false, baseRepo.DefaultBranch, srcRef, title) + elem := strings.Split(test.RedirectURL(resp), "/") + // return pull request ID + return elem[4] +} + +func prepareRepoPR(t *testing.T, baseSession, headSession *TestSession, baseRepo, headRepo *repo_model.Repository) { + // create opening PR + testCreateBranch(t, headSession, headRepo.OwnerName, headRepo.Name, "branch/new-commit", "opening-pr", http.StatusSeeOther) + testCreatePullToDefaultBranch(t, baseSession, baseRepo, headRepo, "opening-pr", "opening pr") + + // create closed PR + testCreateBranch(t, headSession, headRepo.OwnerName, headRepo.Name, "branch/new-commit", "closed-pr", http.StatusSeeOther) + prID := testCreatePullToDefaultBranch(t, baseSession, baseRepo, headRepo, "closed-pr", "closed pr") + testIssueClose(t, baseSession, baseRepo.OwnerName, baseRepo.Name, prID) + + // create closed PR with deleted branch + testCreateBranch(t, headSession, headRepo.OwnerName, headRepo.Name, "branch/new-commit", "closed-pr-deleted", http.StatusSeeOther) + prID = testCreatePullToDefaultBranch(t, baseSession, baseRepo, headRepo, "closed-pr-deleted", "closed pr with deleted branch") + testIssueClose(t, baseSession, baseRepo.OwnerName, baseRepo.Name, prID) + testUIDeleteBranch(t, headSession, headRepo.OwnerName, headRepo.Name, "closed-pr-deleted") + + // create merged PR + testCreateBranch(t, headSession, headRepo.OwnerName, headRepo.Name, "branch/new-commit", "merged-pr", http.StatusSeeOther) + prID = testCreatePullToDefaultBranch(t, baseSession, baseRepo, headRepo, "merged-pr", "merged pr") + testAPINewFile(t, headSession, headRepo.OwnerName, headRepo.Name, "merged-pr", fmt.Sprintf("new-commit-%s.txt", headRepo.Name), "new-commit") + testPullMerge(t, baseSession, baseRepo.OwnerName, baseRepo.Name, prID, repo_model.MergeStyleRebaseMerge, false) + + // create merged PR with deleted branch + testCreateBranch(t, headSession, headRepo.OwnerName, headRepo.Name, "branch/new-commit", "merged-pr-deleted", http.StatusSeeOther) + prID = testCreatePullToDefaultBranch(t, baseSession, baseRepo, headRepo, "merged-pr-deleted", "merged pr with deleted branch") + testAPINewFile(t, headSession, headRepo.OwnerName, headRepo.Name, "merged-pr-deleted", fmt.Sprintf("new-commit-%s-2.txt", headRepo.Name), "new-commit") + testPullMerge(t, baseSession, baseRepo.OwnerName, baseRepo.Name, prID, repo_model.MergeStyleRebaseMerge, true) +} + +func checkRecentlyPushedNewBranches(t *testing.T, session *TestSession, repoPath string, expected []string) { + branches := make([]string, 0, 2) + req := NewRequest(t, "GET", repoPath) + resp := session.MakeRequest(t, req, http.StatusOK) + doc := NewHTMLParser(t, resp.Body) + doc.doc.Find(".ui.positive.message div a").Each(func(index int, branch *goquery.Selection) { + branches = append(branches, branch.Text()) + }) + assert.Equal(t, expected, branches) +} + +func TestRecentlyPushedNewBranches(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + onGiteaRun(t, func(t *testing.T, u *url.URL) { + user1Session := loginUser(t, "user1") + user2Session := loginUser(t, "user2") + user12Session := loginUser(t, "user12") + user13Session := loginUser(t, "user13") + + // prepare branch and PRs in original repo + repo10 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10}) + prepareBranch(t, user12Session, repo10) + prepareRepoPR(t, user12Session, user12Session, repo10, repo10) + + // outdated new branch should not be displayed + checkRecentlyPushedNewBranches(t, user12Session, "user12/repo10", []string{"new-commit"}) + + // create a fork repo in public org + testRepoFork(t, user12Session, repo10.OwnerName, repo10.Name, "org25", "org25_fork_repo10", "new-commit") + orgPublicForkRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: 25, Name: "org25_fork_repo10"}) + prepareRepoPR(t, user12Session, user12Session, repo10, orgPublicForkRepo) + + // user12 is the owner of the repo10 and the organization org25 + // in repo10, user12 has opening/closed/merged pr and closed/merged pr with deleted branch + checkRecentlyPushedNewBranches(t, user12Session, "user12/repo10", []string{"org25/org25_fork_repo10:new-commit", "new-commit"}) + + userForkRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 11}) + testCtx := NewAPITestContext(t, repo10.OwnerName, repo10.Name, auth_model.AccessTokenScopeWriteRepository) + t.Run("AddUser13AsCollaborator", doAPIAddCollaborator(testCtx, "user13", perm.AccessModeWrite)) + prepareBranch(t, user13Session, userForkRepo) + prepareRepoPR(t, user13Session, user13Session, repo10, userForkRepo) + + // create branch with same name in different repo by user13 + testCreateBranch(t, user13Session, repo10.OwnerName, repo10.Name, "branch/new-commit", "same-name-branch", http.StatusSeeOther) + testCreateBranch(t, user13Session, userForkRepo.OwnerName, userForkRepo.Name, "branch/new-commit", "same-name-branch", http.StatusSeeOther) + testCreatePullToDefaultBranch(t, user13Session, repo10, userForkRepo, "same-name-branch", "same name branch pr") + + // user13 pushed 2 branches with the same name in repo10 and repo11 + // and repo11's branch has a pr, but repo10's branch doesn't + // in this case, we should get repo10's branch but not repo11's branch + checkRecentlyPushedNewBranches(t, user13Session, "user12/repo10", []string{"same-name-branch", "user13/repo11:new-commit"}) + + // create a fork repo in private org + testRepoFork(t, user1Session, repo10.OwnerName, repo10.Name, "private_org35", "org35_fork_repo10", "new-commit") + orgPrivateForkRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: 35, Name: "org35_fork_repo10"}) + prepareRepoPR(t, user1Session, user1Session, repo10, orgPrivateForkRepo) + + // user1 is the owner of private_org35 and no write permission to repo10 + // so user1 can only see the branch in org35_fork_repo10 + checkRecentlyPushedNewBranches(t, user1Session, "user12/repo10", []string{"private_org35/org35_fork_repo10:new-commit"}) + + // user2 push a branch in private_org35 + testCreateBranch(t, user2Session, orgPrivateForkRepo.OwnerName, orgPrivateForkRepo.Name, "branch/new-commit", "user-read-permission", http.StatusSeeOther) + // convert write permission to read permission for code unit + token := getTokenForLoggedInUser(t, user1Session, auth_model.AccessTokenScopeWriteOrganization) + req := NewRequestWithJSON(t, "PATCH", fmt.Sprintf("/api/v1/teams/%d", 24), &api.EditTeamOption{ + Name: "team24", + UnitsMap: map[string]string{"repo.code": "read"}, + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusOK) + teamUnit := unittest.AssertExistsAndLoadBean(t, &org_model.TeamUnit{TeamID: 24, Type: unit.TypeCode}) + assert.Equal(t, perm.AccessModeRead, teamUnit.AccessMode) + // user2 can see the branch as it is created by user2 + checkRecentlyPushedNewBranches(t, user2Session, "user12/repo10", []string{"private_org35/org35_fork_repo10:user-read-permission"}) + }) +} diff --git a/tests/integration/repo_fork_test.go b/tests/integration/repo_fork_test.go index ca5d61ecc2a90..feebebf062081 100644 --- a/tests/integration/repo_fork_test.go +++ b/tests/integration/repo_fork_test.go @@ -16,7 +16,7 @@ import ( "github.com/stretchr/testify/assert" ) -func testRepoFork(t *testing.T, session *TestSession, ownerName, repoName, forkOwnerName, forkRepoName string) *httptest.ResponseRecorder { +func testRepoFork(t *testing.T, session *TestSession, ownerName, repoName, forkOwnerName, forkRepoName, forkBranch string) *httptest.ResponseRecorder { forkOwner := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: forkOwnerName}) // Step0: check the existence of the to-fork repo @@ -41,9 +41,10 @@ func testRepoFork(t *testing.T, session *TestSession, ownerName, repoName, forkO _, exists = htmlDoc.doc.Find(fmt.Sprintf(".owner.dropdown .item[data-value=\"%d\"]", forkOwner.ID)).Attr("data-value") assert.True(t, exists, fmt.Sprintf("Fork owner '%s' is not present in select box", forkOwnerName)) req = NewRequestWithValues(t, "POST", link, map[string]string{ - "_csrf": htmlDoc.GetCSRF(), - "uid": fmt.Sprintf("%d", forkOwner.ID), - "repo_name": forkRepoName, + "_csrf": htmlDoc.GetCSRF(), + "uid": fmt.Sprintf("%d", forkOwner.ID), + "repo_name": forkRepoName, + "fork_single_branch": forkBranch, }) session.MakeRequest(t, req, http.StatusSeeOther) @@ -57,13 +58,13 @@ func testRepoFork(t *testing.T, session *TestSession, ownerName, repoName, forkO func TestRepoFork(t *testing.T) { defer tests.PrepareTestEnv(t)() session := loginUser(t, "user1") - testRepoFork(t, session, "user2", "repo1", "user1", "repo1") + testRepoFork(t, session, "user2", "repo1", "user1", "repo1", "") } func TestRepoForkToOrg(t *testing.T) { defer tests.PrepareTestEnv(t)() session := loginUser(t, "user2") - testRepoFork(t, session, "user2", "repo1", "org3", "repo1") + testRepoFork(t, session, "user2", "repo1", "org3", "repo1", "") // Check that no more forking is allowed as user2 owns repository // and org3 organization that owner user2 is also now has forked this repository From 3066114c2481b5f3a5e4cda65fdd22e768359e94 Mon Sep 17 00:00:00 2001 From: GiteaBot Date: Wed, 22 May 2024 00:25:10 +0000 Subject: [PATCH 4/5] [skip ci] Updated translations via Crowdin --- options/locale/locale_pt-PT.ini | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/options/locale/locale_pt-PT.ini b/options/locale/locale_pt-PT.ini index f4c77e4981cd3..ea4c2d26dcec3 100644 --- a/options/locale/locale_pt-PT.ini +++ b/options/locale/locale_pt-PT.ini @@ -807,7 +807,7 @@ add_new_key=Adicionar Chave SSH add_new_gpg_key=Adicionar chave GPG key_content_ssh_placeholder=Começa com 'ssh-ed25519', 'ssh-rsa', 'ecdsa-sha2-nistp256', 'ecdsa-sha2-nistp384', 'ecdsa-sha2-nistp521', 'sk-ecdsa-sha2-nistp256@openssh.com', ou 'sk-ssh-ed25519@openssh.com' key_content_gpg_placeholder=Começa com '-----BEGIN PGP PUBLIC KEY BLOCK-----' -add_new_principal=Adicional Protagonista +add_new_principal=Adicionar protagonista ssh_key_been_used=Esta chave SSH já tinha sido adicionada ao servidor. ssh_key_name_used=Já existe uma chave SSH com o mesmo nome na sua conta. ssh_principal_been_used=Este protagonista já tinha sido adicionado ao servidor. @@ -3348,6 +3348,7 @@ mirror_sync_create=sincronizou a nova referência %[3]s para mirror_sync_delete=sincronizou e eliminou a referência %[2]s em %[3]s da réplica approve_pull_request=`aprovou %[3]s#%[2]s` reject_pull_request=`sugeriu modificações para %[3]s#%[2]s` +publish_release=`lançou "%[4]s" em %[3]s` review_dismissed=`descartou a revisão de %[4]s para %[3]s#%[2]s` review_dismissed_reason=Motivo: create_branch=criou o ramo %[3]s em %[4]s @@ -3414,6 +3415,7 @@ error.unit_not_allowed=Não tem permissão para aceder a esta parte do repositó title=Pacotes desc=Gerir pacotes do repositório. empty=Ainda não há pacotes. +no_metadata=Sem metadados. empty.documentation=Para obter mais informação sobre o registo de pacotes, veja a documentação. empty.repo=Carregou um pacote mas este não é apresentado aqui? Vá às configurações do pacote e ligue-o a este repositório. registry.documentation=Para mais informação sobre o registo %s, veja a documentação. From de6f0488a67ad65bd2ac40356b08a78a365414cd Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 22 May 2024 02:47:18 +0200 Subject: [PATCH 5/5] Add nix flake for dev shell (#30967) To try it you need **nix** installed `nix-daemon ` running and your user has to be member of the **nix-users** group. Or use NixOS. then by just: ```sh nix develop -c $SHELL ``` a dedicated development environment with all needed packages will be created. --- flake.lock | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ flake.nix | 37 +++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+) create mode 100644 flake.lock create mode 100644 flake.nix diff --git a/flake.lock b/flake.lock new file mode 100644 index 0000000000000..0b2278f080265 --- /dev/null +++ b/flake.lock @@ -0,0 +1,61 @@ +{ + "nodes": { + "flake-utils": { + "inputs": { + "systems": "systems" + }, + "locked": { + "lastModified": 1710146030, + "narHash": "sha256-SZ5L6eA7HJ/nmkzGG7/ISclqe6oZdOZTNoesiInkXPQ=", + "owner": "numtide", + "repo": "flake-utils", + "rev": "b1d9ab70662946ef0850d488da1c9019f3a9752a", + "type": "github" + }, + "original": { + "owner": "numtide", + "repo": "flake-utils", + "type": "github" + } + }, + "nixpkgs": { + "locked": { + "lastModified": 1715534503, + "narHash": "sha256-5ZSVkFadZbFP1THataCaSf0JH2cAH3S29hU9rrxTEqk=", + "owner": "nixos", + "repo": "nixpkgs", + "rev": "2057814051972fa1453ddfb0d98badbea9b83c06", + "type": "github" + }, + "original": { + "owner": "nixos", + "ref": "nixos-unstable", + "repo": "nixpkgs", + "type": "github" + } + }, + "root": { + "inputs": { + "flake-utils": "flake-utils", + "nixpkgs": "nixpkgs" + } + }, + "systems": { + "locked": { + "lastModified": 1681028828, + "narHash": "sha256-Vy1rq5AaRuLzOxct8nz4T6wlgyUR7zLU309k9mBC768=", + "owner": "nix-systems", + "repo": "default", + "rev": "da67096a3b9bf56a91d16901293e51ba5b49a27e", + "type": "github" + }, + "original": { + "owner": "nix-systems", + "repo": "default", + "type": "github" + } + } + }, + "root": "root", + "version": 7 +} diff --git a/flake.nix b/flake.nix new file mode 100644 index 0000000000000..c6e915e9db4f7 --- /dev/null +++ b/flake.nix @@ -0,0 +1,37 @@ +{ + inputs = { + nixpkgs.url = "github:nixos/nixpkgs?ref=nixos-unstable"; + flake-utils.url = "github:numtide/flake-utils"; + }; + outputs = + { nixpkgs, flake-utils, ... }: + flake-utils.lib.eachDefaultSystem ( + system: + let + pkgs = nixpkgs.legacyPackages.${system}; + in + { + devShells.default = pkgs.mkShell { + buildInputs = with pkgs; [ + # generic + git + git-lfs + gnumake + gnused + gnutar + gzip + + # frontend + nodejs_20 + + # linting + python312 + poetry + + # backend + go_1_22 + ]; + }; + } + ); +}