fix

Author: wxiaoguang

services/issue/comments.go

@@ -15,6 +15,7 @@ import (
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/gitrepo"
 	"code.gitea.io/gitea/modules/json"
+	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/timeutil"
 	git_service "code.gitea.io/gitea/services/git"
 	notify_service "code.gitea.io/gitea/services/notify"
@@ -151,33 +152,31 @@ func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_m
 }
 
 // LoadCommentPushCommits Load push commits
-func LoadCommentPushCommits(ctx context.Context, c *issues_model.Comment) (err error) {
+func LoadCommentPushCommits(ctx context.Context, c *issues_model.Comment) error {
 	if c.Content == "" || c.Commits != nil || c.Type != issues_model.CommentTypePullRequestPush {
 		return nil
 	}
 
 	var data issues_model.PushActionContent
-	err = json.Unmarshal([]byte(c.Content), &data)
-	if err != nil {
-		return err
+	if err := json.Unmarshal([]byte(c.Content), &data); err != nil {
+		log.Debug("Unmarshal: %v", err) // no need to show 500 error to end user when the JSON is broken
+		return nil
 	}
 
 	c.IsForcePush = data.IsForcePush
-
-	if err := c.LoadIssue(ctx); err != nil {
-		return err
-	}
-	if err := c.Issue.LoadRepo(ctx); err != nil {
-		return err
-	}
-
 	if c.IsForcePush {
 		if len(data.CommitIDs) != 2 {
 			return nil
 		}
-		c.OldCommit = data.CommitIDs[0]
-		c.NewCommit = data.CommitIDs[1]
+		c.OldCommit, c.NewCommit = data.CommitIDs[0], data.CommitIDs[1]
 	} else {
+		if err := c.LoadIssue(ctx); err != nil {
+			return err
+		}
+		if err := c.Issue.LoadRepo(ctx); err != nil {
+			return err
+		}
+
 		gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, c.Issue.Repo)
 		if err != nil {
 			return err
@@ -186,10 +185,11 @@ func LoadCommentPushCommits(ctx context.Context, c *issues_model.Comment) (err e
 
 		c.Commits, err = git_service.ConvertFromGitCommit(ctx, gitRepo.GetCommitsFromIDs(data.CommitIDs), c.Issue.Repo)
 		if err != nil {
-			return err
+			log.Debug("GetCommitsFromIDs: %v", err) // no need to show 500 error to end user when the commit does not exist
+		} else {
+			c.CommitsNum = int64(len(c.Commits))
 		}
-		c.CommitsNum = int64(len(c.Commits))
 	}
 
-	return err
+	return nil
 }

services/pull/pull.go

@@ -374,9 +374,6 @@ type TestPullRequestOptions struct {
 func AddTestPullRequestTask(opts TestPullRequestOptions) {
 	log.Trace("AddTestPullRequestTask [head_repo_id: %d, head_branch: %s]: finding pull requests", opts.RepoID, opts.Branch)
 	graceful.GetManager().RunWithShutdownContext(func(ctx context.Context) {
-		// There is no sensible way to shut this down ":-("
-		// If you don't let it run all the way then you will lose data
-
 		repo, err := repo_model.GetRepositoryByID(ctx, opts.RepoID)
 		if err != nil {
 			log.Error("GetRepositoryByID: %v", err)

services/repository/push.go

@@ -310,16 +310,15 @@ func pushUpdateBranch(_ context.Context, repo *repo_model.Repository, pusher *us
 	}
 
 	// only update branch can trigger pull request task because the pull request hasn't been created yet when creating a branch
-	testPullRequestOpts := pull_service.TestPullRequestOptions{
+	go pull_service.AddTestPullRequestTask(pull_service.TestPullRequestOptions{
 		RepoID:      repo.ID,
 		Doer:        pusher,
 		Branch:      branch,
 		IsSync:      true,
 		IsForcePush: isForcePush,
 		OldCommitID: opts.OldCommitID,
 		NewCommitID: opts.NewCommitID,
-	}
-	go pull_service.AddTestPullRequestTask(testPullRequestOpts)
+	})
 
 	if isForcePush {
 		log.Trace("Push %s is a force push", opts.NewCommitID)

tests/integration/pull_comment_test.go

@@ -62,6 +62,7 @@ func testPullCommentRebase(t *testing.T, u *url.URL, session *TestSession) {
 	doGitPushTestRepositoryFail(dstPath, "base-repo", "local-branch/rebase:test-branch/rebase")(t)
 	doGitPushTestRepository(dstPath, "--force", "base-repo", "local-branch/rebase:test-branch/rebase")(t)
 
+	// reload the pr
 	prIssue := testWaitForPullRequestStatus(t, &issues_model.Issue{Title: testPRTitle}, issues_model.PullRequestStatusMergeable)
 	comments, err := issues_model.FindComments(t.Context(), &issues_model.FindCommentsOptions{
 		IssueID: prIssue.ID,