Repository: actions/stale (ts)

Issue:
Remove stale label when issue is tagged with exempt label after marking stale
If an exempt label, e.g. `wip` is added **after** the action ran and marked an issue as `stale` it would be nice to remove the `stale` label by the action in the next run. Right now the code seems to only consider exempt labels for skipping but not removing previously labeled issues.

Pull request description:
Remove the stale label when labeled with an exempt one 
**Changes:**

- [x] Prefix private methods with `_`
- [x] Create a new method to remove the stale label (avoid code duplication)
- [x] Before skipping the process when the issue/pr has an exempt label it will check if it has a stale label and remove it if it does (costing one operation)
- [x] Change the syntax to display the default values in italic in the readme so that prettier is no longer messing with them

Closes #136 

Patch:
diff --git a/README.md b/README.md
index 5d84ebf..869d8b0 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/src/IssueProcessor.ts b/src/IssueProcessor.ts
index e1a8715..2d11067 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<number> {
     // 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,28 +204,33 @@ export class IssueProcessor {
         continue; // don't process locked issues
       }
 
+      // Does this issue have a stale label?
+      let isStale: boolean = isLabeled(issue, staleLabel);
+
+      if (isStale) {
+        issueLogger.info(`This issue has a stale label`);
+      } else {
+        issueLogger.info(`This issue hasn't a stale label`);
+      }
+
       if (
-        exemptLabels.some((exemptLabel: string) =>
+        exemptLabels.some((exemptLabel: Readonly<string>): 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
       }
 
-      // does this issue have a stale label?
-      let isStale: boolean = isLabeled(issue, staleLabel);
-
-      if (isStale) {
-        issueLogger.info(`This issue has a stale label`);
-      } else {
-        issueLogger.info(`This issue hasn't a stale label`);
-      }
-
       // 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<Comment[]> {
@@ -391,7 +393,7 @@ export class IssueProcessor {
   }
 
   // get the actor from the GitHub token or context
-  private async getActor(): Promise<string> {
+  private async _getActor(): Promise<string> {
     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<Issue[]> {
+  private async _getIssues(page: number): Promise<Issue[]> {
     // generate type for response
     const endpoint = this.client.issues.listForRepo;
     type OctoKitIssueList = GetResponseTypeFromEndpointMethod<typeof endpoint>;
@@ -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<PullRequest | undefined> {
+  private async _getPullRequest(
+    issue: Issue
+  ): Promise<PullRequest | undefined> {
     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<void> {
+  private async _deleteBranch(issue: Issue): Promise<void> {
     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<void> {
+  private async _removeLabel(issue: Issue, label: string): Promise<void> {
     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<string | undefined> {
@@ -677,4 +680,17 @@ export class IssueProcessor {
       ? this.options.daysBeforeClose
       : this.options.daysBeforePrClose;
   }
+
+  private async _removeStaleLabel(
+    issue: Readonly<Issue>,
+    staleLabel: Readonly<string>
+  ): Promise<void> {
+    const issueLogger: IssueLogger = new IssueLogger(issue);
+
+    issueLogger.info(
+      `Issue #${issue.number} is no longer stale. Removing stale label.`
+    );
+
+    return this._removeLabel(issue, staleLabel);
+  }
 }


Test patch:
diff --git a/__tests__/main.test.ts b/__tests__/main.test.ts
index 3de60e5..ee388b6 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);