Use CloseIssue and ReopenIssue instead of ChangeStatus

models/issues/dependency_test.go

@@ -49,7 +49,7 @@ func TestCreateIssueDependency(t *testing.T) {
 	assert.False(t, left)
 
 	// Close #2 and check again
-	_, err = issues_model.ChangeIssueStatus(db.DefaultContext, issue2, user1, true)
+	_, err = issues_model.CloseIssue(db.DefaultContext, issue2, user1)
 	assert.NoError(t, err)
 
 	left, err = issues_model.IssueNoDependenciesLeft(db.DefaultContext, issue1)

models/issues/issue_update.go

@@ -119,16 +119,54 @@ func doChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.Use
 	})
 }
 
+// CloseIssue changes issue status to closed.
+func CloseIssue(ctx context.Context, issue *Issue, doer *user_model.User) (*Comment, error) {
+	if err := issue.LoadRepo(ctx); err != nil {
+		return nil, err
+	}
+	if err := issue.LoadPoster(ctx); err != nil {
+		return nil, err
+	}
+
+	ctx, committer, err := db.TxContext(ctx)
+	if err != nil {
+		return nil, err
+	}
+	defer committer.Close()
+
+	comment, err := changeIssueStatus(ctx, issue, doer, true, false)
+	if err != nil {
+		return nil, err
+	}
+	if err := committer.Commit(); err != nil {
+		return nil, err
+	}
+	return comment, nil
+}
+
 // ChangeIssueStatus changes issue status to open or closed.
-func ChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, isClosed bool) (*Comment, error) {
+func ReopenIssue(ctx context.Context, issue *Issue, doer *user_model.User) (*Comment, error) {
 	if err := issue.LoadRepo(ctx); err != nil {
 		return nil, err
 	}
 	if err := issue.LoadPoster(ctx); err != nil {
 		return nil, err
 	}
 
-	return changeIssueStatus(ctx, issue, doer, isClosed, false)
+	ctx, committer, err := db.TxContext(ctx)
+	if err != nil {
+		return nil, err
+	}
+	defer committer.Close()
+
+	comment, err := changeIssueStatus(ctx, issue, doer, false, false)
+	if err != nil {
+		return nil, err
+	}
+	if err := committer.Commit(); err != nil {
+		return nil, err
+	}
+	return comment, nil
 }
 
 // ChangeIssueTitle changes the title of this issue, as the given user.

models/issues/issue_xref_test.go

@@ -98,7 +98,7 @@ func TestXRef_ResolveCrossReferences(t *testing.T) {
 	i1 := testCreateIssue(t, 1, 2, "title1", "content1", false)
 	i2 := testCreateIssue(t, 1, 2, "title2", "content2", false)
 	i3 := testCreateIssue(t, 1, 2, "title3", "content3", false)
-	_, err := issues_model.ChangeIssueStatus(db.DefaultContext, i3, d, true)
+	_, err := issues_model.CloseIssue(db.DefaultContext, i3, d)
 	assert.NoError(t, err)
 
 	pr := testCreatePR(t, 1, 2, "titlepr", fmt.Sprintf("closes #%d", i1.Index))

routers/api/v1/repo/issue.go

@@ -733,7 +733,7 @@ func CreateIssue(ctx *context.APIContext) {
 	}
 
 	if form.Closed {
-		if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", true); err != nil {
+		if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil {
 			if issues_model.IsErrDependenciesLeft(err) {
 				ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this issue because it still has open dependencies")
 				return
@@ -912,24 +912,29 @@ func EditIssue(ctx *context.APIContext) {
 			}
 		}
 
-		var isClosed bool
+		var closeOrReopen bool
 		switch state := api.StateType(*form.State); state {
 		case api.StateOpen:
-			isClosed = false
+			closeOrReopen = false
 		case api.StateClosed:
-			isClosed = true
+			closeOrReopen = true
 		default:
 			ctx.Error(http.StatusPreconditionFailed, "UnknownIssueStateError", fmt.Sprintf("unknown state: %s", state))
 			return
 		}
 
-		if issue.IsClosed != isClosed {
-			if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", isClosed); err != nil {
+		if closeOrReopen && !issue.IsClosed {
+			if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil {
 				if issues_model.IsErrDependenciesLeft(err) {
 					ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this issue because it still has open dependencies")
 					return
 				}
-				ctx.Error(http.StatusInternalServerError, "ChangeStatus", err)
+				ctx.Error(http.StatusInternalServerError, "CloseIssue", err)
+				return
+			}
+		} else if !closeOrReopen && issue.IsClosed {
+			if err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, ""); err != nil {
+				ctx.Error(http.StatusInternalServerError, "ReopenIssue", err)
 				return
 			}
 		}

routers/api/v1/repo/pull.go

@@ -753,24 +753,29 @@ func EditPullRequest(ctx *context.APIContext) {
 			return
 		}
 
-		var isClosed bool
+		var closeOrReopen bool
 		switch state := api.StateType(*form.State); state {
 		case api.StateOpen:
-			isClosed = false
+			closeOrReopen = false
 		case api.StateClosed:
-			isClosed = true
+			closeOrReopen = true
 		default:
 			ctx.Error(http.StatusPreconditionFailed, "UnknownPRStateError", fmt.Sprintf("unknown state: %s", state))
 			return
 		}
 
-		if issue.IsClosed != isClosed {
-			if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", isClosed); err != nil {
+		if closeOrReopen && !issue.IsClosed {
+			if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil {
 				if issues_model.IsErrDependenciesLeft(err) {
 					ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this pull request because it still has open dependencies")
 					return
 				}
-				ctx.Error(http.StatusInternalServerError, "ChangeStatus", err)
+				ctx.Error(http.StatusInternalServerError, "CloseIssue", err)
+				return
+			}
+		} else if !closeOrReopen && issue.IsClosed {
+			if err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, ""); err != nil {
+				ctx.Error(http.StatusInternalServerError, "ReopenIssue", err)
 				return
 			}
 		}

routers/web/repo/issue_comment.go

@@ -154,25 +154,29 @@ func NewComment(ctx *context.Context) {
 			if pr != nil {
 				ctx.Flash.Info(ctx.Tr("repo.pulls.open_unmerged_pull_exists", pr.Index))
 			} else {
-				isClosed := form.Status == "close"
-				if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", isClosed); err != nil {
-					log.Error("ChangeStatus: %v", err)
-
-					if issues_model.IsErrDependenciesLeft(err) {
-						if issue.IsPull {
-							ctx.JSONError(ctx.Tr("repo.issues.dependency.pr_close_blocked"))
-						} else {
-							ctx.JSONError(ctx.Tr("repo.issues.dependency.issue_close_blocked"))
+				closeOrReopen := form.Status == "close"
+				if closeOrReopen && !issue.IsClosed {
+					if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil {
+						log.Error("CloseIssue: %v", err)
+						if issues_model.IsErrDependenciesLeft(err) {
+							if issue.IsPull {
+								ctx.JSONError(ctx.Tr("repo.issues.dependency.pr_close_blocked"))
+							} else {
+								ctx.JSONError(ctx.Tr("repo.issues.dependency.issue_close_blocked"))
+							}
+							return
 						}
-						return
+					} else {
+						if err := stopTimerIfAvailable(ctx, ctx.Doer, issue); err != nil {
+							ctx.ServerError("stopTimerIfAvailable", err)
+							return
+						}
+						log.Trace("Issue [%d] status changed to closed: %v", issue.ID, issue.IsClosed)
 					}
-				} else {
-					if err := stopTimerIfAvailable(ctx, ctx.Doer, issue); err != nil {
-						ctx.ServerError("CreateOrStopIssueStopwatch", err)
-						return
+				} else if !closeOrReopen && issue.IsClosed {
+					if err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, ""); err != nil {
+						log.Error("ReopenIssue: %v", err)
 					}
-
-					log.Trace("Issue [%d] status changed to closed: %v", issue.ID, issue.IsClosed)
 				}
 			}
 		}

routers/web/repo/issue_list.go

@@ -435,14 +435,16 @@ func UpdateIssueStatus(ctx *context.Context) {
 		return
 	}
 
-	var isClosed bool
+	var closeOrReopen bool // true: close, false: reopen
 	switch action := ctx.FormString("action"); action {
 	case "open":
-		isClosed = false
+		closeOrReopen = false
 	case "close":
-		isClosed = true
+		closeOrReopen = true
 	default:
 		log.Warn("Unrecognized action: %s", action)
+		ctx.JSONOK()
+		return
 	}
 
 	if _, err := issues.LoadRepositories(ctx); err != nil {
@@ -458,8 +460,8 @@ func UpdateIssueStatus(ctx *context.Context) {
 		if issue.IsPull && issue.PullRequest.HasMerged {
 			continue
 		}
-		if issue.IsClosed != isClosed {
-			if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", isClosed); err != nil {
+		if closeOrReopen && !issue.IsClosed {
+			if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil {
 				if issues_model.IsErrDependenciesLeft(err) {
 					ctx.JSON(http.StatusPreconditionFailed, map[string]any{
 						"error": ctx.Tr("repo.issues.dependency.issue_batch_close_blocked", issue.Index),
@@ -469,6 +471,11 @@ func UpdateIssueStatus(ctx *context.Context) {
 				ctx.ServerError("ChangeStatus", err)
 				return
 			}
+		} else if !closeOrReopen && issue.IsClosed {
+			if err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, ""); err != nil {
+				ctx.ServerError("ReopenIssue", err)
+				return
+			}
 		}
 	}
 	ctx.JSONOK()

services/issue/commit.go

@@ -188,15 +188,20 @@ func UpdateIssuesCommit(ctx context.Context, doer *user_model.User, repo *repo_m
 					continue
 				}
 			}
-			isClosed := ref.Action == references.XRefActionCloses
-			if isClosed && len(ref.TimeLog) > 0 {
-				if err := issueAddTime(ctx, refIssue, doer, c.Timestamp, ref.TimeLog); err != nil {
+
+			closeOrReopen := ref.Action == references.XRefActionCloses
+			refIssue.Repo = refRepo
+			if closeOrReopen && !refIssue.IsClosed {
+				if len(ref.TimeLog) > 0 {
+					if err := issueAddTime(ctx, refIssue, doer, c.Timestamp, ref.TimeLog); err != nil {
+						return err
+					}
+				}
+				if err := CloseIssue(ctx, refIssue, doer, c.Sha1); err != nil {
 					return err
 				}
-			}
-			if isClosed != refIssue.IsClosed {
-				refIssue.Repo = refRepo
-				if err := ChangeStatus(ctx, refIssue, doer, c.Sha1, isClosed); err != nil {
+			} else if !closeOrReopen && refIssue.IsClosed {
+				if err := ReopenIssue(ctx, refIssue, doer, c.Sha1); err != nil {
 					return err
 				}
 			}

services/issue/status.go

@@ -6,34 +6,54 @@ package issue
 import (
 	"context"
 
+	"code.gitea.io/gitea/models/db"
 	issues_model "code.gitea.io/gitea/models/issues"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/log"
 	notify_service "code.gitea.io/gitea/services/notify"
 )
 
-// ChangeStatus changes issue status to open or closed.
-// closed means the target status
-// Fix me: you should check whether the current issue status is same to the target status before call this function
-// as in function changeIssueStatus we will return WasClosedError, even the issue status and target status are both open
-func ChangeStatus(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, commitID string, closed bool) error {
-	comment, err := issues_model.ChangeIssueStatus(ctx, issue, doer, closed)
+// CloseIssue close and issue.
+func CloseIssue(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, commitID string) error {
+	dbCtx, committer, err := db.TxContext(ctx)
 	if err != nil {
-		if issues_model.IsErrDependenciesLeft(err) && closed {
-			if err := issues_model.FinishIssueStopwatchIfPossible(ctx, doer, issue); err != nil {
+		return err
+	}
+	defer committer.Close()
+
+	comment, err := issues_model.CloseIssue(dbCtx, issue, doer)
+	if err != nil {
+		if issues_model.IsErrDependenciesLeft(err) {
+			if err := issues_model.FinishIssueStopwatchIfPossible(dbCtx, doer, issue); err != nil {
 				log.Error("Unable to stop stopwatch for issue[%d]#%d: %v", issue.ID, issue.Index, err)
 			}
 		}
 		return err
 	}
 
-	if closed {
-		if err := issues_model.FinishIssueStopwatchIfPossible(ctx, doer, issue); err != nil {
-			return err
-		}
+	if err := issues_model.FinishIssueStopwatchIfPossible(dbCtx, doer, issue); err != nil {
+		return err
+	}
+
+	if err := committer.Commit(); err != nil {
+		return err
+	}
+	committer.Close()
+
+	notify_service.IssueChangeStatus(ctx, doer, commitID, issue, comment, true)
+
+	return nil
+}
+
+// ReopenIssue reopen an issue.
+// FIXME: If some issues dependent this one are closed, should we also reopen them?
+func ReopenIssue(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, commitID string) error {
+	comment, err := issues_model.ReopenIssue(ctx, issue, doer)
+	if err != nil {
+		return err
 	}
 
-	notify_service.IssueChangeStatus(ctx, doer, commitID, issue, comment, closed)
+	notify_service.IssueChangeStatus(ctx, doer, commitID, issue, comment, false)
 
 	return nil
 }

services/pull/merge.go

@@ -242,14 +242,18 @@ func handleCloseCrossReferences(ctx context.Context, pr *issues_model.PullReques
 		if err = ref.Issue.LoadRepo(ctx); err != nil {
 			return err
 		}
-		isClosed := ref.RefAction == references.XRefActionCloses
-		if isClosed != ref.Issue.IsClosed {
-			if err = issue_service.ChangeStatus(ctx, ref.Issue, doer, pr.MergedCommitID, isClosed); err != nil {
+		closeOrReopen := ref.RefAction == references.XRefActionCloses
+		if closeOrReopen && !ref.Issue.IsClosed {
+			if err = issue_service.CloseIssue(ctx, ref.Issue, doer, pr.MergedCommitID); err != nil {
 				// Allow ErrDependenciesLeft
 				if !issues_model.IsErrDependenciesLeft(err) {
 					return err
 				}
 			}
+		} else if !closeOrReopen && ref.Issue.IsClosed {
+			if err = issue_service.ReopenIssue(ctx, ref.Issue, doer, pr.MergedCommitID); err != nil {
+				return err
+			}
 		}
 	}
 	return nil

services/pull/pull.go

@@ -685,7 +685,7 @@ func CloseBranchPulls(ctx context.Context, doer *user_model.User, repoID int64,
 
 	var errs errlist
 	for _, pr := range prs {
-		if err = issue_service.ChangeStatus(ctx, pr.Issue, doer, "", true); err != nil && !issues_model.IsErrPullWasClosed(err) && !issues_model.IsErrDependenciesLeft(err) {
+		if err = issue_service.CloseIssue(ctx, pr.Issue, doer, ""); err != nil && !issues_model.IsErrPullWasClosed(err) && !issues_model.IsErrDependenciesLeft(err) {
 			errs = append(errs, err)
 		}
 	}
@@ -719,7 +719,7 @@ func CloseRepoBranchesPulls(ctx context.Context, doer *user_model.User, repo *re
 			if pr.BaseRepoID == repo.ID {
 				continue
 			}
-			if err = issue_service.ChangeStatus(ctx, pr.Issue, doer, "", true); err != nil && !issues_model.IsErrPullWasClosed(err) {
+			if err = issue_service.CloseIssue(ctx, pr.Issue, doer, ""); err != nil && !issues_model.IsErrPullWasClosed(err) {
 				errs = append(errs, err)
 			}
 		}