Skip to content

Commit 1ce6f48

Browse files
lunnylunny
committed
use a standalone function for close/reopen with comment
1 parent 5382308 commit 1ce6f48

File tree

9 files changed

+132
-83
lines changed

9 files changed

+132
-83
lines changed

‎options/locale/locale_en-US.ini‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1956,6 +1956,7 @@ pulls.push_rejected = Push Failed: The push was rejected. Review the Git Hooks f
19561956
pulls.push_rejected_summary = Full Rejection Message
19571957
pulls.push_rejected_no_message = Push Failed: The push was rejected but there was no remote message. Review the Git Hooks for this repository.
19581958
pulls.open_unmerged_pull_exists = You cannot perform a reopen operation because there is a pending pull request (#%d) with identical properties.
1959+
pulls.head_branch_not_exist = The head branch does not exist, cannot reopen the pull request.
19591960
pulls.status_checking = Some checks are pending
19601961
pulls.status_checks_success = All checks were successful
19611962
pulls.status_checks_warning = Some checks reported warnings

‎routers/api/v1/repo/issue.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -733,7 +733,7 @@ func CreateIssue(ctx *context.APIContext) {
733733
}
734734

735735
if form.Closed {
736-
if _, err := issue_service.CloseIssue(ctx, issue, ctx.Doer, "", "", nil); err != nil {
736+
if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil {
737737
if issues_model.IsErrDependenciesLeft(err) {
738738
ctx.APIError(http.StatusPreconditionFailed, "cannot close this issue because it still has open dependencies")
739739
return
@@ -1056,7 +1056,7 @@ func closeOrReopenIssue(ctx *context.APIContext, issue *issues_model.Issue, stat
10561056
}
10571057

10581058
if state == api.StateClosed && !issue.IsClosed {
1059-
if _, err := issue_service.CloseIssue(ctx, issue, ctx.Doer, "", "", nil); err != nil {
1059+
if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil {
10601060
if issues_model.IsErrDependenciesLeft(err) {
10611061
ctx.APIError(http.StatusPreconditionFailed, "cannot close this issue or pull request because it still has open dependencies")
10621062
return
@@ -1065,7 +1065,7 @@ func closeOrReopenIssue(ctx *context.APIContext, issue *issues_model.Issue, stat
10651065
return
10661066
}
10671067
} else if state == api.StateOpen && issue.IsClosed {
1068-
if _, err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, "", "", nil); err != nil {
1068+
if err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, ""); err != nil {
10691069
ctx.APIErrorInternal(err)
10701070
return
10711071
}

‎routers/web/repo/issue_comment.go‎

Lines changed: 74 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@ import (
1010
"net/http"
1111
"strconv"
1212

13+
git_model "code.gitea.io/gitea/models/git"
1314
issues_model "code.gitea.io/gitea/models/issues"
1415
"code.gitea.io/gitea/models/renderhelper"
1516
user_model "code.gitea.io/gitea/models/user"
1617
"code.gitea.io/gitea/modules/git"
17-
"code.gitea.io/gitea/modules/gitrepo"
1818
"code.gitea.io/gitea/modules/log"
1919
"code.gitea.io/gitea/modules/markup/markdown"
2020
repo_module "code.gitea.io/gitea/modules/repository"
@@ -91,47 +91,10 @@ func NewComment(ctx *context.Context) {
9191
return
9292
}
9393

94-
if issue.IsPull && issue.PullRequest.HasMerged {
95-
ctx.JSONError(ctx.Tr("repo.issues.reopen_not_allowed_merged"))
96-
return
97-
}
98-
99-
// check if an opened pull request exists with the same head branch and base branch
100-
pull := issue.PullRequest
101-
var err error
102-
pr, err := issues_model.GetUnmergedPullRequest(ctx, pull.HeadRepoID, pull.BaseRepoID, pull.HeadBranch, pull.BaseBranch, pull.Flow)
103-
if err != nil {
104-
if !issues_model.IsErrPullRequestNotExist(err) {
105-
ctx.JSONError(err.Error())
106-
return
107-
}
108-
}
109-
if pr != nil {
110-
ctx.Flash.Info(ctx.Tr("repo.pulls.open_unmerged_pull_exists", pr.Index))
111-
return
112-
}
113-
114-
createdComment, err = issue_service.ReopenIssue(ctx, issue, ctx.Doer, "", form.Content, attachments)
115-
if err != nil {
116-
if errors.Is(err, user_model.ErrBlockedUser) {
117-
ctx.JSONError(ctx.Tr("repo.issues.comment.blocked_user"))
118-
} else {
119-
ctx.ServerError("ReopenIssue", err)
120-
}
121-
return
122-
}
123-
124-
// check whether the ref of PR <refs/pulls/pr_index/head> in base repo is consistent with the head commit of head branch in the head repo
125-
// get head commit of PR
126-
if pull.Flow == issues_model.PullRequestFlowGithub {
127-
prHeadRef := pull.GetGitHeadRefName()
128-
if err := pull.LoadBaseRepo(ctx); err != nil {
129-
ctx.ServerError("Unable to load base repo", err)
130-
return
131-
}
132-
prHeadCommitID, err := git.GetFullCommitID(ctx, pull.BaseRepo.RepoPath(), prHeadRef)
133-
if err != nil {
134-
ctx.ServerError("Get head commit Id of pr fail", err)
94+
if issue.IsPull {
95+
pull := issue.PullRequest
96+
if pull.HasMerged {
97+
ctx.JSONError(ctx.Tr("repo.issues.reopen_not_allowed_merged"))
13598
return
13699
}
137100

@@ -140,42 +103,88 @@ func NewComment(ctx *context.Context) {
140103
ctx.ServerError("Unable to load head repo", err)
141104
return
142105
}
143-
if ok := gitrepo.IsBranchExist(ctx, pull.HeadRepo, pull.BaseBranch); !ok {
144-
// todo localize
145-
ctx.JSONError("The origin branch is delete, cannot reopen.")
106+
branchExist, err := git_model.IsBranchExist(ctx, pull.HeadRepo.ID, pull.HeadBranch)
107+
if err != nil {
108+
ctx.ServerError("IsBranchExist", err)
146109
return
147110
}
148-
headBranchRef := pull.GetGitHeadBranchRefName()
149-
headBranchCommitID, err := git.GetFullCommitID(ctx, pull.HeadRepo.RepoPath(), headBranchRef)
150-
if err != nil {
151-
ctx.ServerError("Get head commit Id of head branch fail", err)
111+
if !branchExist {
112+
ctx.JSONError(ctx.Tr("repo.pulls.head_branch_not_exist"))
152113
return
153114
}
154115

155-
err = pull.LoadIssue(ctx)
116+
// check if an opened pull request exists with the same head branch and base branch
117+
pr, err := issues_model.GetUnmergedPullRequest(ctx, pull.HeadRepoID, pull.BaseRepoID, pull.HeadBranch, pull.BaseBranch, pull.Flow)
156118
if err != nil {
157-
ctx.ServerError("load the issue of pull request error", err)
119+
if !issues_model.IsErrPullRequestNotExist(err) {
120+
ctx.JSONError(err.Error())
121+
return
122+
}
123+
}
124+
if pr != nil {
125+
ctx.Flash.Info(ctx.Tr("repo.pulls.open_unmerged_pull_exists", pr.Index))
158126
return
159127
}
128+
}
160129

161-
if prHeadCommitID != headBranchCommitID {
162-
// force push to base repo
163-
err := git.Push(ctx, pull.HeadRepo.RepoPath(), git.PushOptions{
164-
Remote: pull.BaseRepo.RepoPath(),
165-
Branch: pull.HeadBranch + ":" + prHeadRef,
166-
Force: true,
167-
Env: repo_module.InternalPushingEnvironment(pull.Issue.Poster, pull.BaseRepo),
168-
})
130+
createdComment, err = issue_service.ReopenIssueWithComment(ctx, issue, ctx.Doer, "", form.Content, attachments)
131+
if err != nil {
132+
if errors.Is(err, user_model.ErrBlockedUser) {
133+
ctx.JSONError(ctx.Tr("repo.issues.comment.blocked_user"))
134+
} else {
135+
ctx.ServerError("ReopenIssue", err)
136+
}
137+
return
138+
}
139+
140+
if issue.IsPull {
141+
pull := issue.PullRequest
142+
// check whether the ref of PR <refs/pulls/pr_index/head> in base repo is consistent with the head commit of head branch in the head repo
143+
// get head commit of PR
144+
if pull.Flow == issues_model.PullRequestFlowGithub {
145+
prHeadRef := pull.GetGitHeadRefName()
146+
if err := pull.LoadBaseRepo(ctx); err != nil {
147+
ctx.ServerError("Unable to load base repo", err)
148+
return
149+
}
150+
prHeadCommitID, err := git.GetFullCommitID(ctx, pull.BaseRepo.RepoPath(), prHeadRef)
151+
if err != nil {
152+
ctx.ServerError("Get head commit Id of pr fail", err)
153+
return
154+
}
155+
156+
headBranchRef := pull.GetGitHeadBranchRefName()
157+
headBranchCommitID, err := git.GetFullCommitID(ctx, pull.HeadRepo.RepoPath(), headBranchRef)
169158
if err != nil {
170-
ctx.ServerError("force push error", err)
159+
ctx.ServerError("Get head commit Id of head branch fail", err)
160+
return
161+
}
162+
163+
if err = pull.LoadIssue(ctx); err != nil {
164+
ctx.ServerError("load the issue of pull request error", err)
171165
return
172166
}
167+
168+
// if the head commit ID of the PR is different from the head branch
169+
if prHeadCommitID != headBranchCommitID {
170+
// force push to base repo
171+
err := git.Push(ctx, pull.HeadRepo.RepoPath(), git.PushOptions{
172+
Remote: pull.BaseRepo.RepoPath(),
173+
Branch: pull.HeadBranch + ":" + prHeadRef,
174+
Force: true,
175+
Env: repo_module.InternalPushingEnvironment(pull.Issue.Poster, pull.BaseRepo),
176+
})
177+
if err != nil {
178+
ctx.ServerError("force push error", err)
179+
return
180+
}
181+
}
173182
}
174-
}
175183

176-
// Regenerate patch and test conflict.
177-
pull.HeadCommitID = ""
178-
pull_service.StartPullRequestCheckImmediately(ctx, pull)
184+
// Regenerate patch and test conflict.
185+
pull.HeadCommitID = ""
186+
pull_service.StartPullRequestCheckImmediately(ctx, pull)
187+
}
179188
case "close":
180189
if issue.IsClosed {
181190
ctx.JSONError(ctx.Tr("repo.issues.already_closed"))
@@ -189,7 +198,7 @@ func NewComment(ctx *context.Context) {
189198
return
190199
}
191200

192-
createdComment, err = issue_service.CloseIssue(ctx, issue, ctx.Doer, "", form.Content, attachments)
201+
createdComment, err = issue_service.CloseIssueWithComment(ctx, issue, ctx.Doer, "", form.Content, attachments)
193202
default:
194203
if len(form.Content) == 0 && len(attachments) == 0 {
195204
ctx.JSONError(ctx.Tr("repo.issues.comment.empty_content"))

‎routers/web/repo/issue_list.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,7 @@ func UpdateIssueStatus(ctx *context.Context) {
434434
continue
435435
}
436436
if action == "close" && !issue.IsClosed {
437-
if _, err := issue_service.CloseIssue(ctx, issue, ctx.Doer, "", "", nil); err != nil {
437+
if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil {
438438
if issues_model.IsErrDependenciesLeft(err) {
439439
ctx.JSON(http.StatusPreconditionFailed, map[string]any{
440440
"error": ctx.Tr("repo.issues.dependency.issue_batch_close_blocked", issue.Index),
@@ -445,7 +445,7 @@ func UpdateIssueStatus(ctx *context.Context) {
445445
return
446446
}
447447
} else if action == "open" && issue.IsClosed {
448-
if _, err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, "", "", nil); err != nil {
448+
if err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, ""); err != nil {
449449
ctx.ServerError("ReopenIssue", err)
450450
return
451451
}

‎services/issue/commit.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,11 +196,11 @@ func UpdateIssuesCommit(ctx context.Context, doer *user_model.User, repo *repo_m
196196
return err
197197
}
198198
}
199-
if _, err := CloseIssue(ctx, refIssue, doer, c.Sha1, "", nil); err != nil {
199+
if err := CloseIssue(ctx, refIssue, doer, c.Sha1); err != nil {
200200
return err
201201
}
202202
} else if ref.Action == references.XRefActionReopens && refIssue.IsClosed {
203-
if _, err := ReopenIssue(ctx, refIssue, doer, c.Sha1, "", nil); err != nil {
203+
if err := ReopenIssue(ctx, refIssue, doer, c.Sha1); err != nil {
204204
return err
205205
}
206206
}

‎services/issue/status.go‎

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,34 @@ import (
1313
notify_service "code.gitea.io/gitea/services/notify"
1414
)
1515

16-
// CloseIssue close an issue.
17-
func CloseIssue(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, commitID, commentContent string, attachments []string) (*issues_model.Comment, error) {
16+
// CloseIssue closes an issue
17+
func CloseIssue(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, commitID string) error {
18+
var comment *issues_model.Comment
19+
if err := db.WithTx(ctx, func(ctx context.Context) error {
20+
var err error
21+
comment, err = issues_model.CloseIssue(ctx, issue, doer)
22+
if err != nil {
23+
if issues_model.IsErrDependenciesLeft(err) {
24+
if _, err := issues_model.FinishIssueStopwatch(ctx, doer, issue); err != nil {
25+
log.Error("Unable to stop stopwatch for issue[%d]#%d: %v", issue.ID, issue.Index, err)
26+
}
27+
}
28+
return err
29+
}
30+
31+
_, err = issues_model.FinishIssueStopwatch(ctx, doer, issue)
32+
return err
33+
}); err != nil {
34+
return err
35+
}
36+
37+
notify_service.IssueChangeStatus(ctx, doer, commitID, issue, comment, true)
38+
39+
return nil
40+
}
41+
42+
// CloseIssueWithComment close an issue with comment
43+
func CloseIssueWithComment(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, commitID, commentContent string, attachments []string) (*issues_model.Comment, error) {
1844
var refComment, createdComment *issues_model.Comment
1945
if err := db.WithTx(ctx, func(ctx context.Context) error {
2046
var err error
@@ -59,9 +85,22 @@ func CloseIssue(ctx context.Context, issue *issues_model.Issue, doer *user_model
5985
return createdComment, nil
6086
}
6187

62-
// ReopenIssue reopen an issue with or without a comment.
88+
// ReopenIssue reopen an issue
89+
// FIXME: If some issues dependent this one are closed, should we also reopen them?
90+
func ReopenIssue(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, commitID string) error {
91+
comment, err := issues_model.ReopenIssue(ctx, issue, doer)
92+
if err != nil {
93+
return err
94+
}
95+
96+
notify_service.IssueChangeStatus(ctx, doer, commitID, issue, comment, false)
97+
98+
return nil
99+
}
100+
101+
// ReopenIssue reopen an issue with a comment.
63102
// FIXME: If some issues dependent this one are closed, should we also reopen them?
64-
func ReopenIssue(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, commitID, commentContent string, attachments []string) (*issues_model.Comment, error) {
103+
func ReopenIssueWithComment(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, commitID, commentContent string, attachments []string) (*issues_model.Comment, error) {
65104
var createdComment *issues_model.Comment
66105
refComment, err := db.WithTx2(ctx, func(ctx context.Context) (*issues_model.Comment, error) {
67106
var err error

‎services/pull/merge.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -309,14 +309,14 @@ func handleCloseCrossReferences(ctx context.Context, pr *issues_model.PullReques
309309
return err
310310
}
311311
if ref.RefAction == references.XRefActionCloses && !ref.Issue.IsClosed {
312-
if _, err = issue_service.CloseIssue(ctx, ref.Issue, doer, pr.MergedCommitID, "", nil); err != nil {
312+
if err = issue_service.CloseIssue(ctx, ref.Issue, doer, pr.MergedCommitID); err != nil {
313313
// Allow ErrDependenciesLeft
314314
if !issues_model.IsErrDependenciesLeft(err) {
315315
return err
316316
}
317317
}
318318
} else if ref.RefAction == references.XRefActionReopens && ref.Issue.IsClosed {
319-
if _, err = issue_service.ReopenIssue(ctx, ref.Issue, doer, pr.MergedCommitID, "", nil); err != nil {
319+
if err = issue_service.ReopenIssue(ctx, ref.Issue, doer, pr.MergedCommitID); err != nil {
320320
return err
321321
}
322322
}

‎services/pull/pull.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -700,7 +700,7 @@ func AdjustPullsCausedByBranchDeleted(ctx context.Context, doer *user_model.User
700700

701701
var errs []error
702702
for _, pr := range prs {
703-
if _, err = issue_service.CloseIssue(ctx, pr.Issue, doer, "", "", nil); err != nil && !issues_model.IsErrIssueIsClosed(err) && !issues_model.IsErrDependenciesLeft(err) {
703+
if err = issue_service.CloseIssue(ctx, pr.Issue, doer, ""); err != nil && !issues_model.IsErrIssueIsClosed(err) && !issues_model.IsErrDependenciesLeft(err) {
704704
errs = append(errs, err)
705705
}
706706
if err == nil {
@@ -740,7 +740,7 @@ func AdjustPullsCausedByBranchDeleted(ctx context.Context, doer *user_model.User
740740
errs = append(errs, err)
741741
}
742742
if err == nil {
743-
if _, err = issue_service.CloseIssue(ctx, pr.Issue, doer, "", "", nil); err != nil && !issues_model.IsErrIssueIsClosed(err) && !issues_model.IsErrDependenciesLeft(err) {
743+
if err = issue_service.CloseIssue(ctx, pr.Issue, doer, ""); err != nil && !issues_model.IsErrIssueIsClosed(err) && !issues_model.IsErrDependenciesLeft(err) {
744744
errs = append(errs, err)
745745
}
746746
}
@@ -772,7 +772,7 @@ func CloseRepoBranchesPulls(ctx context.Context, doer *user_model.User, repo *re
772772
if pr.BaseRepoID == repo.ID {
773773
continue
774774
}
775-
if _, err = issue_service.CloseIssue(ctx, pr.Issue, doer, "", "", nil); err != nil && !issues_model.IsErrIssueIsClosed(err) {
775+
if err = issue_service.CloseIssue(ctx, pr.Issue, doer, ""); err != nil && !issues_model.IsErrIssueIsClosed(err) {
776776
errs = append(errs, err)
777777
}
778778
}

‎tests/integration/actions_trigger_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -565,12 +565,12 @@ jobs:
565565
checkCommitStatusAndInsertFakeStatus(t, repo, sha)
566566

567567
// closed
568-
_, err = issue_service.CloseIssue(t.Context(), pullIssue, user2, "", "", nil)
568+
err = issue_service.CloseIssue(t.Context(), pullIssue, user2, "")
569569
assert.NoError(t, err)
570570
checkCommitStatusAndInsertFakeStatus(t, repo, sha)
571571

572572
// reopened
573-
_, err = issue_service.ReopenIssue(t.Context(), pullIssue, user2, "", "", nil)
573+
err = issue_service.ReopenIssue(t.Context(), pullIssue, user2, "")
574574
assert.NoError(t, err)
575575
checkCommitStatusAndInsertFakeStatus(t, repo, sha)
576576

0 commit comments

Comments
 (0)