diff --git a/projects/github-actions/required-review/README.md b/projects/github-actions/required-review/README.md index 8d43ac727ed5d..1eb49ded89baa 100644 --- a/projects/github-actions/required-review/README.md +++ b/projects/github-actions/required-review/README.md @@ -65,6 +65,10 @@ This action is intended to be triggered by the `pull_request_review` event. # this to instead fail the status checks instead of leaving them pending. fail: true + # By default required reviewers are not requested. Set this to true to + # request reviews. + request-reviews: true + # GitHub Access Token. The user associated with this token will show up # as the "creator" of the status check, and must have access to read # pull request data, create status checks (`repo:status`), and to read diff --git a/projects/github-actions/required-review/action.yml b/projects/github-actions/required-review/action.yml index 548aaf9054d37..53f7d1b3db3ee 100644 --- a/projects/github-actions/required-review/action.yml +++ b/projects/github-actions/required-review/action.yml @@ -23,6 +23,12 @@ inputs: required: false type: boolean default: false + request-reviews: + description: > + Automatically request reviews from teams or users needed to fulfill a requirement. + required: false + type: boolean + default: false token: description: > GitHub Access Token. The user associated with this token will show up diff --git a/projects/github-actions/required-review/changelog/request-review-if-not-satisfied b/projects/github-actions/required-review/changelog/request-review-if-not-satisfied new file mode 100644 index 0000000000000..20eec52e35c2d --- /dev/null +++ b/projects/github-actions/required-review/changelog/request-review-if-not-satisfied @@ -0,0 +1,4 @@ +Significance: minor +Type: added + +Option to request review if requirement is not satisfied. \ No newline at end of file diff --git a/projects/github-actions/required-review/src/main.js b/projects/github-actions/required-review/src/main.js index 296ddfd9db44f..e9f98dcbcc280 100644 --- a/projects/github-actions/required-review/src/main.js +++ b/projects/github-actions/required-review/src/main.js @@ -2,6 +2,7 @@ const fs = require( 'fs' ); const core = require( '@actions/core' ); const yaml = require( 'js-yaml' ); const reporter = require( './reporter.js' ); +const requestReview = require( './request-review.js' ); const Requirement = require( './requirement.js' ); /** @@ -10,9 +11,9 @@ const Requirement = require( './requirement.js' ); * @returns {Requirement[]} Requirements. */ async function getRequirements() { - let reqirementsString = core.getInput( 'requirements' ); + let requirementsString = core.getInput( 'requirements' ); - if ( ! reqirementsString ) { + if ( ! requirementsString ) { const filename = core.getInput( 'requirements-file' ); if ( ! filename ) { throw new reporter.ReportError( @@ -23,7 +24,7 @@ async function getRequirements() { } try { - reqirementsString = fs.readFileSync( filename, 'utf8' ); + requirementsString = fs.readFileSync( filename, 'utf8' ); } catch ( error ) { throw new reporter.ReportError( `Requirements file ${ filename } could not be read`, @@ -36,7 +37,7 @@ async function getRequirements() { } try { - const requirements = yaml.load( reqirementsString, { + const requirements = yaml.load( requirementsString, { onWarning: w => core.warning( `Yaml: ${ w.message }` ), } ); if ( ! Array.isArray( requirements ) ) { @@ -69,7 +70,7 @@ async function main() { core.endGroup(); let matchedPaths = []; - let ok = true; + const teamsNeededForReview = new Set(); for ( let i = 0; i < requirements.length; i++ ) { const r = requirements[ i ]; core.startGroup( `Checking requirement "${ r.name }"...` ); @@ -78,22 +79,27 @@ async function main() { if ( ! applies ) { core.endGroup(); core.info( `Requirement "${ r.name }" does not apply to any files in this PR.` ); - } else if ( await r.isSatisfied( reviewers ) ) { - core.endGroup(); - core.info( `Requirement "${ r.name }" is satisfied by the existing reviews.` ); } else { - ok = false; + const neededForRequirement = await r.needsReviewsFrom( reviewers ); core.endGroup(); - core.error( `Requirement "${ r.name }" is not satisfied by the existing reviews.` ); + if ( neededForRequirement.length === 0 ) { + core.info( `Requirement "${ r.name }" is satisfied by the existing reviews.` ); + } else { + core.error( `Requirement "${ r.name }" is not satisfied by the existing reviews.` ); + neededForRequirement.forEach( teamsNeededForReview.add, teamsNeededForReview ); + } } } - if ( ok ) { + if ( teamsNeededForReview.size === 0 ) { await reporter.status( reporter.STATE_SUCCESS, 'All required reviews have been provided!' ); } else { await reporter.status( core.getBooleanInput( 'fail' ) ? reporter.STATE_FAILURE : reporter.STATE_PENDING, reviewers.length ? 'Awaiting more reviews...' : 'Awaiting reviews...' ); + if ( core.getBooleanInput( 'request-reviews' ) ) { + await requestReview( [ ...teamsNeededForReview ] ); + } } } catch ( error ) { let err, state, description; diff --git a/projects/github-actions/required-review/src/request-review.js b/projects/github-actions/required-review/src/request-review.js new file mode 100644 index 0000000000000..183924ecfda76 --- /dev/null +++ b/projects/github-actions/required-review/src/request-review.js @@ -0,0 +1,40 @@ +const core = require( '@actions/core' ); +const github = require( '@actions/github' ); + +/** + * Request review from the given team + * + * @param {string[]} teams - GitHub team slug, or @ followed by a GitHub user name. + */ +async function requestReviewer( teams ) { + const octokit = github.getOctokit( core.getInput( 'token', { required: true } ) ); + const owner = github.context.payload.repository.owner.login; + const repo = github.context.payload.repository.name; + const pr = github.context.payload.pull_request.number; + + const userReviews = []; + const teamReviews = []; + + for ( const t of teams ) { + if ( t.startsWith( '@' ) ) { + userReviews.push( t.slice( 1 ) ); + } else { + teamReviews.push( t ); + } + } + + try { + await octokit.rest.pulls.requestReviewers( { + owner: owner, + repo: repo, + pull_number: pr, + reviewers: userReviews, + team_reviewers: teamReviews, + } ); + core.info( `Requested review(s) from ${ teams }` ); + } catch ( err ) { + throw new Error( `Unable to request review.\n Error: ${ err }` ); + } +} + +module.exports = requestReviewer; diff --git a/projects/github-actions/required-review/src/requirement.js b/projects/github-actions/required-review/src/requirement.js index 4d6fd5d735363..5d2258fc2caf4 100644 --- a/projects/github-actions/required-review/src/requirement.js +++ b/projects/github-actions/required-review/src/requirement.js @@ -10,12 +10,13 @@ class RequirementError extends SError {} * Prints a result set, then returns it. * * @param {string} label - Label for the set. - * @param {string[]} items - Items to print. If an empty array, will print `` instead. - * @returns {string[]} `items`. + * @param {string[]} teamReviewers - Team members that have reviewed the file. If an empty array, will print `` instead. + * @param {string[]} neededTeams - Teams that have no reviews from it's members. + * @returns {{teamReviewers, neededTeams}} `{teamReviewers, neededTeams}`. */ -function printSet( label, items ) { - core.info( label + ' ' + ( items.length ? items.join( ', ' ) : '' ) ); - return items; +function printSet( label, teamReviewers, neededTeams ) { + core.info( label + ' ' + ( teamReviewers.length ? teamReviewers.join( ', ' ) : '' ) ); + return { teamReviewers, neededTeams }; } /** @@ -31,10 +32,9 @@ function buildReviewerFilter( config, teamConfig, indent ) { const team = teamConfig; return async function ( reviewers ) { const members = await fetchTeamMembers( team ); - return printSet( - `${ indent }Members of ${ team }:`, - reviewers.filter( reviewer => members.includes( reviewer ) ) - ); + const teamReviewers = reviewers.filter( reviewer => members.includes( reviewer ) ); + const neededTeams = teamReviewers.length ? [] : [ team ]; + return printSet( `${ indent }Members of ${ team }:`, teamReviewers, neededTeams ); }; } @@ -81,22 +81,48 @@ function buildReviewerFilter( config, teamConfig, indent ) { if ( op === 'any-of' ) { return async function ( reviewers ) { core.info( `${ indent }Union of these:` ); - return printSet( `${ indent }=>`, [ - ...new Set( - ( await Promise.all( arg.map( f => f( reviewers, `${ indent } ` ) ) ) ).flat( 1 ) - ), - ] ); + const reviewersAny = await Promise.all( arg.map( f => f( reviewers, `${ indent } ` ) ) ); + const requirementsMet = []; + const neededTeams = []; + for ( const requirementResult of reviewersAny ) { + if ( requirementResult.teamReviewers.length !== 0 ) { + requirementsMet.push( requirementResult.teamReviewers ); + } + if ( requirementResult.neededTeams.length !== 0 ) { + neededTeams.push( requirementResult.neededTeams ); + } + } + if ( requirementsMet.length > 0 ) { + // If there are requirements met, zero out the needed teams + neededTeams.length = 0; + } + return printSet( + `${ indent }=>`, + [ ...new Set( requirementsMet.flat( 1 ) ) ], + [ ...new Set( neededTeams.flat( 1 ) ) ] + ); }; } if ( op === 'all-of' ) { return async function ( reviewers ) { core.info( `${ indent }Union of these, if none are empty:` ); - const filtered = await Promise.all( arg.map( f => f( reviewers, `${ indent } ` ) ) ); - if ( filtered.some( a => a.length === 0 ) ) { - return printSet( `${ indent }=>`, [] ); + const reviewersAll = await Promise.all( arg.map( f => f( reviewers, `${ indent } ` ) ) ); + const requirementsMet = []; + const neededTeams = []; + for ( const requirementResult of reviewersAll ) { + if ( requirementResult.teamReviewers.length !== 0 ) { + requirementsMet.push( requirementResult.teamReviewers ); + } + if ( requirementResult.neededTeams.length !== 0 ) { + neededTeams.push( requirementResult.neededTeams ); + } + } + if ( neededTeams.length !== 0 ) { + // If there are needed teams, zero out requirements met + return printSet( `${ indent }=>`, [], [ ...new Set( neededTeams.flat( 1 ) ) ] ); } - return printSet( `${ indent }=>`, [ ...new Set( filtered.flat( 1 ) ) ] ); + return printSet( `${ indent }=>`, [ ...new Set( requirementsMet.flat( 1 ) ) ], [] ); }; } @@ -215,9 +241,10 @@ class Requirement { * @param {string[]} reviewers - Reviewers to test against. * @returns {boolean} Whether the requirement is satisfied. */ - async isSatisfied( reviewers ) { + async needsReviewsFrom( reviewers ) { core.info( 'Checking reviewers...' ); - return ( await this.reviewerFilter( reviewers ) ).length > 0; + const checkNeededTeams = await this.reviewerFilter( reviewers ); + return checkNeededTeams.neededTeams; } }