Merge branch 'lunny/refactor_changestatus' of github.com:lunny/gitea into lunny/refactor_changestatus

models/issues/issue_update.go

@@ -144,7 +144,7 @@ func CloseIssue(ctx context.Context, issue *Issue, doer *user_model.User) (*Comm
 	return comment, nil
 }
 
-// ChangeIssueStatus changes issue status to open or closed.
+// ReopenIssue changes issue status to open.
 func ReopenIssue(ctx context.Context, issue *Issue, doer *user_model.User) (*Comment, error) {
 	if err := issue.LoadRepo(ctx); err != nil {
 		return nil, err

routers/api/v1/repo/issue.go

@@ -912,32 +912,11 @@ func EditIssue(ctx *context.APIContext) {
 			}
 		}
 
-		var closeOrReopen bool
+		state := api.StateType(*form.State)
-		switch state := api.StateType(*form.State); state {
+		closeOrReopenIssue(ctx, issue, state)
-		case api.StateOpen:
+		if ctx.Written() {
-			closeOrReopen = false
-		case api.StateClosed:
-			closeOrReopen = true
-		default:
-			ctx.Error(http.StatusPreconditionFailed, "UnknownIssueStateError", fmt.Sprintf("unknown state: %s", state))
 			return
 		}
-
-		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, "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
-			}
-		}
 	}
 
 	// Refetch from database to assign some automatic values
@@ -1060,3 +1039,26 @@ func UpdateIssueDeadline(ctx *context.APIContext) {
 
 	ctx.JSON(http.StatusCreated, api.IssueDeadline{Deadline: deadlineUnix.AsTimePtr()})
 }
+
+func closeOrReopenIssue(ctx *context.APIContext, issue *issues_model.Issue, state api.StateType) {
+	if state != api.StateOpen && state != api.StateClosed {
+		ctx.Error(http.StatusPreconditionFailed, "UnknownIssueStateError", fmt.Sprintf("unknown state: %s", state))
+		return
+	}
+
+	if state == api.StateClosed && !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 or pull request because it still has open dependencies")
+				return
+			}
+			ctx.Error(http.StatusInternalServerError, "CloseIssue", err)
+			return
+		}
+	} else if state == api.StateOpen && 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

@@ -729,32 +729,11 @@ func EditPullRequest(ctx *context.APIContext) {
 			return
 		}
 
-		var closeOrReopen bool
+		state := api.StateType(*form.State)
-		switch state := api.StateType(*form.State); state {
+		closeOrReopenIssue(ctx, issue, state)
-		case api.StateOpen:
+		if ctx.Written() {
-			closeOrReopen = false
-		case api.StateClosed:
-			closeOrReopen = true
-		default:
-			ctx.Error(http.StatusPreconditionFailed, "UnknownPRStateError", fmt.Sprintf("unknown state: %s", state))
 			return
 		}
-
-		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, "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
-			}
-		}
 	}
 
 	// change pull target branch

routers/web/repo/issue_comment.go

@@ -154,8 +154,7 @@ func NewComment(ctx *context.Context) {
 			if pr != nil {
 				ctx.Flash.Info(ctx.Tr("repo.pulls.open_unmerged_pull_exists", pr.Index))
 			} else {
-				closeOrReopen := form.Status == "close"
+				if form.Status == "close" && !issue.IsClosed {
-				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) {
@@ -173,7 +172,7 @@ func NewComment(ctx *context.Context) {
 						}
 						log.Trace("Issue [%d] status changed to closed: %v", issue.ID, issue.IsClosed)
 					}
-				} else if !closeOrReopen && issue.IsClosed {
+				} else if form.Status == "reopen" && issue.IsClosed {
 					if err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, ""); err != nil {
 						log.Error("ReopenIssue: %v", err)
 					}

routers/web/repo/issue_list.go

@@ -418,13 +418,8 @@ func UpdateIssueStatus(ctx *context.Context) {
 		return
 	}
 
-	var closeOrReopen bool // true: close, false: reopen
+	action := ctx.FormString("action")
-	switch action := ctx.FormString("action"); action {
+	if action != "open" && action != "close" {
-	case "open":
-		closeOrReopen = false
-	case "close":
-		closeOrReopen = true
-	default:
 		log.Warn("Unrecognized action: %s", action)
 		ctx.JSONOK()
 		return
@@ -443,7 +438,7 @@ func UpdateIssueStatus(ctx *context.Context) {
 		if issue.IsPull && issue.PullRequest.HasMerged {
 			continue
 		}
-		if closeOrReopen && !issue.IsClosed {
+		if action == "close" && !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{
@@ -451,10 +446,10 @@ func UpdateIssueStatus(ctx *context.Context) {
 					})
 					return
 				}
-				ctx.ServerError("ChangeStatus", err)
+				ctx.ServerError("CloseIssue", err)
 				return
 			}
-		} else if !closeOrReopen && issue.IsClosed {
+		} else if action == "open" && issue.IsClosed {
 			if err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, ""); err != nil {
 				ctx.ServerError("ReopenIssue", err)
 				return

services/issue/commit.go

@@ -189,9 +189,8 @@ func UpdateIssuesCommit(ctx context.Context, doer *user_model.User, repo *repo_m
 				}
 			}
 
-			closeOrReopen := ref.Action == references.XRefActionCloses
 			refIssue.Repo = refRepo
-			if closeOrReopen && !refIssue.IsClosed {
+			if ref.Action == references.XRefActionCloses && !refIssue.IsClosed {
 				if len(ref.TimeLog) > 0 {
 					if err := issueAddTime(ctx, refIssue, doer, c.Timestamp, ref.TimeLog); err != nil {
 						return err
@@ -200,7 +199,7 @@ func UpdateIssuesCommit(ctx context.Context, doer *user_model.User, repo *repo_m
 				if err := CloseIssue(ctx, refIssue, doer, c.Sha1); err != nil {
 					return err
 				}
-			} else if !closeOrReopen && refIssue.IsClosed {
+			} else if ref.Action == references.XRefActionReopens && refIssue.IsClosed {
 				if err := ReopenIssue(ctx, refIssue, doer, c.Sha1); err != nil {
 					return err
 				}

services/issue/status.go

@@ -13,7 +13,7 @@ import (
 	notify_service "code.gitea.io/gitea/services/notify"
 )
 
-// CloseIssue close and issue.
+// CloseIssue close an 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 {

services/pull/merge.go

@@ -242,15 +242,14 @@ func handleCloseCrossReferences(ctx context.Context, pr *issues_model.PullReques
 		if err = ref.Issue.LoadRepo(ctx); err != nil {
 			return err
 		}
-		closeOrReopen := ref.RefAction == references.XRefActionCloses
+		if ref.RefAction == references.XRefActionCloses && !ref.Issue.IsClosed {
-		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 {
+		} else if ref.RefAction == references.XRefActionReopens && ref.Issue.IsClosed {
 			if err = issue_service.ReopenIssue(ctx, ref.Issue, doer, pr.MergedCommitID); err != nil {
 				return err
 			}