feat(github-bot): avoid triage-pending if there is any tech-staff review

contribs/github-bot/internal/config/config.go

@@ -4,6 +4,7 @@
 	"github.com/gnolang/gno/contribs/github-bot/internal/client"
 	c "github.com/gnolang/gno/contribs/github-bot/internal/conditions"
 	r "github.com/gnolang/gno/contribs/github-bot/internal/requirements"
+	"github.com/gnolang/gno/contribs/github-bot/internal/utils"
 )
 
 type Teams []string
@@ -41,11 +42,11 @@
 			Then: r.And(
 				r.Or(
 					r.AuthorInTeam(gh, "tech-staff"),
-					r.ReviewByTeamMembers(gh, "tech-staff", 1),
+					r.ReviewByTeamMembers(gh, "tech-staff").WithDesiredState(utils.ReviewStateApproved),
 				),
 				r.Or(
 					r.AuthorInTeam(gh, "devrels"),
-					r.ReviewByTeamMembers(gh, "devrels", 1),
+					r.ReviewByTeamMembers(gh, "devrels").WithDesiredState(utils.ReviewStateApproved),
 				),
 			),
 		},
@@ -55,10 +56,14 @@
 			Then:        r.Never(),
 		},
 		{
-			Description: "Pending initial approval by a review team member (and label matches review triage state)",
+			Description: "Pending initial approval by a review team member, or review from tech-staff",
 			If:          c.Not(c.AuthorInTeam(gh, "tech-staff")),
 			Then: r.
-				If(r.Or(r.ReviewByOrgMembers(gh, 1), r.Draft())).
+				If(r.Or(
+					r.ApprovalByOrgMembers(gh, 1),
+					r.ReviewByTeamMembers(gh, "tech-staff"),
+					r.Draft(),
+				)).
 				// Either there was a first approval from a member, and we
 				// assert that the label for triage-pending is removed...
 				Then(r.Not(r.Label(gh, "review/triage-pending", r.LabelRemove))).

contribs/github-bot/internal/requirements/reviewer.go

@@ -2,6 +2,7 @@
 
 import (
 	"fmt"
+	"slices"
 
 	"github.com/gnolang/gno/contribs/github-bot/internal/client"
 	"github.com/gnolang/gno/contribs/github-bot/internal/utils"
@@ -11,16 +12,14 @@
 )
 
 // Reviewer Requirement.
-type reviewByUser struct {
+type approvalByUser struct {
 	gh   *client.GitHub
 	user string
 }
 
-const approvedState = "APPROVED"
+var _ Requirement = &approvalByUser{}
 
-var _ Requirement = &reviewByUser{}
-
-func (r *reviewByUser) IsSatisfied(pr *github.PullRequest, details treeprint.Tree) bool {
+func (r *approvalByUser) IsSatisfied(pr *github.PullRequest, details treeprint.Tree) bool {
 	detail := fmt.Sprintf("This user approved pull request: %s", r.user)
 
 	// If not a dry run, make the user a reviewer if he's not already.
@@ -67,33 +66,46 @@
 	for _, review := range reviews {
 		if review.GetUser().GetLogin() == r.user {
 			r.gh.Logger.Debugf("User %s already reviewed PR %d with state %s", r.user, pr.GetNumber(), review.GetState())
-			return utils.AddStatusNode(review.GetState() == approvedState, detail, details)
+			return utils.AddStatusNode(review.GetState() == utils.ReviewStateApproved, detail, details)
 		}
 	}
 	r.gh.Logger.Debugf("User %s has not reviewed PR %d yet", r.user, pr.GetNumber())
 
 	return utils.AddStatusNode(false, detail, details)
 }
 
-func ReviewByUser(gh *client.GitHub, user string) Requirement {
-	return &reviewByUser{gh, user}
+func ApprovalByUser(gh *client.GitHub, user string) Requirement {
+	return &approvalByUser{gh, user}
 }
 
 // Reviewer Requirement.
-type reviewByTeamMembers struct {
-	gh    *client.GitHub
-	team  string
-	count uint
+type ReviewByTeamMembersRequirement struct {
+	gh           *client.GitHub
+	team         string
+	count        uint
+	desiredState string
 }
 
-var _ Requirement = &reviewByTeamMembers{}
+var _ Requirement = &ReviewByTeamMembersRequirement{}
 
-func (r *reviewByTeamMembers) IsSatisfied(pr *github.PullRequest, details treeprint.Tree) bool {
-	detail := fmt.Sprintf("At least %d user(s) of the team %s approved pull request", r.count, r.team)
+func (r *ReviewByTeamMembersRequirement) IsSatisfied(pr *github.PullRequest, details treeprint.Tree) bool {
+	detail := fmt.Sprintf("At least %d user(s) of the team %s reviewed pull request", r.count, r.team)
+	if r.desiredState != "" {
+		detail += fmt.Sprintf("(with state %q)", r.desiredState)
+	}
 
-	// If not a dry run, make the user a reviewer if he's not already.
+	teamMembers, err := r.gh.ListTeamMembers(r.team)
+	if err != nil {
+		r.gh.Logger.Errorf(err.Error())
+		return utils.AddStatusNode(false, detail, details)
+	}
+
+	// If not a dry run, request a team review if no member has reviewed yet,
+	// and the team review has not been requested.
 	if !r.gh.DryRun {
-		requested := false
+		var teamRequested bool
+		var usersRequested []string
+
 		reviewers, err := r.gh.ListPRReviewers(pr.GetNumber())
 		if err != nil {
 			r.gh.Logger.Errorf("unable to check if team %s review is already requested: %v", r.team, err)
@@ -102,14 +114,28 @@
 
 		for _, team := range reviewers.Teams {
 			if team.GetSlug() == r.team {
-				requested = true
+				teamRequested = true
 				break
 			}
 		}
 
-		if requested {
+		if !teamRequested {
+			for _, user := range reviewers.Users {
+				if slices.ContainsFunc(teamMembers, func(memb *github.User) bool {
+					return memb.GetID() == user.GetID()
+				}) {
+					usersRequested = append(usersRequested, user.GetLogin())
+				}
+			}
+		}
+
+		switch {
+		case teamRequested:
 			r.gh.Logger.Debugf("Review of team %s already requested on PR %d", r.team, pr.GetNumber())
-		} else {
+		case len(usersRequested) > 0:
+			r.gh.Logger.Debugf("Members %v of team %s already requested on (or reviewed) PR %d",
+				usersRequested, r.team, pr.GetNumber())
+		default:
 			r.gh.Logger.Debugf("Requesting review from team %s on PR %d", r.team, pr.GetNumber())
 			if _, _, err := r.gh.Client.PullRequests.RequestReviewers(
 				r.gh.Ctx,
@@ -125,46 +151,74 @@
 		}
 	}
 
-	// Check how many members of this team already approved this PR.
-	approved := uint(0)
+	// Check how many members of this team already reviewed this PR.
+	reviewCount := uint(0)
 	reviews, err := r.gh.ListPRReviews(pr.GetNumber())
 	if err != nil {
-		r.gh.Logger.Errorf("unable to check if a member of team %s already approved this PR: %v", r.team, err)
+		r.gh.Logger.Errorf("unable to check if a member of team %s already reviewed this PR: %v", r.team, err)
 		return utils.AddStatusNode(false, detail, details)
 	}
 
-	teamMembers, err := r.gh.ListTeamMembers(r.team)
-	if err != nil {
-		r.gh.Logger.Errorf(err.Error())
-		return utils.AddStatusNode(false, detail, details)
+	stateStr := ""
+	if r.desiredState != "" {
+		stateStr = fmt.Sprintf("%q ", r.desiredState)
 	}
-
 	for _, review := range reviews {
 		for _, member := range teamMembers {
 			if review.GetUser().GetLogin() == member.GetLogin() {
-				if review.GetState() == approvedState {
-					approved += 1
+				if desired := r.desiredState; desired == "" || desired == review.GetState() {
+					reviewCount += 1
 				}
-				r.gh.Logger.Debugf("Member %s from team %s already reviewed PR %d with state %s (%d/%d required approval(s))", member.GetLogin(), r.team, pr.GetNumber(), review.GetState(), approved, r.count)
+				r.gh.Logger.Debugf(
+					"Member %s from team %s already reviewed PR %d with state %s (%d/%d required %sreview(s))",
+					member.GetLogin(), r.team, pr.GetNumber(), review.GetState(), reviewCount, r.count, stateStr,
+				)
 			}
 		}
 	}
 
-	return utils.AddStatusNode(approved >= r.count, detail, details)
+	return utils.AddStatusNode(reviewCount >= r.count, detail, details)
+}
+
+// WithCount specifies the number of required reviews.
+// By default, this is 1.
+func (r *ReviewByTeamMembersRequirement) WithCount(n uint) *ReviewByTeamMembersRequirement {
+	if n < 1 {
+		panic("number of required reviews should be at least 1")
+	}
+	r.count = n
+	return r
+}
+
+// WithDesiredState specifies the desired state of the PR reviews.
+//
+// If an empty string is passed, then all reviews are counted. This is the default.
+func (r *ReviewByTeamMembersRequirement) WithDesiredState(state string) *ReviewByTeamMembersRequirement {
+	r.desiredState = state
+	return r
 }
 
-func ReviewByTeamMembers(gh *client.GitHub, team string, count uint) Requirement {
-	return &reviewByTeamMembers{gh, team, count}
+// ReviewByTeamMembers specifies that the given pull request should receive at
+// least one review from a member of the given team.
+//
+// The number of required reviews, or the state of the reviews (e.g., to filter
+// only for approval reviews) can be modified using WithCount and WithDesiredState.
+func ReviewByTeamMembers(gh *client.GitHub, team string) *ReviewByTeamMembersRequirement {
+	return &ReviewByTeamMembersRequirement{
+		gh:    gh,
+		team:  team,
+		count: 1,
+	}
 }
 
-type reviewByOrgMembers struct {
+type approvalByOrgMembers struct {
 	gh    *client.GitHub
 	count uint
 }
 
-var _ Requirement = &reviewByOrgMembers{}
+var _ Requirement = &approvalByOrgMembers{}
 
-func (r *reviewByOrgMembers) IsSatisfied(pr *github.PullRequest, details treeprint.Tree) bool {
+func (r *approvalByOrgMembers) IsSatisfied(pr *github.PullRequest, details treeprint.Tree) bool {
 	detail := fmt.Sprintf("At least %d user(s) of the organization approved the pull request", r.count)
 
 	// Check how many members of this team already approved this PR.
@@ -177,7 +231,7 @@
 
 	for _, review := range reviews {
 		if review.GetAuthorAssociation() == "MEMBER" {
-			if review.GetState() == approvedState {
+			if review.GetState() == utils.ReviewStateApproved {
 				approved++
 			}
 			r.gh.Logger.Debugf(
@@ -191,6 +245,6 @@
 	return utils.AddStatusNode(approved >= r.count, detail, details)
 }
 
-func ReviewByOrgMembers(gh *client.GitHub, count uint) Requirement {
-	return &reviewByOrgMembers{gh, count}
+func ApprovalByOrgMembers(gh *client.GitHub, count uint) Requirement {
+	return &approvalByOrgMembers{gh, count}
 }