diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index b91fbc33bf..de6d4f1e7c 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -886,13 +886,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 2b5916d0fc..8917836eb3 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -695,13 +695,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 {