diff --git a/CHANGELOG.md b/CHANGELOG.md index 95c12d37..d8426d6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,14 +2,12 @@ ## 1.0.0 (2024-07-29) - ### Features -* database update step ([5bad80a](https://github.com/ubiquibot/automated-merging/commit/5bad80a8049890dcf16a5661caadfdacc89fdf2b)) -* set db to be sqlite ([2dbe73b](https://github.com/ubiquibot/automated-merging/commit/2dbe73be10f9ae436050f6b3626890db847c166c)) - +- database update step ([5bad80a](https://github.com/ubiquibot/automated-merging/commit/5bad80a8049890dcf16a5661caadfdacc89fdf2b)) +- set db to be sqlite ([2dbe73b](https://github.com/ubiquibot/automated-merging/commit/2dbe73be10f9ae436050f6b3626890db847c166c)) ### Bug Fixes -* changed approval requirement check to use the configuration ([e1f50e9](https://github.com/ubiquibot/automated-merging/commit/e1f50e95576f81ce01196bbdc0890b0617bf23df)) -* fixed imports within main ([bb001cf](https://github.com/ubiquibot/automated-merging/commit/bb001cf3204593a79b2d214941940a9a44675c00)) +- changed approval requirement check to use the configuration ([e1f50e9](https://github.com/ubiquibot/automated-merging/commit/e1f50e95576f81ce01196bbdc0890b0617bf23df)) +- fixed imports within main ([bb001cf](https://github.com/ubiquibot/automated-merging/commit/bb001cf3204593a79b2d214941940a9a44675c00)) diff --git a/README.md b/README.md index 65ce6480..ce6090ba 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # `@ubiquibot/automated-merging` -Automatically merge pull-requests based on the reviewer count, the time elapsed since the last activity, depending +Automatically merge pull-requests based on the reviewer count, the time elapsed since the last activity, depending on the association of the pull-request author. ## Configuration example @@ -17,6 +17,10 @@ on the association of the pull-request author. mergeTimeout: collaborator: "3.5 days" # defaults to 3.5 days contributor: "7 days" # defaults to 7 days + repos: + monitor: ["ubiquibot/automated-merging"] + ignore: ["ubiquibot/automated-merging"] + allowedReviewerRoles: ["COLLABORATOR", "MEMBER", "OWNER"] ``` ## Testing diff --git a/manifest.json b/manifest.json index 084c9285..c7b57209 100644 --- a/manifest.json +++ b/manifest.json @@ -1,5 +1,5 @@ { "name": "Automated merging", "description": "Automatically merge pull-requests.", - "ubiquity:listeners": [ "push", "issue_comment.created" ] + "ubiquity:listeners": ["push", "issue_comment.created"] } diff --git a/package.json b/package.json index 0b981ee1..42f43e05 100644 --- a/package.json +++ b/package.json @@ -84,5 +84,6 @@ "extends": [ "@commitlint/config-conventional" ] - } -} + }, + "packageManager": "yarn@1.22.22" +} \ No newline at end of file diff --git a/src/helpers/github.ts b/src/helpers/github.ts index 1b8e5091..394ecb7c 100644 --- a/src/helpers/github.ts +++ b/src/helpers/github.ts @@ -24,25 +24,32 @@ export interface Requirements { * statuses, the longest timeout is chosen. */ export async function getMergeTimeoutAndApprovalRequiredCount(context: Context, authorAssociation: string): Promise { + const { config: { mergeTimeout, approvalsRequired } } = context; const timeoutCollaborator = { - mergeTimeout: context.config.mergeTimeout.collaborator, - requiredApprovalCount: context.config.approvalsRequired.collaborator, + mergeTimeout: mergeTimeout.collaborator, + requiredApprovalCount: approvalsRequired.collaborator, }; const timeoutContributor = { - mergeTimeout: context.config.mergeTimeout.contributor, - requiredApprovalCount: context.config.approvalsRequired.contributor, + mergeTimeout: mergeTimeout.contributor, + requiredApprovalCount: approvalsRequired.contributor, }; + + /** + * Hardcoded roles here because we need to determine the timeouts + * separate from `allowedReviewerRoles` which introduces + * potential unintended user errors and logic issues. + */ return ["COLLABORATOR", "MEMBER", "OWNER"].includes(authorAssociation) ? timeoutCollaborator : timeoutContributor; } -export async function getApprovalCount({ octokit, logger }: Context, { owner, repo, issue_number: pullNumber }: IssueParams) { +export async function getApprovalCount({ octokit, logger, config: { allowedReviewerRoles } }: Context, { owner, repo, issue_number: pullNumber }: IssueParams) { try { const { data: reviews } = await octokit.rest.pulls.listReviews({ owner, repo, pull_number: pullNumber, }); - return reviews.filter((review) => review.state === "APPROVED").length; + return reviews.filter((review) => allowedReviewerRoles.includes(review.author_association)).filter((review) => review.state === "APPROVED").length; } catch (e) { logger.error(`Error fetching reviews' approvals: ${e}`); return 0; diff --git a/src/types/plugin-inputs.ts b/src/types/plugin-inputs.ts index ed79b997..1db88d1d 100644 --- a/src/types/plugin-inputs.ts +++ b/src/types/plugin-inputs.ts @@ -55,6 +55,8 @@ export const reposSchema = T.Object( { default: {} } ); +const allowedReviewerRoles = T.Array(T.String(), { default: ["COLLABORATOR", "MEMBER", "OWNER"] }) + export const pluginSettingsSchema = T.Object({ approvalsRequired: approvalsRequiredSchema, mergeTimeout: mergeTimeoutSchema, @@ -62,6 +64,9 @@ export const pluginSettingsSchema = T.Object({ * The list of organizations or repositories to watch for updates. */ repos: reposSchema, + allowedReviewerRoles: T.Transform(allowedReviewerRoles) + .Decode((roles) => roles.map((role) => role.toUpperCase())) + .Encode((roles) => roles.map((role) => role.toUpperCase())) }); export const pluginSettingsValidator = new StandardValidator(pluginSettingsSchema); diff --git a/tests/__mocks__/db.ts b/tests/__mocks__/db.ts index d1a12f75..9e2f9392 100644 --- a/tests/__mocks__/db.ts +++ b/tests/__mocks__/db.ts @@ -24,5 +24,6 @@ export const db = factory({ reviews: { id: primaryKey(Number), state: String, + author_association: String, }, }); diff --git a/tests/__mocks__/routes/search-pull-requests.json b/tests/__mocks__/routes/search-pull-requests.json index 7a67c673..59fdef62 100644 --- a/tests/__mocks__/routes/search-pull-requests.json +++ b/tests/__mocks__/routes/search-pull-requests.json @@ -33,15 +33,11 @@ "type": "User", "site_admin": false }, - "labels": [ - - ], + "labels": [], "state": "open", "locked": false, "assignee": null, - "assignees": [ - - ], + "assignees": [], "milestone": null, "comments": 24, "created_at": "2024-07-02T15:25:13Z", diff --git a/tests/__mocks__/seed.json b/tests/__mocks__/seed.json index 708b5266..a7ed423e 100644 --- a/tests/__mocks__/seed.json +++ b/tests/__mocks__/seed.json @@ -18,11 +18,23 @@ "reviews": [ { "id": 1, - "state": "APPROVED" + "state": "APPROVED", + "author_association": "MEMBER" }, { "id": 2, - "state": "APPROVED" + "state": "APPROVED", + "author_association": "COLLABORATOR" + }, + { + "id": 3, + "state": "APPROVED", + "author_association": "CONTRIBUTOR" + }, + { + "id": 4, + "state": "APPROVED", + "author_association": "NONE" } ] -} +} \ No newline at end of file diff --git a/tests/main.test.ts b/tests/main.test.ts index 4ea86634..af730979 100644 --- a/tests/main.test.ts +++ b/tests/main.test.ts @@ -62,6 +62,7 @@ describe("Action tests", () => { eventName: "push", settings: JSON.stringify({ repos: { monitor: [monitor] }, + allowedReviewerRoles: ["COLLABORATOR", "MEMBER", "OWNER"], }), eventPayload: JSON.stringify({ pull_request: { @@ -154,6 +155,59 @@ describe("Action tests", () => { expect(mergePullRequest).toHaveBeenCalled(); }); + it("Should not close a PR if non-approved reviews are present", async () => { + server.use( + http.get( + "https://api.github.com/repos/:org/:repo/pulls/:id/reviews", + () => { + return HttpResponse.json([{ id: 1, state: "COMMENTED", author_association: "CONTRIBUTOR" }, { id: 2, state: "APPROVED", author_association: "NONE" }]); + }, + { once: true } + ) + ); + jest.mock(actionsGithubPackage, () => ({ + context: { + repo: { + owner: { + login: "ubiquibot", + }, + }, + workflow, + payload: { + inputs: { + eventName: "push", + settings: JSON.stringify({ + repos: { monitor: [monitor] }, + }), + eventPayload: JSON.stringify({ + pull_request: { + html_url: htmlUrl, + }, + repository: { + owner: "ubiquibot", + }, + }), + env: { + workflowName: workflow, + }, + }, + }, + }, + })); + const mergePullRequest = jest.fn(); + jest.mock(githubHelpersPath, () => { + const actualModule = jest.requireActual(githubHelpersPath) as object; + return { + __esModule: true, + ...actualModule, + mergePullRequest, + }; + }); + const run = (await import("../src/action")).run; + await expect(run()).resolves.toMatchObject({ status: 200 }); + expect(mergePullRequest).not.toHaveBeenCalled(); + }); + it("Should pick the timeout according to the assignees status", async () => { const contributorMergeTimeout = "7 days"; const collaboratorMergeTimeout = "3.5 days"; @@ -177,6 +231,7 @@ describe("Action tests", () => { collaborator: collaboratorMinimumApprovalsRequired, contributor: contributorMinimumApprovalsRequired, }, + allowedReviewerRoles: ["COLLABORATOR", "MEMBER", "OWNER"], }, octokit: new Octokit(), } as unknown as Context; @@ -228,7 +283,9 @@ describe("Action tests", () => { }, }, workflow: "other workflow", - config: {}, + config: { + allowedReviewerRoles: ["COLLABORATOR", "MEMBER", "OWNER"], + }, octokit: new Octokit(), env: { workflowName: workflow, diff --git a/tsconfig.json b/tsconfig.json index 2481e795..b88b59b3 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -12,8 +12,8 @@ "target": "ES2022" /* Set the JavaScript language version for emitted JavaScript and include compatible library declarations. */, // "lib": [], /* Specify a set of bundled library declaration files that describe the target runtime environment. */ // "jsx": "preserve", /* Specify what JSX code is generated. */ - "experimentalDecorators": true, /* Enable experimental support for legacy experimental decorators. */ - "emitDecoratorMetadata": true, /* Emit design-type metadata for decorated declarations in source files. */ + "experimentalDecorators": true /* Enable experimental support for legacy experimental decorators. */, + "emitDecoratorMetadata": true /* Emit design-type metadata for decorated declarations in source files. */, // "jsxFactory": "", /* Specify the JSX factory function used when targeting React JSX emit, e.g. 'React.createElement' or 'h'. */ // "jsxFragmentFactory": "", /* Specify the JSX Fragment reference used for fragments when targeting React JSX emit e.g. 'React.Fragment' or 'Fragment'. */ // "jsxImportSource": "", /* Specify module specifier used to import the JSX factory functions when using 'jsx: react-jsx*'. */ @@ -80,7 +80,7 @@ // "strictNullChecks": true, /* When type checking, take into account 'null' and 'undefined'. */ // "strictFunctionTypes": true, /* When assigning functions, check to ensure parameters and the return values are subtype-compatible. */ // "strictBindCallApply": true, /* Check that the arguments for 'bind', 'call', and 'apply' methods match the original function. */ - "strictPropertyInitialization": false, /* Check for class properties that are declared but not set in the constructor. */ + "strictPropertyInitialization": false /* Check for class properties that are declared but not set in the constructor. */, // "noImplicitThis": true, /* Enable error reporting when 'this' is given the type 'any'. */ // "useUnknownInCatchVariables": true, /* Default catch clause variables as 'unknown' instead of 'any'. */ // "alwaysStrict": true, /* Ensure 'use strict' is always emitted. */