pr-approval-check: refactor, fix pagination, fix approval logic

.github/workflows/pr-approval-check.yml

@@ -2,128 +2,116 @@ name: PR Approval Check
 
 on:
   pull_request_review:
-    types: [submitted]
+    types: [submitted, dismissed]
   pull_request:
     types: [opened, reopened, synchronize]
 
 jobs:
   check-product-eng-approval:
     name: Check Product Eng Approval
     runs-on: ubuntu-latest
-
     steps:
       - name: Check for Product Engineering approval
         uses: actions/github-script@v7
+        env:
+          ORG: trufflesecurity
+          APPROVER_TEAM: product-eng
         with:
           github-token: ${{ secrets.GITHUB_TOKEN }}
           script: |
-            const { data: reviews } = await github.rest.pulls.listReviews({
-              owner: context.repo.owner,
-              repo: context.repo.repo,
-              pull_number: context.payload.pull_request.number
-            });
-
-            console.log(`Found ${reviews.length} reviews`);
-
-            // Get approved reviews
-            const approvals = reviews.filter(review => review.state === 'APPROVED');
-            console.log(`Found ${approvals.length} approvals`);
-
-            if (approvals.length === 0) {
-              console.log('No approvals found');
+            const { ORG: org, APPROVER_TEAM: team} = process.env
+            const prettyTeamName = `@${org}/${team}`
+
+            async function status(state, msg) {
               await github.rest.repos.createCommitStatus({
                 owner: context.repo.owner,
                 repo: context.repo.repo,
                 sha: context.payload.pull_request.head.sha,
-                state: 'pending',
-                context: 'product-eng-approval',
-                description: 'Waiting for approval from @trufflesecurity/product-eng team member'
+                state: state,
+                context: 'pr-approval-check',
+                description: msg,
               });
-              return;
             }
-
-            // Helper function to get all teams to check (parent + children)
-            async function getTeamsToCheck() {
-              const teamsToCheck = ['product-eng']; // Start with parent team
-
-              try {
-                // Get child teams of product-eng
-                const { data: childTeams } = await github.rest.teams.listChildInOrg({
-                  org: 'trufflesecurity',
-                  team_slug: 'product-eng'
-                });
-
-                // Add child team slugs
-                childTeams.forEach(team => {
-                  teamsToCheck.push(team.slug);
-                });
-
-                console.log(`Teams to check: ${teamsToCheck.join(', ')}`);
-              } catch (error) {
-                console.log('Error fetching child teams, will only check parent team:', error.message);
-              }
-
-              return teamsToCheck;
+
+            async function fail(msg) {
+              core.setOutput(msg);
+              console.error(msg)
+              await status('failure', msg)
+              process.exit(0);
             }
-
-            // Helper function to check if user is member of any team
-            async function isUserMemberOfAnyTeam(username, teams) {
-              for (const teamSlug of teams) {
-                try {
-                  const { data: membership } = await github.rest.teams.getMembershipForUserInOrg({
-                    org: 'trufflesecurity',
-                    team_slug: teamSlug,
-                    username: username
-                  });
-
-                  if (membership.state === 'active') {
-                    console.log(`✅ Found active member of @trufflesecurity/${teamSlug}: ${username}`);
-                    return { isMember: true, teamSlug };
-                  }
-                } catch (error) {
-                  // User is not a member of this team (404) or other error
-                  console.log(`❌ ${username} is not a member of @trufflesecurity/${teamSlug}`);
-                }
-              }
-              return { isMember: false, teamSlug: null };
+
+            async function succeed(msg) {
+              core.setOutput(msg);
+              console.info(msg)
+              await status('success', msg)
+              process.exit(0);
             }
-
-            // Get all teams to check (parent + children)
-            const teamsToCheck = await getTeamsToCheck();
-
-            // Check if any approver is a member of product-eng or its child teams
-            let hasProductEngApproval = false;
-            let approverInfo = null;
-
-            for (const approval of approvals) {
-              const membershipCheck = await isUserMemberOfAnyTeam(approval.user.login, teamsToCheck);
-
-              if (membershipCheck.isMember) {
-                hasProductEngApproval = true;
-                approverInfo = {
-                  username: approval.user.login,
-                  teamSlug: membershipCheck.teamSlug
-                };
-                break;
-              }
+
+            async function fatal(msg) {
+              core.setFailed(msg);
+              console.error(msg)
+              await status('failure', msg)
+              process.exit(1);
             }
-
-            if (hasProductEngApproval) {
-              await github.rest.repos.createCommitStatus({
+
+
+            await status('pending', `Waiting for approval from ${prettyTeamName} team member`)
+
+            // reviews maps reviewer logins to their latest review
+            let reviews = new Map();
+            try {
+              const iter = octokit.paginate.iterator(github.rest.pulls.listReviews, {
                 owner: context.repo.owner,
                 repo: context.repo.repo,
-                sha: context.payload.pull_request.head.sha,
-                state: 'success',
-                context: 'product-eng-approval',
-                description: `✅ Approved by @trufflesecurity/${approverInfo.teamSlug} member (${approverInfo.username})`
+                pull_number: context.payload.pull_request.number,
               });
+
+              let pages = [];
+              for await (const page of iter) {
+                pages.push(page)
+              }
+
+              const latestReview = (a, b) => new Date(a.submitted_at) > new Date(b.submitted_at) ? a : b;
+              for await (const page of pages) {
+                for (const review of page.data) {
+                  const login = review.user.login;
+                  reviews.set(login, reviews.has(login) ? latestReview(reviews.get(login), review) : review);
+                }
+              }
+            } catch (error) {
+              await fatal(`⚠️ Could not get reviews: ${error.status}`);
+            }
+
+            let approved = false;
+            let changeRequesters = [];
+            for (const [login, review] of reviews) {
+              try {
+                const membership = await github.rest.teams.getMembershipForUserInOrg({
+                  org: org,
+                  team_slug: team,
+                  username: login,
+                });
+
+                if (membership.data.state === 'active') {
+                  if (review.state === 'APPROVED') {
+                    approved = true;
+                  } else if (review.state === 'CHANGES_REQUESTED') {
+                    changeRequesters.push(login);
+                  }
+                }
+
+              } catch (error) {
+                if (error.status != 404) {
+                  await fatal(`⚠️ Could not determine membership for ${login} in ${prettyTeamName}: ${error.status}`)
+                }
+              }
+            }
+
+            if (changeRequesters.length > 0) {
+              await fail(`⚠️ Changes requested by: ${changeRequesters.map(login=>`@${login}`).join(", ")} on behalf of ${prettyTeamName}`);
+            } else if (approved) {
+              await succeed(`✅ Approved by ${prettyTeamName}`)
             } else {
-              await github.rest.repos.createCommitStatus({
-                owner: context.repo.owner,
-                repo: context.repo.repo,
-                sha: context.payload.pull_request.head.sha,
-                state: 'failure',
-                context: 'product-eng-approval',
-                description: '❌ Requires approval from @trufflesecurity/product-eng team member'
-              });
-            }
+              await fail(`⚠️ Requires approval from ${prettyTeamName}`)
+            }
+