aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--models/issues/issue.go33
-rw-r--r--models/issues/issue_update.go121
-rw-r--r--models/issues/pull.go16
-rw-r--r--routers/private/hook_post_receive.go22
-rw-r--r--services/pull/check.go7
-rw-r--r--services/pull/merge.go63
-rw-r--r--services/pull/pull.go5
7 files changed, 121 insertions, 146 deletions
diff --git a/models/issues/issue.go b/models/issues/issue.go
index fe347c2715..1777fbb6a6 100644
--- a/models/issues/issue.go
+++ b/models/issues/issue.go
@@ -46,23 +46,6 @@ func (err ErrIssueNotExist) Unwrap() error {
return util.ErrNotExist
}
-// ErrIssueIsClosed represents a "IssueIsClosed" kind of error.
-type ErrIssueIsClosed struct {
- ID int64
- RepoID int64
- Index int64
-}
-
-// IsErrIssueIsClosed checks if an error is a ErrIssueNotExist.
-func IsErrIssueIsClosed(err error) bool {
- _, ok := err.(ErrIssueIsClosed)
- return ok
-}
-
-func (err ErrIssueIsClosed) Error() string {
- return fmt.Sprintf("issue is closed [id: %d, repo_id: %d, index: %d]", err.ID, err.RepoID, err.Index)
-}
-
// ErrNewIssueInsert is used when the INSERT statement in newIssue fails
type ErrNewIssueInsert struct {
OriginalError error
@@ -78,22 +61,6 @@ func (err ErrNewIssueInsert) Error() string {
return err.OriginalError.Error()
}
-// ErrIssueWasClosed is used when close a closed issue
-type ErrIssueWasClosed struct {
- ID int64
- Index int64
-}
-
-// IsErrIssueWasClosed checks if an error is a ErrIssueWasClosed.
-func IsErrIssueWasClosed(err error) bool {
- _, ok := err.(ErrIssueWasClosed)
- return ok
-}
-
-func (err ErrIssueWasClosed) Error() string {
- return fmt.Sprintf("Issue [%d] %d was already closed", err.ID, err.Index)
-}
-
var ErrIssueAlreadyChanged = util.NewInvalidArgumentErrorf("the issue is already changed")
// Issue represents an issue or pull request of repository.
diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go
index 479834045c..7b3fe04aa5 100644
--- a/models/issues/issue_update.go
+++ b/models/issues/issue_update.go
@@ -28,38 +28,40 @@ import (
// UpdateIssueCols updates cols of issue
func UpdateIssueCols(ctx context.Context, issue *Issue, cols ...string) error {
- if _, err := db.GetEngine(ctx).ID(issue.ID).Cols(cols...).Update(issue); err != nil {
- return err
- }
- return nil
+ _, err := db.GetEngine(ctx).ID(issue.ID).Cols(cols...).Update(issue)
+ return err
}
-func ChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, isClosed, isMergePull bool) (*Comment, error) {
- // Reload the issue
- currentIssue, err := GetIssueByID(ctx, issue.ID)
- if err != nil {
- return nil, err
- }
+// ErrIssueIsClosed is used when close a closed issue
+type ErrIssueIsClosed struct {
+ ID int64
+ RepoID int64
+ Index int64
+ IsPull bool
+}
- // Nothing should be performed if current status is same as target status
- if currentIssue.IsClosed == isClosed {
- if !issue.IsPull {
- return nil, ErrIssueWasClosed{
- ID: issue.ID,
- }
- }
- return nil, ErrPullWasClosed{
- ID: issue.ID,
- }
- }
+// IsErrIssueIsClosed checks if an error is a ErrIssueIsClosed.
+func IsErrIssueIsClosed(err error) bool {
+ _, ok := err.(ErrIssueIsClosed)
+ return ok
+}
- issue.IsClosed = isClosed
- return doChangeIssueStatus(ctx, issue, doer, isMergePull)
+func (err ErrIssueIsClosed) Error() string {
+ return fmt.Sprintf("%s [id: %d, repo_id: %d, index: %d] is already closed", util.Iif(err.IsPull, "Pull Request", "Issue"), err.ID, err.RepoID, err.Index)
}
-func doChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, isMergePull bool) (*Comment, error) {
+func SetIssueAsClosed(ctx context.Context, issue *Issue, doer *user_model.User, isMergePull bool) (*Comment, error) {
+ if issue.IsClosed {
+ return nil, ErrIssueIsClosed{
+ ID: issue.ID,
+ RepoID: issue.RepoID,
+ Index: issue.Index,
+ IsPull: issue.IsPull,
+ }
+ }
+
// Check for open dependencies
- if issue.IsClosed && issue.Repo.IsDependenciesEnabled(ctx) {
+ if issue.Repo.IsDependenciesEnabled(ctx) {
// only check if dependencies are enabled and we're about to close an issue, otherwise reopening an issue would fail when there are unsatisfied dependencies
noDeps, err := IssueNoDependenciesLeft(ctx, issue)
if err != nil {
@@ -71,16 +73,63 @@ func doChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.Use
}
}
- if issue.IsClosed {
- issue.ClosedUnix = timeutil.TimeStampNow()
- } else {
- issue.ClosedUnix = 0
+ issue.IsClosed = true
+ issue.ClosedUnix = timeutil.TimeStampNow()
+
+ if cnt, err := db.GetEngine(ctx).ID(issue.ID).Cols("is_closed", "closed_unix").
+ Where("is_closed = ?", false).
+ Update(issue); err != nil {
+ return nil, err
+ } else if cnt != 1 {
+ return nil, ErrIssueAlreadyChanged
}
- if err := UpdateIssueCols(ctx, issue, "is_closed", "closed_unix"); err != nil {
+ return updateIssueNumbers(ctx, issue, doer, util.Iif(isMergePull, CommentTypeMergePull, CommentTypeClose))
+}
+
+// ErrIssueIsOpen is used when reopen an opened issue
+type ErrIssueIsOpen struct {
+ ID int64
+ RepoID int64
+ IsPull bool
+ Index int64
+}
+
+// IsErrIssueIsOpen checks if an error is a ErrIssueIsOpen.
+func IsErrIssueIsOpen(err error) bool {
+ _, ok := err.(ErrIssueIsOpen)
+ return ok
+}
+
+func (err ErrIssueIsOpen) Error() string {
+ return fmt.Sprintf("%s [id: %d, repo_id: %d, index: %d] is already open", util.Iif(err.IsPull, "Pull Request", "Issue"), err.ID, err.RepoID, err.Index)
+}
+
+func setIssueAsReopen(ctx context.Context, issue *Issue, doer *user_model.User) (*Comment, error) {
+ if !issue.IsClosed {
+ return nil, ErrIssueIsOpen{
+ ID: issue.ID,
+ RepoID: issue.RepoID,
+ Index: issue.Index,
+ IsPull: issue.IsPull,
+ }
+ }
+
+ issue.IsClosed = false
+ issue.ClosedUnix = 0
+
+ if cnt, err := db.GetEngine(ctx).ID(issue.ID).Cols("is_closed", "closed_unix").
+ Where("is_closed = ?", true).
+ Update(issue); err != nil {
return nil, err
+ } else if cnt != 1 {
+ return nil, ErrIssueAlreadyChanged
}
+ return updateIssueNumbers(ctx, issue, doer, CommentTypeReopen)
+}
+
+func updateIssueNumbers(ctx context.Context, issue *Issue, doer *user_model.User, cmtType CommentType) (*Comment, error) {
// Update issue count of labels
if err := issue.LoadLabels(ctx); err != nil {
return nil, err
@@ -103,14 +152,6 @@ func doChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.Use
return nil, err
}
- // New action comment
- cmtType := CommentTypeClose
- if !issue.IsClosed {
- cmtType = CommentTypeReopen
- } else if isMergePull {
- cmtType = CommentTypeMergePull
- }
-
return CreateComment(ctx, &CreateCommentOptions{
Type: cmtType,
Doer: doer,
@@ -134,7 +175,7 @@ func CloseIssue(ctx context.Context, issue *Issue, doer *user_model.User) (*Comm
}
defer committer.Close()
- comment, err := ChangeIssueStatus(ctx, issue, doer, true, false)
+ comment, err := SetIssueAsClosed(ctx, issue, doer, false)
if err != nil {
return nil, err
}
@@ -159,7 +200,7 @@ func ReopenIssue(ctx context.Context, issue *Issue, doer *user_model.User) (*Com
}
defer committer.Close()
- comment, err := ChangeIssueStatus(ctx, issue, doer, false, false)
+ comment, err := setIssueAsReopen(ctx, issue, doer)
if err != nil {
return nil, err
}
diff --git a/models/issues/pull.go b/models/issues/pull.go
index 786b2aa130..e3af00224d 100644
--- a/models/issues/pull.go
+++ b/models/issues/pull.go
@@ -80,22 +80,6 @@ func (err ErrPullRequestAlreadyExists) Unwrap() error {
return util.ErrAlreadyExist
}
-// ErrPullWasClosed is used close a closed pull request
-type ErrPullWasClosed struct {
- ID int64
- Index int64
-}
-
-// IsErrPullWasClosed checks if an error is a ErrErrPullWasClosed.
-func IsErrPullWasClosed(err error) bool {
- _, ok := err.(ErrPullWasClosed)
- return ok
-}
-
-func (err ErrPullWasClosed) Error() string {
- return fmt.Sprintf("Pull request [%d] %d was already closed", err.ID, err.Index)
-}
-
// PullRequestType defines pull request type
type PullRequestType int
diff --git a/routers/private/hook_post_receive.go b/routers/private/hook_post_receive.go
index af40cb3988..dba6aef9a3 100644
--- a/routers/private/hook_post_receive.go
+++ b/routers/private/hook_post_receive.go
@@ -8,11 +8,9 @@ import (
"fmt"
"net/http"
- "code.gitea.io/gitea/models/db"
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"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/cache"
@@ -22,7 +20,7 @@ import (
"code.gitea.io/gitea/modules/private"
repo_module "code.gitea.io/gitea/modules/repository"
"code.gitea.io/gitea/modules/setting"
- timeutil "code.gitea.io/gitea/modules/timeutil"
+ "code.gitea.io/gitea/modules/timeutil"
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/modules/web"
gitea_context "code.gitea.io/gitea/services/context"
@@ -359,21 +357,9 @@ func handlePullRequestMerging(ctx *gitea_context.PrivateContext, opts *private.H
return
}
- pr.MergedCommitID = updates[len(updates)-1].NewCommitID
- pr.MergedUnix = timeutil.TimeStampNow()
- pr.Merger = pusher
- pr.MergerID = pusher.ID
- err = db.WithTx(ctx, func(ctx context.Context) error {
- // Removing an auto merge pull and ignore if not exist
- if err := pull_model.DeleteScheduledAutoMerge(ctx, pr.ID); err != nil && !db.IsErrNotExist(err) {
- return fmt.Errorf("DeleteScheduledAutoMerge[%d]: %v", opts.PullRequestID, err)
- }
- if _, err := pull_service.SetMerged(ctx, pr); err != nil {
- return fmt.Errorf("SetMerged failed: %s/%s Error: %v", ownerName, repoName, err)
- }
- return nil
- })
- if err != nil {
+ // FIXME: Maybe we need a `PullRequestStatusMerged` status for PRs that are merged, currently we use the previous status
+ // here to keep it as before, that maybe PullRequestStatusMergeable
+ if _, err := pull_service.SetMerged(ctx, pr, updates[len(updates)-1].NewCommitID, timeutil.TimeStampNow(), pusher, pr.Status); err != nil {
log.Error("Failed to update PR to merged: %v", err)
ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{Err: "Failed to update PR to merged"})
}
diff --git a/services/pull/check.go b/services/pull/check.go
index f7aa8eb369..e1adc3ca3b 100644
--- a/services/pull/check.go
+++ b/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 {
diff --git a/services/pull/merge.go b/services/pull/merge.go
index 879fe5a540..9c909ef795 100644
--- a/services/pull/merge.go
+++ b/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)
}
- 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)
+ 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 _, err := sess.Exec("UPDATE `issue` SET `repo_id` = `repo_id` WHERE `id` = ?", pr.IssueID); err != nil {
- return false, err
+ 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)
}
- 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 {
- return false, fmt.Errorf("ChangeIssueStatus: %w", err)
+ // 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)
}
- // reset the conflicted files as there cannot be any if we're merged
- pr.ConflictedFiles = []string{}
+ // Set issue as closed
+ if _, err := issues_model.SetIssueAsClosed(ctx, pr.Issue, pr.Merger, true); err != nil {
+ return false, fmt.Errorf("ChangeIssueStatus: %w", err)
+ }
// 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
diff --git a/services/pull/pull.go b/services/pull/pull.go
index 85c36bb16a..52abf35cec 100644
--- a/services/pull/pull.go
+++ b/services/pull/pull.go
@@ -265,6 +265,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,
}
}
@@ -707,7 +708,7 @@ func CloseBranchPulls(ctx context.Context, doer *user_model.User, repoID int64,
var errs errlist
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)
}
}
@@ -741,7 +742,7 @@ 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)
}
}
generated by cgit v1.2.3 (git 2.39.1) at 2025-01-27 06:39:41 +0000