Skip to content

Commit

Permalink
feat(ng-dev): disallow merging if no members of the organization have…
Browse files Browse the repository at this point in the history
… approved the pull request (angular#1744)

This is in large part meant to prevent someone being able to merge a pull request that is unreviewed.

PR Close angular#1744
  • Loading branch information
josephperrott committed Jan 16, 2024
1 parent cef5d14 commit 8e21b74
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 5 deletions.
30 changes: 25 additions & 5 deletions .github/local-actions/branch-manager/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -74093,6 +74093,7 @@ var PullRequestValidationConfig = class {
this.assertPassingCi = true;
this.assertCompletedReviews = true;
this.assertEnforcedStatuses = true;
this.assertMinimumReviews = true;
}
static create(config) {
return Object.assign(new PullRequestValidationConfig(), config);
Expand Down Expand Up @@ -78567,6 +78568,13 @@ var PR_SCHEMA = {
reviewRequests: {
totalCount: import_typed_graphqlify2.types.number
},
reviews: (0, import_typed_graphqlify2.params)({ last: 100, states: "APPROVED" }, {
nodes: [
{
authorAssociation: import_typed_graphqlify2.types.custom()
}
]
}),
maintainerCanModify: import_typed_graphqlify2.types.boolean,
viewerDidAuthor: import_typed_graphqlify2.types.boolean,
headRefOid: import_typed_graphqlify2.types.string,
Expand Down Expand Up @@ -78989,8 +78997,19 @@ var Validation5 = class extends PullRequestValidation {
};

//
var passingCiValidation = createPullRequestValidation({ name: "assertPassingCi", canBeForceIgnored: true }, () => Validation6);
var minimumReviewsValidation = createPullRequestValidation({ name: "assertMinimumReviews", canBeForceIgnored: false }, () => Validation6);
var Validation6 = class extends PullRequestValidation {
assert(pullRequest) {
const totalCount = pullRequest.reviews.nodes.filter(({ authorAssociation }) => authorAssociation === "MEMBER").length;
if (totalCount === 0) {
throw this._createError(`Pull request cannot be merged without at least one review from a team member`);
}
}
};

//
var passingCiValidation = createPullRequestValidation({ name: "assertPassingCi", canBeForceIgnored: true }, () => Validation7);
var Validation7 = class extends PullRequestValidation {
assert(pullRequest) {
const { combinedStatus } = getStatusesForPullRequest(pullRequest);
if (combinedStatus === PullRequestStatus.PENDING) {
Expand All @@ -79003,8 +79022,8 @@ var Validation6 = class extends PullRequestValidation {
};

//
var pendingStateValidation = createPullRequestValidation({ name: "assertPending", canBeForceIgnored: false }, () => Validation7);
var Validation7 = class extends PullRequestValidation {
var pendingStateValidation = createPullRequestValidation({ name: "assertPending", canBeForceIgnored: false }, () => Validation8);
var Validation8 = class extends PullRequestValidation {
assert(pullRequest) {
if (pullRequest.isDraft) {
throw this._createError("Pull request is still a draft.");
Expand All @@ -79021,9 +79040,9 @@ var Validation7 = class extends PullRequestValidation {
//
var signedClaValidation = createPullRequestValidation(
{ name: "assertSignedCla", canBeForceIgnored: true },
() => Validation8
() => Validation9
);
var Validation8 = class extends PullRequestValidation {
var Validation9 = class extends PullRequestValidation {
assert(pullRequest) {
const passing = getStatusesForPullRequest(pullRequest).statuses.some(({ name, status }) => {
return name === "cla/google" && status === PullRequestStatus.PASSING;
Expand All @@ -79041,6 +79060,7 @@ async function assertValidPullRequest(pullRequest, validationConfig, ngDevConfig
return parseCommitMessage(n.commit.message);
});
const validationResults = [
minimumReviewsValidation.run(validationConfig, pullRequest),
completedReviewsValidation.run(validationConfig, pullRequest),
mergeReadyValidation.run(validationConfig, pullRequest),
signedClaValidation.run(validationConfig, pullRequest),
Expand Down
7 changes: 7 additions & 0 deletions github-actions/slash-commands/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -78804,6 +78804,13 @@ var PR_SCHEMA = {
reviewRequests: {
totalCount: import_typed_graphqlify4.types.number
},
reviews: (0, import_typed_graphqlify4.params)({ last: 100, states: "APPROVED" }, {
nodes: [
{
authorAssociation: import_typed_graphqlify4.types.custom()
}
]
}),
maintainerCanModify: import_typed_graphqlify4.types.boolean,
viewerDidAuthor: import_typed_graphqlify4.types.boolean,
headRefOid: import_typed_graphqlify4.types.string,
Expand Down
11 changes: 11 additions & 0 deletions ng-dev/pr/common/fetch-pull-request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
MergeableState,
PullRequestState,
StatusState,
CommentAuthorAssociation,
} from '@octokit/graphql-schema';
import {getPendingPrs, getPr} from '../../utils/github.js';
import {alias, types as graphqlTypes, onUnion, optional, params} from 'typed-graphqlify';
Expand Down Expand Up @@ -81,6 +82,16 @@ export const PR_SCHEMA = {
reviewRequests: {
totalCount: graphqlTypes.number,
},
reviews: params(
{last: 100, states: 'APPROVED'},
{
nodes: [
{
authorAssociation: graphqlTypes.custom<CommentAuthorAssociation>(),
},
],
},
),
maintainerCanModify: graphqlTypes.boolean,
viewerDidAuthor: graphqlTypes.boolean,
headRefOid: graphqlTypes.string,
Expand Down
29 changes: 29 additions & 0 deletions ng-dev/pr/common/validation/assert-minimum-reviews.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/**
* @license
* Copyright Google LLC
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {PullRequestFromGithub} from '../fetch-pull-request.js';
import {createPullRequestValidation, PullRequestValidation} from './validation-config.js';

/** Assert the pull request has completed all requested reviews. */
export const minimumReviewsValidation = createPullRequestValidation(
{name: 'assertMinimumReviews', canBeForceIgnored: false},
() => Validation,
);

class Validation extends PullRequestValidation {
assert(pullRequest: PullRequestFromGithub) {
const totalCount = pullRequest.reviews.nodes.filter(
({authorAssociation}) => authorAssociation === 'MEMBER',
).length;
if (totalCount === 0) {
throw this._createError(
`Pull request cannot be merged without at least one review from a team member`,
);
}
}
}
2 changes: 2 additions & 0 deletions ng-dev/pr/common/validation/validate-pull-request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {breakingChangeInfoValidation} from './assert-breaking-change-info.js';
import {completedReviewsValidation} from './assert-completed-reviews.js';
import {enforcedStatusesValidation} from './assert-enforced-statuses.js';
import {mergeReadyValidation} from './assert-merge-ready.js';
import {minimumReviewsValidation} from './assert-minimum-reviews.js';
import {passingCiValidation} from './assert-passing-ci.js';
import {pendingStateValidation} from './assert-pending.js';
import {signedClaValidation} from './assert-signed-cla.js';
Expand All @@ -42,6 +43,7 @@ export async function assertValidPullRequest(
});

const validationResults = [
minimumReviewsValidation.run(validationConfig, pullRequest),
completedReviewsValidation.run(validationConfig, pullRequest),
mergeReadyValidation.run(validationConfig, pullRequest),
signedClaValidation.run(validationConfig, pullRequest),
Expand Down
1 change: 1 addition & 0 deletions ng-dev/pr/common/validation/validation-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export class PullRequestValidationConfig {
assertPassingCi = true;
assertCompletedReviews = true;
assertEnforcedStatuses = true;
assertMinimumReviews = true;

static create(config: Partial<PullRequestValidationConfig>) {
return Object.assign(new PullRequestValidationConfig(), config);
Expand Down

0 comments on commit 8e21b74

Please sign in to comment.