Merge branch 'main' into lunny/issue_dev

services/pull/check.go

@@ -282,9 +282,6 @@ func manuallyMerged(ctx context.Context, pr *issues_model.PullRequest) bool {
 		return false
 	}
 
-	pr.MergedCommitID = commit.ID.String()
-	pr.MergedUnix = timeutil.TimeStamp(commit.Author.When.Unix())
-	pr.Status = issues_model.PullRequestStatusManuallyMerged
 	merger, _ := user_model.GetUserByEmail(ctx, commit.Author.Email)
 
 	// When the commit author is unknown set the BaseRepo owner as merger
@@ -297,10 +294,8 @@ func manuallyMerged(ctx context.Context, pr *issues_model.PullRequest) bool {
 		}
 		merger = pr.BaseRepo.Owner
 	}
-	pr.Merger = merger
-	pr.MergerID = merger.ID
 
-	if merged, err := SetMerged(ctx, pr); err != nil {
+	if merged, err := SetMerged(ctx, pr, commit.ID.String(), timeutil.TimeStamp(commit.Author.When.Unix()), merger, issues_model.PullRequestStatusManuallyMerged); err != nil {
 		log.Error("%-v setMerged : %v", pr, err)
 		return false
 	} else if !merged {

services/pull/merge.go

@@ -17,6 +17,7 @@ import (
 	git_model "code.gitea.io/gitea/models/git"
 	issues_model "code.gitea.io/gitea/models/issues"
 	access_model "code.gitea.io/gitea/models/perm/access"
+	pull_model "code.gitea.io/gitea/models/pull"
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unit"
 	user_model "code.gitea.io/gitea/models/user"
@@ -632,14 +633,8 @@ func MergedManually(ctx context.Context, pr *issues_model.PullRequest, doer *use
 			return fmt.Errorf("Wrong commit ID")
 		}
 
-		pr.MergedCommitID = commitID
-		pr.MergedUnix = timeutil.TimeStamp(commit.Author.When.Unix())
-		pr.Status = issues_model.PullRequestStatusManuallyMerged
-		pr.Merger = doer
-		pr.MergerID = doer.ID
-
 		var merged bool
-		if merged, err = SetMerged(ctx, pr); err != nil {
+		if merged, err = SetMerged(ctx, pr, commitID, timeutil.TimeStamp(commit.Author.When.Unix()), doer, issues_model.PullRequestStatusManuallyMerged); err != nil {
 			return err
 		} else if !merged {
 			return fmt.Errorf("SetMerged failed")
@@ -658,41 +653,35 @@ func MergedManually(ctx context.Context, pr *issues_model.PullRequest, doer *use
 }
 
 // SetMerged sets a pull request to merged and closes the corresponding issue
-func SetMerged(ctx context.Context, pr *issues_model.PullRequest) (bool, error) {
+func SetMerged(ctx context.Context, pr *issues_model.PullRequest, mergedCommitID string, mergedTimeStamp timeutil.TimeStamp, merger *user_model.User, mergeStatus issues_model.PullRequestStatus) (bool, error) {
 	if pr.HasMerged {
 		return false, fmt.Errorf("PullRequest[%d] already merged", pr.Index)
 	}
+
+	pr.HasMerged = true
+	pr.MergedCommitID = mergedCommitID
+	pr.MergedUnix = mergedTimeStamp
+	pr.Merger = merger
+	pr.MergerID = merger.ID
+	pr.Status = mergeStatus
+	// reset the conflicted files as there cannot be any if we're merged
+	pr.ConflictedFiles = []string{}
+
 	if pr.MergedCommitID == "" || pr.MergedUnix == 0 || pr.Merger == nil {
 		return false, fmt.Errorf("unable to merge PullRequest[%d], some required fields are empty", pr.Index)
 	}
 
-	pr.HasMerged = true
-	sess := db.GetEngine(ctx)
-
-	if _, err := sess.Exec("UPDATE `issue` SET `repo_id` = `repo_id` WHERE `id` = ?", pr.IssueID); err != nil {
-		return false, err
-	}
-
-	if _, err := sess.Exec("UPDATE `pull_request` SET `issue_id` = `issue_id` WHERE `id` = ?", pr.ID); err != nil {
+	ctx, committer, err := db.TxContext(ctx)
+	if err != nil {
 		return false, err
 	}
+	defer committer.Close()
 
 	pr.Issue = nil
 	if err := pr.LoadIssue(ctx); err != nil {
 		return false, err
 	}
 
-	if tmpPr, err := issues_model.GetPullRequestByID(ctx, pr.ID); err != nil {
-		return false, err
-	} else if tmpPr.HasMerged {
-		if pr.Issue.IsClosed {
-			return false, nil
-		}
-		return false, fmt.Errorf("PullRequest[%d] already merged but it's associated issue [%d] is not closed", pr.Index, pr.IssueID)
-	} else if pr.Issue.IsClosed {
-		return false, fmt.Errorf("PullRequest[%d] already closed", pr.Index)
-	}
-
 	if err := pr.Issue.LoadRepo(ctx); err != nil {
 		return false, err
 	}
@@ -701,16 +690,28 @@ func SetMerged(ctx context.Context, pr *issues_model.PullRequest) (bool, error)
 		return false, err
 	}
 
-	if _, err := issues_model.ChangeIssueStatus(ctx, pr.Issue, pr.Merger, true, true); err != nil {
+	// Removing an auto merge pull and ignore if not exist
+	if err := pull_model.DeleteScheduledAutoMerge(ctx, pr.ID); err != nil && !db.IsErrNotExist(err) {
+		return false, fmt.Errorf("DeleteScheduledAutoMerge[%d]: %v", pr.ID, err)
+	}
+
+	// Set issue as closed
+	if _, err := issues_model.SetIssueAsClosed(ctx, pr.Issue, pr.Merger, true); err != nil {
 		return false, fmt.Errorf("ChangeIssueStatus: %w", err)
 	}
 
-	// reset the conflicted files as there cannot be any if we're merged
-	pr.ConflictedFiles = []string{}
-
 	// We need to save all of the data used to compute this merge as it may have already been changed by TestPatch. FIXME: need to set some state to prevent TestPatch from running whilst we are merging.
-	if _, err := sess.Where("id = ?", pr.ID).Cols("has_merged, status, merge_base, merged_commit_id, merger_id, merged_unix, conflicted_files").Update(pr); err != nil {
+	if cnt, err := db.GetEngine(ctx).Where("id = ?", pr.ID).
+		And("has_merged = ?", false).
+		Cols("has_merged, status, merge_base, merged_commit_id, merger_id, merged_unix, conflicted_files").
+		Update(pr); err != nil {
 		return false, fmt.Errorf("failed to update pr[%d]: %w", pr.ID, err)
+	} else if cnt != 1 {
+		return false, issues_model.ErrIssueAlreadyChanged
+	}
+
+	if err := committer.Commit(); err != nil {
+		return false, err
 	}
 
 	return true, nil

services/pull/pull.go

@@ -6,6 +6,7 @@ package pull
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -283,6 +284,7 @@ func ChangeTargetBranch(ctx context.Context, pr *issues_model.PullRequest, doer
 			ID:     pr.Issue.ID,
 			RepoID: pr.Issue.RepoID,
 			Index:  pr.Issue.Index,
+			IsPull: true,
 		}
 	}
 
@@ -653,33 +655,9 @@ func UpdateRef(ctx context.Context, pr *issues_model.PullRequest) (err error) {
 	return err
 }
 
-type errlist []error
-
-func (errs errlist) Error() string {
-	if len(errs) > 0 {
-		var buf strings.Builder
-		for i, err := range errs {
-			if i > 0 {
-				buf.WriteString(", ")
-			}
-			buf.WriteString(err.Error())
-		}
-		return buf.String()
-	}
-	return ""
-}
-
-// RetargetChildrenOnMerge retarget children pull requests on merge if possible
-func RetargetChildrenOnMerge(ctx context.Context, doer *user_model.User, pr *issues_model.PullRequest) error {
-	if setting.Repository.PullRequest.RetargetChildrenOnMerge && pr.BaseRepoID == pr.HeadRepoID {
-		return RetargetBranchPulls(ctx, doer, pr.HeadRepoID, pr.HeadBranch, pr.BaseBranch)
-	}
-	return nil
-}
-
-// RetargetBranchPulls change target branch for all pull requests whose base branch is the branch
+// retargetBranchPulls change target branch for all pull requests whose base branch is the branch
 // Both branch and targetBranch must be in the same repo (for security reasons)
-func RetargetBranchPulls(ctx context.Context, doer *user_model.User, repoID int64, branch, targetBranch string) error {
+func retargetBranchPulls(ctx context.Context, doer *user_model.User, repoID int64, branch, targetBranch string) error {
 	prs, err := issues_model.GetUnmergedPullRequestsByBaseInfo(ctx, repoID, branch)
 	if err != nil {
 		return err
@@ -689,7 +667,7 @@ func RetargetBranchPulls(ctx context.Context, doer *user_model.User, repoID int6
 		return err
 	}
 
-	var errs errlist
+	var errs []error
 	for _, pr := range prs {
 		if err = pr.Issue.LoadRepo(ctx); err != nil {
 			errs = append(errs, err)
@@ -699,40 +677,75 @@ func RetargetBranchPulls(ctx context.Context, doer *user_model.User, repoID int6
 			errs = append(errs, err)
 		}
 	}
-
-	if len(errs) > 0 {
-		return errs
-	}
-	return nil
+	return errors.Join(errs...)
 }
 
-// CloseBranchPulls close all the pull requests who's head branch is the branch
-func CloseBranchPulls(ctx context.Context, doer *user_model.User, repoID int64, branch string) error {
-	prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(ctx, repoID, branch)
+// AdjustPullsCausedByBranchDeleted close all the pull requests who's head branch is the branch
+// Or Close all the plls who's base branch is the branch if setting.Repository.PullRequest.RetargetChildrenOnMerge is false.
+// If it's true, Retarget all these pulls to the default branch.
+func AdjustPullsCausedByBranchDeleted(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, branch string) error {
+	// branch as head branch
+	prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(ctx, repo.ID, branch)
 	if err != nil {
 		return err
 	}
 
-	prs2, err := issues_model.GetUnmergedPullRequestsByBaseInfo(ctx, repoID, branch)
-	if err != nil {
-		return err
-	}
-
-	prs = append(prs, prs2...)
 	if err := issues_model.PullRequestList(prs).LoadAttributes(ctx); err != nil {
 		return err
 	}
+	issues_model.PullRequestList(prs).SetHeadRepo(repo)
+	if err := issues_model.PullRequestList(prs).LoadRepositories(ctx); err != nil {
+		return err
+	}
 
-	var errs errlist
+	var errs []error
 	for _, pr := range prs {
-		if err = issue_service.CloseIssue(ctx, pr.Issue, doer, ""); err != nil && !issues_model.IsErrPullWasClosed(err) && !issues_model.IsErrDependenciesLeft(err) {
+		if err = issue_service.CloseIssue(ctx, pr.Issue, doer, ""); err != nil && !issues_model.IsErrIssueIsClosed(err) && !issues_model.IsErrDependenciesLeft(err) {
 			errs = append(errs, err)
 		}
+		if err == nil {
+			if err := issues_model.AddDeletePRBranchComment(ctx, doer, pr.BaseRepo, pr.Issue.ID, pr.HeadBranch); err != nil {
+				log.Error("AddDeletePRBranchComment: %v", err)
+				errs = append(errs, err)
+			}
+		}
 	}
-	if len(errs) > 0 {
-		return errs
+
+	if setting.Repository.PullRequest.RetargetChildrenOnMerge {
+		if err := retargetBranchPulls(ctx, doer, repo.ID, branch, repo.DefaultBranch); err != nil {
+			log.Error("retargetBranchPulls failed: %v", err)
+			errs = append(errs, err)
+		}
+		return errors.Join(errs...)
 	}
-	return nil
+
+	// branch as base branch
+	prs, err = issues_model.GetUnmergedPullRequestsByBaseInfo(ctx, repo.ID, branch)
+	if err != nil {
+		return err
+	}
+
+	if err := issues_model.PullRequestList(prs).LoadAttributes(ctx); err != nil {
+		return err
+	}
+	issues_model.PullRequestList(prs).SetBaseRepo(repo)
+	if err := issues_model.PullRequestList(prs).LoadRepositories(ctx); err != nil {
+		return err
+	}
+
+	errs = nil
+	for _, pr := range prs {
+		if err = issues_model.AddDeletePRBranchComment(ctx, doer, pr.BaseRepo, pr.Issue.ID, pr.BaseBranch); err != nil {
+			log.Error("AddDeletePRBranchComment: %v", err)
+			errs = append(errs, err)
+		}
+		if err == nil {
+			if err = issue_service.CloseIssue(ctx, pr.Issue, doer, ""); err != nil && !issues_model.IsErrIssueIsClosed(err) && !issues_model.IsErrDependenciesLeft(err) {
+				errs = append(errs, err)
+			}
+		}
+	}
+	return errors.Join(errs...)
 }
 
 // CloseRepoBranchesPulls close all pull requests which head branches are in the given repository, but only whose base repo is not in the given repository
@@ -742,7 +755,7 @@ func CloseRepoBranchesPulls(ctx context.Context, doer *user_model.User, repo *re
 		return err
 	}
 
-	var errs errlist
+	var errs []error
 	for _, branch := range branches {
 		prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(ctx, repo.ID, branch.Name)
 		if err != nil {
@@ -759,16 +772,13 @@ func CloseRepoBranchesPulls(ctx context.Context, doer *user_model.User, repo *re
 			if pr.BaseRepoID == repo.ID {
 				continue
 			}
-			if err = issue_service.CloseIssue(ctx, pr.Issue, doer, ""); err != nil && !issues_model.IsErrPullWasClosed(err) {
+			if err = issue_service.CloseIssue(ctx, pr.Issue, doer, ""); err != nil && !issues_model.IsErrIssueIsClosed(err) {
 				errs = append(errs, err)
 			}
 		}
 	}
 
-	if len(errs) > 0 {
-		return errs
-	}
-	return nil
+	return errors.Join(errs...)
 }
 
 var commitMessageTrailersPattern = regexp.MustCompile(`(?:^|\n\n)(?:[\w-]+[ \t]*:[^\n]+\n*(?:[ \t]+[^\n]+\n*)*)+$`)