diff --git a/services/agit/agit.go b/services/agit/agit.go index bd8cc82e55..a18f9ef728 100644 --- a/services/agit/agit.go +++ b/services/agit/agit.go @@ -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{ diff --git a/services/pull/pull.go b/services/pull/pull.go index a38522977b..82ca0d7047 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -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) { diff --git a/tests/integration/git_test.go b/tests/integration/git_test.go index 120e4834b6..2c46fdde68 100644 --- a/tests/integration/git_test.go +++ b/tests/integration/git_test.go @@ -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)