thepaperpilot/forgejo
1
0
Fork 0
Code Issues Pull requests Releases Activity Actions

fix(agit): run full pr checks on force-push

Browse source
This commit is contained in:
Michael Kriese 2024-08-08 12:34:51 +02:00
parent 0d2efa2c4a
commit 2d05e922a2
No known key found for this signature in database
GPG key ID: F8D7748549A5986A
3 changed files with 63 additions and 37 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
services/agit/agit.go
View file

@ -211,6 +211,8 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.
return nil, fmt.Errorf("failed to update the reference of the pull request: %w", err)
}
// TODO: refactor to unify with `pull_service.AddTestPullRequestTask`
// Add the pull request to the merge conflicting checker queue.
pull_service.AddToTaskQueue(ctx, pr)
@ -218,12 +220,19 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.
return nil, fmt.Errorf("failed to load the issue of the pull request: %w", err)
}
// Validate pull request.
pull_service.ValidatePullRequest(ctx, pr, oldCommitID, opts.NewCommitIDs[i], pusher)
// TODO: call `InvalidateCodeComments`
// Create and notify about the new commits.
comment, err := pull_service.CreatePushPullComment(ctx, pusher, pr, oldCommitID, opts.NewCommitIDs[i])
if err == nil && comment != nil {
notify_service.PullRequestPushCommits(ctx, pusher, pr, comment)
}
notify_service.PullRequestSynchronized(ctx, pusher, pr)
// this always seems to be false
isForcePush := comment != nil && comment.IsForcePush
results = append(results, private.HookProcReceiveRefResult{

78
services/pull/pull.go
View file

@ -356,43 +356,7 @@ func TestPullRequest(ctx context.Context, doer *user_model.User, repoID, olderTh
}
if err == nil {
for _, pr := range prs {
objectFormat := git.ObjectFormatFromName(pr.BaseRepo.ObjectFormatName)
if newCommitID != "" && newCommitID != objectFormat.EmptyObjectID().String() {
changed, err := checkIfPRContentChanged(ctx, pr, oldCommitID, newCommitID)
if err != nil {
log.Error("checkIfPRContentChanged: %v", err)
}
if changed {
// Mark old reviews as stale if diff to mergebase has changed
if err := issues_model.MarkReviewsAsStale(ctx, pr.IssueID); err != nil {
log.Error("MarkReviewsAsStale: %v", err)
}
// dismiss all approval reviews if protected branch rule item enabled.
pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch)
if err != nil {
log.Error("GetFirstMatchProtectedBranchRule: %v", err)
}
if pb != nil && pb.DismissStaleApprovals {
if err := DismissApprovalReviews(ctx, doer, pr); err != nil {
log.Error("DismissApprovalReviews: %v", err)
}
}
}
if err := issues_model.MarkReviewsAsNotStale(ctx, pr.IssueID, newCommitID); err != nil {
log.Error("MarkReviewsAsNotStale: %v", err)
}
divergence, err := GetDiverging(ctx, pr)
if err != nil {
log.Error("GetDiverging: %v", err)
} else {
err = pr.UpdateCommitDivergence(ctx, divergence.Ahead, divergence.Behind)
if err != nil {
log.Error("UpdateCommitDivergence: %v", err)
}
}
}
ValidatePullRequest(ctx, pr, newCommitID, oldCommitID, doer)
notify_service.PullRequestSynchronized(ctx, doer, pr)
}
}
@ -422,6 +386,46 @@ func TestPullRequest(ctx context.Context, doer *user_model.User, repoID, olderTh
}
}
// Mark old reviews as stale if diff to mergebase has changed.
// Dismiss all approval reviews if protected branch rule item enabled.
// Update commit divergence.
func ValidatePullRequest(ctx context.Context, pr *issues_model.PullRequest, newCommitID, oldCommitID string, doer *user_model.User) {
objectFormat := git.ObjectFormatFromName(pr.BaseRepo.ObjectFormatName)
if newCommitID != "" && newCommitID != objectFormat.EmptyObjectID().String() {
changed, err := checkIfPRContentChanged(ctx, pr, oldCommitID, newCommitID)
if err != nil {
log.Error("checkIfPRContentChanged: %v", err)
}
if changed {
if err := issues_model.MarkReviewsAsStale(ctx, pr.IssueID); err != nil {
log.Error("MarkReviewsAsStale: %v", err)
}
pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch)
if err != nil {
log.Error("GetFirstMatchProtectedBranchRule: %v", err)
}
if pb != nil && pb.DismissStaleApprovals {
if err := DismissApprovalReviews(ctx, doer, pr); err != nil {
log.Error("DismissApprovalReviews: %v", err)
}
}
}
if err := issues_model.MarkReviewsAsNotStale(ctx, pr.IssueID, newCommitID); err != nil {
log.Error("MarkReviewsAsNotStale: %v", err)
}
divergence, err := GetDiverging(ctx, pr)
if err != nil {
log.Error("GetDiverging: %v", err)
} else {
err = pr.UpdateCommitDivergence(ctx, divergence.Ahead, divergence.Behind)
if err != nil {
log.Error("UpdateCommitDivergence: %v", err)
}
}
}
}
// checkIfPRContentChanged checks if diff to target branch has changed by push
// A commit can be considered to leave the PR untouched if the patch/diff with its merge base is unchanged
func checkIfPRContentChanged(ctx context.Context, pr *issues_model.PullRequest, oldCommitID, newCommitID string) (hasChanged bool, err error) {

13
tests/integration/git_test.go
View file

@ -829,6 +829,9 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, headBranch string
if !assert.NotEmpty(t, pr1) {
return
}
assert.Equal(t, 1, pr1.CommitsAhead)
assert.Equal(t, 0, pr1.CommitsBehind)
prMsg, err := doAPIGetPullRequest(*ctx, ctx.Username, ctx.Reponame, pr1.Index)(t)
require.NoError(t, err)
@ -849,6 +852,8 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, headBranch string
if !assert.NotEmpty(t, pr2) {
return
}
assert.Equal(t, 1, pr2.CommitsAhead)
assert.Equal(t, 0, pr2.CommitsBehind)
prMsg, err = doAPIGetPullRequest(*ctx, ctx.Username, ctx.Reponame, pr2.Index)(t)
require.NoError(t, err)
@ -907,6 +912,14 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, headBranch string
assert.False(t, prMsg.HasMerged)
assert.Equal(t, commit, prMsg.Head.Sha)
pr1 = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{
HeadRepoID: repo.ID,
Flow: issues_model.PullRequestFlowAGit,
Index: pr1.Index,
})
assert.Equal(t, 2, pr1.CommitsAhead)
assert.Equal(t, 0, pr1.CommitsBehind)
_, _, err = git.NewCommand(git.DefaultContext, "push", "origin").AddDynamicArguments("HEAD:refs/for/master/test/" + headBranch).RunStdString(&git.RunOpts{Dir: dstPath})
require.NoError(t, err)