feat(draft-pr): add new option to not process PRs which are in draft (#539)

* chore(assignees): add logs

* feat(draft-pr): add new option to not process PRs which are in draft

* refactor(draft-pr): create a dedicated class to handle the logic

* chore(index): update index file
This commit is contained in:
Geoffrey Testelin 2021-09-20 15:37:32 +02:00 committed by GitHub
parent 303465a5d2
commit 9912fa74d1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 367 additions and 72 deletions

View File

@ -75,6 +75,7 @@ Every argument is optional.
| [exempt-all-assignees](#exempt-all-assignees) | Exempt all issues/PRs with assignees from stale | `false` |
| [exempt-all-issue-assignees](#exempt-all-issue-assignees) | Override [exempt-all-assignees](#exempt-all-assignees) for issues only | |
| [exempt-all-pr-assignees](#exempt-all-pr-assignees) | Override [exempt-all-assignees](#exempt-all-assignees) for PRs only | |
| [exempt-draft-pr](#exempt-draft-pr) | Skip the stale action for draft PRs | `false` |
| [enable-statistics](#enable-statistics) | Display statistics in the logs | `true` |
| [ignore-updates](#ignore-updates) | Any update (update/comment) can reset the stale idle time on the issues/PRs | `false` |
| [ignore-issue-updates](#ignore-issue-updates) | Override [ignore-updates](#ignore-updates) for issues only | |
@ -472,6 +473,14 @@ Override [exempt-all-assignees](#exempt-all-assignees) but only to exempt the pu
Default value: unset
#### exempt-draft-pr
If set to `true`, the pull requests currently in draft will not be marked as stale automatically.
⚠️ This option consume one operation per pull request to process because we need to fetch the pull request with the GitHub API to know if it's a draft one or not.
Default value: `false`
Required Permission: `pull-requests: read`
#### enable-statistics
Collects and display statistics at the end of the stale workflow logs to get a summary of what happened during the run.

View File

@ -2,6 +2,7 @@ import {Issue} from '../../src/classes/issue';
import {IssuesProcessor} from '../../src/classes/issues-processor';
import {IComment} from '../../src/interfaces/comment';
import {IIssuesProcessorOptions} from '../../src/interfaces/issues-processor-options';
import {IPullRequest} from '../../src/interfaces/pull-request';
export class IssuesProcessorMock extends IssuesProcessor {
constructor(
@ -14,7 +15,8 @@ export class IssuesProcessorMock extends IssuesProcessor {
getLabelCreationDate?: (
issue: Issue,
label: string
) => Promise<string | undefined>
) => Promise<string | undefined>,
getPullRequest?: (issue: Issue) => Promise<IPullRequest | undefined | void>
) {
super(options);
@ -29,5 +31,9 @@ export class IssuesProcessorMock extends IssuesProcessor {
if (getLabelCreationDate) {
this.getLabelCreationDate = getLabelCreationDate;
}
if (getPullRequest) {
this.getPullRequest = getPullRequest;
}
}
}

View File

@ -49,5 +49,6 @@ export const DefaultProcessorOptions: IIssuesProcessorOptions = Object.freeze({
labelsToAddWhenUnstale: '',
ignoreUpdates: false,
ignoreIssueUpdates: undefined,
ignorePrUpdates: undefined
ignorePrUpdates: undefined,
exemptDraftPr: false
});

View File

@ -0,0 +1,139 @@
import {Issue} from '../src/classes/issue';
import {IIssue} from '../src/interfaces/issue';
import {IIssuesProcessorOptions} from '../src/interfaces/issues-processor-options';
import {IPullRequest} from '../src/interfaces/pull-request';
import {IssuesProcessorMock} from './classes/issues-processor-mock';
import {DefaultProcessorOptions} from './constants/default-processor-options';
import {generateIssue} from './functions/generate-issue';
let issuesProcessorBuilder: IssuesProcessorBuilder;
let issuesProcessor: IssuesProcessorMock;
describe('exempt-draft-pr option', (): void => {
beforeEach((): void => {
issuesProcessorBuilder = new IssuesProcessorBuilder();
});
describe('when the option "exempt-draft-pr" is disabled', (): void => {
beforeEach((): void => {
issuesProcessorBuilder.processDraftPr();
});
test('should stale the pull request', async (): Promise<void> => {
expect.assertions(1);
issuesProcessor = issuesProcessorBuilder
.toStalePrs([
{
number: 10
}
])
.build();
await issuesProcessor.processIssues();
expect(issuesProcessor.staleIssues).toHaveLength(1);
});
});
describe('when the option "exempt-draft-pr" is enabled', (): void => {
beforeEach((): void => {
issuesProcessorBuilder.exemptDraftPr();
});
test('should not stale the pull request', async (): Promise<void> => {
expect.assertions(1);
issuesProcessor = issuesProcessorBuilder
.toStalePrs([
{
number: 20
}
])
.build();
await issuesProcessor.processIssues();
expect(issuesProcessor.staleIssues).toHaveLength(0);
});
});
});
class IssuesProcessorBuilder {
private _options: IIssuesProcessorOptions = {
...DefaultProcessorOptions
};
private _issues: Issue[] = [];
processDraftPr(): IssuesProcessorBuilder {
this._options.exemptDraftPr = false;
return this;
}
exemptDraftPr(): IssuesProcessorBuilder {
this._options.exemptDraftPr = true;
return this;
}
issuesOrPrs(issues: Partial<IIssue>[]): IssuesProcessorBuilder {
this._issues = issues.map(
(issue: Readonly<Partial<IIssue>>, index: Readonly<number>): Issue =>
generateIssue(
this._options,
issue.number ?? index,
issue.title ?? 'dummy-title',
issue.updated_at ?? new Date().toDateString(),
issue.created_at ?? new Date().toDateString(),
!!issue.pull_request,
issue.labels ? issue.labels.map(label => label.name) : []
)
);
return this;
}
prs(issues: Partial<IIssue>[]): IssuesProcessorBuilder {
this.issuesOrPrs(
issues.map((issue: Readonly<Partial<IIssue>>): Partial<IIssue> => {
return {
...issue,
pull_request: {key: 'value'}
};
})
);
return this;
}
toStalePrs(issues: Partial<IIssue>[]): IssuesProcessorBuilder {
this.prs(
issues.map((issue: Readonly<Partial<IIssue>>): Partial<IIssue> => {
return {
...issue,
updated_at: '2020-01-01T17:00:00Z',
created_at: '2020-01-01T17:00:00Z'
};
})
);
return this;
}
build(): IssuesProcessorMock {
return new IssuesProcessorMock(
this._options,
async p => (p === 1 ? this._issues : []),
async () => [],
async () => new Date().toDateString(),
async (): Promise<IPullRequest> => {
return Promise.resolve({
number: 0,
draft: true,
head: {
ref: 'ref'
}
});
}
);
}
}

View File

@ -164,6 +164,10 @@ inputs:
description: 'Exempt all pull requests with assignees from being marked as stale. Override "exempt-all-assignees" option regarding only the pull requests.'
default: ''
required: false
exempt-draft-pr:
description: 'Exempt draft pull requests from being marked as stale. Default to false.'
default: 'false'
required: false
enable-statistics:
description: 'Display some statistics at the end regarding the stale workflow (only when the logs are enabled).'
default: 'true'

126
dist/index.js vendored
View File

@ -141,6 +141,55 @@ class Assignees {
exports.Assignees = Assignees;
/***/ }),
/***/ 854:
/***/ (function(__unused_webpack_module, exports, __nccwpck_require__) {
"use strict";
var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) {
function adopt(value) { return value instanceof P ? value : new P(function (resolve) { resolve(value); }); }
return new (P || (P = Promise))(function (resolve, reject) {
function fulfilled(value) { try { step(generator.next(value)); } catch (e) { reject(e); } }
function rejected(value) { try { step(generator["throw"](value)); } catch (e) { reject(e); } }
function step(result) { result.done ? resolve(result.value) : adopt(result.value).then(fulfilled, rejected); }
step((generator = generator.apply(thisArg, _arguments || [])).next());
});
};
Object.defineProperty(exports, "__esModule", ({ value: true }));
exports.ExemptDraftPullRequest = void 0;
const option_1 = __nccwpck_require__(5931);
const logger_service_1 = __nccwpck_require__(1973);
const issue_logger_1 = __nccwpck_require__(2984);
class ExemptDraftPullRequest {
constructor(options, issue) {
this._options = options;
this._issue = issue;
this._issueLogger = new issue_logger_1.IssueLogger(issue);
}
shouldExemptDraftPullRequest(pullRequestCallback) {
return __awaiter(this, void 0, void 0, function* () {
if (this._issue.isPullRequest) {
if (this._options.exemptDraftPr) {
this._issueLogger.info(`The option ${this._issueLogger.createOptionLink(option_1.Option.ExemptDraftPr)} is enabled`);
const pullRequest = yield pullRequestCallback();
if ((pullRequest === null || pullRequest === void 0 ? void 0 : pullRequest.draft) === true) {
this._issueLogger.info(logger_service_1.LoggerService.white('└──'), `Skip the $$type draft checks`);
return true;
}
else {
this._issueLogger.info(logger_service_1.LoggerService.white('└──'), `Continuing the process for this $$type because it is not a draft`);
}
}
}
return false;
});
}
}
exports.ExemptDraftPullRequest = ExemptDraftPullRequest;
/***/ }),
/***/ 2935:
@ -298,6 +347,7 @@ const should_mark_when_stale_1 = __nccwpck_require__(2461);
const words_to_list_1 = __nccwpck_require__(1883);
const assignees_1 = __nccwpck_require__(7236);
const ignore_updates_1 = __nccwpck_require__(2935);
const exempt_draft_pull_request_1 = __nccwpck_require__(854);
const issue_1 = __nccwpck_require__(4783);
const issue_logger_1 = __nccwpck_require__(2984);
const logger_1 = __nccwpck_require__(6212);
@ -404,6 +454,16 @@ class IssuesProcessor {
const daysBeforeStale = issue.isPullRequest
? this._getDaysBeforePrStale()
: this._getDaysBeforeIssueStale();
if (issue.state === 'closed') {
issueLogger.info(`Skipping this $$type because it is closed`);
IssuesProcessor._endIssueProcessing(issue);
return; // Don't process closed issues
}
if (issue.locked) {
issueLogger.info(`Skipping this $$type because it is locked`);
IssuesProcessor._endIssueProcessing(issue);
return; // Don't process locked issues
}
const onlyLabels = words_to_list_1.wordsToList(this._getOnlyLabels(issue));
if (onlyLabels.length > 0) {
issueLogger.info(`The option ${issueLogger.createOptionLink(option_1.Option.OnlyLabels)} was specified to only process issues and pull requests with all those labels (${logger_service_1.LoggerService.cyan(onlyLabels.length)})`);
@ -426,16 +486,6 @@ class IssuesProcessor {
}
issueLogger.info(`Days before $$type stale: ${logger_service_1.LoggerService.cyan(daysBeforeStale)}`);
const shouldMarkAsStale = should_mark_when_stale_1.shouldMarkWhenStale(daysBeforeStale);
if (issue.state === 'closed') {
issueLogger.info(`Skipping this $$type because it is closed`);
IssuesProcessor._endIssueProcessing(issue);
return; // Don't process closed issues
}
if (issue.locked) {
issueLogger.info(`Skipping this $$type because it is locked`);
IssuesProcessor._endIssueProcessing(issue);
return; // Don't process locked issues
}
// Try to remove the close label when not close/locked issue or PR
yield this._removeCloseLabel(issue, closeLabel);
if (this.options.startDate) {
@ -503,6 +553,16 @@ class IssuesProcessor {
IssuesProcessor._endIssueProcessing(issue);
return; // Don't process exempt assignees
}
// Ignore draft PR
// Note that this check is so far below because it cost one read operation
// So it's simply better to do all the stale checks which don't cost more operation before this one
const exemptDraftPullRequest = new exempt_draft_pull_request_1.ExemptDraftPullRequest(this.options, issue);
if (yield exemptDraftPullRequest.shouldExemptDraftPullRequest(() => __awaiter(this, void 0, void 0, function* () {
return this.getPullRequest(issue);
}))) {
IssuesProcessor._endIssueProcessing(issue);
return; // Don't process draft PR
}
// Determine if this issue needs to be marked stale first
if (!issue.isStale) {
issueLogger.info(`This $$type is not stale`);
@ -624,6 +684,25 @@ class IssuesProcessor {
return staleLabeledEvent.created_at;
});
}
getPullRequest(issue) {
var _a;
return __awaiter(this, void 0, void 0, function* () {
const issueLogger = new issue_logger_1.IssueLogger(issue);
try {
this._consumeIssueOperation(issue);
(_a = this._statistics) === null || _a === void 0 ? void 0 : _a.incrementFetchedPullRequestsCount();
const pullRequest = yield this.client.pulls.get({
owner: github_1.context.repo.owner,
repo: github_1.context.repo.repo,
pull_number: issue.number
});
return pullRequest.data;
}
catch (error) {
issueLogger.error(`Error when getting this $$type: ${error.message}`);
}
});
}
// handle all of the stale issue logic when we find a stale issue
_processStaleIssue(issue, staleLabel, staleMessage, labelsToAddWhenUnstale, labelsToRemoveWhenUnstale, closeMessage, closeLabel) {
return __awaiter(this, void 0, void 0, function* () {
@ -664,7 +743,7 @@ class IssuesProcessor {
issueLogger.info(`Closing $$type because it was last updated on: ${logger_service_1.LoggerService.cyan(issue.updated_at)}`);
yield this._closeIssue(issue, closeMessage, closeLabel);
if (this.options.deleteBranch && issue.pull_request) {
issueLogger.info(`Deleting the branch since the option ${issueLogger.createOptionLink(option_1.Option.DeleteBranch)} was specified`);
issueLogger.info(`Deleting the branch since the option ${issueLogger.createOptionLink(option_1.Option.DeleteBranch)} is enabled`);
yield this._deleteBranch(issue);
this.deletedBranchIssues.push(issue);
}
@ -795,32 +874,13 @@ class IssuesProcessor {
}
});
}
_getPullRequest(issue) {
var _a;
return __awaiter(this, void 0, void 0, function* () {
const issueLogger = new issue_logger_1.IssueLogger(issue);
try {
this._consumeIssueOperation(issue);
(_a = this._statistics) === null || _a === void 0 ? void 0 : _a.incrementFetchedPullRequestsCount();
const pullRequest = yield this.client.pulls.get({
owner: github_1.context.repo.owner,
repo: github_1.context.repo.repo,
pull_number: issue.number
});
return pullRequest.data;
}
catch (error) {
issueLogger.error(`Error when getting this $$type: ${error.message}`);
}
});
}
// Delete the branch on closed pull request
_deleteBranch(issue) {
var _a;
return __awaiter(this, void 0, void 0, function* () {
const issueLogger = new issue_logger_1.IssueLogger(issue);
issueLogger.info(`Delete branch from closed $$type - ${issue.title}`);
const pullRequest = yield this._getPullRequest(issue);
const pullRequest = yield this.getPullRequest(issue);
if (!pullRequest) {
issueLogger.info(`Not deleting this branch as no pull request was found for this $$type`);
return;
@ -1807,6 +1867,7 @@ var Option;
Option["IgnoreUpdates"] = "ignore-updates";
Option["IgnoreIssueUpdates"] = "ignore-issue-updates";
Option["IgnorePrUpdates"] = "ignore-pr-updates";
Option["ExemptDraftPr"] = "exempt-draft-pr";
})(Option = exports.Option || (exports.Option = {}));
@ -2116,7 +2177,8 @@ function _getAndValidateArgs() {
labelsToAddWhenUnstale: core.getInput('labels-to-add-when-unstale'),
ignoreUpdates: core.getInput('ignore-updates') === 'true',
ignoreIssueUpdates: _toOptionalBoolean('ignore-issue-updates'),
ignorePrUpdates: _toOptionalBoolean('ignore-pr-updates')
ignorePrUpdates: _toOptionalBoolean('ignore-pr-updates'),
exemptDraftPr: core.getInput('exempt-draft-pr') === 'true'
};
for (const numberInput of [
'days-before-stale',

View File

@ -0,0 +1,51 @@
import {Option} from '../enums/option';
import {IIssuesProcessorOptions} from '../interfaces/issues-processor-options';
import {IPullRequest} from '../interfaces/pull-request';
import {LoggerService} from '../services/logger.service';
import {Issue} from './issue';
import {IssueLogger} from './loggers/issue-logger';
export class ExemptDraftPullRequest {
private readonly _options: IIssuesProcessorOptions;
private readonly _issue: Issue;
private readonly _issueLogger: IssueLogger;
constructor(options: Readonly<IIssuesProcessorOptions>, issue: Issue) {
this._options = options;
this._issue = issue;
this._issueLogger = new IssueLogger(issue);
}
async shouldExemptDraftPullRequest(
pullRequestCallback: () => Promise<IPullRequest | undefined | void>
): Promise<boolean> {
if (this._issue.isPullRequest) {
if (this._options.exemptDraftPr) {
this._issueLogger.info(
`The option ${this._issueLogger.createOptionLink(
Option.ExemptDraftPr
)} is enabled`
);
const pullRequest: IPullRequest | undefined | void =
await pullRequestCallback();
if (pullRequest?.draft === true) {
this._issueLogger.info(
LoggerService.white('└──'),
`Skip the $$type draft checks`
);
return true;
} else {
this._issueLogger.info(
LoggerService.white('└──'),
`Continuing the process for this $$type because it is not a draft`
);
}
}
}
return false;
}
}

View File

@ -60,7 +60,8 @@ describe('Issue', (): void => {
labelsToAddWhenUnstale: '',
ignoreUpdates: false,
ignoreIssueUpdates: undefined,
ignorePrUpdates: undefined
ignorePrUpdates: undefined,
exemptDraftPr: false
};
issueInterface = {
title: 'dummy-title',

View File

@ -17,6 +17,7 @@ import {IIssuesProcessorOptions} from '../interfaces/issues-processor-options';
import {IPullRequest} from '../interfaces/pull-request';
import {Assignees} from './assignees';
import {IgnoreUpdates} from './ignore-updates';
import {ExemptDraftPullRequest} from './exempt-draft-pull-request';
import {Issue} from './issue';
import {IssueLogger} from './loggers/issue-logger';
import {Logger} from './loggers/logger';
@ -207,6 +208,19 @@ export class IssuesProcessor {
const daysBeforeStale: number = issue.isPullRequest
? this._getDaysBeforePrStale()
: this._getDaysBeforeIssueStale();
if (issue.state === 'closed') {
issueLogger.info(`Skipping this $$type because it is closed`);
IssuesProcessor._endIssueProcessing(issue);
return; // Don't process closed issues
}
if (issue.locked) {
issueLogger.info(`Skipping this $$type because it is locked`);
IssuesProcessor._endIssueProcessing(issue);
return; // Don't process locked issues
}
const onlyLabels: string[] = wordsToList(this._getOnlyLabels(issue));
if (onlyLabels.length > 0) {
@ -260,18 +274,6 @@ export class IssuesProcessor {
const shouldMarkAsStale: boolean = shouldMarkWhenStale(daysBeforeStale);
if (issue.state === 'closed') {
issueLogger.info(`Skipping this $$type because it is closed`);
IssuesProcessor._endIssueProcessing(issue);
return; // Don't process closed issues
}
if (issue.locked) {
issueLogger.info(`Skipping this $$type because it is locked`);
IssuesProcessor._endIssueProcessing(issue);
return; // Don't process locked issues
}
// Try to remove the close label when not close/locked issue or PR
await this._removeCloseLabel(issue, closeLabel);
@ -397,6 +399,23 @@ export class IssuesProcessor {
return; // Don't process exempt assignees
}
// Ignore draft PR
// Note that this check is so far below because it cost one read operation
// So it's simply better to do all the stale checks which don't cost more operation before this one
const exemptDraftPullRequest: ExemptDraftPullRequest =
new ExemptDraftPullRequest(this.options, issue);
if (
await exemptDraftPullRequest.shouldExemptDraftPullRequest(
async (): Promise<IPullRequest | undefined | void> => {
return this.getPullRequest(issue);
}
)
) {
IssuesProcessor._endIssueProcessing(issue);
return; // Don't process draft PR
}
// Determine if this issue needs to be marked stale first
if (!issue.isStale) {
issueLogger.info(`This $$type is not stale`);
@ -574,6 +593,25 @@ export class IssuesProcessor {
return staleLabeledEvent.created_at;
}
async getPullRequest(issue: Issue): Promise<IPullRequest | undefined | void> {
const issueLogger: IssueLogger = new IssueLogger(issue);
try {
this._consumeIssueOperation(issue);
this._statistics?.incrementFetchedPullRequestsCount();
const pullRequest = await this.client.pulls.get({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: issue.number
});
return pullRequest.data;
} catch (error) {
issueLogger.error(`Error when getting this $$type: ${error.message}`);
}
}
// handle all of the stale issue logic when we find a stale issue
private async _processStaleIssue(
issue: Issue,
@ -666,7 +704,7 @@ export class IssuesProcessor {
issueLogger.info(
`Deleting the branch since the option ${issueLogger.createOptionLink(
Option.DeleteBranch
)} was specified`
)} is enabled`
);
await this._deleteBranch(issue);
this.deletedBranchIssues.push(issue);
@ -830,34 +868,14 @@ export class IssuesProcessor {
}
}
private async _getPullRequest(
issue: Issue
): Promise<IPullRequest | undefined | void> {
const issueLogger: IssueLogger = new IssueLogger(issue);
try {
this._consumeIssueOperation(issue);
this._statistics?.incrementFetchedPullRequestsCount();
const pullRequest = await this.client.pulls.get({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: issue.number
});
return pullRequest.data;
} catch (error) {
issueLogger.error(`Error when getting this $$type: ${error.message}`);
}
}
// Delete the branch on closed pull request
private async _deleteBranch(issue: Issue): Promise<void> {
const issueLogger: IssueLogger = new IssueLogger(issue);
issueLogger.info(`Delete branch from closed $$type - ${issue.title}`);
const pullRequest = await this._getPullRequest(issue);
const pullRequest: IPullRequest | undefined | void =
await this.getPullRequest(issue);
if (!pullRequest) {
issueLogger.info(

View File

@ -45,5 +45,6 @@ export enum Option {
LabelsToAddWhenUnstale = 'labels-to-add-when-unstale',
IgnoreUpdates = 'ignore-updates',
IgnoreIssueUpdates = 'ignore-issue-updates',
IgnorePrUpdates = 'ignore-pr-updates'
IgnorePrUpdates = 'ignore-pr-updates',
ExemptDraftPr = 'exempt-draft-pr'
}

View File

@ -50,4 +50,5 @@ export interface IIssuesProcessorOptions {
ignoreUpdates: boolean;
ignoreIssueUpdates: boolean | undefined;
ignorePrUpdates: boolean | undefined;
exemptDraftPr: boolean;
}

View File

@ -3,4 +3,5 @@ export interface IPullRequest {
head: {
ref: string;
};
draft: boolean;
}

View File

@ -86,7 +86,8 @@ function _getAndValidateArgs(): IIssuesProcessorOptions {
labelsToAddWhenUnstale: core.getInput('labels-to-add-when-unstale'),
ignoreUpdates: core.getInput('ignore-updates') === 'true',
ignoreIssueUpdates: _toOptionalBoolean('ignore-issue-updates'),
ignorePrUpdates: _toOptionalBoolean('ignore-pr-updates')
ignorePrUpdates: _toOptionalBoolean('ignore-pr-updates'),
exemptDraftPr: core.getInput('exempt-draft-pr') === 'true'
};
for (const numberInput of [