Fix Add/Remove WIP on pull request title failure (go-gitea#29999)

Fix go-gitea#29997
lunny · Mar 25, 2024 · 099fdef · 099fdef
1 parent 78795dd
commit 099fdef
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 22 deletions.
diff --git a/services/issue/issue.go b/services/issue/issue.go
@@ -16,6 +16,7 @@ import (
 	system_model "code.gitea.io/gitea/models/system"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/git"
+	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/storage"
 	notify_service "code.gitea.io/gitea/services/notify"
 )
@@ -70,23 +71,17 @@ func ChangeTitle(ctx context.Context, issue *issues_model.Issue, doer *user_mode
 		return err
 	}
 
-	var reviewNotifers []*ReviewRequestNotifier
-
-	if err := db.WithTx(ctx, func(ctx context.Context) error {
-		if err := issues_model.ChangeIssueTitle(ctx, issue, doer, oldTitle); err != nil {
-			return err
-		}
+	if err := issues_model.ChangeIssueTitle(ctx, issue, doer, oldTitle); err != nil {
+		return err
+	}
 
-		if issue.IsPull && issues_model.HasWorkInProgressPrefix(oldTitle) && !issues_model.HasWorkInProgressPrefix(title) {
-			var err error
-			reviewNotifers, err = PullRequestCodeOwnersReview(ctx, issue, issue.PullRequest)
-			if err != nil {
-				return err
-			}
+	var reviewNotifers []*ReviewRequestNotifier
+	if issue.IsPull && issues_model.HasWorkInProgressPrefix(oldTitle) && !issues_model.HasWorkInProgressPrefix(title) {
+		var err error
+		reviewNotifers, err = PullRequestCodeOwnersReview(ctx, issue, issue.PullRequest)
+		if err != nil {
+			log.Error("PullRequestCodeOwnersReview: %v", err)
 		}
-		return nil
-	}); err != nil {
-		return err
 	}
 
 	notify_service.IssueChangeTitle(ctx, doer, issue, oldTitle)

diff --git a/services/issue/pull.go b/services/issue/pull.go
@@ -39,7 +39,7 @@ type ReviewRequestNotifier struct {
 	ReviewTeam *org_model.Team
 }
 
-func PullRequestCodeOwnersReview(ctx context.Context, pull *issues_model.Issue, pr *issues_model.PullRequest) ([]*ReviewRequestNotifier, error) {
+func PullRequestCodeOwnersReview(ctx context.Context, issue *issues_model.Issue, pr *issues_model.PullRequest) ([]*ReviewRequestNotifier, error) {
 	files := []string{"CODEOWNERS", "docs/CODEOWNERS", ".gitea/CODEOWNERS"}
 
 	if pr.IsWorkInProgress(ctx) {
@@ -89,7 +89,7 @@ func PullRequestCodeOwnersReview(ctx context.Context, pull *issues_model.Issue,
 
 	// https://github.com/go-gitea/gitea/issues/29763, we need to get the files changed
 	// between the merge base and the head commit but not the base branch and the head commit
-	changedFiles, err := repo.GetFilesChangedBetween(mergeBase, pr.HeadCommitID)
+	changedFiles, err := repo.GetFilesChangedBetween(mergeBase, pr.GetGitRefName())
 	if err != nil {
 		return nil, err
 	}
@@ -111,22 +111,26 @@ func PullRequestCodeOwnersReview(ctx context.Context, pull *issues_model.Issue,
 
 	notifiers := make([]*ReviewRequestNotifier, 0, len(uniqUsers)+len(uniqTeams))
 
+	if err := issue.LoadPoster(ctx); err != nil {
+		return nil, err
+	}
+
 	for _, u := range uniqUsers {
-		if u.ID != pull.Poster.ID {
-			comment, err := issues_model.AddReviewRequest(ctx, pull, u, pull.Poster)
+		if u.ID != issue.Poster.ID {
+			comment, err := issues_model.AddReviewRequest(ctx, issue, u, issue.Poster)
 			if err != nil {
 				log.Warn("Failed add assignee user: %s to PR review: %s#%d, error: %s", u.Name, pr.BaseRepo.Name, pr.ID, err)
 				return nil, err
 			}
 			notifiers = append(notifiers, &ReviewRequestNotifier{
 				Comment: comment,
 				IsAdd:   true,
-				Reviwer: pull.Poster,
+				Reviwer: u,
 			})
 		}
 	}
 	for _, t := range uniqTeams {
-		comment, err := issues_model.AddTeamReviewRequest(ctx, pull, t, pull.Poster)
+		comment, err := issues_model.AddTeamReviewRequest(ctx, issue, t, issue.Poster)
 		if err != nil {
 			log.Warn("Failed add assignee team: %s to PR review: %s#%d, error: %s", t.Name, pr.BaseRepo.Name, pr.ID, err)
 			return nil, err

diff --git a/tests/integration/pull_review_test.go b/tests/integration/pull_review_test.go
@@ -14,6 +14,7 @@ import (
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/test"
+	issue_service "code.gitea.io/gitea/services/issue"
 	repo_service "code.gitea.io/gitea/services/repository"
 	files_service "code.gitea.io/gitea/services/repository/files"
 	"code.gitea.io/gitea/tests"
@@ -85,8 +86,21 @@ func TestPullView_CodeOwner(t *testing.T) {
 			session := loginUser(t, "user2")
 			createPullRequest(t, session, "user2", "test_codeowner", repo.DefaultBranch, "codeowner-basebranch", "Test Pull Request")
 
-			pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadBranch: "codeowner-basebranch"})
+			pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadRepoID: repo.ID, HeadBranch: "codeowner-basebranch"})
 			unittest.AssertExistsIf(t, true, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 5})
+			assert.NoError(t, pr.LoadIssue(db.DefaultContext))
+
+			err := issue_service.ChangeTitle(db.DefaultContext, pr.Issue, user2, "[WIP] Test Pull Request")
+			assert.NoError(t, err)
+			prUpdated1 := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID})
+			assert.NoError(t, prUpdated1.LoadIssue(db.DefaultContext))
+			assert.EqualValues(t, "[WIP] Test Pull Request", prUpdated1.Issue.Title)
+
+			err = issue_service.ChangeTitle(db.DefaultContext, prUpdated1.Issue, user2, "Test Pull Request2")
+			assert.NoError(t, err)
+			prUpdated2 := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID})
+			assert.NoError(t, prUpdated2.LoadIssue(db.DefaultContext))
+			assert.EqualValues(t, "Test Pull Request2", prUpdated2.Issue.Title)
 		})
 
 		// change the default branch CODEOWNERS file to change README.md's codeowner