From eba6bfbe150d88a04e71521e2a0b4b804da159ff Mon Sep 17 00:00:00 2001
From: yp05327 <576951401@qq.com>
Date: Mon, 2 Sep 2024 03:08:27 +0900
Subject: [PATCH] Fix 500 error when `state` params is set when editing
issue/PR by API (#31880)
A quick fix for #31871
---
routers/api/v1/repo/issue.go | 24 +++++++++++++++++++-----
routers/api/v1/repo/pull.go | 24 +++++++++++++++++++-----
services/issue/status.go | 3 +++
3 files changed, 41 insertions(+), 10 deletions(-)
diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go
index 108504ebb4..c53eb07190 100644
--- a/routers/api/v1/repo/issue.go
+++ b/routers/api/v1/repo/issue.go
@@ -891,13 +891,27 @@ func EditIssue(ctx *context.APIContext) {
return
}
}
- if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", api.StateClosed == api.StateType(*form.State)); err != nil {
- if issues_model.IsErrDependenciesLeft(err) {
- ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this issue because it still has open dependencies")
+
+ var isClosed bool
+ switch state := api.StateType(*form.State); state {
+ case api.StateOpen:
+ isClosed = false
+ case api.StateClosed:
+ isClosed = 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 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)
return
}
- ctx.Error(http.StatusInternalServerError, "ChangeStatus", err)
- return
}
}
diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go
index 148b6ed637..de67057fdd 100644
--- a/routers/api/v1/repo/pull.go
+++ b/routers/api/v1/repo/pull.go
@@ -716,13 +716,27 @@ func EditPullRequest(ctx *context.APIContext) {
ctx.Error(http.StatusPreconditionFailed, "MergedPRState", "cannot change state of this pull request, it was already merged")
return
}
- if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", api.StateClosed == api.StateType(*form.State)); err != nil {
- if issues_model.IsErrDependenciesLeft(err) {
- ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this pull request because it still has open dependencies")
+
+ var isClosed bool
+ switch state := api.StateType(*form.State); state {
+ case api.StateOpen:
+ isClosed = false
+ case api.StateClosed:
+ isClosed = 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 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)
return
}
- ctx.Error(http.StatusInternalServerError, "ChangeStatus", err)
- return
}
}
diff --git a/services/issue/status.go b/services/issue/status.go
index 9b6c683f4f..967c29bd22 100644
--- a/services/issue/status.go
+++ b/services/issue/status.go
@@ -13,6 +13,9 @@ import (
)
// 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)
if err != nil {