Skip to content

Commit f0df738

Browse files
j2fwj2fw
authored
pr-approval-check: refactor, fix pagination, fix approval logic (#4519)
1 parent 349d1c7 commit f0df738

File tree

1 file changed

+87
-99
lines changed

1 file changed

+87
-99
lines changed

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

Lines changed: 87 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -2,128 +2,116 @@ name: PR Approval Check
22

33
on:
44
pull_request_review:
5-
types: [submitted]
5+
types: [submitted, dismissed]
66
pull_request:
77
types: [opened, reopened, synchronize]
88

99
jobs:
1010
check-product-eng-approval:
1111
name: Check Product Eng Approval
1212
runs-on: ubuntu-latest
13-
1413
steps:
1514
- name: Check for Product Engineering approval
1615
uses: actions/github-script@v7
16+
env:
17+
ORG: trufflesecurity
18+
APPROVER_TEAM: product-eng
1719
with:
1820
github-token: ${{ secrets.GITHUB_TOKEN }}
1921
script: |
20-
const { data: reviews } = await github.rest.pulls.listReviews({
21-
owner: context.repo.owner,
22-
repo: context.repo.repo,
23-
pull_number: context.payload.pull_request.number
24-
});
25-
26-
console.log(`Found ${reviews.length} reviews`);
27-
28-
// Get approved reviews
29-
const approvals = reviews.filter(review => review.state === 'APPROVED');
30-
console.log(`Found ${approvals.length} approvals`);
31-
32-
if (approvals.length === 0) {
33-
console.log('No approvals found');
22+
const { ORG: org, APPROVER_TEAM: team} = process.env
23+
const prettyTeamName = `@${org}/${team}`
24+
25+
async function status(state, msg) {
3426
await github.rest.repos.createCommitStatus({
3527
owner: context.repo.owner,
3628
repo: context.repo.repo,
3729
sha: context.payload.pull_request.head.sha,
38-
state: 'pending',
39-
context: 'product-eng-approval',
40-
description: 'Waiting for approval from @trufflesecurity/product-eng team member'
30+
state: state,
31+
context: 'pr-approval-check',
32+
description: msg,
4133
});
42-
return;
4334
}
44-
45-
// Helper function to get all teams to check (parent + children)
46-
async function getTeamsToCheck() {
47-
const teamsToCheck = ['product-eng']; // Start with parent team
48-
49-
try {
50-
// Get child teams of product-eng
51-
const { data: childTeams } = await github.rest.teams.listChildInOrg({
52-
org: 'trufflesecurity',
53-
team_slug: 'product-eng'
54-
});
55-
56-
// Add child team slugs
57-
childTeams.forEach(team => {
58-
teamsToCheck.push(team.slug);
59-
});
60-
61-
console.log(`Teams to check: ${teamsToCheck.join(', ')}`);
62-
} catch (error) {
63-
console.log('Error fetching child teams, will only check parent team:', error.message);
64-
}
65-
66-
return teamsToCheck;
35+
36+
async function fail(msg) {
37+
core.setOutput(msg);
38+
console.error(msg)
39+
await status('failure', msg)
40+
process.exit(0);
6741
}
68-
69-
// Helper function to check if user is member of any team
70-
async function isUserMemberOfAnyTeam(username, teams) {
71-
for (const teamSlug of teams) {
72-
try {
73-
const { data: membership } = await github.rest.teams.getMembershipForUserInOrg({
74-
org: 'trufflesecurity',
75-
team_slug: teamSlug,
76-
username: username
77-
});
78-
79-
if (membership.state === 'active') {
80-
console.log(`✅ Found active member of @trufflesecurity/${teamSlug}: ${username}`);
81-
return { isMember: true, teamSlug };
82-
}
83-
} catch (error) {
84-
// User is not a member of this team (404) or other error
85-
console.log(`❌ ${username} is not a member of @trufflesecurity/${teamSlug}`);
86-
}
87-
}
88-
return { isMember: false, teamSlug: null };
42+
43+
async function succeed(msg) {
44+
core.setOutput(msg);
45+
console.info(msg)
46+
await status('success', msg)
47+
process.exit(0);
8948
}
90-
91-
// Get all teams to check (parent + children)
92-
const teamsToCheck = await getTeamsToCheck();
93-
94-
// Check if any approver is a member of product-eng or its child teams
95-
let hasProductEngApproval = false;
96-
let approverInfo = null;
97-
98-
for (const approval of approvals) {
99-
const membershipCheck = await isUserMemberOfAnyTeam(approval.user.login, teamsToCheck);
100-
101-
if (membershipCheck.isMember) {
102-
hasProductEngApproval = true;
103-
approverInfo = {
104-
username: approval.user.login,
105-
teamSlug: membershipCheck.teamSlug
106-
};
107-
break;
108-
}
49+
50+
async function fatal(msg) {
51+
core.setFailed(msg);
52+
console.error(msg)
53+
await status('failure', msg)
54+
process.exit(1);
10955
}
110-
111-
if (hasProductEngApproval) {
112-
await github.rest.repos.createCommitStatus({
56+
57+
58+
await status('pending', `Waiting for approval from ${prettyTeamName} team member`)
59+
60+
// reviews maps reviewer logins to their latest review
61+
let reviews = new Map();
62+
try {
63+
const iter = octokit.paginate.iterator(github.rest.pulls.listReviews, {
11364
owner: context.repo.owner,
11465
repo: context.repo.repo,
115-
sha: context.payload.pull_request.head.sha,
116-
state: 'success',
117-
context: 'product-eng-approval',
118-
description: `✅ Approved by @trufflesecurity/${approverInfo.teamSlug} member (${approverInfo.username})`
66+
pull_number: context.payload.pull_request.number,
11967
});
68+
69+
let pages = [];
70+
for await (const page of iter) {
71+
pages.push(page)
72+
}
73+
74+
const latestReview = (a, b) => new Date(a.submitted_at) > new Date(b.submitted_at) ? a : b;
75+
for await (const page of pages) {
76+
for (const review of page.data) {
77+
const login = review.user.login;
78+
reviews.set(login, reviews.has(login) ? latestReview(reviews.get(login), review) : review);
79+
}
80+
}
81+
} catch (error) {
82+
await fatal(`⚠️ Could not get reviews: ${error.status}`);
83+
}
84+
85+
let approved = false;
86+
let changeRequesters = [];
87+
for (const [login, review] of reviews) {
88+
try {
89+
const membership = await github.rest.teams.getMembershipForUserInOrg({
90+
org: org,
91+
team_slug: team,
92+
username: login,
93+
});
94+
95+
if (membership.data.state === 'active') {
96+
if (review.state === 'APPROVED') {
97+
approved = true;
98+
} else if (review.state === 'CHANGES_REQUESTED') {
99+
changeRequesters.push(login);
100+
}
101+
}
102+
103+
} catch (error) {
104+
if (error.status != 404) {
105+
await fatal(`⚠️ Could not determine membership for ${login} in ${prettyTeamName}: ${error.status}`)
106+
}
107+
}
108+
}
109+
110+
if (changeRequesters.length > 0) {
111+
await fail(`⚠️ Changes requested by: ${changeRequesters.map(login=>`@${login}`).join(", ")} on behalf of ${prettyTeamName}`);
112+
} else if (approved) {
113+
await succeed(`✅ Approved by ${prettyTeamName}`)
120114
} else {
121-
await github.rest.repos.createCommitStatus({
122-
owner: context.repo.owner,
123-
repo: context.repo.repo,
124-
sha: context.payload.pull_request.head.sha,
125-
state: 'failure',
126-
context: 'product-eng-approval',
127-
description: '❌ Requires approval from @trufflesecurity/product-eng team member'
128-
});
129-
}
115+
await fail(`⚠️ Requires approval from ${prettyTeamName}`)
116+
}
117+

0 commit comments

Comments
 (0)