Adding Simulation API

policy/common/actor.go

@@ -26,17 +26,17 @@ import (
 // team and organization memberships. The set of allowed actors is the union of
 // all conditions in this structure.
 type Actors struct {
-	Users         []string `yaml:"users"`
-	Teams         []string `yaml:"teams"`
-	Organizations []string `yaml:"organizations"`
+	Users         []string `yaml:"users" json:"users"`
+	Teams         []string `yaml:"teams" json:"teams"`
+	Organizations []string `yaml:"organizations" json:"organizations"`
 
 	// Deprecated: use Permissions with "admin" or "write"
 	Admins             bool `yaml:"admins"`
 	WriteCollaborators bool `yaml:"write_collaborators"`
 
 	// A list of GitHub collaborator permissions that are allowed. Values may
 	// be any of "admin", "maintain", "write", "triage", and "read".
-	Permissions []pull.Permission
+	Permissions []pull.Permission `json:"permissions"`
 }
 
 // IsEmpty returns true if no conditions for actors are defined.

policy/simulated/context.go

@@ -52,13 +52,8 @@ func (c *Context) filterIgnoredComments(prCtx pull.Context, comments []*pull.Com
 	}
 
 	var filteredComments []*pull.Comment
-	actors, err := c.options.IgnoreComments.toCommonActors()
-	if err != nil {
-		return nil, err
-	}
-
 	for _, comment := range comments {
-		isActor, err := actors.IsActor(c.ctx, prCtx, comment.Author)
+		isActor, err := c.options.IgnoreComments.IsActor(c.ctx, prCtx, comment.Author)
 		if err != nil {
 			return nil, err
 		}
@@ -76,7 +71,7 @@ func (c *Context) filterIgnoredComments(prCtx pull.Context, comments []*pull.Com
 func (c *Context) addApprovalComment(comments []*pull.Comment) []*pull.Comment {
 	var commentsToAdd []*pull.Comment
 	for _, comment := range c.options.AddComments {
-		commentsToAdd = append(commentsToAdd, comment.toPullComment())
+		commentsToAdd = append(commentsToAdd, &comment)
 	}
 
 	return append(comments, commentsToAdd...)
@@ -103,13 +98,8 @@ func (c *Context) filterIgnoredReviews(prCtx pull.Context, reviews []*pull.Revie
 	}
 
 	var filteredReviews []*pull.Review
-	actors, err := c.options.IgnoreReviews.toCommonActors()
-	if err != nil {
-		return nil, err
-	}
-
 	for _, review := range reviews {
-		isActor, err := actors.IsActor(c.ctx, prCtx, review.Author)
+		isActor, err := c.options.IgnoreReviews.IsActor(c.ctx, prCtx, review.Author)
 		if err != nil {
 			return nil, err
 		}
@@ -127,10 +117,9 @@ func (c *Context) filterIgnoredReviews(prCtx pull.Context, reviews []*pull.Revie
 func (c *Context) addApprovalReview(reviews []*pull.Review) []*pull.Review {
 	var reviewsToAdd []*pull.Review
 	for i, review := range c.options.AddReviews {
-		reviewID := fmt.Sprintf("simulated-reviewID-%d", i)
-		reviewSHA := fmt.Sprintf("simulated-reviewSHA-%d", i)
-
-		reviewsToAdd = append(reviewsToAdd, review.toPullReview(reviewID, reviewSHA))
+		review.ID = fmt.Sprintf("simulated-reviewID-%d", i)
+		review.SHA = fmt.Sprintf("simulated-reviewSHA-%d", i)
+		reviewsToAdd = append(reviewsToAdd, &review)
 	}
 
 	return append(reviews, reviewsToAdd...)

policy/simulated/context_test.go

@@ -18,6 +18,7 @@ import (
 	"sort"
 	"testing"
 
+	"github.com/palantir/policy-bot/policy/common"
 	"github.com/palantir/policy-bot/pull"
 	"github.com/palantir/policy-bot/pull/pulltest"
 	"github.com/stretchr/testify/assert"
@@ -41,7 +42,7 @@ func TestComments(t *testing.T) {
 			},
 			ExpectedCommentAuthors: []string{"rrandom"},
 			Options: Options{
-				IgnoreComments: &Actors{
+				IgnoreComments: &common.Actors{
 					Users: []string{"iignore"},
 				},
 			},
@@ -52,7 +53,7 @@ func TestComments(t *testing.T) {
 				{Author: "iignore"},
 			},
 			Options: Options{
-				IgnoreComments: &Actors{
+				IgnoreComments: &common.Actors{
 					Teams: []string{"test-team-1"},
 				},
 			},
@@ -67,7 +68,7 @@ func TestComments(t *testing.T) {
 				{Author: "iignore"},
 			},
 			Options: Options{
-				IgnoreComments: &Actors{
+				IgnoreComments: &common.Actors{
 					Organizations: []string{"test-org-1"},
 				},
 			},
@@ -82,8 +83,8 @@ func TestComments(t *testing.T) {
 				{Author: "iignore"},
 			},
 			Options: Options{
-				IgnoreComments: &Actors{
-					Permissions: []string{"read"},
+				IgnoreComments: &common.Actors{
+					Permissions: []pull.Permission{pull.PermissionRead},
 				},
 			},
 			Collaborators: []*pull.Collaborator{
@@ -93,23 +94,6 @@ func TestComments(t *testing.T) {
 			},
 			ExpectedCommentAuthors: []string{"rrandom"},
 		},
-		"return error when invalid permission used": {
-			Comments: []*pull.Comment{
-				{Author: "rrandom"},
-				{Author: "iignore"},
-			},
-			Options: Options{
-				IgnoreComments: &Actors{
-					Permissions: []string{"bread"},
-				},
-			},
-			Collaborators: []*pull.Collaborator{
-				{Name: "iignore", Permissions: []pull.CollaboratorPermission{
-					{Permission: pull.PermissionRead},
-				}},
-			},
-			ExpectedError: true,
-		},
 		"do not ignore any comments": {
 			Comments: []*pull.Comment{
 				{Author: "rrandom"},
@@ -123,7 +107,7 @@ func TestComments(t *testing.T) {
 				{Author: "iignore"},
 			},
 			Options: Options{
-				AddComments: []Comment{
+				AddComments: []pull.Comment{
 					{Author: "sperson", Body: ":+1:"},
 				},
 			},
@@ -135,10 +119,10 @@ func TestComments(t *testing.T) {
 				{Author: "iignore"},
 			},
 			Options: Options{
-				AddComments: []Comment{
+				AddComments: []pull.Comment{
 					{Author: "sperson", Body: ":+1:"},
 				},
-				IgnoreComments: &Actors{
+				IgnoreComments: &common.Actors{
 					Users: []string{"iignore"},
 				},
 			},
@@ -197,7 +181,7 @@ func TestReviews(t *testing.T) {
 			},
 			ExpectedReviewAuthors: []string{"rrandom"},
 			Options: Options{
-				IgnoreReviews: &Actors{
+				IgnoreReviews: &common.Actors{
 					Users: []string{"iignore"},
 				},
 			},
@@ -208,7 +192,7 @@ func TestReviews(t *testing.T) {
 				{Author: "iignore"},
 			},
 			Options: Options{
-				IgnoreReviews: &Actors{
+				IgnoreReviews: &common.Actors{
 					Teams: []string{"test-team-1"},
 				},
 			},
@@ -223,7 +207,7 @@ func TestReviews(t *testing.T) {
 				{Author: "iignore"},
 			},
 			Options: Options{
-				IgnoreReviews: &Actors{
+				IgnoreReviews: &common.Actors{
 					Organizations: []string{"test-org-1"},
 				},
 			},
@@ -238,8 +222,8 @@ func TestReviews(t *testing.T) {
 				{Author: "iignore"},
 			},
 			Options: Options{
-				IgnoreReviews: &Actors{
-					Permissions: []string{"read"},
+				IgnoreReviews: &common.Actors{
+					Permissions: []pull.Permission{pull.PermissionRead},
 				},
 			},
 			Collaborators: []*pull.Collaborator{
@@ -249,23 +233,6 @@ func TestReviews(t *testing.T) {
 			},
 			ExpectedReviewAuthors: []string{"rrandom"},
 		},
-		"return error when invalid permission used": {
-			Reviews: []*pull.Review{
-				{Author: "rrandom"},
-				{Author: "iignore"},
-			},
-			Options: Options{
-				IgnoreReviews: &Actors{
-					Permissions: []string{"bread"},
-				},
-			},
-			Collaborators: []*pull.Collaborator{
-				{Name: "iignore", Permissions: []pull.CollaboratorPermission{
-					{Permission: pull.PermissionRead},
-				}},
-			},
-			ExpectedError: true,
-		},
 		"do not ignore any reviews": {
 			Reviews: []*pull.Review{
 				{Author: "rrandom"},
@@ -279,7 +246,7 @@ func TestReviews(t *testing.T) {
 				{Author: "iignore"},
 			},
 			Options: Options{
-				AddReviews: []Review{
+				AddReviews: []pull.Review{
 					{Author: "sperson", State: "approved"},
 				},
 			},
@@ -291,10 +258,10 @@ func TestReviews(t *testing.T) {
 				{Author: "iignore"},
 			},
 			Options: Options{
-				AddReviews: []Review{
+				AddReviews: []pull.Review{
 					{Author: "sperson", State: "approved"},
 				},
-				IgnoreReviews: &Actors{
+				IgnoreReviews: &common.Actors{
 					Users: []string{"iignore"},
 				},
 			},

policy/simulated/options.go

@@ -26,11 +26,11 @@ import (
 
 // Options should contain optional data that can be used to modify the results of the methods on the simulated Context.
 type Options struct {
-	IgnoreComments *Actors   `json:"ignore_comments"`
-	IgnoreReviews  *Actors   `json:"ignore_reviews"`
-	AddComments    []Comment `json:"add_comments"`
-	AddReviews     []Review  `json:"add_reviews"`
-	BaseBranch     string    `json:"base_branch"`
+	IgnoreComments *common.Actors `json:"ignore_comments"`
+	IgnoreReviews  *common.Actors `json:"ignore_reviews"`
+	AddComments    []pull.Comment `json:"add_comments"`
+	AddReviews     []pull.Review  `json:"add_reviews"`
+	BaseBranch     string         `json:"base_branch"`
 }
 
 func NewOptionsFromRequest(r *http.Request) (Options, error) {
@@ -50,68 +50,29 @@ func NewOptionsFromRequest(r *http.Request) (Options, error) {
 // setDefaults sets any values for the options that were not intentionally set in the request body but which should have
 // consistent values for the length of the simulation, such as the created time for a comment or review.
 func (o *Options) setDefaults() {
+	now := time.Now()
 	for i, review := range o.AddReviews {
-		review.setDefaults()
-		o.AddReviews[i] = review
-	}
-
-	for i, comment := range o.AddComments {
-		comment.setDefaults()
-		o.AddComments[i] = comment
-	}
-}
-
-type Actors struct {
-	Users         []string `json:"users"`
-	Teams         []string `json:"teams"`
-	Organizations []string `json:"organizations"`
-	Permissions   []string `json:"permissions"`
-}
+		if review.CreatedAt.IsZero() {
+			review.CreatedAt = now
+		}
 
-func (a *Actors) toCommonActors() (common.Actors, error) {
-	var permissions []pull.Permission
-	for _, p := range a.Permissions {
-		permission, err := pull.ParsePermission(p)
-		if err != nil {
-			return common.Actors{}, err
+		if review.LastEditedAt.IsZero() {
+			review.LastEditedAt = now
 		}
 
-		permissions = append(permissions, permission)
+		o.AddReviews[i] = review
 	}
 
-	return common.Actors{
-		Users:         a.Users,
-		Teams:         a.Teams,
-		Organizations: a.Organizations,
-		Permissions:   permissions,
-	}, nil
-}
-
-type Comment struct {
-	CreatedAt    *time.Time `json:"created_at"`
-	LastEditedAt *time.Time `json:"last_edited_at"`
-	Author       string     `json:"author"`
-	Body         string     `json:"body"`
-}
-
-// setDefaults sets the createdAt and lastEdtedAt values to time.Now() if they are otherwise unset
-func (c *Comment) setDefaults() {
-	now := time.Now()
-	if c.CreatedAt == nil {
-		c.CreatedAt = &now
-	}
+	for i, comment := range o.AddComments {
+		if comment.CreatedAt.IsZero() {
+			comment.CreatedAt = now
+		}
 
-	if c.LastEditedAt == nil {
-		c.LastEditedAt = &now
-	}
-}
+		if comment.LastEditedAt.IsZero() {
+			comment.LastEditedAt = now
+		}
 
-func (c *Comment) toPullComment() *pull.Comment {
-	return &pull.Comment{
-		CreatedAt:    *c.CreatedAt,
-		LastEditedAt: *c.LastEditedAt,
-		Author:       c.Author,
-		Body:         c.Body,
+		o.AddComments[i] = comment
 	}
 }
 

policy/simulated/options_test.go

@@ -31,13 +31,13 @@ func TestOptionsFromRequest(t *testing.T) {
 			"users":["iignore"],
 			"teams":[""],
 			"organizations":[""],
-			"permissions":[""]
+			"permissions":["read"]
 		},
 		"ignore_reviews":{
 			"users":["iignore"],
 			"teams":[""],
 			"organizations":[""],
-			"permissions":[""]
+			"permissions":["read"]
 		},
 		"add_comments":[
 			{"author":"iignore", "body":":+1:"}
@@ -55,25 +55,27 @@ func TestOptionsFromRequest(t *testing.T) {
 	require.NoError(t, err)
 
 	assert.Equal(t, []string{"iignore"}, opt.IgnoreComments.Users)
+	assert.Equal(t, pull.PermissionRead, opt.IgnoreComments.Permissions[0])
 	assert.Equal(t, []string{"iignore"}, opt.IgnoreReviews.Users)
+	assert.Equal(t, pull.PermissionRead, opt.IgnoreReviews.Permissions[0])
 
 	assert.Equal(t, "iignore", opt.AddComments[0].Author)
 	assert.Equal(t, ":+1:", opt.AddComments[0].Body)
 
 	assert.Equal(t, "iignore", opt.AddReviews[0].Author)
 	assert.Equal(t, ":+1:", opt.AddReviews[0].Body)
 
-	assert.Equal(t, pull.ReviewApproved, opt.AddReviews[0].toPullReview("test-id", "test-sha").State)
+	assert.Equal(t, pull.ReviewApproved, opt.AddReviews[0].State)
 	assert.Equal(t, "test-base", opt.BaseBranch)
 }
 
 func TestOptionDefaults(t *testing.T) {
 	options := Options{
-		AddComments: []Comment{
+		AddComments: []pull.Comment{
 			{Author: "aperson", Body: ":+1:"},
 			{Author: "otherperson", Body: ":+1:"},
 		},
-		AddReviews: []Review{
+		AddReviews: []pull.Review{
 			{Author: "aperson", Body: ":+1:"},
 			{Author: "otherperson", Body: ":+1:"},
 		},