Merge pull request #172 from johnmartel/feature/#166-abort-on-non-def…

…ault-branch-or-non-organization-repository

Feature/#166 abort on non default branch or non organization repository

__tests__/action.test.ts

@@ -20,6 +20,18 @@ jest.mock('signale');
 jest.mock('../src/githubOrganization');
 jest.mock('../src/pushPayload');
 
+function initializeToolkit(): Toolkit {
+  const tools = new Toolkit({
+    logger: signale,
+  });
+  // @ts-ignore
+  tools.exit.success = jest.fn();
+  // @ts-ignore
+  tools.exit.failure = jest.fn();
+
+  return tools;
+}
+
 function givenAddOrUpdateOperationsWithResults(success: boolean): void {
   let singleResult: OperationResult;
 
@@ -49,53 +61,97 @@ function givenRemoveOperationsWithResults(success: boolean): void {
 }
 
 describe('action test suite', () => {
-  let tools: Toolkit;
-
   beforeEach(() => {
     Object.assign(process.env, {
       GITHUB_REPOSITORY: 'johnmartel/organization-membership-action',
       GITHUB_ACTION: 'organization-membership-action',
       GITHUB_EVENT_PATH: path.join(__dirname, 'fixtures', 'pushEventPayload.json'),
       GITHUB_WORKSPACE: path.join(__dirname, 'fixtures'),
     });
-
-    tools = new Toolkit({
-      logger: signale,
-    });
-    // @ts-ignore
-    tools.exit.success = jest.fn();
-    // @ts-ignore
-    tools.exit.failure = jest.fn();
   });
 
   afterEach(() => {
     jest.resetAllMocks();
   });
 
-  describe('given members file was not modified', () => {
-    beforeEach(() => {
+  describe('given an organization repository', () => {
+    it('should continue processing the event payload', async () => {
+      PushPayload.prototype.isOrganizationOwned = jest.fn().mockReturnValue(true);
+      PushPayload.prototype.isDefaultBranch = jest.fn().mockReturnValue(false);
+      const tools = initializeToolkit();
+
+      await action(tools);
+
+      expect(PushPayload.prototype.isDefaultBranch).toHaveBeenCalled();
+    });
+  });
+
+  describe('given a user repository', () => {
+    it('should halt event payload processing and exit with failure', async () => {
+      PushPayload.prototype.isOrganizationOwned = jest.fn().mockReturnValue(false);
+      const tools = initializeToolkit();
+
+      await action(tools);
+
+      expect(PushPayload.prototype.isDefaultBranch).not.toHaveBeenCalled();
+      expect(tools.exit.failure).toHaveBeenCalledWith(expect.stringContaining('Not an organization repository'));
+    });
+  });
+
+  describe('given push on default branch', () => {
+    it('should continue processing the event payload', async () => {
+      PushPayload.prototype.isOrganizationOwned = jest.fn().mockReturnValue(true);
+      PushPayload.prototype.isDefaultBranch = jest.fn().mockReturnValue(true);
       PushPayload.prototype.fileWasModified = jest.fn().mockResolvedValue(false);
+      const tools = initializeToolkit();
+
+      await action(tools);
+
+      expect(PushPayload.prototype.fileWasModified).toHaveBeenCalled();
+    });
+  });
+
+  describe('given push on any non-default branch', () => {
+    it('should halt event payload processing and exit successfully', async () => {
+      PushPayload.prototype.isOrganizationOwned = jest.fn().mockReturnValue(true);
+      PushPayload.prototype.isDefaultBranch = jest.fn().mockReturnValue(false);
+      const tools = initializeToolkit();
+
+      await action(tools);
+
+      expect(PushPayload.prototype.fileWasModified).not.toHaveBeenCalled();
+      expect(tools.exit.success).toHaveBeenCalledWith(expect.stringContaining('Not working on default branch'));
     });
+  });
 
+  describe('given members file was not modified', () => {
     it('should exit successfully', async () => {
+      PushPayload.prototype.isOrganizationOwned = jest.fn().mockReturnValue(true);
+      PushPayload.prototype.isDefaultBranch = jest.fn().mockReturnValue(true);
+      PushPayload.prototype.fileWasModified = jest.fn().mockResolvedValue(false);
+      const tools = initializeToolkit();
+
       await action(tools);
 
       expect(tools.exit.success).toHaveBeenCalled();
     });
   });
 
   describe('given members file was modified', () => {
+    const SUCCESS = true;
+    const FAILURE = false;
+
     beforeEach(() => {
+      PushPayload.prototype.isOrganizationOwned = jest.fn().mockReturnValue(true);
+      PushPayload.prototype.isDefaultBranch = jest.fn().mockReturnValue(true);
       PushPayload.prototype.fileWasModified = jest.fn().mockResolvedValue(true);
-      tools.readFile = jest.fn().mockReturnValue(VALID_FILE);
     });
 
     describe('given a file with no members', () => {
-      beforeEach(() => {
+      it('should exit with failure', async () => {
+        const tools = initializeToolkit();
         tools.readFile = jest.fn().mockReturnValue(VALID_FILE_WITH_EMPTY_MEMBERS);
-      });
 
-      it('should exit with failure', async () => {
         await action(tools);
 
         expect(GithubOrganization.prototype.inviteNewMembers).not.toHaveBeenCalled();
@@ -105,12 +161,12 @@ describe('action test suite', () => {
     });
 
     describe('given all operations are successful', () => {
-      beforeEach(() => {
-        givenAddOrUpdateOperationsWithResults(true);
-        givenRemoveOperationsWithResults(true);
-      });
-
       it('should exit successfully', async () => {
+        const tools = initializeToolkit();
+        tools.readFile = jest.fn().mockReturnValue(VALID_FILE);
+        givenAddOrUpdateOperationsWithResults(SUCCESS);
+        givenRemoveOperationsWithResults(SUCCESS);
+
         await action(tools);
 
         expect(GithubOrganization.prototype.inviteNewMembers).toHaveBeenCalledTimes(1);
@@ -120,12 +176,12 @@ describe('action test suite', () => {
     });
 
     describe('given add/update operations are not successful', () => {
-      beforeEach(() => {
-        givenAddOrUpdateOperationsWithResults(false);
-        givenRemoveOperationsWithResults(true);
-      });
-
       it('should exit with failure', async () => {
+        const tools = initializeToolkit();
+        tools.readFile = jest.fn().mockReturnValue(VALID_FILE);
+        givenAddOrUpdateOperationsWithResults(FAILURE);
+        givenRemoveOperationsWithResults(SUCCESS);
+
         await action(tools);
 
         expect(GithubOrganization.prototype.removeMembers).toHaveBeenCalledTimes(1);
@@ -134,21 +190,20 @@ describe('action test suite', () => {
     });
 
     describe('given removal operations are not successful', () => {
-      beforeEach(() => {
-        givenAddOrUpdateOperationsWithResults(true);
-        givenRemoveOperationsWithResults(false);
-      });
-
       it('should exit with failure', async () => {
+        const tools = initializeToolkit();
+        tools.readFile = jest.fn().mockReturnValue(VALID_FILE);
+        givenAddOrUpdateOperationsWithResults(SUCCESS);
+        givenRemoveOperationsWithResults(FAILURE);
+
         await action(tools);
 
         expect(GithubOrganization.prototype.inviteNewMembers).toHaveBeenCalledTimes(1);
         expect(tools.exit.failure).toHaveBeenCalled();
       });
     });
 
-    async function runActionAndThrow(error: object): Promise<void> {
-      PushPayload.prototype.fileWasModified = jest.fn().mockResolvedValue(true);
+    async function runActionAndThrow(tools: Toolkit, error: object): Promise<void> {
       tools.readFile = jest.fn().mockImplementation(() => {
         throw error;
       });
@@ -159,35 +214,47 @@ describe('action test suite', () => {
     describe('given an unknown error occurs', () => {
       const expectedError = { message: 'test' };
 
-      beforeEach(async () => {
-        await runActionAndThrow(expectedError);
-      });
+      it('should log the error', async () => {
+        const tools = initializeToolkit();
+
+        await runActionAndThrow(tools, expectedError);
 
-      it('should log the error', () => {
         expect(tools.log.error).toHaveBeenCalledWith('test', expectedError);
       });
 
-      it('should exit with failure', () => {
+      it('should exit with failure', async () => {
+        const tools = initializeToolkit();
+
+        await runActionAndThrow(tools, expectedError);
+
         expect(tools.exit.failure).toHaveBeenCalled();
       });
     });
 
     describe('given an unknown error with details occurs', () => {
       const expectedError = { message: 'test', errors: ['more details'] };
 
-      beforeEach(async () => {
-        await runActionAndThrow(expectedError);
-      });
+      it('should log the error', async () => {
+        const tools = initializeToolkit();
+
+        await runActionAndThrow(tools, expectedError);
 
-      it('should log the error', () => {
         expect(tools.log.error).toHaveBeenCalledWith('test', expectedError);
       });
 
-      it('should log the error details', () => {
+      it('should log the error details', async () => {
+        const tools = initializeToolkit();
+
+        await runActionAndThrow(tools, expectedError);
+
         expect(tools.log.error).toHaveBeenCalledWith(['more details']);
       });
 
-      it('should exit with failure', () => {
+      it('should exit with failure', async () => {
+        const tools = initializeToolkit();
+
+        await runActionAndThrow(tools, expectedError);
+
         expect(tools.exit.failure).toHaveBeenCalled();
       });
     });

__tests__/pushPayload.test.ts

@@ -1,4 +1,5 @@
 import { Octokit } from '@octokit/rest';
+import cloneDeep from 'lodash/cloneDeep';
 import nock from 'nock';
 import PushPayload from '../src/pushPayload';
 import MembersFile from '../src/membersFile';
@@ -7,14 +8,10 @@ import commitComparisonWithMembersFile from './fixtures/commitComparisonWithMemb
 import commitComparisonWithoutMembersFile from './fixtures/commitComparisonWithoutMembersFile.json';
 
 describe('PushPayload test suite', () => {
-  let payload: PushPayload;
-
   describe('when reading organizationLogin', () => {
-    beforeEach(() => {
-      payload = new PushPayload(pushEventPayload);
-    });
-
     it('should return the login of the repository owner', () => {
+      const payload = new PushPayload(pushEventPayload);
+
       expect(payload.organizationLogin).toEqual('coglinc');
     });
   });
@@ -23,17 +20,13 @@ describe('PushPayload test suite', () => {
     const repo = { owner: 'coglinc', repo: '.github' };
     const github = new Octokit();
 
-    beforeEach(() => {
-      // @ts-ignore
-      payload = new PushPayload(pushEventPayload);
-    });
-
     afterEach(() => {
       nock.cleanAll();
     });
 
     describe('given file was modified', () => {
       it('should return true', async () => {
+        const payload = new PushPayload(pushEventPayload);
         nock('https://api.github.com')
           .get(/\/repos\/.*\/.*\/compare/)
           .reply(200, () => {
@@ -48,6 +41,7 @@ describe('PushPayload test suite', () => {
 
     describe('given file was not modified', () => {
       it('should return false', async () => {
+        const payload = new PushPayload(pushEventPayload);
         nock('https://api.github.com')
           .get(/\/repos\/.*\/.*\/compare/)
           .reply(200, () => {
@@ -60,4 +54,44 @@ describe('PushPayload test suite', () => {
       });
     });
   });
+
+  describe('when verifying if push is on default branch', () => {
+    describe('given push to default branch', () => {
+      it('should return true', () => {
+        const payload = new PushPayload(pushEventPayload);
+
+        expect(payload.isDefaultBranch()).toBe(true);
+      });
+    });
+
+    describe('given push to any non-default branch', () => {
+      it('should return false', () => {
+        const pushToFeatureBranchEventPayload = cloneDeep(pushEventPayload);
+        pushToFeatureBranchEventPayload.ref = 'refs/heads/feature/test';
+        const payload = new PushPayload(pushToFeatureBranchEventPayload);
+
+        expect(payload.isDefaultBranch()).toBe(false);
+      });
+    });
+  });
+
+  describe('when verifying if repository is owned by an organization', () => {
+    describe('given an organization repository', () => {
+      it('should return true', () => {
+        const payload = new PushPayload(pushEventPayload);
+
+        expect(payload.isOrganizationOwned()).toBe(true);
+      });
+    });
+
+    describe('given a user repository', () => {
+      it('should return false', () => {
+        const pushToAUserRepositoryEventPayload = cloneDeep(pushEventPayload);
+        pushToAUserRepositoryEventPayload.repository.owner.type = 'User';
+        const payload = new PushPayload(pushToAUserRepositoryEventPayload);
+
+        expect(payload.isOrganizationOwned()).toBe(false);
+      });
+    });
+  });
 });

src/action.ts

@@ -51,6 +51,17 @@ export default async function (tools: Toolkit): Promise<void> {
     tools.log('Verify if organization membership file was modified');
 
     const payloadWrapper: PushPayload = new PushPayload(tools.context.payload as EventPayloads.WebhookPayloadPush);
+
+    if (!payloadWrapper.isOrganizationOwned()) {
+      tools.exit.failure('Not an organization repository, nothing to do');
+      return;
+    }
+
+    if (!payloadWrapper.isDefaultBranch()) {
+      tools.exit.success('Not working on default branch, nothing to do');
+      return;
+    }
+
     const membersFileModified: boolean = await payloadWrapper.fileWasModified(
       MembersFile.FILENAME,
       tools.context.repo,

src/pushPayload.ts

@@ -14,6 +14,14 @@ export default class PushPayload {
     return this.payload.repository.owner.login;
   }
 
+  isDefaultBranch(): boolean {
+    return this.payload.ref === `refs/heads/${this.payload.repository.default_branch}`;
+  }
+
+  isOrganizationOwned(): boolean {
+    return this.payload.repository.owner.type.toLowerCase() === 'organization';
+  }
+
   async fileWasModified(
     filename: string,
     repo: {