Merge pull request '[Port] gitea#29999: Fix Add/Remove WIP on pull request title failure' (#2873) from gusted/forgejo-port-gitea-prs into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/2873
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
This commit is contained in:
Earl Warren 2024-03-29 12:12:19 +00:00
commit 6548f8feae
3 changed files with 44 additions and 23 deletions

View file

@ -17,6 +17,7 @@ import (
system_model "code.gitea.io/gitea/models/system" system_model "code.gitea.io/gitea/models/system"
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/storage"
"code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/timeutil"
notify_service "code.gitea.io/gitea/services/notify" notify_service "code.gitea.io/gitea/services/notify"
@ -64,24 +65,26 @@ func ChangeTitle(ctx context.Context, issue *issues_model.Issue, doer *user_mode
return nil return nil
} }
var reviewNotifers []*ReviewRequestNotifier if err := issue.LoadRepo(ctx); err != nil {
return err
}
if user_model.IsBlockedMultiple(ctx, []int64{issue.PosterID, issue.Repo.OwnerID}, doer.ID) {
return user_model.ErrBlockedByUser
}
if err := db.WithTx(ctx, func(ctx context.Context) error {
if err := issues_model.ChangeIssueTitle(ctx, issue, doer, oldTitle); err != nil { if err := issues_model.ChangeIssueTitle(ctx, issue, doer, oldTitle); err != nil {
return err return err
} }
var reviewNotifers []*ReviewRequestNotifier
if issue.IsPull && issues_model.HasWorkInProgressPrefix(oldTitle) && !issues_model.HasWorkInProgressPrefix(title) { if issue.IsPull && issues_model.HasWorkInProgressPrefix(oldTitle) && !issues_model.HasWorkInProgressPrefix(title) {
var err error var err error
reviewNotifers, err = PullRequestCodeOwnersReview(ctx, issue, issue.PullRequest) reviewNotifers, err = PullRequestCodeOwnersReview(ctx, issue, issue.PullRequest)
if err != nil { if err != nil {
return err log.Error("PullRequestCodeOwnersReview: %v", err)
} }
} }
return nil
}); err != nil {
return err
}
notify_service.IssueChangeTitle(ctx, doer, issue, oldTitle) notify_service.IssueChangeTitle(ctx, doer, issue, oldTitle)
ReviewRequestNotify(ctx, issue, issue.Poster, reviewNotifers) ReviewRequestNotify(ctx, issue, issue.Poster, reviewNotifers)

View file

@ -40,7 +40,7 @@ type ReviewRequestNotifier struct {
ReviewTeam *org_model.Team 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"} files := []string{"CODEOWNERS", "docs/CODEOWNERS", ".gitea/CODEOWNERS"}
if pr.IsWorkInProgress(ctx) { if pr.IsWorkInProgress(ctx) {
@ -90,7 +90,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 // 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 // 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 { if err != nil {
return nil, err return nil, err
} }
@ -112,9 +112,13 @@ func PullRequestCodeOwnersReview(ctx context.Context, pull *issues_model.Issue,
notifiers := make([]*ReviewRequestNotifier, 0, len(uniqUsers)+len(uniqTeams)) notifiers := make([]*ReviewRequestNotifier, 0, len(uniqUsers)+len(uniqTeams))
if err := issue.LoadPoster(ctx); err != nil {
return nil, err
}
for _, u := range uniqUsers { for _, u := range uniqUsers {
if u.ID != pull.Poster.ID { if u.ID != issue.Poster.ID {
comment, err := issues_model.AddReviewRequest(ctx, pull, u, pull.Poster) comment, err := issues_model.AddReviewRequest(ctx, issue, u, issue.Poster)
if err != nil { 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) 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 return nil, err
@ -122,12 +126,12 @@ func PullRequestCodeOwnersReview(ctx context.Context, pull *issues_model.Issue,
notifiers = append(notifiers, &ReviewRequestNotifier{ notifiers = append(notifiers, &ReviewRequestNotifier{
Comment: comment, Comment: comment,
IsAdd: true, IsAdd: true,
Reviwer: pull.Poster, Reviwer: u,
}) })
} }
} }
for _, t := range uniqTeams { 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 { 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) 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 return nil, err

View file

@ -16,6 +16,7 @@ import (
"code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/git"
issue_service "code.gitea.io/gitea/services/issue"
repo_service "code.gitea.io/gitea/services/repository" repo_service "code.gitea.io/gitea/services/repository"
files_service "code.gitea.io/gitea/services/repository/files" files_service "code.gitea.io/gitea/services/repository/files"
"code.gitea.io/gitea/tests" "code.gitea.io/gitea/tests"
@ -303,8 +304,21 @@ func TestPullView_CodeOwner(t *testing.T) {
session := loginUser(t, "user2") session := loginUser(t, "user2")
testPullCreate(t, session, "user2", "test_codeowner", false, repo.DefaultBranch, "codeowner-basebranch", "Test Pull Request") testPullCreate(t, session, "user2", "test_codeowner", false, 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}) 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 // change the default branch CODEOWNERS file to change README.md's codeowner