mirror of
https://github.com/go-gitea/gitea.git
synced 2026-02-21 14:07:32 +01:00
actions: report commit status for pull_request_review events (#36589)
Workflows triggered by pull_request_review events (approved, rejected, comment) complete successfully but never create a commit status on the PR. This makes them invisible in the merge checks UI, breaking any CI gate that re-evaluates on review submission. The commit status handler's switch statement was missing the three review event types, so they fell through to the default case which returned empty strings. Additionally, review events use PullRequestPayload but IsPullRequest() returns false for them (Event() returns "pull_request_approved" etc. instead of "pull_request"), so GetPullRequestEventPayload() refuses to parse their payload. Signed-off-by: Jörg Thalheim <joerg@thalheim.io> Co-authored-by: silverwind <me@silverwind.io>
This commit is contained in:
@@ -168,7 +168,7 @@ func (run *ActionRun) GetPushEventPayload() (*api.PushPayload, error) {
|
||||
}
|
||||
|
||||
func (run *ActionRun) GetPullRequestEventPayload() (*api.PullRequestPayload, error) {
|
||||
if run.Event.IsPullRequest() {
|
||||
if run.Event.IsPullRequest() || run.Event.IsPullRequestReview() {
|
||||
var payload api.PullRequestPayload
|
||||
if err := json.Unmarshal([]byte(run.EventPayload), &payload); err != nil {
|
||||
return nil, err
|
||||
|
||||
@@ -98,6 +98,20 @@ func (h HookEventType) IsPullRequest() bool {
|
||||
return h.Event() == "pull_request"
|
||||
}
|
||||
|
||||
// IsPullRequestReview returns true for pull request review events
|
||||
// (approved, rejected, comment). These events use the same PullRequestPayload
|
||||
// as regular pull_request events.
|
||||
func (h HookEventType) IsPullRequestReview() bool {
|
||||
switch h {
|
||||
case HookEventPullRequestReviewApproved,
|
||||
HookEventPullRequestReviewRejected,
|
||||
HookEventPullRequestReviewComment:
|
||||
return true
|
||||
default:
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
// HookType is the type of a webhook
|
||||
type HookType = string
|
||||
|
||||
|
||||
@@ -115,6 +115,21 @@ 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,
|
||||
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
|
||||
case webhook_module.HookEventRelease:
|
||||
event = string(run.Event)
|
||||
commitID = run.CommitSHA
|
||||
|
||||
@@ -691,6 +691,144 @@ func insertFakeStatus(t *testing.T, repo *repo_model.Repository, sha, targetURL,
|
||||
assert.NoError(t, err)
|
||||
}
|
||||
|
||||
func TestPullRequestReviewCommitStatusEvent(t *testing.T) {
|
||||
onGiteaRun(t, func(t *testing.T, u *url.URL) {
|
||||
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) // owner of the repo
|
||||
user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) // reviewer
|
||||
|
||||
// create a repo
|
||||
repo, err := repo_service.CreateRepository(t.Context(), user2, user2, repo_service.CreateRepoOptions{
|
||||
Name: "repo-pull-request-review",
|
||||
Description: "test pull-request-review commit status",
|
||||
AutoInit: true,
|
||||
Gitignores: "Go",
|
||||
License: "MIT",
|
||||
Readme: "Default",
|
||||
DefaultBranch: "main",
|
||||
IsPrivate: false,
|
||||
})
|
||||
assert.NoError(t, err)
|
||||
assert.NotEmpty(t, repo)
|
||||
|
||||
// add user4 as collaborator so they can review
|
||||
ctx := NewAPITestContext(t, repo.OwnerName, repo.Name, auth_model.AccessTokenScopeWriteRepository)
|
||||
t.Run("AddUser4AsCollaboratorWithWriteAccess", doAPIAddCollaborator(ctx, "user4", perm.AccessModeWrite))
|
||||
|
||||
// add workflow file that triggers on pull_request_review
|
||||
addWorkflow, err := files_service.ChangeRepoFiles(t.Context(), repo, user2, &files_service.ChangeRepoFilesOptions{
|
||||
Files: []*files_service.ChangeRepoFile{
|
||||
{
|
||||
Operation: "create",
|
||||
TreePath: ".gitea/workflows/pr-review.yml",
|
||||
ContentReader: strings.NewReader(`name: test
|
||||
on:
|
||||
pull_request_review:
|
||||
types: [submitted]
|
||||
jobs:
|
||||
test:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- run: echo helloworld
|
||||
`),
|
||||
},
|
||||
},
|
||||
Message: "add workflow",
|
||||
OldBranch: "main",
|
||||
NewBranch: "main",
|
||||
Author: &files_service.IdentityOptions{
|
||||
GitUserName: user2.Name,
|
||||
GitUserEmail: user2.Email,
|
||||
},
|
||||
Committer: &files_service.IdentityOptions{
|
||||
GitUserName: user2.Name,
|
||||
GitUserEmail: user2.Email,
|
||||
},
|
||||
Dates: &files_service.CommitDateOptions{
|
||||
Author: time.Now(),
|
||||
Committer: time.Now(),
|
||||
},
|
||||
})
|
||||
assert.NoError(t, err)
|
||||
assert.NotEmpty(t, addWorkflow)
|
||||
|
||||
// create a branch and a PR
|
||||
testBranch := "test-review-branch"
|
||||
err = repo_service.CreateNewBranch(t.Context(), user2, repo, "main", testBranch)
|
||||
assert.NoError(t, err)
|
||||
|
||||
// add a file on the test branch so the PR has changes
|
||||
addFileResp, err := files_service.ChangeRepoFiles(t.Context(), repo, user2, &files_service.ChangeRepoFilesOptions{
|
||||
Files: []*files_service.ChangeRepoFile{
|
||||
{
|
||||
Operation: "create",
|
||||
TreePath: "test.txt",
|
||||
ContentReader: strings.NewReader("test content"),
|
||||
},
|
||||
},
|
||||
Message: "add test file",
|
||||
OldBranch: testBranch,
|
||||
NewBranch: testBranch,
|
||||
Author: &files_service.IdentityOptions{
|
||||
GitUserName: user2.Name,
|
||||
GitUserEmail: user2.Email,
|
||||
},
|
||||
Committer: &files_service.IdentityOptions{
|
||||
GitUserName: user2.Name,
|
||||
GitUserEmail: user2.Email,
|
||||
},
|
||||
Dates: &files_service.CommitDateOptions{
|
||||
Author: time.Now(),
|
||||
Committer: time.Now(),
|
||||
},
|
||||
})
|
||||
assert.NoError(t, err)
|
||||
assert.NotEmpty(t, addFileResp)
|
||||
sha := addFileResp.Commit.SHA
|
||||
|
||||
pullIssue := &issues_model.Issue{
|
||||
RepoID: repo.ID,
|
||||
Title: "A test PR for review",
|
||||
PosterID: user2.ID,
|
||||
Poster: user2,
|
||||
IsPull: true,
|
||||
}
|
||||
pullRequest := &issues_model.PullRequest{
|
||||
HeadRepoID: repo.ID,
|
||||
BaseRepoID: repo.ID,
|
||||
HeadBranch: testBranch,
|
||||
BaseBranch: "main",
|
||||
HeadRepo: repo,
|
||||
BaseRepo: repo,
|
||||
Type: issues_model.PullRequestGitea,
|
||||
}
|
||||
prOpts := &pull_service.NewPullRequestOptions{Repo: repo, Issue: pullIssue, PullRequest: pullRequest}
|
||||
err = pull_service.NewPullRequest(t.Context(), prOpts)
|
||||
assert.NoError(t, err)
|
||||
|
||||
// submit an approval review as user4
|
||||
gitRepo, err := gitrepo.OpenRepository(t.Context(), repo)
|
||||
assert.NoError(t, err)
|
||||
defer gitRepo.Close()
|
||||
|
||||
_, _, err = pull_service.SubmitReview(t.Context(), user4, gitRepo, pullIssue, issues_model.ReviewTypeApprove, "lgtm", sha, nil)
|
||||
assert.NoError(t, err)
|
||||
|
||||
// verify that a commit status was created for the review event
|
||||
assert.Eventually(t, func() bool {
|
||||
latestCommitStatuses, err := git_model.GetLatestCommitStatus(t.Context(), repo.ID, sha, db.ListOptionsAll)
|
||||
assert.NoError(t, err)
|
||||
if len(latestCommitStatuses) == 0 {
|
||||
return false
|
||||
}
|
||||
if latestCommitStatuses[0].State == commitstatus.CommitStatusPending {
|
||||
insertFakeStatus(t, repo, sha, latestCommitStatuses[0].TargetURL, latestCommitStatuses[0].Context)
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}, 1*time.Second, 100*time.Millisecond)
|
||||
})
|
||||
}
|
||||
|
||||
func TestWorkflowDispatchPublicApi(t *testing.T) {
|
||||
onGiteaRun(t, func(t *testing.T, u *url.URL) {
|
||||
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
|
||||
|
||||
Reference in New Issue
Block a user