fix: allow max approvals to be 0 (#50)

* Functionality updates

* update

* Update readme

.github/workflows/auto-approve.yml

@@ -0,0 +1,18 @@
+name: Auto approve
+
+on:
+  pull_request:
+
+jobs:
+  approve:
+    runs-on: ubuntu-latest
+    permissions:
+      pull-requests: write
+    steps:
+      - name: Sleep for 30 seconds
+        run: sleep 30
+
+      - name: Approve Pull Request
+        uses: hmarr/auto-approve-action@v3
+        with:
+          github-token: ${{ secrets.GITHUB_TOKEN }}

.github/workflows/test.yml

@@ -1,6 +1,11 @@
 name: Build and test
 on:
   pull_request:
+  pull_request_review:
+
+permissions:
+  checks: write
+  pull-requests: read
 
 jobs:
   build:
@@ -13,6 +18,7 @@ jobs:
           yarn all
 
   self-test:
+    name: Check required approvals
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v3

.github/workflows/update-tags-post-release.yml

@@ -0,0 +1,19 @@
+# As per the GitHub recommendations, we should update
+# the major and minor tags (ie v1, v1.1) to point to the
+# latest release.
+# See: https://docs.github.com/en/actions/creating-actions/about-custom-actions#using-tags-for-release-management
+name: Update major and minor tags post release
+
+on:
+  push:
+    branches-ignore:
+      - '**'
+    tags:
+      - 'v*.*.*'
+
+jobs:
+  update-major-and-minor-tags:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: haya14busa/action-update-semver@v1

README.md

@@ -10,6 +10,32 @@ require more than 1 reviewer, and we dont want to force multiple approvals on al
 
 With this action, you can set paths and approval requirements based on file change globs, and then make this check required.
 
+## How
+
+In order to avoid pull requests showing a red ❌ when the required approvals are not met (which may deter futher reviewers),
+this action always succeeds. If the approval requirements are met, the action will **add an additional check to the pull request
+with the name "Required number of approvals met"**.
+
+If the required approvals are not met, the additional status check will _not be added_, and the PR will show in a pending state.
+
+Users of this action should set this additional check (`Required number of approvals met`) as required in the branch protection settings.
+
+### Example of a PR without the required approvals
+
+<img width="500" src="images/pending_checks_pr_list.png" />
+<img width="500" src="images/pending_checks.png" />
+
+### Example of a PR with required approvals and check
+
+<img width="500" src="images/completed_checks.png" />
+
+### Pitfalls
+
+When there are multiple workflows running in parallel, the check will be randomly attached
+to one of the workflows, instead of the originating workflow.
+
+If you want to fix this, you must use a GitHub App token. See for example: https://github.com/LouisBrunner/checks-action/issues/26#issuecomment-1232948025
+
 ## Usage
 
 Here's an example workflow configuration:
@@ -21,6 +47,10 @@ on:
   pull_request:
   pull_request_review:
 
+permissions:
+  checks: write
+  pull-requests: read
+
 jobs:
   required-approvals:
     runs-on: ubuntu-latest
@@ -40,7 +70,7 @@ jobs:
           github-token: ${{ secrets.GITHUB_TOKEN }}
 ```
 
-You would then make `required-approvals` a required check in the repository's branch protection.
+You would then make `Required number of approvals met` a required check in the repository's branch protection.
 
 ## Inputs
 
@@ -61,6 +91,7 @@ You would then make `required-approvals` a required check in the repository's br
 
 ## Behavior
 
-- If none of the files in the specified patterns are touched in the pull request, the action passes.
+- If none of the files in the specified patterns are touched in the pull request, the action sets the custom check.
 - If files in the specified patterns are touched, the action checks if the pull request has the required number of approvals.
-- If the required number of approvals is not met, the action fails.
+- If the required number of approvals is not met, the action does not set the custom check.
+- Always succeed on the `pull_request` event when there are no reviews yet, so that you can set this check as `required` in branch protection.

__tests__/main.test.ts

@@ -3,15 +3,18 @@ import * as core from '@actions/core'
 
 const listReviewsMock = jest.fn()
 const listFilesMock = jest.fn()
+const createCheckMock = jest.fn()
+const updateCheckMock = jest.fn()
+const listForRefMock = jest.fn()
 
 jest.mock('@actions/core', () => ({
   info: jest.fn(),
-  setFailed: jest.fn(),
 }))
 
 jest.mock('@actions/github', () => {
   return {
     context: {
+      eventName: 'pull_request',
       repo: {
         owner: 'mocked-owner-value',
         repo: 'mocked-repo-value',
@@ -28,6 +31,11 @@ jest.mock('@actions/github', () => {
           listReviews: listReviewsMock,
           listFiles: listFilesMock,
         },
+        checks: {
+          create: createCheckMock,
+          update: updateCheckMock,
+          listForRef: listForRefMock,
+        },
       },
     })),
   }
@@ -45,11 +53,19 @@ function mockNumberOfReviews(number: number) {
   })
 }
 
+function mockCheckAlreadyExisting(alreadyExists: boolean) {
+  listForRefMock.mockReturnValue({
+    data: alreadyExists
+      ? {check_runs: [{name: 'Required number of approvals met'}]}
+      : {check_runs: []},
+  })
+}
+
 beforeEach(() => {
   jest.clearAllMocks()
 })
 
-it('should pass when no files are matched, and the PR is not approved', async () => {
+it('should "pass" and set the check when no files are matched, and the PR is not approved', async () => {
   mockFileList(['foo', 'bar'])
   mockNumberOfReviews(0)
 
@@ -63,11 +79,47 @@ it('should pass when no files are matched, and the PR is not approved', async ()
     token: 'foo',
   })
 
-  expect(core.setFailed).not.toBeCalled()
-  expect(core.info).toHaveBeenCalledWith('All checks passed!')
+  expect(core.info).toHaveBeenCalledWith(
+    'No reviews yet, setting check to successful.',
+  )
+  expect(createCheckMock).toBeCalledWith(
+    expect.objectContaining({
+      conclusion: 'success',
+      output: expect.objectContaining({
+        title: 'No reviews yet',
+      }),
+    }),
+  )
 })
 
-it('should pass when files are matched and approvals are met', async () => {
+it('should "pass" and set the check if the event is pull_request and there are no reviews yet, even with matching files', async () => {
+  mockFileList(['src/test.js'])
+  mockNumberOfReviews(0)
+
+  await checkRequiredApprovals({
+    requirements: [
+      {
+        patterns: ['src/**/*'],
+        requiredApprovals: 1,
+      },
+    ],
+    token: 'foo',
+  })
+
+  expect(core.info).toHaveBeenCalledWith(
+    'No reviews yet, setting check to successful.',
+  )
+  expect(createCheckMock).toBeCalledWith(
+    expect.objectContaining({
+      conclusion: 'success',
+      output: expect.objectContaining({
+        title: 'No reviews yet',
+      }),
+    }),
+  )
+})
+
+it('should "pass" and set the check when files are matched and approvals are met', async () => {
   mockFileList(['src/test.js'])
   mockNumberOfReviews(1)
 
@@ -81,11 +133,18 @@ it('should pass when files are matched and approvals are met', async () => {
     token: 'foo',
   })
 
-  expect(core.setFailed).not.toBeCalled()
   expect(core.info).toHaveBeenCalledWith('All checks passed!')
+  expect(createCheckMock).toBeCalledWith(
+    expect.objectContaining({
+      conclusion: 'success',
+      output: expect.objectContaining({
+        title: '1/1 approvals',
+      }),
+    }),
+  )
 })
 
-it('should pass when multiple patterns are met', async () => {
+it('should "pass" and set the check when multiple patterns are met', async () => {
   mockFileList(['src/test.js', '.github/workflows/test.yml'])
   mockNumberOfReviews(2)
 
@@ -103,12 +162,20 @@ it('should pass when multiple patterns are met', async () => {
     token: 'foo',
   })
 
-  expect(core.setFailed).not.toBeCalled()
   expect(core.info).toHaveBeenCalledWith('All checks passed!')
+  expect(createCheckMock).toBeCalledWith(
+    expect.objectContaining({
+      conclusion: 'success',
+      output: expect.objectContaining({
+        title: '2/2 approvals',
+      }),
+    }),
+  )
 })
 
-it('should fail if there is one requirement that is not met', async () => {
+it('should "fail" and not set the check, if there is one requirement that is not met', async () => {
   mockFileList(['src/test.js', '.github/workflows/test.yml'])
+  mockCheckAlreadyExisting(false)
   mockNumberOfReviews(1)
 
   await checkRequiredApprovals({
@@ -125,11 +192,35 @@ it('should fail if there is one requirement that is not met', async () => {
     token: 'foo',
   })
 
-  expect(core.setFailed).toHaveBeenCalledWith(
-    'Required approvals not met for one or more patterns',
-  )
   expect(core.info).toHaveBeenCalledWith(
     'Required approvals not met for files matching patterns (1/2): .github/**/*',
   )
   expect(core.info).not.toHaveBeenCalledWith('All checks passed!')
+  expect(createCheckMock).not.toBeCalled()
+  expect(updateCheckMock).not.toBeCalled()
+})
+
+it('should "fail" and update the check to in progress when failing, if it already exists', async () => {
+  mockFileList(['src/test.js'])
+  mockCheckAlreadyExisting(true)
+  mockNumberOfReviews(1)
+
+  await checkRequiredApprovals({
+    requirements: [
+      {
+        patterns: ['src/**/*'],
+        requiredApprovals: 2,
+      },
+    ],
+    token: 'foo',
+  })
+
+  expect(updateCheckMock).toBeCalled()
+  expect(core.info).toHaveBeenCalledWith(
+    'Setting existing check to in_progress',
+  )
+  expect(core.info).toHaveBeenCalledWith(
+    'Required approvals not met for files matching patterns (1/2): src/**/*',
+  )
+  expect(createCheckMock).not.toBeCalled()
 })

dist/index.js

@@ -62,9 +62,9 @@ function hasChangedFilesMatchingPatterns(patterns, filenames) {
 // The main action function.
 // Checks if the required approvals are met for the patterns
 function checkRequiredApprovals(config) {
-    var _a, _b;
+    var _a, _b, _c, _d, _e, _f, _g, _h;
     return __awaiter(this, void 0, void 0, function* () {
-        let actionFailed = false;
+        let requiredApprovalsMet = true;
         const octokit = (0, github_1.getOctokit)(config.token);
         const filenames = yield getPRFilenames(octokit);
         const { data: reviews } = yield octokit.rest.pulls.listReviews({
@@ -72,21 +72,73 @@ function checkRequiredApprovals(config) {
             repo: github_1.context.repo.repo,
             pull_number: (_b = (_a = github_1.context.payload.pull_request) === null || _a === void 0 ? void 0 : _a.number) !== null && _b !== void 0 ? _b : 0,
         });
+        const approvals = reviews.filter(review => review.state === 'APPROVED').length;
+        // Otherwise ensure all the checks are met
+        let maxApprovalsRequired = 0;
         for (const requirement of config.requirements) {
             const hasChanges = hasChangedFilesMatchingPatterns(requirement.patterns, filenames);
             if (hasChanges) {
-                const approvals = reviews.filter(review => review.state === 'APPROVED').length;
+                maxApprovalsRequired = Math.max(maxApprovalsRequired, requirement.requiredApprovals);
                 if (approvals < requirement.requiredApprovals) {
-                    actionFailed = true;
+                    requiredApprovalsMet = false;
                     core.info(`Required approvals not met for files matching patterns (${approvals}/${requirement.requiredApprovals}): ${requirement.patterns.join(', ')}`);
                 }
             }
         }
-        if (actionFailed) {
-            core.setFailed(`Required approvals not met for one or more patterns`);
+        const noReviewsYet = github_1.context.eventName === 'pull_request' && reviews.length === 0;
+        const checkTitle = noReviewsYet
+            ? 'No reviews yet'
+            : `${approvals}/${maxApprovalsRequired} approvals`;
+        if (
+        // Always succeed on pull_request events when there are no reviews yet.
+        // That way we will not get red Xs on the PR right away.
+        requiredApprovalsMet ||
+            noReviewsYet) {
+            if (noReviewsYet) {
+                core.info('No reviews yet, setting check to successful.');
+            }
+            else {
+                core.info('All checks passed!');
+            }
+            yield octokit.rest.checks.create({
+                owner: github_1.context.repo.owner,
+                repo: github_1.context.repo.repo,
+                name: 'Required number of approvals met',
+                head_sha: (_e = (_d = (_c = github_1.context.payload.pull_request) === null || _c === void 0 ? void 0 : _c.head) === null || _d === void 0 ? void 0 : _d.sha) !== null && _e !== void 0 ? _e : github_1.context.sha,
+                status: 'completed',
+                conclusion: 'success',
+                started_at: new Date().toISOString(),
+                completed_at: new Date().toISOString(),
+                output: {
+                    title: checkTitle,
+                    summary: 'All required approvals have been met.',
+                },
+            });
         }
         else {
-            core.info('All checks passed!');
+            core.info(`Required approvals not met for one or more patterns.`);
+            // If the check already exists, update it so its pending and the
+            // PR cannot be merged. Otherwise leave the check missing.
+            const checks = yield octokit.rest.checks.listForRef({
+                owner: github_1.context.repo.owner,
+                repo: github_1.context.repo.repo,
+                ref: (_h = (_g = (_f = github_1.context.payload.pull_request) === null || _f === void 0 ? void 0 : _f.head) === null || _g === void 0 ? void 0 : _g.sha) !== null && _h !== void 0 ? _h : github_1.context.sha,
+            });
+            const requiredApprovalsCheck = checks.data.check_runs.find(check => check.name === 'Required number of approvals met');
+            if (requiredApprovalsCheck) {
+                core.info(`Setting existing check to in_progress`);
+                yield octokit.rest.checks.update({
+                    owner: github_1.context.repo.owner,
+                    repo: github_1.context.repo.repo,
+                    check_run_id: requiredApprovalsCheck.id,
+                    status: 'in_progress',
+                    started_at: new Date().toISOString(),
+                    output: {
+                        title: checkTitle,
+                        summary: `${maxApprovalsRequired} approvals are required, but only ${approvals} have been met.`,
+                    },
+                });
+            }
         }
     });
 }
@@ -136,6 +188,7 @@ Object.defineProperty(exports, "__esModule", ({ value: true }));
 exports.run = void 0;
 const core = __importStar(__nccwpck_require__(2186));
 const yaml = __importStar(__nccwpck_require__(4083));
+const github_1 = __nccwpck_require__(5438);
 const check_required_approvals_1 = __nccwpck_require__(2782);
 const parseRequirementsYaml = (requirements) => {
     const parsed = yaml.parse(requirements);
@@ -168,7 +221,13 @@ function run() {
                 })),
                 token: core.getInput('github-token', { required: true }),
             };
-            yield (0, check_required_approvals_1.checkRequiredApprovals)(config);
+            const eventName = github_1.context.eventName;
+            if (eventName === 'pull_request' || eventName === 'pull_request_review') {
+                yield (0, check_required_approvals_1.checkRequiredApprovals)(config);
+            }
+            else {
+                core.warning(`This action only supports 'pull_request', and 'pull_request_review' events. Received '${eventName}'.`);
+            }
         }
         catch (error) {
             if (error instanceof Error) {

src/check-required-approvals.ts

@@ -38,7 +38,7 @@ function hasChangedFilesMatchingPatterns(
 // The main action function.
 // Checks if the required approvals are met for the patterns
 export async function checkRequiredApprovals(config: Config): Promise<void> {
-  let actionFailed = false
+  let requiredApprovalsMet = true
   const octokit = getOctokit(config.token)
   const filenames = await getPRFilenames(octokit)
 
@@ -48,6 +48,10 @@ export async function checkRequiredApprovals(config: Config): Promise<void> {
     pull_number: context.payload.pull_request?.number ?? 0,
   })
 
+  const approvals = reviews.filter(review => review.state === 'APPROVED').length
+
+  // Otherwise ensure all the checks are met
+  let maxApprovalsRequired = 0
   for (const requirement of config.requirements) {
     const hasChanges = hasChangedFilesMatchingPatterns(
       requirement.patterns,
@@ -55,12 +59,13 @@ export async function checkRequiredApprovals(config: Config): Promise<void> {
     )
 
     if (hasChanges) {
-      const approvals = reviews.filter(
-        review => review.state === 'APPROVED',
-      ).length
+      maxApprovalsRequired = Math.max(
+        maxApprovalsRequired,
+        requirement.requiredApprovals,
+      )
 
       if (approvals < requirement.requiredApprovals) {
-        actionFailed = true
+        requiredApprovalsMet = false
         core.info(
           `Required approvals not met for files matching patterns (${approvals}/${
             requirement.requiredApprovals
@@ -70,9 +75,68 @@ export async function checkRequiredApprovals(config: Config): Promise<void> {
     }
   }
 
-  if (actionFailed) {
-    core.setFailed(`Required approvals not met for one or more patterns`)
+  const noReviewsYet =
+    context.eventName === 'pull_request' && reviews.length === 0
+
+  const checkTitle = noReviewsYet
+    ? 'No reviews yet'
+    : `${approvals}/${maxApprovalsRequired} approvals`
+
+  if (
+    // Always succeed on pull_request events when there are no reviews yet.
+    // That way we will not get red Xs on the PR right away.
+    requiredApprovalsMet ||
+    noReviewsYet
+  ) {
+    if (noReviewsYet) {
+      core.info('No reviews yet, setting check to successful.')
+    } else {
+      core.info('All checks passed!')
+    }
+
+    await octokit.rest.checks.create({
+      owner: context.repo.owner,
+      repo: context.repo.repo,
+      name: 'Required number of approvals met',
+      head_sha: context.payload.pull_request?.head?.sha ?? context.sha,
+      status: 'completed',
+      conclusion: 'success',
+      started_at: new Date().toISOString(),
+      completed_at: new Date().toISOString(),
+      output: {
+        title: checkTitle,
+        summary: 'All required approvals have been met.',
+      },
+    })
   } else {
-    core.info('All checks passed!')
+    core.info(`Required approvals not met for one or more patterns.`)
+
+    // If the check already exists, update it so its pending and the
+    // PR cannot be merged. Otherwise leave the check missing.
+    const checks = await octokit.rest.checks.listForRef({
+      owner: context.repo.owner,
+      repo: context.repo.repo,
+      ref: context.payload.pull_request?.head?.sha ?? context.sha,
+    })
+
+    const requiredApprovalsCheck = checks.data.check_runs.find(
+      check => check.name === 'Required number of approvals met',
+    )
+
+    if (requiredApprovalsCheck) {
+      core.info(`Setting existing check to in_progress`)
+
+      await octokit.rest.checks.update({
+        owner: context.repo.owner,
+        repo: context.repo.repo,
+        check_run_id: requiredApprovalsCheck.id,
+        status: 'in_progress',
+        started_at: new Date().toISOString(),
+        output: {
+          title: checkTitle,
+          summary: `${maxApprovalsRequired} approvals are required, but only ${approvals} have been met.`,
+        },
+      })
+    }
   }
 }

src/main.ts

@@ -1,5 +1,6 @@
 import * as core from '@actions/core'
 import * as yaml from 'yaml'
+import {context} from '@actions/github'
 import {
   checkRequiredApprovals,
   Requirement,
@@ -44,7 +45,14 @@ export async function run(): Promise<void> {
       token: core.getInput('github-token', {required: true}),
     }
 
-    await checkRequiredApprovals(config)
+    const eventName = context.eventName
+    if (eventName === 'pull_request' || eventName === 'pull_request_review') {
+      await checkRequiredApprovals(config)
+    } else {
+      core.warning(
+        `This action only supports 'pull_request', and 'pull_request_review' events. Received '${eventName}'.`,
+      )
+    }
   } catch (error) {
     if (error instanceof Error) {
       core.setFailed(`Action failed with error: ${error.message}`)