Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(approvals-satisfied): add users support #581

Merged
merged 18 commits into from
Apr 2, 2024
Merged
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ Each of the following helpers are defined in a file of the same name in `src/hel

### [approvals-satisfied](.github/workflows/approvals-satisfied.yml)

- Returns `true` if the PR has been approved by the specified GitHub team(s) and `false` otherwise
- If GitHub teams are omitted, uses `CODEOWNERS.md` to determine teams to use
- Returns `true` if the PR has been approved by the specified GitHub team(s) or user(s) and `false` otherwise
- If GitHub teams are omitted, uses `CODEOWNERS.md` to determine teams and/or users to use

### [approve-pr](.github/workflows/approve-pr.yml)

Expand Down
37 changes: 23 additions & 14 deletions dist/431.index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/431.index.js.map

Large diffs are not rendered by default.

37 changes: 23 additions & 14 deletions dist/676.index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/676.index.js.map

Large diffs are not rendered by default.

38 changes: 24 additions & 14 deletions src/helpers/approvals-satisfied.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,12 @@ import { paginateAllReviews } from '../utils/paginate-all-reviews';

export class ApprovalsSatisfied extends HelperInputs {
teams?: string;
users?: string;
number_of_reviewers?: string;
pull_number?: string;
}

export const approvalsSatisfied = async ({ teams, number_of_reviewers = '1', pull_number }: ApprovalsSatisfied = {}) => {
export const approvalsSatisfied = async ({ teams, users, number_of_reviewers = '1', pull_number }: ApprovalsSatisfied = {}) => {
const prNumber = pull_number ? Number(pull_number) : context.issue.number;
const reviews = await paginateAllReviews(prNumber);
const approverLogins = reviews
Expand All @@ -37,23 +38,22 @@ export const approvalsSatisfied = async ({ teams, number_of_reviewers = '1', pul
core.debug(`PR already approved by: ${approverLogins.toString()}`);

const teamsList = teams?.split('\n');
const requiredCodeOwnersEntries = teamsList ? createArtificialCodeOwnersEntry(teamsList) : await getRequiredCodeOwnersEntries(prNumber);
const usersList = users?.split('\n');
const requiredCodeOwnersEntries =
teamsList || usersList ? createArtificialCodeOwnersEntry(teamsList, usersList) : await getRequiredCodeOwnersEntries(prNumber);
const requiredCodeOwnersEntriesWithOwners = requiredCodeOwnersEntries.filter(({ owners }) => owners.length);

const codeOwnersEntrySatisfiesApprovals = async (entry: Pick<CodeOwnersEntry, 'owners'>) => {
const teamsAndLoginsLists = await map(entry.owners, async team => {
const { data } = await octokit.teams.listMembersInOrg({
org: context.repo.owner,
team_slug: convertToTeamSlug(team),
per_page: 100
});
return data.map(({ login }) => ({ team, login }));
const loginsLists = await map(entry.owners, async teamOrUser => {
if (isTeam(teamOrUser)) {
return await fetchTeamLogins(teamOrUser);
} else {
return [teamOrUser];
}
});
const codeOwnerLogins = teamsAndLoginsLists.flat().map(({ login }) => login);
const codeOwnerLogins = distinct(loginsLists.flat());

const numberOfCollectiveApprovalsAcrossTeams = approverLogins.filter(login => codeOwnerLogins.includes(login)).length;
const numberOfApprovalsForSingleTeam = codeOwnerLogins.filter(login => approverLogins.includes(login)).length;
const numberOfApprovals = entry.owners.length > 1 ? numberOfCollectiveApprovalsAcrossTeams : numberOfApprovalsForSingleTeam;
const numberOfApprovals = approverLogins.filter(login => codeOwnerLogins.includes(login)).length;

core.debug(`Current number of approvals satisfied for ${entry.owners}: ${numberOfApprovals}`);

Expand All @@ -65,4 +65,14 @@ export const approvalsSatisfied = async ({ teams, number_of_reviewers = '1', pul
return booleans.every(Boolean);
};

const createArtificialCodeOwnersEntry = (teams: string[]) => teams.map(team => ({ owners: [team] }));
const createArtificialCodeOwnersEntry = (teams?: string[], users?: string[]) => [{ owners: (teams || []).concat(users || []) }];
vsingal-p marked this conversation as resolved.
Show resolved Hide resolved
const distinct = (arrayWithDuplicates: string[]) => arrayWithDuplicates.filter((n, i) => arrayWithDuplicates.indexOf(n) === i);
const isTeam = (teamOrUser: string) => teamOrUser.includes('/');
vsingal-p marked this conversation as resolved.
Show resolved Hide resolved
const fetchTeamLogins = async (team: string) => {
const { data } = await octokit.teams.listMembersInOrg({
org: context.repo.owner,
team_slug: convertToTeamSlug(team),
per_page: 100
});
return data.map(({ login }) => login);
};
112 changes: 112 additions & 0 deletions test/helpers/approvals-satisfied.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,88 @@ describe('approvalsSatisfied', () => {
expect(result).toBe(true);
});

it('should return false when passing users override and required approvals are not met', async () => {
mockPagination({
data: [
{
state: 'APPROVED',
user: { login: 'user3' }
}
]
});

const result = await approvalsSatisfied({ users: 'user1', pull_number: '12345' });
expect(octokit.pulls.listReviews).toHaveBeenCalledWith({ pull_number: 12345, repo: 'repo', owner: 'owner', page: 1, per_page: 100 });
expect(getRequiredCodeOwnersEntries).not.toHaveBeenCalled();
expect(result).toBe(false);
});

it('should return true when passing users override and required approvals are met', async () => {
mockPagination({
data: [
{
state: 'APPROVED',
user: { login: 'user1' }
}
]
});
const result = await approvalsSatisfied({ users: 'user1', pull_number: '12345' });
expect(octokit.pulls.listReviews).toHaveBeenCalledWith({ pull_number: 12345, repo: 'repo', owner: 'owner', page: 1, per_page: 100 });
expect(getRequiredCodeOwnersEntries).not.toHaveBeenCalled();
expect(result).toBe(true);
});

it('should return true when passing users override and required approvals are met when count > 1', async () => {
mockPagination({
data: [
{
state: 'APPROVED',
user: { login: 'user1' }
},
{
state: 'APPROVED',
user: { login: 'user3' }
},
{
state: 'APPROVED',
user: { login: 'user5' }
}
]
});
const result = await approvalsSatisfied({ users: 'user1\nuser2\nuser3', pull_number: '12345', number_of_reviewers: '2' });
expect(octokit.pulls.listReviews).toHaveBeenCalledWith({ pull_number: 12345, repo: 'repo', owner: 'owner', page: 1, per_page: 100 });
expect(getRequiredCodeOwnersEntries).not.toHaveBeenCalled();
expect(result).toBe(true);
});

it('should return true when passing users and teams override and required approvals are met when count > 1', async () => {
mockPagination({
data: [
{
state: 'APPROVED',
user: { login: 'user1' }
},
{
state: 'APPROVED',
user: { login: 'user3' }
},
{
state: 'APPROVED',
user: { login: 'user5' }
}
]
});
const result = await approvalsSatisfied({
users: 'user4\nuser2\nuser3',
teams: 'team1',
pull_number: '12345',
number_of_reviewers: '2'
});
expect(octokit.pulls.listReviews).toHaveBeenCalledWith({ pull_number: 12345, repo: 'repo', owner: 'owner', page: 1, per_page: 100 });
expect(getRequiredCodeOwnersEntries).not.toHaveBeenCalled();
expect(result).toBe(true);
});

it('should return false when a core member has not approved', async () => {
(getRequiredCodeOwnersEntries as jest.Mock).mockResolvedValue([{ owners: ['@ExpediaGroup/team1'] }]);
mockPagination({
Expand Down Expand Up @@ -159,6 +241,36 @@ describe('approvalsSatisfied', () => {
expect(result).toBe(true);
});

it('should return true when a member from each owner group (teams and users) has approved', async () => {
(getRequiredCodeOwnersEntries as jest.Mock).mockResolvedValue([
{ owners: ['@ExpediaGroup/team1'] },
{ owners: ['@ExpediaGroup/team2'] },
{ owners: ['@ExpediaGroup/team4', 'user10'] }
]);
mockPagination({
data: [
{
state: 'APPROVED',
user: { login: 'user1' }
},
{
state: 'APPROVED',
user: { login: 'user2' }
},
{
state: 'CHANGES_REQUESTED',
user: { login: 'user3' }
},
{
state: 'APPROVED',
user: { login: 'user10' }
}
]
});
const result = await approvalsSatisfied();
expect(result).toBe(true);
});

it('should return false when not enough members from core teams have approved', async () => {
(getRequiredCodeOwnersEntries as jest.Mock).mockResolvedValue([
{ owners: ['@ExpediaGroup/team2'] },
Expand Down
Loading