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
This commit is contained in:
Geoffrey Testelin 2020-11-20 12:48:33 +01:00 committed by GitHub
parent 9b82e8c1ef
commit 324009e5d0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 407 additions and 19 deletions

1
.gitignore vendored
View File

@ -2,3 +2,4 @@
node_modules/ node_modules/
lib/ lib/
__tests__/runner/* __tests__/runner/*
.idea

View File

@ -2,10 +2,10 @@ module.exports = {
clearMocks: true, clearMocks: true,
moduleFileExtensions: ['js', 'ts'], moduleFileExtensions: ['js', 'ts'],
testEnvironment: 'node', testEnvironment: 'node',
testMatch: ['**/*.test.ts'], testMatch: ['**/*.test.ts', '**/*.spec.ts'],
testRunner: 'jest-circus/runner', testRunner: 'jest-circus/runner',
transform: { transform: {
'^.+\\.ts$': 'ts-jest' '^.+\\.ts$': 'ts-jest'
}, },
verbose: true verbose: true
} };

20
package-lock.json generated
View File

@ -1337,6 +1337,21 @@
"integrity": "sha1-7ihweulOEdK4J7y+UnC86n8+ce4=", "integrity": "sha1-7ihweulOEdK4J7y+UnC86n8+ce4=",
"dev": true "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": { "@types/node": {
"version": "14.10.0", "version": "14.10.0",
"resolved": "https://registry.npmjs.org/@types/node/-/node-14.10.0.tgz", "resolved": "https://registry.npmjs.org/@types/node/-/node-14.10.0.tgz",
@ -6423,6 +6438,11 @@
"integrity": "sha512-JNvd8XER9GQX0v2qJgsaN/mzFCNA5BRe/j8JN9d+tWyGLSodKQHKFicdwNYzWwI3wjRnaKPsGj1XkBjx/F96DQ==", "integrity": "sha512-JNvd8XER9GQX0v2qJgsaN/mzFCNA5BRe/j8JN9d+tWyGLSodKQHKFicdwNYzWwI3wjRnaKPsGj1XkBjx/F96DQ==",
"dev": true "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": { "lodash.memoize": {
"version": "4.1.2", "version": "4.1.2",
"resolved": "https://registry.npmjs.org/lodash.memoize/-/lodash.memoize-4.1.2.tgz", "resolved": "https://registry.npmjs.org/lodash.memoize/-/lodash.memoize-4.1.2.tgz",

View File

@ -9,6 +9,7 @@
"format": "prettier --write **/*.ts", "format": "prettier --write **/*.ts",
"format-check": "prettier --check **/*.ts", "format-check": "prettier --check **/*.ts",
"lint": "eslint src/**/*.ts", "lint": "eslint src/**/*.ts",
"lint:fix": "eslint src/**/*.ts --fix",
"pack": "ncc build", "pack": "ncc build",
"test": "jest", "test": "jest",
"all": "npm run build && npm run format && npm run lint && npm run pack && npm test" "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/core": "^1.2.6",
"@actions/github": "^4.0.0", "@actions/github": "^4.0.0",
"@octokit/rest": "^18.0.4", "@octokit/rest": "^18.0.4",
"lodash.deburr": "^4.1.0",
"semver": "^7.3.2" "semver": "^7.3.2"
}, },
"devDependencies": { "devDependencies": {
"@types/semver": "^7.3.1",
"@types/jest": "^26.0.10", "@types/jest": "^26.0.10",
"@types/lodash.deburr": "^4.1.6",
"@types/node": "^14.10.0", "@types/node": "^14.10.0",
"@types/semver": "^7.3.1",
"@typescript-eslint/parser": "^3.10.1", "@typescript-eslint/parser": "^3.10.1",
"@vercel/ncc": "^0.24.0", "@vercel/ncc": "^0.24.0",
"eslint": "^7.7.0", "eslint": "^7.7.0",

View File

@ -1,6 +1,8 @@
import * as core from '@actions/core'; import * as core from '@actions/core';
import {context, getOctokit} from '@actions/github'; import {context, getOctokit} from '@actions/github';
import {GetResponseTypeFromEndpointMethod} from '@octokit/types'; import {GetResponseTypeFromEndpointMethod} from '@octokit/types';
import {isLabeled} from './functions/is-labeled';
import {labelsToList} from './functions/labels-to-list';
export interface Issue { export interface Issue {
title: string; title: string;
@ -131,7 +133,7 @@ export class IssueProcessor {
const closeLabel: string = isPr const closeLabel: string = isPr
? this.options.closePrLabel ? this.options.closePrLabel
: this.options.closeIssueLabel; : this.options.closeIssueLabel;
const exemptLabels = IssueProcessor.parseCommaSeparatedString( const exemptLabels: string[] = labelsToList(
isPr ? this.options.exemptPrLabels : this.options.exemptIssueLabels isPr ? this.options.exemptPrLabels : this.options.exemptIssueLabels
); );
const skipMessage = isPr const skipMessage = isPr
@ -157,7 +159,7 @@ export class IssueProcessor {
if ( if (
exemptLabels.some((exemptLabel: string) => exemptLabels.some((exemptLabel: string) =>
IssueProcessor.isLabeled(issue, exemptLabel) isLabeled(issue, exemptLabel)
) )
) { ) {
core.info(`Skipping ${issueType} because it has an exempt label`); core.info(`Skipping ${issueType} because it has an exempt label`);
@ -165,7 +167,7 @@ export class IssueProcessor {
} }
// does this issue have a stale label? // does this issue have a stale label?
let isStale = IssueProcessor.isLabeled(issue, staleLabel); let isStale = isLabeled(issue, staleLabel);
// should this issue be marked stale? // should this issue be marked stale?
const shouldBeStale = !IssueProcessor.updatedSince( const shouldBeStale = !IssueProcessor.updatedSince(
@ -486,12 +488,6 @@ export class IssueProcessor {
return staleLabeledEvent.created_at; 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 { private static updatedSince(timestamp: string, num_days: number): boolean {
const daysInMillis = 1000 * 60 * 60 * 24 * num_days; const daysInMillis = 1000 * 60 * 60 * 24 * num_days;
const millisSinceLastUpdated = const millisSinceLastUpdated =
@ -499,11 +495,4 @@ export class IssueProcessor {
return millisSinceLastUpdated <= daysInMillis; 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());
}
} }

View File

@ -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);
});
});
});
});

View File

@ -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>} issue A GitHub issue containing some labels
* @param {Readonly<string>} 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<Issue>,
label: Readonly<string>
): boolean {
return !!issue.labels.find((issueLabel: Readonly<Label>): boolean => {
return cleanLabel(label) === cleanLabel(issueLabel.name);
});
}
function cleanLabel(label: Readonly<string>): string {
return deburr(label.toLowerCase());
}

View File

@ -0,0 +1,141 @@
import {labelsToList} from './labels-to-list';
describe('labelsToList()', (): void => {
let labels: string;
describe('when the given labels is empty', (): void => {
beforeEach((): void => {
labels = '';
});
it('should return an empty list of labels', (): void => {
expect.assertions(1);
const result = labelsToList(labels);
expect(result).toStrictEqual([]);
});
});
describe('when the given labels is a simple label', (): void => {
beforeEach((): void => {
labels = 'label';
});
it('should return a list of one label', (): void => {
expect.assertions(1);
const result = labelsToList(labels);
expect(result).toStrictEqual(['label']);
});
});
describe('when the given labels is a label with extra spaces before and after', (): void => {
beforeEach((): void => {
labels = ' label ';
});
it('should return a list of one label and remove all spaces before and after', (): void => {
expect.assertions(1);
const result = labelsToList(labels);
expect(result).toStrictEqual(['label']);
});
});
describe('when the given labels is a kebab case label', (): void => {
beforeEach((): void => {
labels = 'kebab-case-label';
});
it('should return a list of one label', (): void => {
expect.assertions(1);
const result = labelsToList(labels);
expect(result).toStrictEqual(['kebab-case-label']);
});
});
describe('when the given labels is two kebab case labels separated with a comma', (): void => {
beforeEach((): void => {
labels = 'kebab-case-label-1,kebab-case-label-2';
});
it('should return a list of two labels', (): void => {
expect.assertions(1);
const result = labelsToList(labels);
expect(result).toStrictEqual([
'kebab-case-label-1',
'kebab-case-label-2'
]);
});
});
describe('when the given labels is a multiple word label', (): void => {
beforeEach((): void => {
labels = 'label like a sentence';
});
it('should return a list of one label', (): void => {
expect.assertions(1);
const result = labelsToList(labels);
expect(result).toStrictEqual(['label like a sentence']);
});
});
describe('when the given labels is two multiple word labels separated with a comma', (): void => {
beforeEach((): void => {
labels = 'label like a sentence, another label like a sentence';
});
it('should return a list of two labels', (): void => {
expect.assertions(1);
const result = labelsToList(labels);
expect(result).toStrictEqual([
'label like a sentence',
'another label like a sentence'
]);
});
});
describe('when the given labels is a multiple word label with %20 spaces', (): void => {
beforeEach((): void => {
labels = 'label%20like%20a%20sentence';
});
it('should return a list of one label', (): void => {
expect.assertions(1);
const result = labelsToList(labels);
expect(result).toStrictEqual(['label%20like%20a%20sentence']);
});
});
describe('when the given labels is two multiple word labels with %20 spaces separated with a comma', (): void => {
beforeEach((): void => {
labels =
'label%20like%20a%20sentence,another%20label%20like%20a%20sentence';
});
it('should return a list of two labels', (): void => {
expect.assertions(1);
const result = labelsToList(labels);
expect(result).toStrictEqual([
'label%20like%20a%20sentence',
'another%20label%20like%20a%20sentence'
]);
});
});
});

View File

@ -0,0 +1,23 @@
/**
* @description
* Transform a string of comma separated labels
* to an array of labels
*
* @example
* labelsToList('label') => ['label']
* labelsToList('label,label') => ['label', 'label']
* labelsToList('kebab-label') => ['kebab-label']
* labelsToList('kebab%20label') => ['kebab%20label']
* labelsToList('label with words') => ['label with words']
*
* @param {Readonly<string>} labels A string of comma separated labels
*
* @return {string[]} A list of labels
*/
export function labelsToList(labels: Readonly<string>): string[] {
if (!labels.length) {
return [];
}
return labels.split(',').map(l => l.trim());
}