dont evaluate unless we need to

server/handler/issue_comment.go

@@ -92,6 +92,11 @@ func (h *IssueComment) Handle(ctx context.Context, eventType, deliveryID string,
 		return nil
 	}
 
+	if !h.affectsApproval(event, evalCtx.Config.Config.ApprovalRules) {
+		logger.Debug().Msg("Skipping evaluation because this comment does not impact approval")
+		return nil
+	}
+
 	result, err := evalCtx.EvaluatePolicy(ctx, evaluator)
 	if err != nil {
 		return err
@@ -104,15 +109,7 @@ func (h *IssueComment) Handle(ctx context.Context, eventType, deliveryID string,
 func (h *IssueComment) detectAndLogTampering(ctx context.Context, evalCtx *EvalContext, event github.IssueCommentEvent) bool {
 	logger := zerolog.Ctx(ctx)
 
-	var originalBody string
-	switch event.GetAction() {
-	case "edited":
-		originalBody = event.GetChanges().GetBody().GetFrom()
-
-	case "deleted":
-		originalBody = event.GetComment().GetBody()
-
-	default:
+	if event.GetAction() == "created" {
 		return false
 	}
 
@@ -122,7 +119,7 @@ func (h *IssueComment) detectAndLogTampering(ctx context.Context, evalCtx *EvalC
 		return false
 	}
 
-	if h.affectsApproval(originalBody, evalCtx.Config.Config.ApprovalRules) {
+	if h.affectsApproval(event, evalCtx.Config.Config.ApprovalRules) {
 		msg := fmt.Sprintf("Entity %s edited approval comment by %s", eventAuthor, commentAuthor)
 		logger.Warn().Str(LogKeyAudit, "issue_comment").Msg(msg)
 
@@ -134,9 +131,20 @@ func (h *IssueComment) detectAndLogTampering(ctx context.Context, evalCtx *EvalC
 	return true
 }
 
-func (h *IssueComment) affectsApproval(actualComment string, rules []*approval.Rule) bool {
+func (h *IssueComment) affectsApproval(event github.IssueCommentEvent, rules []*approval.Rule) bool {
+	var body, originalBody string
+	switch event.GetAction() {
+	case "edited":
+		body = event.GetComment().GetBody()
+		originalBody = event.GetChanges().GetBody().GetFrom()
+	default:
+		body = event.GetComment().GetBody()
+		originalBody = body
+	}
+
 	for _, rule := range rules {
-		if rule.Options.GetMethods().CommentMatches(actualComment) {
+		methods := rule.Options.GetMethods()
+		if methods.CommentMatches(body) || (body != originalBody && methods.CommentMatches(originalBody)) {
 			return true
 		}
 	}

server/handler/pull_request_review.go

@@ -20,6 +20,7 @@ import (
 
 	"github.com/google/go-github/v47/github"
 	"github.com/palantir/go-githubapp/githubapp"
+	"github.com/palantir/policy-bot/policy/approval"
 	"github.com/palantir/policy-bot/policy/common"
 	"github.com/palantir/policy-bot/pull"
 	"github.com/pkg/errors"
@@ -44,13 +45,52 @@ func (h *PullRequestReview) Handle(ctx context.Context, eventType, deliveryID st
 		return nil
 	}
 
+	pr := event.GetPullRequest()
+	repo := event.GetRepo()
+	owner := repo.GetOwner().GetLogin()
+	number := event.GetPullRequest().GetNumber()
 	installationID := githubapp.GetInstallationIDFromEvent(&event)
-	ctx, _ = h.PreparePRContext(ctx, installationID, event.GetPullRequest())
 
-	return h.Evaluate(ctx, installationID, common.TriggerReview, pull.Locator{
-		Owner:  event.GetRepo().GetOwner().GetLogin(),
-		Repo:   event.GetRepo().GetName(),
-		Number: event.GetPullRequest().GetNumber(),
-		Value:  event.GetPullRequest(),
+	ctx, logger := h.PreparePRContext(ctx, installationID, pr)
+
+	evalCtx, err := h.NewEvalContext(ctx, installationID, pull.Locator{
+		Owner:  owner,
+		Repo:   repo.GetName(),
+		Number: number,
+		Value:  pr,
 	})
+	if err != nil {
+		return err
+	}
+
+	evaluator, err := evalCtx.ParseConfig(ctx, common.TriggerComment)
+	if err != nil {
+		return err
+	}
+	if evaluator == nil {
+		return nil
+	}
+
+	reviewState := pull.ReviewState(event.GetReview().GetState())
+	if !h.affectsApproval(reviewState, evalCtx.Config.Config.ApprovalRules) {
+		logger.Debug().Msg("Skipping evaluation because this review does not impact approval")
+		return nil
+	}
+
+	result, err := evalCtx.EvaluatePolicy(ctx, evaluator)
+	if err != nil {
+		return err
+	}
+
+	evalCtx.RunPostEvaluateActions(ctx, result, common.TriggerComment)
+	return nil
+}
+
+func (h *PullRequestReview) affectsApproval(reviewState pull.ReviewState, rules []*approval.Rule) bool {
+	for _, rule := range rules {
+		if reviewState == rule.Options.GetMethods().GithubReviewState {
+			return true
+		}
+	}
+	return false
 }