From 324009e5d028d042dc995e77140cfb983b7f0528 Mon Sep 17 00:00:00 2001 From: Geoffrey Testelin Date: Fri, 20 Nov 2020 12:48:33 +0100 Subject: [PATCH] fix(label): allow to use spaces inside the labels (#199) * chore(git): ignore .idea folder to avoid adding WebStorm files * test(jest): find all spec files as well * refactor(labels): create a dedicated function to parse the labels at first I thought that the parseCommaSeparatedString method was causing the issue so I move it to a dedicated file to test it since it was private also because I think that this repo could have more clean code and code splitting anyway this was not the root of the #98 issue :/ * fix(label): allow to use spaces inside the labels * docs(isLabeled): add JSDoc * chore(npm): add lint:fix script --- .gitignore | 1 + jest.config.js | 4 +- package-lock.json | 20 +++ package.json | 5 +- src/IssueProcessor.ts | 21 +-- src/functions/is-labeled.spec.ts | 187 +++++++++++++++++++++++++++ src/functions/is-labeled.ts | 24 ++++ src/functions/labels-to-list.spec.ts | 141 ++++++++++++++++++++ src/functions/labels-to-list.ts | 23 ++++ 9 files changed, 407 insertions(+), 19 deletions(-) create mode 100644 src/functions/is-labeled.spec.ts create mode 100644 src/functions/is-labeled.ts create mode 100644 src/functions/labels-to-list.spec.ts create mode 100644 src/functions/labels-to-list.ts diff --git a/.gitignore b/.gitignore index c8b37f36..1ae337ac 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ node_modules/ lib/ __tests__/runner/* +.idea diff --git a/jest.config.js b/jest.config.js index 563d4ccb..a2737419 100644 --- a/jest.config.js +++ b/jest.config.js @@ -2,10 +2,10 @@ module.exports = { clearMocks: true, moduleFileExtensions: ['js', 'ts'], testEnvironment: 'node', - testMatch: ['**/*.test.ts'], + testMatch: ['**/*.test.ts', '**/*.spec.ts'], testRunner: 'jest-circus/runner', transform: { '^.+\\.ts$': 'ts-jest' }, verbose: true -} \ No newline at end of file +}; diff --git a/package-lock.json b/package-lock.json index c66d409a..387ba7e4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1337,6 +1337,21 @@ "integrity": "sha1-7ihweulOEdK4J7y+UnC86n8+ce4=", "dev": true }, + "@types/lodash": { + "version": "4.14.162", + "resolved": "https://registry.npmjs.org/@types/lodash/-/lodash-4.14.162.tgz", + "integrity": "sha512-alvcho1kRUnnD1Gcl4J+hK0eencvzq9rmzvFPRmP5rPHx9VVsJj6bKLTATPVf9ktgv4ujzh7T+XWKp+jhuODig==", + "dev": true + }, + "@types/lodash.deburr": { + "version": "4.1.6", + "resolved": "https://registry.npmjs.org/@types/lodash.deburr/-/lodash.deburr-4.1.6.tgz", + "integrity": "sha512-LsdZUIBM/YDQ4+w4/PSq2KTRRuCmBic48vzzKD7PHw1uIsKMWizwDtk0fMdqYmo9/iLMhHiFh2ol0eqhjT0VTg==", + "dev": true, + "requires": { + "@types/lodash": "*" + } + }, "@types/node": { "version": "14.10.0", "resolved": "https://registry.npmjs.org/@types/node/-/node-14.10.0.tgz", @@ -6423,6 +6438,11 @@ "integrity": "sha512-JNvd8XER9GQX0v2qJgsaN/mzFCNA5BRe/j8JN9d+tWyGLSodKQHKFicdwNYzWwI3wjRnaKPsGj1XkBjx/F96DQ==", "dev": true }, + "lodash.deburr": { + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/lodash.deburr/-/lodash.deburr-4.1.0.tgz", + "integrity": "sha1-3bG7s+8HRYwBd7oH3hRCLLAz/5s=" + }, "lodash.memoize": { "version": "4.1.2", "resolved": "https://registry.npmjs.org/lodash.memoize/-/lodash.memoize-4.1.2.tgz", diff --git a/package.json b/package.json index 8de99c69..54092208 100644 --- a/package.json +++ b/package.json @@ -9,6 +9,7 @@ "format": "prettier --write **/*.ts", "format-check": "prettier --check **/*.ts", "lint": "eslint src/**/*.ts", + "lint:fix": "eslint src/**/*.ts --fix", "pack": "ncc build", "test": "jest", "all": "npm run build && npm run format && npm run lint && npm run pack && npm test" @@ -28,12 +29,14 @@ "@actions/core": "^1.2.6", "@actions/github": "^4.0.0", "@octokit/rest": "^18.0.4", + "lodash.deburr": "^4.1.0", "semver": "^7.3.2" }, "devDependencies": { - "@types/semver": "^7.3.1", "@types/jest": "^26.0.10", + "@types/lodash.deburr": "^4.1.6", "@types/node": "^14.10.0", + "@types/semver": "^7.3.1", "@typescript-eslint/parser": "^3.10.1", "@vercel/ncc": "^0.24.0", "eslint": "^7.7.0", diff --git a/src/IssueProcessor.ts b/src/IssueProcessor.ts index 28faf975..77f3b09f 100644 --- a/src/IssueProcessor.ts +++ b/src/IssueProcessor.ts @@ -1,6 +1,8 @@ import * as core from '@actions/core'; import {context, getOctokit} from '@actions/github'; import {GetResponseTypeFromEndpointMethod} from '@octokit/types'; +import {isLabeled} from './functions/is-labeled'; +import {labelsToList} from './functions/labels-to-list'; export interface Issue { title: string; @@ -131,7 +133,7 @@ export class IssueProcessor { const closeLabel: string = isPr ? this.options.closePrLabel : this.options.closeIssueLabel; - const exemptLabels = IssueProcessor.parseCommaSeparatedString( + const exemptLabels: string[] = labelsToList( isPr ? this.options.exemptPrLabels : this.options.exemptIssueLabels ); const skipMessage = isPr @@ -157,7 +159,7 @@ export class IssueProcessor { if ( exemptLabels.some((exemptLabel: string) => - IssueProcessor.isLabeled(issue, exemptLabel) + isLabeled(issue, exemptLabel) ) ) { core.info(`Skipping ${issueType} because it has an exempt label`); @@ -165,7 +167,7 @@ export class IssueProcessor { } // does this issue have a stale label? - let isStale = IssueProcessor.isLabeled(issue, staleLabel); + let isStale = isLabeled(issue, staleLabel); // should this issue be marked stale? const shouldBeStale = !IssueProcessor.updatedSince( @@ -486,12 +488,6 @@ export class IssueProcessor { return staleLabeledEvent.created_at; } - private static isLabeled(issue: Issue, label: string): boolean { - const labelComparer: (l: Label) => boolean = l => - label.localeCompare(l.name, undefined, {sensitivity: 'accent'}) === 0; - return issue.labels.filter(labelComparer).length > 0; - } - private static updatedSince(timestamp: string, num_days: number): boolean { const daysInMillis = 1000 * 60 * 60 * 24 * num_days; const millisSinceLastUpdated = @@ -499,11 +495,4 @@ export class IssueProcessor { return millisSinceLastUpdated <= daysInMillis; } - - private static parseCommaSeparatedString(s: string): string[] { - // String.prototype.split defaults to [''] when called on an empty string - // In this case, we'd prefer to just return an empty array indicating no labels - if (!s.length) return []; - return s.split(',').map(l => l.trim()); - } } diff --git a/src/functions/is-labeled.spec.ts b/src/functions/is-labeled.spec.ts new file mode 100644 index 00000000..fbabb3db --- /dev/null +++ b/src/functions/is-labeled.spec.ts @@ -0,0 +1,187 @@ +import {Issue} from '../IssueProcessor'; +import {isLabeled} from './is-labeled'; + +describe('isLabeled()', (): void => { + let issue: Issue; + let label: string; + + describe('when the given issue contains no label', (): void => { + beforeEach((): void => { + issue = ({ + labels: [] + } as unknown) as Issue; + }); + + describe('when the given label is a simple label', (): void => { + beforeEach((): void => { + label = 'label'; + }); + + it('should return false', (): void => { + expect.assertions(1); + + const result = isLabeled(issue, label); + + expect(result).toStrictEqual(false); + }); + }); + }); + + describe('when the given issue contains a simple label', (): void => { + beforeEach((): void => { + issue = { + labels: [ + { + name: 'label' + } + ] + } as Issue; + }); + + describe('when the given label is a simple label', (): void => { + beforeEach((): void => { + label = 'label'; + }); + + it('should return true', (): void => { + expect.assertions(1); + + const result = isLabeled(issue, label); + + expect(result).toStrictEqual(true); + }); + }); + }); + + describe('when the given issue contains a kebab case label', (): void => { + beforeEach((): void => { + issue = { + labels: [ + { + name: 'kebab-case-label' + } + ] + } as Issue; + }); + + describe('when the given label is a kebab case label', (): void => { + beforeEach((): void => { + label = 'kebab-case-label'; + }); + + it('should return true', (): void => { + expect.assertions(1); + + const result = isLabeled(issue, label); + + expect(result).toStrictEqual(true); + }); + }); + }); + + describe('when the given issue contains a multiple word label', (): void => { + beforeEach((): void => { + issue = { + labels: [ + { + name: 'label like a sentence' + } + ] + } as Issue; + }); + + describe('when the given label is a multiple word label', (): void => { + beforeEach((): void => { + label = 'label like a sentence'; + }); + + it('should return true', (): void => { + expect.assertions(1); + + const result = isLabeled(issue, label); + + expect(result).toStrictEqual(true); + }); + }); + }); + + describe('when the given issue contains a multiple word label with %20 spaces', (): void => { + beforeEach((): void => { + issue = { + labels: [ + { + name: 'label%20like%20a%20sentence' + } + ] + } as Issue; + }); + + describe('when the given label is a multiple word label with %20 spaces', (): void => { + beforeEach((): void => { + label = 'label%20like%20a%20sentence'; + }); + + it('should return true', (): void => { + expect.assertions(1); + + const result = isLabeled(issue, label); + + expect(result).toStrictEqual(true); + }); + }); + }); + + describe('when the given issue contains a label wih diacritical marks', (): void => { + beforeEach((): void => { + issue = { + labels: [ + { + name: 'déjà vu' + } + ] + } as Issue; + }); + + describe('when the given issue contains a label', (): void => { + beforeEach((): void => { + label = 'deja vu'; + }); + + it('should return true', (): void => { + expect.assertions(1); + + const result = isLabeled(issue, label); + + expect(result).toStrictEqual(true); + }); + }); + + describe('when the given issue contains an uppercase label', (): void => { + beforeEach((): void => { + label = 'DEJA VU'; + }); + + it('should return true', (): void => { + expect.assertions(1); + + const result = isLabeled(issue, label); + + expect(result).toStrictEqual(true); + }); + }); + + describe('when the given issue contains a label wih diacritical marks', (): void => { + beforeEach((): void => { + label = 'déjà vu'; + }); + + it('should return true', (): void => { + expect.assertions(1); + + const result = isLabeled(issue, label); + + expect(result).toStrictEqual(true); + }); + }); + }); +}); diff --git a/src/functions/is-labeled.ts b/src/functions/is-labeled.ts new file mode 100644 index 00000000..97ee5e37 --- /dev/null +++ b/src/functions/is-labeled.ts @@ -0,0 +1,24 @@ +import deburr from 'lodash.deburr'; +import {Issue, Label} from '../IssueProcessor'; + +/** + * @description + * Check if the label is listed as a label of the issue + * + * @param {Readonly} issue A GitHub issue containing some labels + * @param {Readonly} label The label to check the presence with + * + * @return {boolean} Return true when the given label is also in the issue labels + */ +export function isLabeled( + issue: Readonly, + label: Readonly +): boolean { + return !!issue.labels.find((issueLabel: Readonly