chore: validate rules per section

internal/api/approval/approval.go

@@ -75,10 +75,67 @@ func (a *Approval) ComputeApprovalStatus(ctx context.Context, pr *PR) (*Result,
 		return nil, err
 	}
 
-	rules, err := a.computeRulesForTargetBranch(cfg, pr)
+	// Grab the list of teams under the current organisation.
+	teams, err := a.client.GetTeams(ctx, pr.OwnerLogin)
+	if err != nil {
+		return nil, err
+	}
+
+	// Grab the list of all the reviews for the current PR.
+	reviews, err := a.client.GetPullRequestReviews(ctx, pr.OwnerLogin, pr.RepoName, pr.Number)
 	if err != nil {
 		return nil, err
 	}
+
+	results := make([]*Result, 0)
+	for _, prCfg := range cfg.PullRequestApprovalRules {
+		if len(prCfg.TargetBranches) == 0 || indexOf(prCfg.TargetBranches, pr.TargetBranch) >= 0 {
+			result, err := a.computeApprovalStatus(ctx, pr, teams, reviews, prCfg.Rules)
+			if err != nil {
+				return nil, err
+			}
+
+			results = append(results, result)
+		}
+	}
+
+	if len(results) == 0 {
+		return &Result{
+			status:      StatusEventStatusSuccess,
+			description: statusEventDescriptionNoRulesForTargetBranch,
+		}, nil
+	}
+
+	firstFailedResult := firstFailedResult(results)
+	if firstFailedResult != nil {
+		ignoredReviewers := results[0].IgnoredReviewers()
+		invalidReviewers := results[0].InvalidReviewers()
+		for _, result := range results[1:] {
+			ignoredReviewers = intersection(ignoredReviewers, result.IgnoredReviewers())
+			invalidReviewers = intersection(invalidReviewers, result.InvalidReviewers())
+		}
+
+		if err := a.client.ReportIgnoredReviews(ctx, pr.OwnerLogin, pr.RepoName, pr.Number, ignoredReviewers); err != nil {
+			return nil, err
+		}
+		if err := a.client.ReportInvalidReviews(ctx, pr.OwnerLogin, pr.RepoName, pr.Number, invalidReviewers); err != nil {
+			return nil, err
+		}
+	}
+
+	return firstFailedResult, nil
+}
+
+func firstFailedResult(results []*Result) *Result {
+	for _, result := range results {
+		if result.Status() != StatusEventStatusSuccess {
+			return result
+		}
+	}
+	return nil
+}
+
+func (a *Approval) computeApprovalStatus(ctx context.Context, pr *PR, teams []*github.Team, reviews []*github.PullRequestReview, rules []configuration.Rule) (*Result, error) {
 	if len(rules) > 0 {
 		a.log.Tracef("A total of %d rules apply to target branch %q", len(rules), pr.TargetBranch)
 	} else {
@@ -91,18 +148,6 @@ func (a *Approval) ComputeApprovalStatus(ctx context.Context, pr *PR) (*Result,
 		return status, nil
 	}
 
-	// Grab the list of teams under the current organisation.
-	teams, err := a.client.GetTeams(ctx, pr.OwnerLogin)
-	if err != nil {
-		return nil, err
-	}
-
-	// Grab the list of all the reviews for the current PR.
-	reviews, err := a.client.GetPullRequestReviews(ctx, pr.OwnerLogin, pr.RepoName, pr.Number)
-	if err != nil {
-		return nil, err
-	}
-
 	state := newState()
 	state.setApprovingReviewers(reviews)
 
@@ -168,21 +213,7 @@ func (a *Approval) ComputeApprovalStatus(ctx context.Context, pr *PR) (*Result,
 
 	state.updateInvalidReviewers(allAllowedMembers)
 
-	result := state.result(a.log, teams) // state should not be consumed past this point
-
-	if result.pendingReviewsWaiting() {
-		if err := a.client.ReportIgnoredReviews(
-			ctx, pr.OwnerLogin, pr.RepoName, pr.Number, result.IgnoredReviewers()); err != nil {
-			return nil, err
-		}
-
-		if err := a.client.ReportInvalidReviews(
-			ctx, pr.OwnerLogin, pr.RepoName, pr.Number, result.InvalidReviewers()); err != nil {
-			return nil, err
-		}
-	}
-
-	return result, nil
+	return state.result(a.log, teams), nil // state should not be consumed past this point
 }
 
 func addMembers(allowed map[string]bool, members []*github.User) {

internal/api/approval/string_slices.go

@@ -39,3 +39,18 @@ func indexOf(s []string, v string) int {
 	}
 	return -1
 }
+
+func intersection(a, b []string) []string {
+	m := map[string]bool{}
+	for _, e := range a {
+		m[e] = true
+	}
+	result := make([]string, 0, len(a))
+	for _, e := range b {
+		if _, ok := m[e]; ok {
+			result = append(result, e)
+			delete(m, e) // avoid duplicates
+		}
+	}
+	return result
+}

internal/api/approval/string_slices_test.go

@@ -153,3 +153,54 @@ func Test_uniqueAppend(t *testing.T) {
 			})
 	}
 }
+
+func Test_intersection(t *testing.T) {
+	tests := map[string]struct {
+		a    []string
+		b    []string
+		want []string
+	}{
+		"intersection of nil and empty slice": {
+			nil,
+			[]string{},
+			[]string{},
+		},
+		"intersection of empty slice and nil": {
+			[]string{},
+			nil,
+			[]string{},
+		},
+		"intersection of empty slices": {
+			[]string{},
+			[]string{},
+			[]string{},
+		},
+		"intersection of empty slice and empty slice": {
+			[]string{},
+			[]string{},
+			[]string{},
+		},
+		"intersection of slices with no common elements": {
+			[]string{"foo", "bar", "baz"},
+			[]string{"qux", "quux", "corge"},
+			[]string{},
+		},
+		"intersection of slices with common elements": {
+			[]string{"foo", "bar", "baz"},
+			[]string{"foo", "bar", "qux"},
+			[]string{"foo", "bar"},
+		},
+		"intersection of slices with duplicates": {
+			[]string{"foo", "foo", "bar", "baz"},
+			[]string{"foo", "bar", "bar", "qux"},
+			[]string{"foo", "bar"},
+		},
+	}
+	for name, tt := range tests {
+		t.Run(name,
+			func(t *testing.T) {
+				require.Equal(t, intersection(tt.a, tt.b), tt.want)
+			},
+		)
+	}
+}