From 7f340a46f348b7b0cfb3581e6ab6324b82882428 Mon Sep 17 00:00:00 2001 From: Geoffrey Testelin Date: Sun, 17 Jan 2021 02:13:19 +0100 Subject: [PATCH] Remove the stale label when labeled with an exempt one (#268) * refactor(issue): create a dedicated function to remove the stale label * refactor: prefix private methods with _ to make it consistent with others methods * feat(label): remove the stale label when labeled with an exempt one closes #136 * chore: fix logger issues due to rebase @hross I think there is a room for improvement regarding the class creation of the issue logger (code duplication) but I do not see how to do it without changing a lot of stuff; do you have an idea? * test: use strict equal and move the new test in a more logical position * docs(readme): fix parsing error of the default values in the table prettier was not liking the previous syntax --- README.md | 18 +++---- __tests__/main.test.ts | 46 ++++++++++++++++-- src/IssueProcessor.ts | 106 ++++++++++++++++++++++++----------------- 3 files changed, 112 insertions(+), 58 deletions(-) diff --git a/README.md b/README.md index 5d84ebfd..869d8b00 100644 --- a/README.md +++ b/README.md @@ -27,29 +27,29 @@ $ npm test | Input | Description | Usage | | --------------------------- | ------------------------------------------------------------------------------------ | -------- | | `repo-token` | PAT(Personal Access Token) for authorizing repository. | Optional | -| `days-before-stale` | Idle number of days before marking an issue/pr as stale. \*Defaults to **60** | Optional | +| `days-before-stale` | Idle number of days before marking an issue/pr as stale. _Defaults to **60**_ | Optional | | `days-before-issue-stale` | Idle number of days before marking an issue as stale (override `days-before-stale`). | Optional | | `days-before-pr-stale` | Idle number of days before marking an pr as stale (override `days-before-stale`). | Optional | -| `days-before-close` | Idle number of days before closing an stale issue/pr. \*Defaults to **7\*** | Optional | +| `days-before-close` | Idle number of days before closing an stale issue/pr. _Defaults to **7**_ | Optional | | `days-before-issue-close` | Idle number of days before closing an stale issue (override `days-before-close`). | Optional | | `days-before-pr-close` | Idle number of days before closing an stale pr (override `days-before-close`). | Optional | | `stale-issue-message` | Message to post on the stale issue. | Optional | | `stale-pr-message` | Message to post on the stale pr. | Optional | | `close-issue-message` | Message to post on the stale issue while closing it. | Optional | | `close-pr-message` | Message to post on the stale pr while closing it. | Optional | -| `stale-issue-label` | Label to apply on the stale issue. \*Defaults to **stale\*** | Optional | +| `stale-issue-label` | Label to apply on the stale issue. _Defaults to **stale**_ | Optional | | `close-issue-label` | Label to apply on closing issue. | Optional | | `stale-pr-label` | Label to apply on the stale pr. | Optional | | `close-pr-label` | Label to apply on the closing pr. | Optional | | `exempt-issue-labels` | Labels on an issue exempted from being marked as stale. | Optional | | `exempt-pr-labels` | Labels on the pr exempted from being marked as stale. | Optional | | `only-labels` | Only labels checked for stale issue/pr. | Optional | -| `operations-per-run` | Maximum number of operations per run. \*Defaults to **30\*** | Optional | -| `remove-stale-when-updated` | Remove stale label from issue/pr on updates or comments. \*Defaults to **true\*** | Optional | -| `debug-only` | Dry-run on action. \*Defaults to **false\*** | Optional | -| `ascending` | Order to get issues/pr. \*Defaults to **false\*** | Optional | -| `skip-stale-issue-message` | Skip adding stale message on stale issue. \*Defaults to **false\*** | Optional | -| `skip-stale-pr-message` | Skip adding stale message on stale pr. \*Defaults to **false\*** | Optional | +| `operations-per-run` | Maximum number of operations per run. _Defaults to **30**_ | Optional | +| `remove-stale-when-updated` | Remove stale label from issue/pr on updates or comments. _Defaults to **true**_ | Optional | +| `debug-only` | Dry-run on action. _Defaults to **false**_ | Optional | +| `ascending` | Order to get issues/pr. _Defaults to **false**_ | Optional | +| `skip-stale-issue-message` | Skip adding stale message on stale issue. _Defaults to **false**_ | Optional | +| `skip-stale-pr-message` | Skip adding stale message on stale pr. _Defaults to **false**_ | Optional | ### Usage diff --git a/__tests__/main.test.ts b/__tests__/main.test.ts index 3de60e56..ee388b6f 100644 --- a/__tests__/main.test.ts +++ b/__tests__/main.test.ts @@ -797,6 +797,7 @@ test('stale locked prs will not be closed', async () => { }); test('exempt issue labels will not be marked stale', async () => { + expect.assertions(3); const TestIssueList: Issue[] = [ generateIssue(1, 'My first issue', '2020-01-01T17:00:00Z', false, [ 'Exempt' @@ -817,8 +818,9 @@ test('exempt issue labels will not be marked stale', async () => { // process our fake issue list await processor.processIssues(1); - expect(processor.staleIssues.length).toEqual(0); - expect(processor.closedIssues.length).toEqual(0); + expect(processor.staleIssues.length).toStrictEqual(0); + expect(processor.closedIssues.length).toStrictEqual(0); + expect(processor.removedLabelIssues.length).toStrictEqual(0); }); test('exempt issue labels will not be marked stale (multi issue label with spaces)', async () => { @@ -892,6 +894,42 @@ test('exempt pr labels will not be marked stale', async () => { expect(processor.staleIssues.length).toEqual(2); // PR should get processed even though it has an exempt **issue** label }); +test('exempt issue labels will not be marked stale and will remove the existing stale label', async () => { + expect.assertions(3); + + const TestIssueList: Issue[] = [ + generateIssue(1, 'My first issue', '2020-01-01T17:00:00Z', false, [ + 'Exempt', + 'Stale' + ]) + ]; + + const opts = {...DefaultProcessorOptions}; + opts.exemptIssueLabels = 'Exempt'; + + const processor = new IssueProcessor( + opts, + async () => 'abot', + async p => (p == 1 ? TestIssueList : []), + async (num: number, dt: string) => [ + { + user: { + login: 'notme', + type: 'User' + } + } + ], // return a fake comment to indicate there was an update + async (issue: Issue, label: string) => new Date().toDateString() + ); + + // process our fake issue list + await processor.processIssues(1); + + expect(processor.staleIssues.length).toStrictEqual(0); + expect(processor.closedIssues.length).toStrictEqual(0); + expect(processor.removedLabelIssues.length).toStrictEqual(1); +}); + test('stale issues should not be closed if days is set to -1', async () => { const TestIssueList: Issue[] = [ generateIssue(1, 'My first issue', '2020-01-01T17:00:00Z', false, [ @@ -1150,7 +1188,7 @@ test('skips stale message on issues when skip-stale-issue-message is set', async ); // for sake of testing, mocking private function - const markSpy = jest.spyOn(processor as any, 'markStale'); + const markSpy = jest.spyOn(processor as any, '_markStale'); await processor.processIssues(1); @@ -1195,7 +1233,7 @@ test('skips stale message on prs when skip-stale-pr-message is set', async () => ); // for sake of testing, mocking private function - const markSpy = jest.spyOn(processor as any, 'markStale'); + const markSpy = jest.spyOn(processor as any, '_markStale'); await processor.processIssues(1); diff --git a/src/IssueProcessor.ts b/src/IssueProcessor.ts index e1a87152..2d110679 100644 --- a/src/IssueProcessor.ts +++ b/src/IssueProcessor.ts @@ -80,7 +80,7 @@ const logger: Logger = new Logger(); * Handle processing of issues for staleness/closure. */ export class IssueProcessor { - private static updatedSince(timestamp: string, num_days: number): boolean { + private static _updatedSince(timestamp: string, num_days: number): boolean { const daysInMillis = 1000 * 60 * 60 * 24 * num_days; const millisSinceLastUpdated = new Date().getTime() - new Date(timestamp).getTime(); @@ -114,19 +114,19 @@ export class IssueProcessor { this.client = getOctokit(options.repoToken); if (getActor) { - this.getActor = getActor; + this._getActor = getActor; } if (getIssues) { - this.getIssues = getIssues; + this._getIssues = getIssues; } if (listIssueComments) { - this.listIssueComments = listIssueComments; + this._listIssueComments = listIssueComments; } if (getLabelCreationDate) { - this.getLabelCreationDate = getLabelCreationDate; + this._getLabelCreationDate = getLabelCreationDate; } if (this.options.debugOnly) { @@ -138,10 +138,10 @@ export class IssueProcessor { async processIssues(page = 1): Promise { // get the next batch of issues - const issues: Issue[] = await this.getIssues(page); + const issues: Issue[] = await this._getIssues(page); this.operationsLeft -= 1; - const actor: string = await this.getActor(); + const actor: string = await this._getActor(); if (issues.length <= 0) { logger.info('---'); @@ -204,18 +204,7 @@ export class IssueProcessor { continue; // don't process locked issues } - if ( - exemptLabels.some((exemptLabel: string) => - isLabeled(issue, exemptLabel) - ) - ) { - issueLogger.info( - `Skipping ${issueType} because it has an exempt label` - ); - continue; // don't process exempt issues - } - - // does this issue have a stale label? + // Does this issue have a stale label? let isStale: boolean = isLabeled(issue, staleLabel); if (isStale) { @@ -224,8 +213,24 @@ export class IssueProcessor { issueLogger.info(`This issue hasn't a stale label`); } + if ( + exemptLabels.some((exemptLabel: Readonly): boolean => + isLabeled(issue, exemptLabel) + ) + ) { + if (isStale) { + issueLogger.info(`An exempt label was added after the stale label.`); + await this._removeStaleLabel(issue, staleLabel); + } + + issueLogger.info( + `Skipping ${issueType} because it has an exempt label` + ); + continue; // don't process exempt issues + } + // should this issue be marked stale? - const shouldBeStale = !IssueProcessor.updatedSince( + const shouldBeStale = !IssueProcessor._updatedSince( issue.updated_at, this.options.daysBeforeStale ); @@ -235,14 +240,14 @@ export class IssueProcessor { issueLogger.info( `Marking ${issueType} stale because it was last updated on ${issue.updated_at} and it does not have a stale label` ); - await this.markStale(issue, staleMessage, staleLabel, skipMessage); + await this._markStale(issue, staleMessage, staleLabel, skipMessage); isStale = true; // this issue is now considered stale } // process the issue if it was marked stale if (isStale) { issueLogger.info(`Found a stale ${issueType}`); - await this.processStaleIssue( + await this._processStaleIssue( issue, issueType, staleLabel, @@ -263,7 +268,7 @@ export class IssueProcessor { } // handle all of the stale issue logic when we find a stale issue - private async processStaleIssue( + private async _processStaleIssue( issue: Issue, issueType: IssueType, staleLabel: string, @@ -273,12 +278,12 @@ export class IssueProcessor { ) { const issueLogger: IssueLogger = new IssueLogger(issue); const markedStaleOn: string = - (await this.getLabelCreationDate(issue, staleLabel)) || issue.updated_at; + (await this._getLabelCreationDate(issue, staleLabel)) || issue.updated_at; issueLogger.info( `Issue #${issue.number} marked stale on: ${markedStaleOn}` ); - const issueHasComments: boolean = await this.hasCommentsSince( + const issueHasComments: boolean = await this._hasCommentsSince( issue, markedStaleOn, actor @@ -298,7 +303,7 @@ export class IssueProcessor { issueLogger.info(`Days before issue close: ${daysBeforeClose}`); } - const issueHasUpdate: boolean = IssueProcessor.updatedSince( + const issueHasUpdate: boolean = IssueProcessor._updatedSince( issue.updated_at, daysBeforeClose ); @@ -308,10 +313,7 @@ export class IssueProcessor { // should we un-stale this issue? if (this.options.removeStaleWhenUpdated && issueHasComments) { - issueLogger.info( - `Issue #${issue.number} is no longer stale. Removing stale label.` - ); - await this.removeLabel(issue, staleLabel); + await this._removeStaleLabel(issue, staleLabel); } // now start closing logic @@ -323,13 +325,13 @@ export class IssueProcessor { issueLogger.info( `Closing ${issueType} because it was last updated on ${issue.updated_at}` ); - await this.closeIssue(issue, closeMessage, closeLabel); + await this._closeIssue(issue, closeMessage, closeLabel); if (this.options.deleteBranch && issue.pull_request) { issueLogger.info( `Deleting branch for #${issue.number} as delete-branch option was specified` ); - await this.deleteBranch(issue); + await this._deleteBranch(issue); this.deletedBranchIssues.push(issue); } } else { @@ -340,7 +342,7 @@ export class IssueProcessor { } // checks to see if a given issue is still stale (has had activity on it) - private async hasCommentsSince( + private async _hasCommentsSince( issue: Issue, sinceDate: string, actor: string @@ -356,7 +358,7 @@ export class IssueProcessor { } // find any comments since the date - const comments = await this.listIssueComments(issue.number, sinceDate); + const comments = await this._listIssueComments(issue.number, sinceDate); const filteredComments = comments.filter( comment => comment.user.type === 'User' && comment.user.login !== actor @@ -371,7 +373,7 @@ export class IssueProcessor { } // grab comments for an issue since a given date - private async listIssueComments( + private async _listIssueComments( issueNumber: number, sinceDate: string ): Promise { @@ -391,7 +393,7 @@ export class IssueProcessor { } // get the actor from the GitHub token or context - private async getActor(): Promise { + private async _getActor(): Promise { let actor; try { actor = await this.client.users.getAuthenticated(); @@ -403,7 +405,7 @@ export class IssueProcessor { } // grab issues from github in baches of 100 - private async getIssues(page: number): Promise { + private async _getIssues(page: number): Promise { // generate type for response const endpoint = this.client.issues.listForRepo; type OctoKitIssueList = GetResponseTypeFromEndpointMethod; @@ -428,7 +430,7 @@ export class IssueProcessor { } // Mark an issue as stale with a comment and a label - private async markStale( + private async _markStale( issue: Issue, staleMessage: string, staleLabel: string, @@ -477,7 +479,7 @@ export class IssueProcessor { } // Close an issue based on staleness - private async closeIssue( + private async _closeIssue( issue: Issue, closeMessage?: string, closeLabel?: string @@ -532,9 +534,10 @@ export class IssueProcessor { } } - private async getPullRequest(issue: Issue): Promise { + private async _getPullRequest( + issue: Issue + ): Promise { const issueLogger: IssueLogger = new IssueLogger(issue); - this.operationsLeft -= 1; try { @@ -553,7 +556,7 @@ export class IssueProcessor { } // Delete the branch on closed pull request - private async deleteBranch(issue: Issue): Promise { + private async _deleteBranch(issue: Issue): Promise { const issueLogger: IssueLogger = new IssueLogger(issue); issueLogger.info( @@ -564,7 +567,7 @@ export class IssueProcessor { return; } - const pullRequest = await this.getPullRequest(issue); + const pullRequest = await this._getPullRequest(issue); if (!pullRequest) { issueLogger.info( @@ -594,7 +597,7 @@ export class IssueProcessor { } // Remove a label from an issue - private async removeLabel(issue: Issue, label: string): Promise { + private async _removeLabel(issue: Issue, label: string): Promise { const issueLogger: IssueLogger = new IssueLogger(issue); issueLogger.info(`Removing label "${label}" from issue #${issue.number}`); @@ -622,7 +625,7 @@ export class IssueProcessor { // returns the creation date of a given label on an issue (or nothing if no label existed) ///see https://developer.github.com/v3/activity/events/ - private async getLabelCreationDate( + private async _getLabelCreationDate( issue: Issue, label: string ): Promise { @@ -677,4 +680,17 @@ export class IssueProcessor { ? this.options.daysBeforeClose : this.options.daysBeforePrClose; } + + private async _removeStaleLabel( + issue: Readonly, + staleLabel: Readonly + ): Promise { + const issueLogger: IssueLogger = new IssueLogger(issue); + + issueLogger.info( + `Issue #${issue.number} is no longer stale. Removing stale label.` + ); + + return this._removeLabel(issue, staleLabel); + } }