diff --git a/models/issues/dependency_test.go b/models/issues/dependency_test.go index 6eed483cc9..b8c21f3d10 100644 --- a/models/issues/dependency_test.go +++ b/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) diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go index 31d76be5e0..21458e458c 100644 --- a/models/issues/issue_update.go +++ b/models/issues/issue_update.go @@ -118,8 +118,8 @@ func doChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.Use }) } -// ChangeIssueStatus changes issue status to open or closed. -func ChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, isClosed bool) (*Comment, error) { +// 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 } @@ -127,7 +127,19 @@ func ChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, return nil, err } - return changeIssueStatus(ctx, issue, doer, isClosed, false) + return changeIssueStatus(ctx, issue, doer, true, false) +} + +// ChangeIssueStatus changes issue status to open or closed. +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, false, false) } // ChangeIssueTitle changes the title of this issue, as the given user. diff --git a/models/issues/issue_xref_test.go b/models/issues/issue_xref_test.go index f1b1bb2a6b..7f257330b7 100644 --- a/models/issues/issue_xref_test.go +++ b/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)) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index deda5d1d13..94165b1518 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -3166,7 +3166,7 @@ func NewComment(ctx *context.Context) { ctx.Flash.Info(ctx.Tr("repo.pulls.open_unmerged_pull_exists", pr.Index)) } else { closeOrReopen := form.Status == "close" - if closeOrReopen { + 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) { @@ -3184,7 +3184,7 @@ func NewComment(ctx *context.Context) { } log.Trace("Issue [%d] status changed to closed: %v", issue.ID, issue.IsClosed) } - } else { + } else if !closeOrReopen && issue.IsClosed { if err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, ""); err != nil { log.Error("ReopenIssue: %v", err) } diff --git a/services/issue/commit.go b/services/issue/commit.go index cd64be4410..805cdd8672 100644 --- a/services/issue/commit.go +++ b/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 := CloseIssue(ctx, refIssue, doer, c.Sha1); err != nil { + } else if !closeOrReopen && refIssue.IsClosed { + if err := ReopenIssue(ctx, refIssue, doer, c.Sha1); err != nil { return err } } diff --git a/services/issue/status.go b/services/issue/status.go index b6ee54082f..a7f3ce8f7b 100644 --- a/services/issue/status.go +++ b/services/issue/status.go @@ -14,7 +14,7 @@ import ( // 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) + comment, err := issues_model.CloseIssue(ctx, issue, doer) if err != nil { if issues_model.IsErrDependenciesLeft(err) { if err := issues_model.FinishIssueStopwatchIfPossible(ctx, doer, issue); err != nil { @@ -36,15 +36,11 @@ func CloseIssue(ctx context.Context, issue *issues_model.Issue, doer *user_model // 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) + comment, err := issues_model.ReopenIssue(ctx, issue, doer) 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 diff --git a/services/pull/merge.go b/services/pull/merge.go index 5c68e6ef2c..14dfe235ab 100644 --- a/services/pull/merge.go +++ b/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 { + 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