Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions .npmignore

This file was deleted.

39 changes: 19 additions & 20 deletions lib/bot.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ const {
/**
* @typedef { import('./types').Octokit } Octokit
* @typedef { import('./types').Probot } Probot
* @typedef { import('./types').Context } AnyContext
* @typedef { import('./types').Context<any> } AnyContext
*
* @typedef { import('probot').Context<import('@octokit/webhooks').EventPayloads.WebhookPayloadCheckSuite> } CheckSuiteContext
* @typedef { import('probot').Context<import('@octokit/webhooks').EventPayloads.WebhookPayloadCheckSuite> } CheckSuiteCompletedContext
* @typedef { import('probot').Context<import('@octokit/webhooks').EventPayloads.WebhookPayloadPullRequest> } PullRequestContext
* @typedef { import('probot').Context<import('@octokit/webhooks').EventPayloads.WebhookPayloadPullRequestReview> } PullRequestReviewContext
* @typedef { import('probot').Context<import('@octokit/webhooks').EventPayloads.WebhookPayloadPullRequestReview> } PullRequestReviewSubmittedContext
* @typedef { import('probot').Context<import('@octokit/webhooks').EventPayloads.WebhookPayloadStatus> } StatusContext
*/

Expand All @@ -41,15 +41,16 @@ module.exports = app => {

app.on('status', handleStatus);

app.on('*', handleAnyEvent);
app.onAny(handleAnyEvent);
};

/**
* Handle `check_suite.completed` event.
*
* @param {CheckSuiteContext} context
* @param {CheckSuiteCompletedContext} context
*/
async function handleCheckSuiteCompleted(context) {

logEvent(context);

const {
Expand Down Expand Up @@ -92,7 +93,7 @@ async function handleCheckSuiteCompleted(context) {
/**
* Handle `pull_request_review.submitted` event.
*
* @param {PullRequestReviewContext} context
* @param {PullRequestReviewSubmittedContext} context
*/
async function handlePullRequestReviewSubmitted(context) {
logEvent(context);
Expand All @@ -102,17 +103,23 @@ async function handlePullRequestReviewSubmitted(context) {
pull_request
} = context.payload;

if (review.state !== 'approved') {
context.log.info(context.repo({
pull_number: pull_request.number
}), `skipping: review in state ${review.state}`);
const ctx = context.repo({
pull_number: pull_request.number
});

if (review.state !== 'approved') {
context.log.info(ctx, `skipping: review in state ${review.state}`);
return;
}

// fetch pull request with full details
const pullRequest = await findPullRequestByShallowRef(context, pull_request);

if (!pullRequest) {
context.log.warn(ctx, 'skipping: no PR found for shallow ref');
return;
}

// check, whether PR can be merged
await checkMerge(context, pullRequest);
}
Expand Down Expand Up @@ -172,11 +179,7 @@ async function handleAnyEvent(context) {
payload
} = context;

const {
action
} = payload;

const eventName = action ? `${name}.${action}` : name;
const eventName = 'action' in payload ? `${name}.${payload.action}` : name;

log(eventName, {
type: 'event',
Expand All @@ -194,11 +197,7 @@ function logEvent(context) {
payload
} = context;

const {
action
} = payload;

const eventName = action ? `${name}.${action}` : name;
const eventName = 'action' in payload ? `${name}.${payload.action}` : name;

context.log.debug(`processing ${eventName}`);
}
52 changes: 42 additions & 10 deletions lib/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ async function findPullRequestByStatus(context, status) {
* @param {Context} context
* @param {{ number: number }} pullRequestReference
*
* @return {Promise<PullRequest>}
* @return {Promise<PullRequest|null>}
*/
async function findPullRequestByShallowRef(context, pullRequestReference) {

Expand All @@ -223,19 +223,21 @@ async function findPullRequestByShallowRef(context, pullRequestReference) {
pull_number: number
}));

return pullRequest;
}
// TODO(nikku): assert + cleanup types
if (!pullRequest.user) {
return null;
}

return /** @type PullRequest */ (pullRequest);
}

/**
* Return the collaborator reviews present on the given pull request
*
* @param {Context} context
* @param {PullRequest} pullRequest
*
* @return {Promise<Review[]>} reviews
*/
async function getCollaboratorReviews(context, pullRequest) {
async function listReviews(context, pullRequest) {

const {
number: pull_number
Expand All @@ -249,6 +251,23 @@ async function getCollaboratorReviews(context, pullRequest) {
per_page: 100
}));

return /** @type Review[] */ (
allReviews.filter(review => review.user && review.user.login)
);
}

/**
* Return the collaborator reviews present on the given pull request
*
* @param {Context} context
* @param {PullRequest} pullRequest
*
* @return {Promise<Review[]>} reviews
*/
async function getCollaboratorReviews(context, pullRequest) {

const allReviews = await listReviews(context, pullRequest);

const effectiveReviews = getEffectiveReviews(allReviews);

const collaboratorReviews = [];
Expand Down Expand Up @@ -432,7 +451,11 @@ function isStatusRelevantSuite(suite, pullRequest) {
conclusion
} = suite;

const pullRequests = /** @type { { number: number }[] } */ (suite.pull_requests);
const pullRequests = suite.pull_requests || [];

if (status === null) {
return false;
}

// discard queued checks that do not relate to the
// target pull request, for example bogus dependabot checks
Expand Down Expand Up @@ -542,6 +565,10 @@ async function getStatusApproval(context, pullRequest) {
}
}

context.log.trace({
chech_suites: relevantSuites
}, 'examining suites');

// at this point, we got at least a single check_suite
const checkSuitesStatus = relevantSuites.reduce(combineSuiteStatus, null);

Expand Down Expand Up @@ -719,8 +746,7 @@ async function checkMerge(context, pullRequest) {
} else if (isMergeCheckError(error)) {
context.log.debug(ctx, `skipping: ${error.message}`);
} else {
context.log.error(ctx, 'skipping: merge check error');
context.log.error(error);
throw error;
}

return false;
Expand Down Expand Up @@ -928,6 +954,10 @@ function getEffectiveReviewTeams(teams, reviewers, requestedTeams) {
*/
async function getTeamsWithMembers(context, pullRequest, reviewTeams) {

const ctx = context.repo({
pull_number: pullRequest.number
});

const org = getPullRequestTargetOrg(pullRequest);

const teamMembers = await Promise.all(reviewTeams.map(async (teamSlug) => {
Expand All @@ -939,6 +969,8 @@ async function getTeamsWithMembers(context, pullRequest, reviewTeams) {
team_slug: teamSlug
}).catch(error => {

context.log.debug(ctx, `failed to fetch team ${teamSlug}`, error);

// app is missing missing permissions
if (error.status === 403) {
throw MergeCheckError(`failed to fetch team ${org}/${teamSlug} (status=403)`);
Expand All @@ -952,7 +984,7 @@ async function getTeamsWithMembers(context, pullRequest, reviewTeams) {
throw error;
});

return members;
return /** @type { { login: string }[] } */ (members.filter(m => m));
}));

return reviewTeams.map(
Expand Down
14 changes: 9 additions & 5 deletions lib/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ import {

export type Octokit = InstanceType<typeof ProbotOctokit>;

export { Context, Octokit, Probot };
export {
Context,
Octokit,
Probot
};

export type Review = {
id: number;
Expand Down Expand Up @@ -51,15 +55,15 @@ export type Status = {
export type Suite = {
id: number;
head_sha: string;
status: string;
conclusion: string;
status: string | null;
conclusion: string | null;
repository: {
name: string;
owner: {
login: string;
};
} | null;
};
pull_requests: unknown[];
pull_requests: { number: number }[] | null;
};

export type PullRequest = {
Expand Down
Loading