From ab74b59fab68a681fe74f759dd688db2be00c83e Mon Sep 17 00:00:00 2001 From: Javyer Date: Thu, 26 Jan 2023 13:33:04 +0100 Subject: [PATCH] feat: added support for labels (#53) Added support for labeling. The system can now evaluate the new issues based on the existence of labels, allowing us to combine unlabeled and labeled issue behavior. This commit closes #45 Updated `README` to explain how to set up the labeling system and showing an example of how to combine different configurations in one file. Extended unit tests to handle all predicted behaviors. --- README.md | 99 +++++++++++++++++++++++++- action.yml | 4 ++ src/github/issueKit.ts | 3 +- src/github/types.ts | 4 +- src/main.ts | 15 ++-- src/synchronizer.ts | 126 +++++++++++++++++++++++++++++++--- src/test/synchronizer.test.ts | 123 +++++++++++++++++++++++++++++++++ 7 files changed, 350 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index f88056f..79717ce 100644 --- a/README.md +++ b/README.md @@ -33,7 +33,6 @@ on: issues: types: - opened - - reopened - labeled workflow_dispatch: inputs: @@ -54,12 +53,49 @@ jobs: # This is a Personal Access Token and it needs to have the following permissions # - "read:org": used to read the project's board # - "write:org": used to assign issues to the project's board - PROJECT_TOKEN: ${{ steps.generate_token.outputs.token }} + PROJECT_TOKEN: ${{ secrets.PROJECT_TOKEN }} # The number of the project which the issues will be synced to # You can find this in https://github.com/orgs/@ORGANIZATION/projects/ project: 4 + # Optional, the project field to modify with a new value + # Found more in https://docs.github.com/en/issues/planning-and-tracking-with-projects/understanding-fields/about-single-select-fields + project_field: Status + # Optional unless that project_field was set up. Then this field is required. + # The value to modify in the project field + project_value: To do + # Optional, labels to work with. Read below to see how to configure it. + # If this value is set, the action will be applied only to issues with such label(s). + labels: | + duplicate + bug + invalid ``` You can generate a new token [in your user's token dashboard](https://github.com/settings/tokens/new). + +### Warning about labels field +The labels field accepts an array or a single value, [but only with some particular format](https://github.com/actions/toolkit/issues/184#issuecomment-1198653452), so it is important to follow it. +It accepts either: +```yml +labels: my label name +``` +or an array of labels using a `pipe`: +```yml +labels: | + some label + another label + third label +``` +It **does not** support the following type of arrays: +```yml +# not this one +labels: + - some label + - another one + +# also doesn't support this one +labels: ["some label", "another one"] +``` + ### Using a GitHub app instead of a PAT In some cases, specially in big organizations, it is more organized to use a GitHub app to authenticate, as it allows us to give it permissions per repository and we can fine-grain them even better. If you wish to do that, you need to create a GitHub app with the following permissions: - Repository permissions: @@ -88,6 +124,65 @@ Because this project is intended to be used with a token we need to do an extra PROJECT_TOKEN: ${{ steps.generate_token.outputs.token }} ``` +## Combining labels and different fields + +As the system works different when there are labels available, you can set up steps to work with different cases. +Let's do an example: +- You have 3 cases you want to handle: + - When an new issue is created, assign it to `project 1` and set the `Status` to `To do`. + - When an issue is labeled as `DevOps` or `CI` assign it to `project 2` and set the `Status` to `Needs reviewing`. + - When an issue is labeled as `Needs planning` assign it to `project 1` and set the `Condition` to `Review on next sprint`. + +```yml +name: GitHub Issue Sync + +on: + issues: + types: + - opened + - labeled + workflow_dispatch: + inputs: + excludeClosed: + description: 'Exclude closed issues in the sync.' + type: boolean + default: true + +jobs: + sync: + runs-on: ubuntu-latest + steps: + - name: Sync new issues + uses: paritytech/github-issue-sync@master + with: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PROJECT_TOKEN: ${{ secrets.PROJECT_TOKEN }} + project: 1 + project_field: Status + project_value: To do + - name: Sync DevOps issues + uses: paritytech/github-issue-sync@master + with: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PROJECT_TOKEN: ${{ secrets.PROJECT_TOKEN }} + project: 2 + project_field: Status + project_value: Needs reviewing + labels: | + DevOps + CI + - name: Sync issues for the next sprint + uses: paritytech/github-issue-sync@master + with: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PROJECT_TOKEN: ${{ secrets.PROJECT_TOKEN }} + project: 1 + project_field: Condition + project_value: Review on next sprint + labels: Needs planning +``` +With this configuration you will be able to handle all of the aforementioned cases. + ## Development To work on this app, you require - `Node 18.x` diff --git a/action.yml b/action.yml index 0adf031..59d2f7d 100644 --- a/action.yml +++ b/action.yml @@ -17,6 +17,10 @@ inputs: required: false description: The value which will be set in the project_field type: string + labels: + required: false + description: array of labels required to execute the action. See Readme for input format. + type: string GITHUB_TOKEN: required: true type: string diff --git a/src/github/issueKit.ts b/src/github/issueKit.ts index a60299f..101b8f6 100644 --- a/src/github/issueKit.ts +++ b/src/github/issueKit.ts @@ -12,10 +12,11 @@ export class IssueApi implements IIssues { return issueData.data.state === "open" ? "open" : "closed"; } - async getAllIssues(excludeClosed: boolean): Promise { + async getAllIssues(excludeClosed: boolean, labels?: string[]): Promise { const allIssues = await this.octokit.rest.issues.listForRepo({ ...this.repoData, state: excludeClosed ? "open" : "all", + labels: labels?.join(","), }); return allIssues.data; } diff --git a/src/github/types.ts b/src/github/types.ts index e75955f..71ceb71 100644 --- a/src/github/types.ts +++ b/src/github/types.ts @@ -1,6 +1,6 @@ export type Repository = { owner: string; repo: string }; -export type Issue = { number: number; node_id: string }; +export type Issue = { number: number; node_id?: string; labels?: (string | { name?: string })[] }; /** Key value pair with the name/id of a field and the name/id of its value */ export type FieldValues = { field: string; value: string }; @@ -43,7 +43,7 @@ export interface IIssues { * Returns the node_id for all the issues available in the repository * @param includeClosed exclude issues which are closed from the data agregation. */ - getAllIssues(excludeClosed: boolean): Promise; + getAllIssues(excludeClosed: boolean, labels?: string[]): Promise; } export interface ILogger { diff --git a/src/main.ts b/src/main.ts index dc39852..0dd5197 100644 --- a/src/main.ts +++ b/src/main.ts @@ -1,4 +1,4 @@ -import { debug, error, getInput, info, setFailed } from "@actions/core"; +import { debug, error, getInput, getMultilineInput, info, setFailed } from "@actions/core"; import { context, getOctokit } from "@actions/github"; import { CoreLogger } from "./github/CoreLogger"; @@ -17,6 +17,8 @@ const getProjectFieldValues = (): { field: string; value: string } | undefined = } }; +const getRequiredLabels = (): string[] => getMultilineInput("labels"); + //* * Generates the class that will handle the project logic */ const generateSynchronizer = (): Synchronizer => { const repoToken = getInput("GITHUB_TOKEN", { required: true }); @@ -36,17 +38,14 @@ const generateSynchronizer = (): Synchronizer => { }; const synchronizer = generateSynchronizer(); +const labels = getRequiredLabels(); const projectFields = getProjectFieldValues(); -const { issue } = context.payload; +const { payload } = context; const parsedContext: GitHubContext = { eventName: context.eventName, - payload: { - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - inputs: context.payload.inputs, - issue: issue ? { number: issue.number, node_id: issue.node_id as string } : undefined, - }, - config: { projectField: projectFields }, + payload, + config: { projectField: projectFields, labels }, }; const errorHandler = (e: Error) => { diff --git a/src/synchronizer.ts b/src/synchronizer.ts index 0119bd4..e6f7beb 100644 --- a/src/synchronizer.ts +++ b/src/synchronizer.ts @@ -1,20 +1,30 @@ import { FieldValues, IIssues, ILogger, IProjectApi, Issue, NodeData } from "./github/types"; -// type IssueEvent = "opened" | "deleted" | "closed" | "reopened" | "labeled" | "unlabeled" | "transfered"; +export type IssueEvent = "opened" | "deleted" | "closed" | "reopened" | "labeled" | "unlabeled" | "transfered"; type EventNames = "workflow_dispatch" | "issues" | string; +type Payload = { + action?: IssueEvent | string; + inputs?: { excludeClosed?: "true" | "false" }; + issue?: Issue; + label?: { + id: number; + name: string; + }; +}; + export type GitHubContext = { eventName: EventNames; - payload: { - inputs?: { excludeClosed?: "true" | "false" }; - issue?: Issue; - }; + payload: Payload; config?: { projectField?: FieldValues; + labels?: string[]; }; }; +const toLowerCase = (array: string[]): string[] => array.map((a) => a.toLowerCase()); + export class Synchronizer { constructor( private readonly issueKit: IIssues, @@ -26,15 +36,21 @@ export class Synchronizer { if (context.eventName === "workflow_dispatch") { const excludeClosed = context.payload.inputs?.excludeClosed === "true"; this.logger.notice(excludeClosed ? "Closed issues will NOT be synced." : "Closed issues will be synced."); - return await this.updateAllIssues(excludeClosed, context.config?.projectField); + return await this.updateAllIssues(excludeClosed, context.config?.projectField, context.config?.labels); } else if (context.eventName === "issues") { + this.logger.debug(`Required labels are: '${JSON.stringify(context.config?.labels)}'`); + this.logger.debug("Payload received: " + JSON.stringify(context.payload)); const { issue } = context.payload; if (!issue) { throw new Error("Issue payload object was null"); } - this.logger.debug(`Received issue ${JSON.stringify(issue)}`); - this.logger.info(`Assigning issue #${issue.number} to project`); - return await this.updateOneIssue(issue, context.config?.projectField); + this.logger.debug(`Received event: ${context.eventName}`); + if (this.shouldAssignIssue(context.payload, context.config?.labels)) { + this.logger.info(`Assigning issue #${issue.number} to project`); + return await this.updateOneIssue(issue, context.config?.projectField); + } else { + return this.logger.info("Skipped assigment as it didn't fullfill requirements."); + } } else { const failMessage = `Event '${context.eventName}' is not expected. Failing.`; this.logger.warning(failMessage); @@ -42,6 +58,90 @@ export class Synchronizer { } } + /** + * Labels can be either an array of objects or an array of string (or maybe both?) + * This functions cleans them and returns all the labels names as a string array + */ + convertLabelArray(labels?: (string | { name?: string })[]): string[] { + if (!labels || labels.length === 0) { + return []; + } + const list: string[] = []; + + labels.forEach((label) => { + if (typeof label === "string" || label instanceof String) { + list.push(label as string); + } else if (label.name) { + list.push(label.name); + } + }); + + return list; + } + + /** + * Method which takes all of the (predicted) cases and calculates if the issue should be assigned or skipped + * @param payload object which contains both the event, the issue type and it's information + * @param labels labels required for the action. Can be null or empty + * @returns true if the label should be assigned, false if it should be skipped + */ + shouldAssignIssue(payload: Payload, labels?: string[]): boolean { + const action = payload.action as IssueEvent; + + if (action === "labeled") { + const labelName = payload.label?.name; + // Shouldn't happen. Throw and find out what is this kind of event. + if (!labelName) { + throw new Error("No label found in a labeling event!"); + } + + this.logger.info(`Label ${labelName} was added to the issue.`); + + // If this is a labeling event but there are no labels in the config we skip them + if (!labels || labels.length === 0) { + this.logger.notice("No required labels found for event. Skipping assignment."); + return false; + } + + if (toLowerCase(labels).indexOf(labelName.toLowerCase()) > -1) { + this.logger.info(`Found matching label '${labelName}' in required labels.`); + return true; + } + this.logger.notice( + `Label '${labelName}' does not match any of the labels '${JSON.stringify(labels)}'. Skipping.`, + ); + return false; + } else if (action === "unlabeled") { + this.logger.warning("No support for 'unlabeled' event. Skipping"); + return false; + } + + // if no labels are required and this is not a labeling event, assign the issue. + if (!labels || labels.length === 0) { + this.logger.info("Matching requirements: not a labeling event and no labels found in the configuration."); + return true; + } + // if the issue in this event has labels and a matching label config, assign it. + const issueLabels = payload.issue?.labels ?? null; + if (labels.length > 0 && issueLabels && issueLabels.length > 0) { + // complex query. Sanitizing everything to a lower case string array first + const parsedLabels = toLowerCase(this.convertLabelArray(issueLabels)); + const requiredLabels = toLowerCase(labels); + // checking if an element in one array is included in the second one + const matchingElement = parsedLabels.some((pl) => requiredLabels.includes(pl)); + if (matchingElement) { + this.logger.info( + `Found matching element between ${JSON.stringify(parsedLabels)} and ${JSON.stringify(labels)}`, + ); + return true; + } + return false; + } + + this.logger.debug(`Case ${action} not considered. Accepted with the following payload: ${JSON.stringify(payload)}`); + return true; + } + /** * Gets the field node data ids to set custom fields * This method will fail if the field or value are not available. @@ -61,8 +161,12 @@ export class Synchronizer { } } - private async updateAllIssues(excludeClosed: boolean = false, customField?: FieldValues): Promise | never { - const issues = await this.issueKit.getAllIssues(excludeClosed); + private async updateAllIssues( + excludeClosed: boolean = false, + customField?: FieldValues, + labels?: string[], + ): Promise | never { + const issues = await this.issueKit.getAllIssues(excludeClosed, labels); if (issues?.length === 0) { return this.logger.notice("No issues found"); } diff --git a/src/test/synchronizer.test.ts b/src/test/synchronizer.test.ts index 6d73d09..708f75e 100644 --- a/src/test/synchronizer.test.ts +++ b/src/test/synchronizer.test.ts @@ -94,7 +94,101 @@ describe("Synchronizer tests", () => { ).rejects.toThrow("Failed fetching project values"); expect(projectKit.changeIssueStateInProject).toHaveBeenCalledTimes(0); }); + + describe("label logic", () => { + test("should throw error if the event is labeled and there is no label", async () => { + ctx.payload.action = "labeled"; + await expect(synchronizer.synchronizeIssue(ctx)).rejects.toThrowError("No label found in a labeling event!"); + }); + + test("should skip on labeling event when config labels are null", async () => { + ctx.payload.action = "labeled"; + ctx.payload.label = { name: "example1", id: 1 }; + await synchronizer.synchronizeIssue(ctx); + expect(projectKit.fetchProjectData).toHaveBeenCalledTimes(0); + expect(logger.info).toHaveBeenCalledWith(`Label ${ctx.payload.label.name} was added to the issue.`); + expect(logger.info).toHaveBeenCalledWith("Skipped assigment as it didn't fullfill requirements."); + expect(logger.notice).toHaveBeenCalledWith("No required labels found for event. Skipping assignment."); + }); + + test("should skip on labeling event when config labels are empty", async () => { + ctx.payload.action = "labeled"; + ctx.payload.label = { name: "example1", id: 1 }; + ctx.config = { labels: [] }; + await synchronizer.synchronizeIssue(ctx); + expect(projectKit.fetchProjectData).toHaveBeenCalledTimes(0); + expect(logger.info).toHaveBeenCalledWith(`Label ${ctx.payload.label.name} was added to the issue.`); + expect(logger.info).toHaveBeenCalledWith("Skipped assigment as it didn't fullfill requirements."); + expect(logger.notice).toHaveBeenCalledWith("No required labels found for event. Skipping assignment."); + }); + + test("should assign on labeling event when config labels match assigned label", async () => { + ctx.payload.action = "labeled"; + ctx.payload.label = { name: "example2", id: 1 }; + ctx.config = { labels: ["example1", "example2"] }; + await synchronizer.synchronizeIssue(ctx); + expect(logger.info).toHaveBeenCalledWith(`Label ${ctx.payload.label.name} was added to the issue.`); + expect(logger.info).toHaveBeenCalledWith( + `Found matching label '${ctx.payload.label.name}' in required labels.`, + ); + expect(projectKit.fetchProjectData).toHaveBeenCalled(); + }); + + test("should skip on labeling event when config labels do not match assigned label", async () => { + ctx.payload.action = "labeled"; + ctx.payload.label = { name: "example3", id: 1 }; + ctx.config = { labels: ["example1", "example2"] }; + await synchronizer.synchronizeIssue(ctx); + expect(logger.info).toHaveBeenCalledWith(`Label ${ctx.payload.label.name} was added to the issue.`); + expect(logger.notice).toHaveBeenCalledWith( + `Label '${ctx.payload.label.name}' does not match any of the labels '${JSON.stringify( + ctx.config.labels, + )}'. Skipping.`, + ); + expect(logger.info).toHaveBeenCalledWith("Skipped assigment as it didn't fullfill requirements."); + expect(projectKit.fetchProjectData).toHaveBeenCalledTimes(0); + }); + + test("should skip on unlabeling event ", async () => { + ctx.payload.action = "unlabeled"; + await synchronizer.synchronizeIssue(ctx); + expect(logger.warning).toHaveBeenCalledWith("No support for 'unlabeled' event. Skipping"); + expect(logger.info).toHaveBeenCalledWith("Skipped assigment as it didn't fullfill requirements."); + expect(projectKit.fetchProjectData).toHaveBeenCalledTimes(0); + }); + + test("should assign on non labeling event when no labels are in the config", async () => { + await synchronizer.synchronizeIssue(ctx); + expect(logger.info).toHaveBeenCalledWith( + "Matching requirements: not a labeling event and no labels found in the configuration.", + ); + expect(projectKit.fetchProjectData).toHaveBeenCalled(); + }); + + test("should assign when config labels match labels in issue", async () => { + ctx.payload.issue = { node_id: "test_with_labels", number: issueNumber, labels: [{ name: "example3" }] }; + ctx.config = { labels: ["example3", "example2"] }; + await synchronizer.synchronizeIssue(ctx); + expect(logger.info).toHaveBeenCalledWith( + `Found matching element between ["example3"] and ${JSON.stringify(ctx.config.labels)}`, + ); + expect(projectKit.fetchProjectData).toHaveBeenCalled(); + }); + + test("should skip assigment when config labels do not match labels in issue", async () => { + ctx.payload.issue = { + node_id: "test_with_different_labels", + number: issueNumber, + labels: [{ name: "example4" }, { name: "random label" }], + }; + ctx.config = { labels: ["example3", "example2"] }; + await synchronizer.synchronizeIssue(ctx); + expect(logger.info).toHaveBeenCalledWith("Skipped assigment as it didn't fullfill requirements."); + expect(projectKit.fetchProjectData).toHaveBeenCalledTimes(0); + }); + }); }); + describe("update all issues", () => { let nodeData: NodeData; let ctx: GitHubContext; @@ -167,4 +261,33 @@ describe("Synchronizer tests", () => { expect(projectKit.changeIssueStateInProject).toHaveBeenCalledTimes(0); }); }); + + test("convertLabelArray should parse string array", () => { + const array = ["asd", "dsa", "rew"]; + expect(synchronizer.convertLabelArray(array)).toEqual(array); + }); + + test("convertLabelArray should parse object array", () => { + const array = [{ name: "rep" }, { name: "lol" }, { name: "asd" }]; + expect(synchronizer.convertLabelArray(array)).toEqual(["rep", "lol", "asd"]); + }); + + test("convertLabelArray should parse mixed object & string array", () => { + const array = [ + "hola", + { name: "chau" }, + "buenos dias", + { name: "buenas tardes" }, + { name: "buenas noches" }, + "arrivederchi", + ]; + expect(synchronizer.convertLabelArray(array)).toEqual([ + "hola", + "chau", + "buenos dias", + "buenas tardes", + "buenas noches", + "arrivederchi", + ]); + }); });