Skip to content

Commit d45456b

Browse files
lunnylunny
authored
Move SetMerged to service layer (#33045)
No code change. Extract from #32178
1 parent 8eecca3 commit d45456b

File tree

5 files changed

+65
-65
lines changed

5 files changed

+65
-65
lines changed

‎models/issues/issue_update.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ func UpdateIssueCols(ctx context.Context, issue *Issue, cols ...string) error {
3434
return nil
3535
}
3636

37-
func changeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, isClosed, isMergePull bool) (*Comment, error) {
37+
func ChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, isClosed, isMergePull bool) (*Comment, error) {
3838
// Reload the issue
3939
currentIssue, err := GetIssueByID(ctx, issue.ID)
4040
if err != nil {
@@ -134,7 +134,7 @@ func CloseIssue(ctx context.Context, issue *Issue, doer *user_model.User) (*Comm
134134
}
135135
defer committer.Close()
136136

137-
comment, err := changeIssueStatus(ctx, issue, doer, true, false)
137+
comment, err := ChangeIssueStatus(ctx, issue, doer, true, false)
138138
if err != nil {
139139
return nil, err
140140
}
@@ -159,7 +159,7 @@ func ReopenIssue(ctx context.Context, issue *Issue, doer *user_model.User) (*Com
159159
}
160160
defer committer.Close()
161161

162-
comment, err := changeIssueStatus(ctx, issue, doer, false, false)
162+
comment, err := ChangeIssueStatus(ctx, issue, doer, false, false)
163163
if err != nil {
164164
return nil, err
165165
}

‎models/issues/pull.go

Lines changed: 0 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -499,65 +499,6 @@ func (pr *PullRequest) IsFromFork() bool {
499499
return pr.HeadRepoID != pr.BaseRepoID
500500
}
501501

502-
// SetMerged sets a pull request to merged and closes the corresponding issue
503-
func (pr *PullRequest) SetMerged(ctx context.Context) (bool, error) {
504-
if pr.HasMerged {
505-
return false, fmt.Errorf("PullRequest[%d] already merged", pr.Index)
506-
}
507-
if pr.MergedCommitID == "" || pr.MergedUnix == 0 || pr.Merger == nil {
508-
return false, fmt.Errorf("Unable to merge PullRequest[%d], some required fields are empty", pr.Index)
509-
}
510-
511-
pr.HasMerged = true
512-
sess := db.GetEngine(ctx)
513-
514-
if _, err := sess.Exec("UPDATE `issue` SET `repo_id` = `repo_id` WHERE `id` = ?", pr.IssueID); err != nil {
515-
return false, err
516-
}
517-
518-
if _, err := sess.Exec("UPDATE `pull_request` SET `issue_id` = `issue_id` WHERE `id` = ?", pr.ID); err != nil {
519-
return false, err
520-
}
521-
522-
pr.Issue = nil
523-
if err := pr.LoadIssue(ctx); err != nil {
524-
return false, err
525-
}
526-
527-
if tmpPr, err := GetPullRequestByID(ctx, pr.ID); err != nil {
528-
return false, err
529-
} else if tmpPr.HasMerged {
530-
if pr.Issue.IsClosed {
531-
return false, nil
532-
}
533-
return false, fmt.Errorf("PullRequest[%d] already merged but it's associated issue [%d] is not closed", pr.Index, pr.IssueID)
534-
} else if pr.Issue.IsClosed {
535-
return false, fmt.Errorf("PullRequest[%d] already closed", pr.Index)
536-
}
537-
538-
if err := pr.Issue.LoadRepo(ctx); err != nil {
539-
return false, err
540-
}
541-
542-
if err := pr.Issue.Repo.LoadOwner(ctx); err != nil {
543-
return false, err
544-
}
545-
546-
if _, err := changeIssueStatus(ctx, pr.Issue, pr.Merger, true, true); err != nil {
547-
return false, fmt.Errorf("Issue.changeStatus: %w", err)
548-
}
549-
550-
// reset the conflicted files as there cannot be any if we're merged
551-
pr.ConflictedFiles = []string{}
552-
553-
// 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.
554-
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 {
555-
return false, fmt.Errorf("Failed to update pr[%d]: %w", pr.ID, err)
556-
}
557-
558-
return true, nil
559-
}
560-
561502
// NewPullRequest creates new pull request with labels for repository.
562503
func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *Issue, labelIDs []int64, uuids []string, pr *PullRequest) (err error) {
563504
ctx, committer, err := db.TxContext(ctx)

‎routers/private/hook_post_receive.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ func handlePullRequestMerging(ctx *gitea_context.PrivateContext, opts *private.H
368368
if err := pull_model.DeleteScheduledAutoMerge(ctx, pr.ID); err != nil && !db.IsErrNotExist(err) {
369369
return fmt.Errorf("DeleteScheduledAutoMerge[%d]: %v", opts.PullRequestID, err)
370370
}
371-
if _, err := pr.SetMerged(ctx); err != nil {
371+
if _, err := pull_service.SetMerged(ctx, pr); err != nil {
372372
return fmt.Errorf("SetMerged failed: %s/%s Error: %v", ownerName, repoName, err)
373373
}
374374
return nil

‎services/pull/check.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ func manuallyMerged(ctx context.Context, pr *issues_model.PullRequest) bool {
300300
pr.Merger = merger
301301
pr.MergerID = merger.ID
302302

303-
if merged, err := pr.SetMerged(ctx); err != nil {
303+
if merged, err := SetMerged(ctx, pr); err != nil {
304304
log.Error("%-v setMerged : %v", pr, err)
305305
return false
306306
} else if !merged {

‎services/pull/merge.go

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -639,7 +639,7 @@ func MergedManually(ctx context.Context, pr *issues_model.PullRequest, doer *use
639639
pr.MergerID = doer.ID
640640

641641
var merged bool
642-
if merged, err = pr.SetMerged(ctx); err != nil {
642+
if merged, err = SetMerged(ctx, pr); err != nil {
643643
return err
644644
} else if !merged {
645645
return fmt.Errorf("SetMerged failed")
@@ -656,3 +656,62 @@ func MergedManually(ctx context.Context, pr *issues_model.PullRequest, doer *use
656656

657657
return handleCloseCrossReferences(ctx, pr, doer)
658658
}
659+
660+
// SetMerged sets a pull request to merged and closes the corresponding issue
661+
func SetMerged(ctx context.Context, pr *issues_model.PullRequest) (bool, error) {
662+
if pr.HasMerged {
663+
return false, fmt.Errorf("PullRequest[%d] already merged", pr.Index)
664+
}
665+
if pr.MergedCommitID == "" || pr.MergedUnix == 0 || pr.Merger == nil {
666+
return false, fmt.Errorf("unable to merge PullRequest[%d], some required fields are empty", pr.Index)
667+
}
668+
669+
pr.HasMerged = true
670+
sess := db.GetEngine(ctx)
671+
672+
if _, err := sess.Exec("UPDATE `issue` SET `repo_id` = `repo_id` WHERE `id` = ?", pr.IssueID); err != nil {
673+
return false, err
674+
}
675+
676+
if _, err := sess.Exec("UPDATE `pull_request` SET `issue_id` = `issue_id` WHERE `id` = ?", pr.ID); err != nil {
677+
return false, err
678+
}
679+
680+
pr.Issue = nil
681+
if err := pr.LoadIssue(ctx); err != nil {
682+
return false, err
683+
}
684+
685+
if tmpPr, err := issues_model.GetPullRequestByID(ctx, pr.ID); err != nil {
686+
return false, err
687+
} else if tmpPr.HasMerged {
688+
if pr.Issue.IsClosed {
689+
return false, nil
690+
}
691+
return false, fmt.Errorf("PullRequest[%d] already merged but it's associated issue [%d] is not closed", pr.Index, pr.IssueID)
692+
} else if pr.Issue.IsClosed {
693+
return false, fmt.Errorf("PullRequest[%d] already closed", pr.Index)
694+
}
695+
696+
if err := pr.Issue.LoadRepo(ctx); err != nil {
697+
return false, err
698+
}
699+
700+
if err := pr.Issue.Repo.LoadOwner(ctx); err != nil {
701+
return false, err
702+
}
703+
704+
if _, err := issues_model.ChangeIssueStatus(ctx, pr.Issue, pr.Merger, true, true); err != nil {
705+
return false, fmt.Errorf("ChangeIssueStatus: %w", err)
706+
}
707+
708+
// reset the conflicted files as there cannot be any if we're merged
709+
pr.ConflictedFiles = []string{}
710+
711+
// 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.
712+
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 {
713+
return false, fmt.Errorf("failed to update pr[%d]: %w", pr.ID, err)
714+
}
715+
716+
return true, nil
717+
}

0 commit comments

Comments
 (0)