Use CloseIssue and ReopenIssue instead of ChagneStatus for issues

This commit is contained in:
Lunny Xiao 2024-11-10 13:12:13 -08:00
parent a928739456
commit 406884a6f4
No known key found for this signature in database
GPG Key ID: C3B7C91B632F738A
7 changed files with 85 additions and 52 deletions

View File

@ -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
}
}

View File

@ -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
}
}

View File

@ -2991,14 +2991,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 {
@ -3014,15 +3016,20 @@ 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),
})
return
}
ctx.ServerError("ChangeStatus", err)
ctx.ServerError("CloseIssue", err)
return
}
} else if !closeOrReopen && issue.IsClosed {
if err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, ""); err != nil {
ctx.ServerError("ReopenIssue", err)
return
}
}
@ -3158,10 +3165,10 @@ 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)
closeOrReopen := form.Status == "close"
if closeOrReopen {
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"))
@ -3172,12 +3179,16 @@ func NewComment(ctx *context.Context) {
}
} else {
if err := stopTimerIfAvailable(ctx, ctx.Doer, issue); err != nil {
ctx.ServerError("CreateOrStopIssueStopwatch", err)
ctx.ServerError("stopTimerIfAvailable", err)
return
}
log.Trace("Issue [%d] status changed to closed: %v", issue.ID, issue.IsClosed)
}
} else {
if err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, ""); err != nil {
log.Error("ReopenIssue: %v", err)
}
}
}
}

View File

@ -194,9 +194,9 @@ func UpdateIssuesCommit(ctx context.Context, doer *user_model.User, repo *repo_m
return err
}
}
if isClosed != refIssue.IsClosed {
if isClosed && !refIssue.IsClosed {
refIssue.Repo = refRepo
if err := ChangeStatus(ctx, refIssue, doer, c.Sha1, isClosed); err != nil {
if err := CloseIssue(ctx, refIssue, doer, c.Sha1); err != nil {
return err
}
}

View File

@ -12,14 +12,11 @@ import (
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 {
comment, err := issues_model.ChangeIssueStatus(ctx, issue, doer, true)
if err != nil {
if issues_model.IsErrDependenciesLeft(err) && closed {
if issues_model.IsErrDependenciesLeft(err) {
if err := issues_model.FinishIssueStopwatchIfPossible(ctx, doer, issue); err != nil {
log.Error("Unable to stop stopwatch for issue[%d]#%d: %v", issue.ID, issue.Index, err)
}
@ -27,13 +24,28 @@ func ChangeStatus(ctx context.Context, issue *issues_model.Issue, doer *user_mod
return err
}
if closed {
if err := issues_model.FinishIssueStopwatchIfPossible(ctx, doer, issue); err != nil {
return err
}
}
notify_service.IssueChangeStatus(ctx, doer, commitID, issue, comment, closed)
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.ChangeIssueStatus(ctx, issue, doer, false)
if err != nil {
return err
}
if err := issues_model.FinishIssueStopwatchIfPossible(ctx, doer, issue); err != nil {
return err
}
notify_service.IssueChangeStatus(ctx, doer, commitID, issue, comment, false)
return nil
}

View File

@ -243,8 +243,8 @@ func handleCloseCrossReferences(ctx context.Context, pr *issues_model.PullReques
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 {
if isClosed && !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

View File

@ -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)
}
}