use github api for automerge approvals (#11)

bot/actions/automerge/automerge.go

@@ -18,6 +18,11 @@ type MergeableEventData interface {
 	Merge(mergeMethod string)
 }
 
+type HasReviewersAPIEventData interface {
+	GetReviewers() map[string]string
+	GetApprovals() []string
+}
+
 func (a *action) merge(meta bot.EventData) {
 	if a.rule.Label == "" {
 		mergeable, ok := meta.(MergeableEventData)
@@ -32,12 +37,28 @@ func (a *action) merge(meta bot.EventData) {
 	}
 }
 
-func (a *action) Apply(config bot.Configuration, meta bot.EventData) {
+func (a *action) getApprovalsFromAPI(meta bot.EventData) (int, bool) {
 	assigneesList := meta.GetAssignees()
-	if len(assigneesList) == 0 {
-		util.Logger.Debug("No assignees to issue - skipping")
-		return
+	approvals := 0
+	assignees := util.StringSet{}
+	assignees.AddAll(assigneesList)
+	reviewApi, ok := meta.(HasReviewersAPIEventData)
+	if !ok {
+		util.Logger.Warning("Event data does not support reviewers API. Check your configuration")
+		a.err = fmt.Errorf("Event data does not support reviewers API")
+	} else {
+		for _, approver := range reviewApi.GetApprovals() {
+			if assignees.Contains(approver) {
+				assignees.Remove(approver)
+				approvals++
+			}
+		}
 	}
+	return approvals, assignees.Len() == 0
+}
+
+func (a *action) getApprovalsFromComments(meta bot.EventData) (int, bool) {
+	assigneesList := meta.GetAssignees()
 	approvals := 0
 	assignees := util.StringSet{}
 	assignees.AddAll(assigneesList)
@@ -51,12 +72,30 @@ func (a *action) Apply(config bot.Configuration, meta bot.EventData) {
 			approvals++
 		}
 	}
-	if a.rule.Require == 0 && assignees.Len() == 0 {
-		util.Logger.Debug("All assignees have approved the PR - merging")
-		a.merge(meta)
-	} else if a.rule.Require > 0 && approvals >= a.rule.Require {
-		util.Logger.Debug("Got %d required approvals for PR - merging", a.rule.Require)
-		a.merge(meta)
+	return approvals, assignees.Len() == 0
+}
+
+func (a *action) Apply(config bot.Configuration, meta bot.EventData) {
+	assigneesList := meta.GetAssignees()
+	if len(assigneesList) == 0 {
+		util.Logger.Debug("No assignees to issue - skipping")
+		return
+	}
+	calls := []func(bot.EventData) (int, bool){
+		a.getApprovalsFromAPI,
+		a.getApprovalsFromComments,
+	}
+	for _, call := range calls {
+		approvals, all := call(meta)
+		if a.rule.Require == 0 && all {
+			util.Logger.Debug("All assignees have approved the PR - merging")
+			a.merge(meta)
+			return
+		} else if a.rule.Require > 0 && approvals >= a.rule.Require {
+			util.Logger.Debug("Got %d required approvals for PR - merging", a.rule.Require)
+			a.merge(meta)
+			return
+		}
 	}
 }
 

bot/actions/automerge/automerge.md

@@ -8,6 +8,12 @@ Assignees must use one of the approval phrases (_Approve_, _Approved_, _LGTM_, _
 
 **Note** If others comment with one of the approval phrases, it will not count as approval
 
+### GitHub Users
+
+Since GitHub API support for pull request reviews API - the lookup will first search the API and only then for comments.
+
+**Note** Approvals can only be read by API or comments. Mixing both approvals from API and comments is not supported.
+
 ## Requirements
 
 None

bot/actions/automerge/automerge_test.go

@@ -1,23 +1,66 @@
 package automerge
 
 import (
+	"testing"
+
 	"github.com/bivas/rivi/bot"
 	"github.com/bivas/rivi/bot/mock"
 	"github.com/stretchr/testify/assert"
-	"testing"
 )
 
 type mockMergableEventData struct {
 	mock.MockEventData
-	merged bool
-	method string
+	merged    bool
+	method    string
+	reviewers map[string]string
+	approvals []string
+}
+
+func (m *mockMergableEventData) GetReviewers() map[string]string {
+	return m.reviewers
+}
+
+func (m *mockMergableEventData) GetApprovals() []string {
+	return m.approvals
 }
 
 func (m *mockMergableEventData) Merge(mergeMethod string) {
 	m.merged = true
 	m.method = mergeMethod
 }
 
+func TestNoReviewersAPI(t *testing.T) {
+	action := action{rule: &rule{}}
+	action.rule.Defaults()
+	config := &mock.MockConfiguration{}
+	meta := &mock.MockEventData{Assignees: []string{"user1"}, Comments: []bot.Comment{
+		{Commenter: "user1", Comment: "approved"},
+	}}
+	action.Apply(config, meta)
+	assert.NotNil(t, action.err, "should be error on api")
+}
+
+func TestMergeWithAPI(t *testing.T) {
+	action := action{rule: &rule{}}
+	action.rule.Defaults()
+	config := &mock.MockConfiguration{}
+	mockEventData := mock.MockEventData{Assignees: []string{"user1"}}
+	meta := &mockMergableEventData{MockEventData: mockEventData, approvals: []string{"user1"}}
+	action.Apply(config, meta)
+	assert.True(t, meta.merged, "should be merged")
+	assert.Equal(t, "merge", meta.method, "default should be merge")
+}
+
+func TestMergeWithAPINoApprovals(t *testing.T) {
+	action := action{rule: &rule{}}
+	action.rule.Defaults()
+	config := &mock.MockConfiguration{}
+	mockEventData := mock.MockEventData{Assignees: []string{"user1"}}
+	meta := &mockMergableEventData{MockEventData: mockEventData, approvals: []string{"user2"}}
+	action.Apply(config, meta)
+	assert.False(t, meta.merged, "should be merged")
+}
+
 func TestNotCapableToMerge(t *testing.T) {
 	action := action{rule: &rule{}}
 	action.rule.Defaults()

bot/connector/github/builder.go

@@ -86,6 +86,7 @@ func (builder *eventDataBuilder) readFromClient(context *builderContext) {
 	context.data.fileNames = fileNames
 	stringSet := util.StringSet{Transformer: filepath.Ext}
 	context.data.fileExt = stringSet.AddAll(fileNames).Values()
+	context.data.reviewers = context.client.GetReviewers(id)
 	context.data.locked = context.client.Locked(id)
 }
 

bot/connector/github/client.go

@@ -177,6 +177,21 @@ func (c *ghClient) AddComment(issue int, comment string) bot.Comment {
 	}
 }
 
+func (c *ghClient) GetReviewers(issue int) map[string]string {
+	result := make(map[string]string)
+	reviews, _, err := c.client.PullRequests.ListReviews(context.Background(), c.owner, c.repo, issue, nil)
+	if err != nil {
+		util.Logger.Error("Unable to get reviewers for issue %d. %s", issue, err)
+		return result
+	}
+	for _, review := range reviews {
+		user := strings.ToLower(*review.User.Login)
+		state := strings.ToLower(*review.State)
+		result[user] = state
+	}
+	return result
+}
+
 func (c *ghClient) Merge(issue int, mergeMethod string) {
 	options := &github.PullRequestOptions{MergeMethod: mergeMethod}
 	_, _, err := c.client.PullRequests.Merge(context.Background(), c.owner, c.repo, issue, "", options)

bot/connector/github/event_data.go

@@ -1,6 +1,9 @@
 package github
 
-import "github.com/bivas/rivi/bot"
+import (
+	"github.com/bivas/rivi/bot"
+	"github.com/bivas/rivi/util"
+)
 
 type eventData struct {
 	client       *ghClient
@@ -21,6 +24,21 @@ type eventData struct {
 	assignees    []string
 	comments     []bot.Comment
 	payload      []byte
+	reviewers    map[string]string
+}
+
+func (d *eventData) GetReviewers() map[string]string {
+	return d.reviewers
+}
+
+func (d *eventData) GetApprovals() []string {
+	result := util.StringSet{}
+	for reviewer, state := range d.reviewers {
+		if state == "approve" {
+			result.Add(reviewer)
+		}
+	}
+	return result.Values()
 }
 
 func (d *eventData) Lock() {