Merge 802efa342d into 993736d838

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.ChangeIssueStatus(db.DefaultContext, issue2, user1, true, false)
 	assert.NoError(t, err)
 
 	left, err = issues_model.IssueNoDependenciesLeft(db.DefaultContext, issue1)

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 bool) (*Comment, error) {
+func ChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, isClosed, isMergePull 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, false)
+	return changeIssueStatus(ctx, issue, doer, isClosed, isMergePull)
 }
 
 // ChangeIssueTitle changes the title of this issue, as the given user.

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.ChangeIssueStatus(db.DefaultContext, i3, d, true, false)
 	assert.NoError(t, err)
 
 	pr := testCreatePR(t, 1, 2, "titlepr", fmt.Sprintf("closes #%d", i1.Index))

models/issues/pull.go

@@ -499,10 +499,6 @@ 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{}
 

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); err != nil {
+		if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", true, false); err != nil {
 			if issues_model.IsErrDependenciesLeft(err) {
 				ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this issue because it still has open dependencies")
 				return

routers/web/repo/issue.go

@@ -242,7 +242,8 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption opt
 	}
 
 	var isShowClosed optional.Option[bool]
-	switch ctx.FormString("state") {
+	stateVal := ctx.FormString("state")
+	switch stateVal {
 	case "closed":
 		isShowClosed = optional.Some(true)
 	case "all":
@@ -2924,7 +2925,7 @@ func UpdateIssueStatus(ctx *context.Context) {
 			continue
 		}
 		if issue.IsClosed != isClosed {
-			if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", isClosed); err != nil {
+			if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", isClosed, false); 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),
@@ -3068,7 +3069,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); err != nil {
+				if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", isClosed, false); err != nil {
 					log.Error("ChangeStatus: %v", err)
 
 					if issues_model.IsErrDependenciesLeft(err) {
@@ -3079,13 +3080,6 @@ 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)
 				}
 			}
 		}

routers/web/repo/pull.go

@@ -1153,13 +1153,6 @@ 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)
@@ -1209,16 +1202,6 @@ 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)

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); err != nil {
+				if err := ChangeStatus(ctx, refIssue, doer, c.Sha1, isClosed, false); err != nil {
 					return err
 				}
 			}

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 bool) error {
-	comment, err := issues_model.ChangeIssueStatus(ctx, issue, doer, 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)
 	if err != nil {
 		if issues_model.IsErrDependenciesLeft(err) && closed {
 			if err := issues_model.FinishIssueStopwatchIfPossible(ctx, doer, issue); err != nil {

services/pull/check.go

@@ -27,6 +27,7 @@ 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"
 )
 
@@ -296,6 +297,10 @@ func manuallyMerged(ctx context.Context, pr *issues_model.PullRequest) bool {
 		return false
 	} else if !merged {
 		return false
+	} else {
+		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)

services/pull/merge.go

@@ -193,8 +193,12 @@ func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.U
 	pr.Merger = doer
 	pr.MergerID = doer.ID
 
-	if _, err := pr.SetMerged(ctx); err != nil {
+	if merged, 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 {
@@ -233,7 +237,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); err != nil {
+			if err = issue_service.ChangeStatus(ctx, ref.Issue, doer, pr.MergedCommitID, isClosed, false); err != nil {
 				// Allow ErrDependenciesLeft
 				if !issues_model.IsErrDependenciesLeft(err) {
 					return err
@@ -530,6 +534,10 @@ 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 {

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); err != nil && !issues_model.IsErrPullWasClosed(err) && !issues_model.IsErrDependenciesLeft(err) {
+		if err = issue_service.ChangeStatus(ctx, pr.Issue, doer, "", true, false); 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); err != nil && !issues_model.IsErrPullWasClosed(err) {
+			if err = issue_service.ChangeStatus(ctx, pr.Issue, doer, "", true, false); err != nil && !issues_model.IsErrPullWasClosed(err) {
 				errs = append(errs, err)
 			}
 		}