Skip to content

Commit

Permalink
Fix login strings for bot accounts (#44)
Browse files Browse the repository at this point in the history
In the V3 API, the login strings for bot accounts had a "[bot]" suffix,
which may be used in policy definitions. In the V4 API, bot accounts
have their own type, so the login string does not include this suffix.
Add the suffix in our code to maintain compatibility with existing
policy definitions.
  • Loading branch information
bluekeyes authored Dec 18, 2018
1 parent 2ab6233 commit 19530c2
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 8 deletions.
22 changes: 16 additions & 6 deletions pull/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ func (ghc *GitHubContext) loadTimeline() error {
state := ReviewState(strings.ToLower(event.PullRequestReview.State))
ghc.reviews = append(ghc.reviews, &Review{
CreatedAt: event.CreatedAt(),
Author: event.PullRequestReview.Author.Login,
Author: event.PullRequestReview.Author.GetV3Login(),
State: state,
Body: event.PullRequestReview.Body,
ID: event.PullRequestReview.ID,
Expand All @@ -304,7 +304,7 @@ func (ghc *GitHubContext) loadTimeline() error {
case "IssueComment":
ghc.comments = append(ghc.comments, &Comment{
CreatedAt: event.CreatedAt(),
Author: event.IssueComment.Author.Login,
Author: event.IssueComment.Author.GetV3Login(),
Body: event.IssueComment.Body,
})
}
Expand Down Expand Up @@ -385,22 +385,32 @@ func (c *v4Commit) ToCommit() *Commit {
SHA: c.OID,
Parents: parents,
CommittedViaWeb: c.CommittedViaWeb,
Author: c.Author.GetLogin(),
Committer: c.Committer.GetLogin(),
Author: c.Author.GetV3Login(),
Committer: c.Committer.GetV3Login(),
}
}

type v4Actor struct {
Type string `graphql:"__typename"`
Login string
}

// GetV3Login returns a V3-compatible login string. These login strings contain
// the "[bot]" suffix for GitHub identities.
func (a v4Actor) GetV3Login() string {
if a.Type == "Bot" {
return a.Login + "[bot]"
}
return a.Login
}

type v4GitActor struct {
User *v4Actor
}

func (ga v4GitActor) GetLogin() string {
func (ga v4GitActor) GetV3Login() string {
if ga.User != nil {
return ga.User.Login
return ga.User.GetV3Login()
}
return ""
}
Expand Down
8 changes: 6 additions & 2 deletions pull/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ func TestComments(t *testing.T) {
comments, err := ctx.Comments()
require.NoError(t, err)

require.Len(t, comments, 1, "incorrect number of comments")
require.Len(t, comments, 2, "incorrect number of comments")
assert.Equal(t, 1, timelineRule.Count, "no http request was made")

expectedTime, err := time.Parse(time.RFC3339, "2018-06-27T20:28:22Z")
Expand All @@ -175,11 +175,15 @@ func TestComments(t *testing.T) {
assert.Equal(t, expectedTime, comments[0].CreatedAt)
assert.Equal(t, ":+1:", comments[0].Body)

assert.Equal(t, "bulldozer[bot]", comments[1].Author)
assert.Equal(t, expectedTime.Add(time.Minute), comments[1].CreatedAt)
assert.Equal(t, "I merge!", comments[1].Body)

// verify that the commit list is cached
comments, err = ctx.Comments()
require.NoError(t, err)

require.Len(t, comments, 1, "incorrect number of comments")
require.Len(t, comments, 2, "incorrect number of comments")
assert.Equal(t, 1, timelineRule.Count, "cached comments were not used")
}

Expand Down
10 changes: 10 additions & 0 deletions pull/testdata/responses/timeline_comments.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,20 @@
{
"__typename": "IssueComment",
"author": {
"__typename": "User",
"login": "bkeyes"
},
"body": ":+1:",
"createdAt": "2018-06-27T20:28:22Z"
},
{
"__typename": "IssueComment",
"author": {
"__typename": "Bot",
"login": "bulldozer"
},
"body": "I merge!",
"createdAt": "2018-06-27T20:29:22Z"
}
]
}
Expand Down

0 comments on commit 19530c2

Please sign in to comment.