diff --git a/models/issues/dependency_test.go b/models/issues/dependency_test.go index 4c40dc2904..6eed483cc9 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, false) + _, err = issues_model.ChangeIssueStatus(db.DefaultContext, issue2, user1, true) 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 0b669fc969..ef96e1ee50 100644 --- a/models/issues/issue_update.go +++ b/models/issues/issue_update.go @@ -119,7 +119,7 @@ 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, isMergePull bool) (*Comment, error) { +func ChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, isClosed bool) (*Comment, error) { if err := issue.LoadRepo(ctx); err != nil { return nil, err } @@ -127,7 +127,7 @@ func ChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, return nil, err } - return changeIssueStatus(ctx, issue, doer, isClosed, isMergePull) + return changeIssueStatus(ctx, issue, doer, isClosed, 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 9e9d6f1dc8..5bcaf75518 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, false) + _, err := issues_model.ChangeIssueStatus(db.DefaultContext, i3, d, true) assert.NoError(t, err) pr := testCreatePR(t, 1, 2, "titlepr", fmt.Sprintf("closes #%d", i1.Index)) diff --git a/models/issues/pull.go b/models/issues/pull.go index bbf958b137..dc1b1b956a 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -499,6 +499,10 @@ func (pr *PullRequest) SetMerged(ctx context.Context) (bool, error) { return false, err } + if _, err := changeIssueStatus(ctx, pr.Issue, pr.Merger, true, true); err != nil { + return false, fmt.Errorf("Issue.changeStatus: %w", err) + } + // reset the conflicted files as there cannot be any if we're merged pr.ConflictedFiles = []string{} diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 57649611e3..dfe6d31f74 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -720,7 +720,7 @@ func CreateIssue(ctx *context.APIContext) { } if form.Closed { - if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", true, false); err != nil { + if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", true); err != nil { if issues_model.IsErrDependenciesLeft(err) { ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this issue because it still has open dependencies") return diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 061a155fbf..95f0cf3d71 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -242,8 +242,7 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption opt } var isShowClosed optional.Option[bool] - stateVal := ctx.FormString("state") - switch stateVal { + switch ctx.FormString("state") { case "closed": isShowClosed = optional.Some(true) case "all": @@ -2925,7 +2924,7 @@ func UpdateIssueStatus(ctx *context.Context) { continue } if issue.IsClosed != isClosed { - if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", isClosed, false); err != nil { + if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", isClosed); 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), @@ -3069,7 +3068,7 @@ func NewComment(ctx *context.Context) { 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, false); err != nil { + if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", isClosed); err != nil { log.Error("ChangeStatus: %v", err) if issues_model.IsErrDependenciesLeft(err) { @@ -3080,6 +3079,13 @@ func NewComment(ctx *context.Context) { } return } + } else { + if err := stopTimerIfAvailable(ctx, ctx.Doer, issue); err != nil { + ctx.ServerError("CreateOrStopIssueStopwatch", err) + return + } + + log.Trace("Issue [%d] status changed to closed: %v", issue.ID, issue.IsClosed) } } } diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index d3ff5522d5..acdba4bcdc 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1153,6 +1153,13 @@ func MergePullRequest(ctx *context.Context) { } log.Trace("Pull request merged: %d", pr.ID) + if err := stopTimerIfAvailable(ctx, ctx.Doer, issue); err != nil { + ctx.ServerError("stopTimerIfAvailable", err) + return + } + + log.Trace("Pull request merged: %d", pr.ID) + if form.DeleteBranchAfterMerge { // Don't cleanup when other pr use this branch as head branch exist, err := issues_model.HasUnmergedPullRequestsByHeadInfo(ctx, pr.HeadRepoID, pr.HeadBranch) @@ -1202,6 +1209,16 @@ func CancelAutoMergePullRequest(ctx *context.Context) { ctx.Redirect(fmt.Sprintf("%s/pulls/%d", ctx.Repo.RepoLink, issue.Index)) } +func stopTimerIfAvailable(ctx *context.Context, user *user_model.User, issue *issues_model.Issue) error { + if issues_model.StopwatchExists(ctx, user.ID, issue.ID) { + if err := issues_model.CreateOrStopIssueStopwatch(ctx, user, issue); err != nil { + return err + } + } + + return nil +} + // CompareAndPullRequestPost response for creating pull request func CompareAndPullRequestPost(ctx *context.Context) { form := web.GetForm(ctx).(*forms.CreateIssueForm) diff --git a/services/issue/commit.go b/services/issue/commit.go index 8ed0191fe0..0579e0f5c5 100644 --- a/services/issue/commit.go +++ b/services/issue/commit.go @@ -196,7 +196,7 @@ func UpdateIssuesCommit(ctx context.Context, doer *user_model.User, repo *repo_m } if isClosed != refIssue.IsClosed { refIssue.Repo = refRepo - if err := ChangeStatus(ctx, refIssue, doer, c.Sha1, isClosed, false); err != nil { + if err := ChangeStatus(ctx, refIssue, doer, c.Sha1, isClosed); err != nil { return err } } diff --git a/services/issue/status.go b/services/issue/status.go index e2e411fc37..9b6c683f4f 100644 --- a/services/issue/status.go +++ b/services/issue/status.go @@ -13,8 +13,8 @@ import ( ) // ChangeStatus changes issue status to open or closed. -func ChangeStatus(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, commitID string, closed, isMergePull bool) error { - comment, err := issues_model.ChangeIssueStatus(ctx, issue, doer, closed, isMergePull) +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 { if issues_model.IsErrDependenciesLeft(err) && closed { if err := issues_model.FinishIssueStopwatchIfPossible(ctx, doer, issue); err != nil { diff --git a/services/pull/check.go b/services/pull/check.go index f3f30fd147..f4dd332b14 100644 --- a/services/pull/check.go +++ b/services/pull/check.go @@ -27,7 +27,6 @@ import ( "code.gitea.io/gitea/modules/queue" "code.gitea.io/gitea/modules/timeutil" asymkey_service "code.gitea.io/gitea/services/asymkey" - issue_service "code.gitea.io/gitea/services/issue" notify_service "code.gitea.io/gitea/services/notify" ) @@ -299,10 +298,6 @@ func manuallyMerged(ctx context.Context, pr *issues_model.PullRequest) bool { return false } - if err = issue_service.ChangeStatus(ctx, pr.Issue, pr.Merger, pr.MergedCommitID, true, true); err != nil { - log.Error("ChangeStatus %-v: %v", pr, err) - } - notify_service.MergePullRequest(ctx, merger, pr) log.Info("manuallyMerged[%-v]: Marked as manually merged into %s/%s by commit id: %s", pr, pr.BaseRepo.Name, pr.BaseBranch, commit.ID.String()) diff --git a/services/pull/merge.go b/services/pull/merge.go index a542c6646c..00f23e1e3a 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -193,12 +193,8 @@ func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.U pr.Merger = doer pr.MergerID = doer.ID - if merged, err := pr.SetMerged(ctx); err != nil { + if _, err := pr.SetMerged(ctx); err != nil { log.Error("SetMerged %-v: %v", pr, err) - } else if merged { - if err = issue_service.ChangeStatus(ctx, pr.Issue, pr.Merger, pr.MergedCommitID, true, true); err != nil { - log.Error("ChangeStatus %-v: %v", pr, err) - } } if err := pr.LoadIssue(ctx); err != nil { @@ -237,7 +233,7 @@ func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.U } isClosed := ref.RefAction == references.XRefActionCloses if isClosed != ref.Issue.IsClosed { - if err = issue_service.ChangeStatus(ctx, ref.Issue, doer, pr.MergedCommitID, isClosed, false); err != nil { + if err = issue_service.ChangeStatus(ctx, ref.Issue, doer, pr.MergedCommitID, isClosed); err != nil { // Allow ErrDependenciesLeft if !issues_model.IsErrDependenciesLeft(err) { return err @@ -534,10 +530,6 @@ func MergedManually(ctx context.Context, pr *issues_model.PullRequest, doer *use return err } else if !merged { return fmt.Errorf("SetMerged failed") - } else { - if err = issue_service.ChangeStatus(ctx, pr.Issue, pr.Merger, pr.MergedCommitID, true, true); err != nil { - log.Error("ChangeStatus %-v: %v", pr, err) - } } return nil }); err != nil { diff --git a/services/pull/pull.go b/services/pull/pull.go index b113508e42..764be5c6e3 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -633,7 +633,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, false); err != nil && !issues_model.IsErrPullWasClosed(err) && !issues_model.IsErrDependenciesLeft(err) { + if err = issue_service.ChangeStatus(ctx, pr.Issue, doer, "", true); err != nil && !issues_model.IsErrPullWasClosed(err) && !issues_model.IsErrDependenciesLeft(err) { errs = append(errs, err) } } @@ -667,7 +667,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, false); err != nil && !issues_model.IsErrPullWasClosed(err) { + if err = issue_service.ChangeStatus(ctx, pr.Issue, doer, "", true); err != nil && !issues_model.IsErrPullWasClosed(err) { errs = append(errs, err) } }