diff options
author | Lunny Xiao <xiaolunwen@gmail.com> | 2024-12-29 23:04:03 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-12-30 07:04:03 +0000 |
commit | d45456b1b5c9ddd37b2980bbf3afaf75f8e3daef (patch) | |
tree | c8b08141969af7a93101700f4e71fa011985b65a | |
parent | 8eecca34783be2a5cae0994b2c91e7d3abcd470e (diff) | |
download | gitea-d45456b1b5c9ddd37b2980bbf3afaf75f8e3daef.tar.gz gitea-d45456b1b5c9ddd37b2980bbf3afaf75f8e3daef.zip |
Move SetMerged to service layer (#33045)
No code change.
Extract from #32178
-rw-r--r-- | models/issues/issue_update.go | 6 | ||||
-rw-r--r-- | models/issues/pull.go | 59 | ||||
-rw-r--r-- | routers/private/hook_post_receive.go | 2 | ||||
-rw-r--r-- | services/pull/check.go | 2 | ||||
-rw-r--r-- | services/pull/merge.go | 61 |
5 files changed, 65 insertions, 65 deletions
diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go index 2a6f3603bc..03863fe968 100644 --- a/models/issues/issue_update.go +++ b/models/issues/issue_update.go @@ -34,7 +34,7 @@ func UpdateIssueCols(ctx context.Context, issue *Issue, cols ...string) error { return nil } -func changeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, isClosed, isMergePull bool) (*Comment, error) { +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 { @@ -134,7 +134,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 := ChangeIssueStatus(ctx, issue, doer, true, false) if err != nil { return nil, err } @@ -159,7 +159,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 := ChangeIssueStatus(ctx, issue, doer, false, false) if err != nil { return nil, err } diff --git a/models/issues/pull.go b/models/issues/pull.go index efb24e8984..8c43eb19dd 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -499,65 +499,6 @@ func (pr *PullRequest) IsFromFork() bool { return pr.HeadRepoID != pr.BaseRepoID } -// SetMerged sets a pull request to merged and closes the corresponding issue -func (pr *PullRequest) SetMerged(ctx context.Context) (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) - - 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 { - return false, err - } - - pr.Issue = nil - if err := pr.LoadIssue(ctx); err != nil { - return false, err - } - - if tmpPr, err := 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 - } - - if err := pr.Issue.Repo.LoadOwner(ctx); err != nil { - return false, err - } - - if _, err := changeIssueStatus(ctx, pr.Issue, pr.Merger, true, true); err != nil { - return false, fmt.Errorf("Issue.changeStatus: %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 { - return false, fmt.Errorf("Failed to update pr[%d]: %w", pr.ID, err) - } - - return true, nil -} - // NewPullRequest creates new pull request with labels for repository. func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *Issue, labelIDs []int64, uuids []string, pr *PullRequest) (err error) { ctx, committer, err := db.TxContext(ctx) diff --git a/routers/private/hook_post_receive.go b/routers/private/hook_post_receive.go index 32c2828739..af40cb3988 100644 --- a/routers/private/hook_post_receive.go +++ b/routers/private/hook_post_receive.go @@ -368,7 +368,7 @@ func handlePullRequestMerging(ctx *gitea_context.PrivateContext, opts *private.H if err := pull_model.DeleteScheduledAutoMerge(ctx, pr.ID); err != nil && !db.IsErrNotExist(err) { return fmt.Errorf("DeleteScheduledAutoMerge[%d]: %v", opts.PullRequestID, err) } - if _, err := pr.SetMerged(ctx); err != nil { + if _, err := pull_service.SetMerged(ctx, pr); err != nil { return fmt.Errorf("SetMerged failed: %s/%s Error: %v", ownerName, repoName, err) } return nil diff --git a/services/pull/check.go b/services/pull/check.go index bffca394a8..f7aa8eb369 100644 --- a/services/pull/check.go +++ b/services/pull/check.go @@ -300,7 +300,7 @@ func manuallyMerged(ctx context.Context, pr *issues_model.PullRequest) bool { pr.Merger = merger pr.MergerID = merger.ID - if merged, err := pr.SetMerged(ctx); err != nil { + if merged, err := SetMerged(ctx, pr); 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 75011a697c..879fe5a540 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -639,7 +639,7 @@ func MergedManually(ctx context.Context, pr *issues_model.PullRequest, doer *use pr.MergerID = doer.ID var merged bool - if merged, err = pr.SetMerged(ctx); err != nil { + if merged, err = SetMerged(ctx, pr); err != nil { return err } else if !merged { return fmt.Errorf("SetMerged failed") @@ -656,3 +656,62 @@ func MergedManually(ctx context.Context, pr *issues_model.PullRequest, doer *use return handleCloseCrossReferences(ctx, pr, doer) } + +// SetMerged sets a pull request to merged and closes the corresponding issue +func SetMerged(ctx context.Context, pr *issues_model.PullRequest) (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) + + 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 { + return false, err + } + + 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 + } + + if err := pr.Issue.Repo.LoadOwner(ctx); err != nil { + return false, err + } + + if _, err := issues_model.ChangeIssueStatus(ctx, pr.Issue, pr.Merger, true, 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 { + return false, fmt.Errorf("failed to update pr[%d]: %w", pr.ID, err) + } + + return true, nil +} |