Skip to content

Commit

Permalink
fix: filter out issues and closed PRs
Browse files Browse the repository at this point in the history
  • Loading branch information
aeddi committed Dec 2, 2024
1 parent ffdce93 commit 241a755
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 32 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/bot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ jobs:
process-pr:
name: Process PR
needs: define-prs-matrix
# Just skip this job if PR numbers matrix is empty (prevent failed state)
if: ${{ needs.define-prs-matrix.outputs.pr-numbers != '[]' && needs.define-prs-matrix.outputs.pr-numbers != '' }}
runs-on: ubuntu-latest
strategy:
matrix:
Expand Down
4 changes: 2 additions & 2 deletions contribs/github-bot/internal/check/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ func execCheck(flags *checkFlags) error {
// (flag or GitHub Action context).
prs = make([]*github.PullRequest, len(flags.PRNums))
for i, prNum := range flags.PRNums {
pr, _, err := gh.Client.PullRequests.Get(gh.Ctx, gh.Owner, gh.Repo, prNum)
pr, err := gh.GetOpenedPullRequest(prNum)

Check warning on line 68 in contribs/github-bot/internal/check/check.go

View check run for this annotation

Codecov / codecov/patch

contribs/github-bot/internal/check/check.go#L66-L68

Added lines #L66 - L68 were not covered by tests
if err != nil {
return fmt.Errorf("unable to retrieve specified pull request (%d): %w", prNum, err)
return fmt.Errorf("unable to process PR list: %w", err)

Check warning on line 70 in contribs/github-bot/internal/check/check.go

View check run for this annotation

Codecov / codecov/patch

contribs/github-bot/internal/check/check.go#L70

Added line #L70 was not covered by tests
}
prs[i] = pr
}
Expand Down
24 changes: 15 additions & 9 deletions contribs/github-bot/internal/check/comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,18 @@ func handleCommentUpdate(gh *client.GitHub, actionCtx *githubactions.GitHubConte
return nil
}

// Get PR number from GitHub Actions context.
prNumFloat, ok := utils.IndexMap(actionCtx.Event, "issue", "number").(float64)
if !ok || prNumFloat <= 0 {
return errors.New("unable to get issue number on issue comment event")
}
prNum := int(prNumFloat)

// Ignore if this comment update is not related to an opened PR.
if _, err := gh.GetOpenedPullRequest(prNum); err != nil {
return nil // May come from an issue or a closed PR
}

// Return if comment was edited by bot (current authenticated user).
authUser, _, err := gh.Client.Users.Get(gh.Ctx, "")
if err != nil {
Expand Down Expand Up @@ -133,17 +145,11 @@ func handleCommentUpdate(gh *client.GitHub, actionCtx *githubactions.GitHubConte
return errors.New("unable to get changes body content on issue comment event")
}

// Get PR number from GitHub Actions context.
prNum, ok := utils.IndexMap(actionCtx.Event, "issue", "number").(float64)
if !ok || prNum <= 0 {
return errors.New("unable to get issue number on issue comment event")
}

// Check if change is only a checkbox being checked or unckecked.
if checkboxes.ReplaceAllString(current, "") != checkboxes.ReplaceAllString(previous, "") {
// If not, restore previous comment body.
if !gh.DryRun {
gh.SetBotComment(previous, int(prNum))
gh.SetBotComment(previous, prNum)
}
return errors.New("bot comment edited outside of checkboxes")
}
Expand Down Expand Up @@ -181,7 +187,7 @@ func handleCommentUpdate(gh *client.GitHub, actionCtx *githubactions.GitHubConte
if len(teams) > 0 {
if !gh.IsUserInTeams(actionCtx.Actor, teams) { // If user not allowed to check the boxes.

Check warning on line 188 in contribs/github-bot/internal/check/comment.go

View check run for this annotation

Codecov / codecov/patch

contribs/github-bot/internal/check/comment.go#L188

Added line #L188 was not covered by tests
if !gh.DryRun {
gh.SetBotComment(previous, int(prNum)) // Then restore previous state.
gh.SetBotComment(previous, prNum) // Then restore previous state.

Check warning on line 190 in contribs/github-bot/internal/check/comment.go

View check run for this annotation

Codecov / codecov/patch

contribs/github-bot/internal/check/comment.go#L190

Added line #L190 was not covered by tests
}
return errors.New("checkbox edited by a user not allowed to")
}
Expand All @@ -203,7 +209,7 @@ func handleCommentUpdate(gh *client.GitHub, actionCtx *githubactions.GitHubConte

// Update comment with username.
if edited != "" && !gh.DryRun {
gh.SetBotComment(edited, int(prNum))
gh.SetBotComment(edited, prNum)

Check warning on line 212 in contribs/github-bot/internal/check/comment.go

View check run for this annotation

Codecov / codecov/patch

contribs/github-bot/internal/check/comment.go#L212

Added line #L212 was not covered by tests
gh.Logger.Debugf("Comment manual checks updated successfully")
}

Expand Down
19 changes: 15 additions & 4 deletions contribs/github-bot/internal/check/comment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,21 @@ func TestCommentUpdateHandler(t *testing.T) {
assert.NoError(t, handleCommentUpdate(gh, actionCtx))
actionCtx.Event["action"] = "deleted"

// Exit with error because Event.issue.number is not set.
assert.Error(t, handleCommentUpdate(gh, actionCtx))
actionCtx.Event = setValue(t, actionCtx.Event, float64(42), "issue", "number")

// Exit without error can't get open pull request associated with PR num.
assert.NoError(t, handleCommentUpdate(gh, actionCtx))
mockOptions = append(mockOptions, mock.WithRequestMatchPages(
mock.EndpointPattern{
Pattern: "/repos/pulls/42",
Method: "GET",
},
github.PullRequest{Number: github.Int(42), State: github.String(utils.PRStateOpen)},
))
gh = newGHClient()

// Exit with error because mock not setup to return authUser.
assert.Error(t, handleCommentUpdate(gh, actionCtx))
mockOptions = append(mockOptions, mock.WithRequestMatchPages(
Expand Down Expand Up @@ -152,10 +167,6 @@ func TestCommentUpdateHandler(t *testing.T) {
assert.Error(t, handleCommentUpdate(gh, actionCtx))
actionCtx.Event = setValue(t, actionCtx.Event, "updated_body", "changes", "body", "from")

// Exit with error because Event.issue.number is not set.
assert.Error(t, handleCommentUpdate(gh, actionCtx))
actionCtx.Event = setValue(t, actionCtx.Event, float64(42), "issue", "number")

// Exit with error because checkboxes are differents.
assert.Error(t, handleCommentUpdate(gh, actionCtx))
actionCtx.Event = setValue(t, actionCtx.Event, "current_body", "changes", "body", "from")
Expand Down
13 changes: 12 additions & 1 deletion contribs/github-bot/internal/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"os"

"github.com/gnolang/gno/contribs/github-bot/internal/logger"

"github.com/gnolang/gno/contribs/github-bot/internal/utils"
"github.com/google/go-github/v64/github"
)

Expand Down Expand Up @@ -130,6 +130,17 @@ func (gh *GitHub) SetBotComment(body string, prNum int) (*github.IssueComment, e
return editComment, nil
}

func (gh *GitHub) GetOpenedPullRequest(prNum int) (*github.PullRequest, error) {
pr, _, err := gh.Client.PullRequests.Get(gh.Ctx, gh.Owner, gh.Repo, prNum)
if err != nil {
return nil, fmt.Errorf("unable to retrieve specified pull request (%d): %w", prNum, err)
} else if pr.GetState() != utils.PRStateOpen {
return nil, fmt.Errorf("pull request %d is not opened, actual state: %s", prNum, pr.GetState())
}

return pr, nil
}

// ListTeamMembers lists the members of the specified team.
func (gh *GitHub) ListTeamMembers(team string) ([]*github.User, error) {
var (
Expand Down
22 changes: 11 additions & 11 deletions contribs/github-bot/internal/matrix/matrix.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,16 +109,6 @@ func getPRListFromEvent(gh *client.GitHub, actionCtx *githubactions.GitHubContex
if err := prList.UnmarshalText([]byte(input)); err != nil {
return nil, fmt.Errorf("invalid PR list provided as input: %w", err)
}

// Then check if all provided PR are opened.
for _, prNum := range prList {
pr, _, err := gh.Client.PullRequests.Get(gh.Ctx, gh.Owner, gh.Repo, prNum)
if err != nil {
return nil, fmt.Errorf("unable to retrieve specified pull request (%d): %w", prNum, err)
} else if pr.GetState() != utils.PRStateOpen {
return nil, fmt.Errorf("pull request %d is not opened, actual state: %s", prNum, pr.GetState())
}
}
}

// Event triggered by an issue / PR comment being created / edited / deleted
Expand All @@ -135,5 +125,15 @@ func getPRListFromEvent(gh *client.GitHub, actionCtx *githubactions.GitHubContex
return nil, fmt.Errorf("unsupported event type: %s", actionCtx.EventName)
}

return prList, nil
// Then only keep provided PR that are opened.
var openedPRList utils.PRList = nil
for _, prNum := range prList {
if _, err := gh.GetOpenedPullRequest(prNum); err != nil {
gh.Logger.Warningf("Can't get PR from event: %v", err)
} else {
openedPRList = append(openedPRList, prNum)
}
}

return openedPRList, nil
}
10 changes: 5 additions & 5 deletions contribs/github-bot/internal/matrix/matrix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func TestProcessEvent(t *testing.T) {
Event: map[string]any{"inputs": map[string]any{"pull-request-list": "all"}},
},
nil,
utils.PRList{},
utils.PRList(nil),
false,
}, {
"valid workflow_dispatch list",
Expand All @@ -124,8 +124,8 @@ func TestProcessEvent(t *testing.T) {
Event: map[string]any{"inputs": map[string]any{"pull-request-list": "1,2,3,4"}},
},
prs,
utils.PRList(nil),
true,
utils.PRList{1, 2, 3},
false,
}, {
"invalid workflow_dispatch list (1 doesn't exist)",
&githubactions.GitHubContext{
Expand All @@ -134,7 +134,7 @@ func TestProcessEvent(t *testing.T) {
},
prs,
utils.PRList(nil),
true,
false,
}, {
"invalid workflow_dispatch list (all closed)",
&githubactions.GitHubContext{
Expand All @@ -143,7 +143,7 @@ func TestProcessEvent(t *testing.T) {
},
prs,
utils.PRList(nil),
true,
false,
}, {
"invalid workflow_dispatch list (empty)",
&githubactions.GitHubContext{
Expand Down

0 comments on commit 241a755

Please sign in to comment.