From f7858425bb87eb3fe4f478152201c941673455bb Mon Sep 17 00:00:00 2001
From: Michael Kriese <michael.kriese@visualon.de>
Date: Thu, 8 Aug 2024 12:34:51 +0200
Subject: [PATCH] fix(agit): run full pr checks on force-push
(cherry picked from commit 2d05e922a2cec0ec7c8e62e7909d0187faefdbb3)
---
services/agit/agit.go | 9 ++++
services/pull/pull.go | 78 ++++++++++++++++++-----------------
tests/integration/git_test.go | 13 ++++++
3 files changed, 63 insertions(+), 37 deletions(-)
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)