fix(actions): run review-triggered workflows on the pull request head (#40099) (#40117)

Backport #40099 by @silverwind

Runs triggered by pull request reviews took their ref and commit from
the review's `commit_id` instead of the pull request. They now use the
pull request head like other pull request events, and their commit
status is set on the commit that ran.

Co-authored-by: silverwind <me@silverwind.io>

services/actions/commit_status.go

@@ -132,21 +132,11 @@ func getCommitStatusEventNameAndCommitID(run *actions_model.ActionRun) (event, c
 			return "", "", errors.New("head of pull request is missing in event payload")
 		}
 		commitID = payload.PullRequest.Head.Sha
-	case // pull_request_review events share the same PullRequestPayload as pull_request
-		webhook_module.HookEventPullRequestReviewApproved,
+	case webhook_module.HookEventPullRequestReviewApproved,
 		webhook_module.HookEventPullRequestReviewRejected,
 		webhook_module.HookEventPullRequestReviewComment:
 		event = run.TriggerEvent
-		payload, err := run.GetPullRequestEventPayload()
-		if err != nil {
-			return "", "", fmt.Errorf("GetPullRequestEventPayload: %w", err)
-		}
-		if payload.PullRequest == nil {
-			return "", "", errors.New("pull request is missing in event payload")
-		} else if payload.PullRequest.Head == nil {
-			return "", "", errors.New("head of pull request is missing in event payload")
-		}
-		commitID = payload.PullRequest.Head.Sha
+		commitID = run.CommitSHA
 	case webhook_module.HookEventRelease:
 		event = string(run.Event)
 		commitID = run.CommitSHA

services/actions/notifier.go

@@ -459,7 +459,8 @@ func (n *actionsNotifier) PullRequestReview(ctx context.Context, pr *issues_mode
 		return
 	}
 
-	newPullRequestReviewNotifyInput(review.Issue.Repo, review.Reviewer, reviewHookType, review.CommitID, pr).
+	newNotifyInput(review.Issue.Repo, review.Reviewer, reviewHookType).
+		WithPullRequest(pr).
 		WithPayload(&api.PullRequestPayload{
 			Action:      api.HookIssueReviewed,
 			Index:       review.Issue.Index,

services/actions/notifier_helper.go

@@ -84,12 +84,6 @@ func newNotifyInputForSchedules(repo *repo_model.Repository) *notifyInput {
 	return newNotifyInput(repo, user_model.NewActionsUser(), webhook_module.HookEventSchedule)
 }
 
-func newPullRequestReviewNotifyInput(repo *repo_model.Repository, reviewer *user_model.User, event webhook_module.HookEventType, commitID string, pr *issues_model.PullRequest) *notifyInput {
-	return newNotifyInput(repo, reviewer, event).
-		WithRef(commitID).
-		WithPullRequest(pr)
-}
-
 func (input *notifyInput) WithDoer(doer *user_model.User) *notifyInput {
 	input.Doer = doer
 	return input

services/actions/notifier_helper_test.go

@@ -122,12 +122,12 @@ func TestGetApprovalUsersAddsForkPullRequestAuthorUnlessDefaultBranchWorkflow(t
 }
 
 func TestFilteredWorkflowCommitStatusForForkPullRequest(t *testing.T) {
-	forkPR := &issues_model.PullRequest{
+	input := new(notifyInput).WithPullRequest(&issues_model.PullRequest{
 		Flow:       issues_model.PullRequestFlowGithub,
 		BaseRepoID: 1,
 		HeadRepoID: 2,
-	}
-	input := newPullRequestReviewNotifyInput(&repo_model.Repository{ID: 1}, &user_model.User{ID: 2}, actions_module.GithubEventPullRequest, "refs/pull/1/head", forkPR)
+		Index:      1,
+	})
 
 	assert.True(t, isForkPullRequestInput(input))
 	assert.Equal(t, "refs/pull/1/head", input.Ref.String())

tests/integration/actions_trigger_test.go

@@ -886,11 +886,12 @@ jobs:
 		assert.NoError(t, err)
 		defer gitRepo.Close()
 
-		_, _, err = pull_service.SubmitReview(t.Context(), user4, gitRepo, pullIssue, issues_model.ReviewTypeApprove, "lgtm", sha, nil)
+		_, _, err = pull_service.SubmitReview(t.Context(), user4, gitRepo, pullIssue, issues_model.ReviewTypeApprove, "lgtm", addWorkflow.Commit.SHA, nil)
 		assert.NoError(t, err)
 
 		// verify that a commit status was created for the review event
 		waitCommitStatusAndInsertFakeStatus(t, repo, sha)
+		assert.Equal(t, sha, unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{RepoID: repo.ID}).CommitSHA)
 	})
 }