Skip to content

Commit 406884a

Browse files
lunnylunny
committed
Use CloseIssue and ReopenIssue instead of ChagneStatus for issues
1 parent a928739 commit 406884a

File tree

7 files changed

+85
-52
lines changed

7 files changed

+85
-52
lines changed

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

Lines changed: 12 additions & 7 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.ChangeStatus(ctx, issue, ctx.Doer, "", true); err != nil {
736+
if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil {
737737
if issues_model.IsErrDependenciesLeft(err) {
738738
ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this issue because it still has open dependencies")
739739
return
@@ -912,24 +912,29 @@ func EditIssue(ctx *context.APIContext) {
912912
}
913913
}
914914

915-
var isClosed bool
915+
var closeOrReopen bool
916916
switch state := api.StateType(*form.State); state {
917917
case api.StateOpen:
918-
isClosed = false
918+
closeOrReopen = false
919919
case api.StateClosed:
920-
isClosed = true
920+
closeOrReopen = true
921921
default:
922922
ctx.Error(http.StatusPreconditionFailed, "UnknownIssueStateError", fmt.Sprintf("unknown state: %s", state))
923923
return
924924
}
925925

926-
if issue.IsClosed != isClosed {
927-
if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", isClosed); err != nil {
926+
if closeOrReopen && !issue.IsClosed {
927+
if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil {
928928
if issues_model.IsErrDependenciesLeft(err) {
929929
ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this issue because it still has open dependencies")
930930
return
931931
}
932-
ctx.Error(http.StatusInternalServerError, "ChangeStatus", err)
932+
ctx.Error(http.StatusInternalServerError, "CloseIssue", err)
933+
return
934+
}
935+
} else if !closeOrReopen && issue.IsClosed {
936+
if err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, ""); err != nil {
937+
ctx.Error(http.StatusInternalServerError, "ReopenIssue", err)
933938
return
934939
}
935940
}

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -753,24 +753,29 @@ func EditPullRequest(ctx *context.APIContext) {
753753
return
754754
}
755755

756-
var isClosed bool
756+
var closeOrReopen bool
757757
switch state := api.StateType(*form.State); state {
758758
case api.StateOpen:
759-
isClosed = false
759+
closeOrReopen = false
760760
case api.StateClosed:
761-
isClosed = true
761+
closeOrReopen = true
762762
default:
763763
ctx.Error(http.StatusPreconditionFailed, "UnknownPRStateError", fmt.Sprintf("unknown state: %s", state))
764764
return
765765
}
766766

767-
if issue.IsClosed != isClosed {
768-
if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", isClosed); err != nil {
767+
if closeOrReopen && !issue.IsClosed {
768+
if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil {
769769
if issues_model.IsErrDependenciesLeft(err) {
770770
ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this pull request because it still has open dependencies")
771771
return
772772
}
773-
ctx.Error(http.StatusInternalServerError, "ChangeStatus", err)
773+
ctx.Error(http.StatusInternalServerError, "CloseIssue", err)
774+
return
775+
}
776+
} else if !closeOrReopen && issue.IsClosed {
777+
if err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, ""); err != nil {
778+
ctx.Error(http.StatusInternalServerError, "ReopenIssue", err)
774779
return
775780
}
776781
}

‎routers/web/repo/issue.go‎

Lines changed: 32 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2991,14 +2991,16 @@ func UpdateIssueStatus(ctx *context.Context) {
29912991
return
29922992
}
29932993

2994-
var isClosed bool
2994+
var closeOrReopen bool // true: close, false: reopen
29952995
switch action := ctx.FormString("action"); action {
29962996
case "open":
2997-
isClosed = false
2997+
closeOrReopen = false
29982998
case "close":
2999-
isClosed = true
2999+
closeOrReopen = true
30003000
default:
30013001
log.Warn("Unrecognized action: %s", action)
3002+
ctx.JSONOK()
3003+
return
30023004
}
30033005

30043006
if _, err := issues.LoadRepositories(ctx); err != nil {
@@ -3014,15 +3016,20 @@ func UpdateIssueStatus(ctx *context.Context) {
30143016
if issue.IsPull && issue.PullRequest.HasMerged {
30153017
continue
30163018
}
3017-
if issue.IsClosed != isClosed {
3018-
if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", isClosed); err != nil {
3019+
if closeOrReopen && !issue.IsClosed {
3020+
if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil {
30193021
if issues_model.IsErrDependenciesLeft(err) {
30203022
ctx.JSON(http.StatusPreconditionFailed, map[string]any{
30213023
"error": ctx.Tr("repo.issues.dependency.issue_batch_close_blocked", issue.Index),
30223024
})
30233025
return
30243026
}
3025-
ctx.ServerError("ChangeStatus", err)
3027+
ctx.ServerError("CloseIssue", err)
3028+
return
3029+
}
3030+
} else if !closeOrReopen && issue.IsClosed {
3031+
if err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, ""); err != nil {
3032+
ctx.ServerError("ReopenIssue", err)
30263033
return
30273034
}
30283035
}
@@ -3158,25 +3165,29 @@ func NewComment(ctx *context.Context) {
31583165
if pr != nil {
31593166
ctx.Flash.Info(ctx.Tr("repo.pulls.open_unmerged_pull_exists", pr.Index))
31603167
} else {
3161-
isClosed := form.Status == "close"
3162-
if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", isClosed); err != nil {
3163-
log.Error("ChangeStatus: %v", err)
3164-
3165-
if issues_model.IsErrDependenciesLeft(err) {
3166-
if issue.IsPull {
3167-
ctx.JSONError(ctx.Tr("repo.issues.dependency.pr_close_blocked"))
3168-
} else {
3169-
ctx.JSONError(ctx.Tr("repo.issues.dependency.issue_close_blocked"))
3168+
closeOrReopen := form.Status == "close"
3169+
if closeOrReopen {
3170+
if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil {
3171+
log.Error("CloseIssue: %v", err)
3172+
if issues_model.IsErrDependenciesLeft(err) {
3173+
if issue.IsPull {
3174+
ctx.JSONError(ctx.Tr("repo.issues.dependency.pr_close_blocked"))
3175+
} else {
3176+
ctx.JSONError(ctx.Tr("repo.issues.dependency.issue_close_blocked"))
3177+
}
3178+
return
31703179
}
3171-
return
3180+
} else {
3181+
if err := stopTimerIfAvailable(ctx, ctx.Doer, issue); err != nil {
3182+
ctx.ServerError("stopTimerIfAvailable", err)
3183+
return
3184+
}
3185+
log.Trace("Issue [%d] status changed to closed: %v", issue.ID, issue.IsClosed)
31723186
}
31733187
} else {
3174-
if err := stopTimerIfAvailable(ctx, ctx.Doer, issue); err != nil {
3175-
ctx.ServerError("CreateOrStopIssueStopwatch", err)
3176-
return
3188+
if err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, ""); err != nil {
3189+
log.Error("ReopenIssue: %v", err)
31773190
}
3178-
3179-
log.Trace("Issue [%d] status changed to closed: %v", issue.ID, issue.IsClosed)
31803191
}
31813192
}
31823193
}

‎services/issue/commit.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,9 +194,9 @@ func UpdateIssuesCommit(ctx context.Context, doer *user_model.User, repo *repo_m
194194
return err
195195
}
196196
}
197-
if isClosed != refIssue.IsClosed {
197+
if isClosed && !refIssue.IsClosed {
198198
refIssue.Repo = refRepo
199-
if err := ChangeStatus(ctx, refIssue, doer, c.Sha1, isClosed); err != nil {
199+
if err := CloseIssue(ctx, refIssue, doer, c.Sha1); err != nil {
200200
return err
201201
}
202202
}

‎services/issue/status.go‎

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,28 +12,40 @@ import (
1212
notify_service "code.gitea.io/gitea/services/notify"
1313
)
1414

15-
// ChangeStatus changes issue status to open or closed.
16-
// closed means the target status
17-
// Fix me: you should check whether the current issue status is same to the target status before call this function
18-
// as in function changeIssueStatus we will return WasClosedError, even the issue status and target status are both open
19-
func ChangeStatus(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, commitID string, closed bool) error {
20-
comment, err := issues_model.ChangeIssueStatus(ctx, issue, doer, closed)
15+
// CloseIssue close and issue.
16+
func CloseIssue(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, commitID string) error {
17+
comment, err := issues_model.ChangeIssueStatus(ctx, issue, doer, true)
2118
if err != nil {
22-
if issues_model.IsErrDependenciesLeft(err) && closed {
19+
if issues_model.IsErrDependenciesLeft(err) {
2320
if err := issues_model.FinishIssueStopwatchIfPossible(ctx, doer, issue); err != nil {
2421
log.Error("Unable to stop stopwatch for issue[%d]#%d: %v", issue.ID, issue.Index, err)
2522
}
2623
}
2724
return err
2825
}
2926

30-
if closed {
31-
if err := issues_model.FinishIssueStopwatchIfPossible(ctx, doer, issue); err != nil {
32-
return err
33-
}
27+
if err := issues_model.FinishIssueStopwatchIfPossible(ctx, doer, issue); err != nil {
28+
return err
29+
}
30+
31+
notify_service.IssueChangeStatus(ctx, doer, commitID, issue, comment, true)
32+
33+
return nil
34+
}
35+
36+
// ReopenIssue reopen an issue.
37+
// FIXME: If some issues dependent this one are closed, should we also reopen them?
38+
func ReopenIssue(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, commitID string) error {
39+
comment, err := issues_model.ChangeIssueStatus(ctx, issue, doer, false)
40+
if err != nil {
41+
return err
42+
}
43+
44+
if err := issues_model.FinishIssueStopwatchIfPossible(ctx, doer, issue); err != nil {
45+
return err
3446
}
3547

36-
notify_service.IssueChangeStatus(ctx, doer, commitID, issue, comment, closed)
48+
notify_service.IssueChangeStatus(ctx, doer, commitID, issue, comment, false)
3749

3850
return nil
3951
}

‎services/pull/merge.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,8 +243,8 @@ func handleCloseCrossReferences(ctx context.Context, pr *issues_model.PullReques
243243
return err
244244
}
245245
isClosed := ref.RefAction == references.XRefActionCloses
246-
if isClosed != ref.Issue.IsClosed {
247-
if err = issue_service.ChangeStatus(ctx, ref.Issue, doer, pr.MergedCommitID, isClosed); err != nil {
246+
if isClosed && !ref.Issue.IsClosed {
247+
if err = issue_service.CloseIssue(ctx, ref.Issue, doer, pr.MergedCommitID); err != nil {
248248
// Allow ErrDependenciesLeft
249249
if !issues_model.IsErrDependenciesLeft(err) {
250250
return err

‎services/pull/pull.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -685,7 +685,7 @@ func CloseBranchPulls(ctx context.Context, doer *user_model.User, repoID int64,
685685

686686
var errs errlist
687687
for _, pr := range prs {
688-
if err = issue_service.ChangeStatus(ctx, pr.Issue, doer, "", true); err != nil && !issues_model.IsErrPullWasClosed(err) && !issues_model.IsErrDependenciesLeft(err) {
688+
if err = issue_service.CloseIssue(ctx, pr.Issue, doer, ""); err != nil && !issues_model.IsErrPullWasClosed(err) && !issues_model.IsErrDependenciesLeft(err) {
689689
errs = append(errs, err)
690690
}
691691
}
@@ -719,7 +719,7 @@ func CloseRepoBranchesPulls(ctx context.Context, doer *user_model.User, repo *re
719719
if pr.BaseRepoID == repo.ID {
720720
continue
721721
}
722-
if err = issue_service.ChangeStatus(ctx, pr.Issue, doer, "", true); err != nil && !issues_model.IsErrPullWasClosed(err) {
722+
if err = issue_service.CloseIssue(ctx, pr.Issue, doer, ""); err != nil && !issues_model.IsErrPullWasClosed(err) {
723723
errs = append(errs, err)
724724
}
725725
}

0 commit comments

Comments
 (0)