assess PR comments

src/routes/remove-member.ts

@@ -1,11 +1,13 @@
 import { Router } from 'express';
 
+import { authentication } from '../middleware/authentication.middleware';
+
 import { get, post } from '../controller/remove-member.controller';
 import * as config from '../config';
 
 const removeMemberRouter = Router();
 
-removeMemberRouter.get(config.REMOVE_MEMBER_URL, get);
+removeMemberRouter.get(config.REMOVE_MEMBER_URL, authentication, get);
 removeMemberRouter.post(config.REMOVE_MEMBER_URL, post);
 
 export default removeMemberRouter;

src/views/layout.html

@@ -3,6 +3,10 @@
 {% from "govuk/components/footer/macro.njk"           import govukFooter %}
 {% from "govuk/components/header/macro.njk"           import govukHeader %}
 {% from "govuk/components/panel/macro.njk"            import govukPanel %}
+{% from "govuk/components/back-link/macro.njk"        import govukBackLink %}
+{% from "govuk/components/input/macro.njk"            import govukInput %}
+{% from "govuk/components/button/macro.njk"           import govukButton %}
+{% from "govuk/components/textarea/macro.njk"         import govukTextarea %}
 
 {% block head %}
 

src/views/remove-member.html

@@ -1,10 +1,5 @@
 {% extends "layout.html" %}
 
-{% from "govuk/components/back-link/macro.njk" import govukBackLink %}
-{% from "govuk/components/input/macro.njk" import govukInput %}
-{% from "govuk/components/button/macro.njk" import govukButton %}
-{% from "govuk/components/textarea/macro.njk" import govukTextarea %}
-
 {% block beforeContent %}
   {{ govukBackLink({
     text: "Back",
@@ -18,7 +13,7 @@
       <h1 class="govuk-heading-l">Remove a GitHub Member</h1>
 
       <p class="govuk-body">
-        When a user no longer requires access to our GitHub orgnizations(e.g they have left The Cabinet Office) they should be removed from the Cabinet Office GitHub orgnization.
+        When a user no longer requires access to our GitHub organisations(e.g they have left The Cabinet Office) they should be removed from the Cabinet Office GitHub organisation.
       </p>
 
       <form method="post" novalidate>
@@ -45,10 +40,11 @@ <h1 class="govuk-heading-l">Remove a GitHub Member</h1>
               text: "Include more details to why this user is getting removed."
             }
           }) }}
+
         {{ govukButton({
           text: "Save"
         }) }}
-
+        
       </form>
     </div>
   </div>

test/integration/routes/remove-member.spec.ts

@@ -1,4 +1,5 @@
 jest.mock('../../../src/middleware/logger.middleware');
+jest.mock('../../../src/middleware/authentication.middleware');
 
 import { jest, beforeEach, describe, expect, test } from '@jest/globals';
 import { Request, Response, NextFunction } from 'express';
@@ -8,6 +9,7 @@ import app from '../../../src/app';
 import * as config from '../../../src/config';
 import { logger } from '../../../src/middleware/logger.middleware';
 import { log } from '../../../src/utils/logger';
+import { authentication } from '../../../src/middleware/authentication.middleware';
 
 import { MOCK_REDIRECT_MESSSAGE, MOCK_GET_REMOVE_MEMBER_RESPONSE, MOCK_POST_REMOVE_MEMBER_RESPONSE } from '../../mock/text.mock';
 import { MOCK_POST_REMOVE_MEMBER } from '../../mock/data';
@@ -20,6 +22,8 @@ jest.mock('../../../src/utils/logger', () => ({
 
 const mockedLogger = logger as jest.Mock<typeof logger>;
 mockedLogger.mockImplementation((req: Request, res: Response, next: NextFunction) => next());
+const mockedAuth = authentication as jest.Mock<typeof authentication>;
+mockedAuth.mockImplementation((_req: Request, _res: Response, next: NextFunction) => next());
 
 describe('Remove-member endpoint integration tests', () => {
     beforeEach(() => {
@@ -33,6 +37,7 @@ describe('Remove-member endpoint integration tests', () => {
             expect(res.status).toEqual(200);
             expect(res.text).toContain(MOCK_GET_REMOVE_MEMBER_RESPONSE);
             expect(mockedLogger).toHaveBeenCalledTimes(1);
+            expect(mockedAuth).toHaveBeenCalledTimes(1);
         });
     });
     describe('POST tests', () => {
@@ -46,7 +51,9 @@ describe('Remove-member endpoint integration tests', () => {
         test('Should log the GitHub Handle and More Details on POST request.', async () => {
             const res = await request(app).post(config.REMOVE_MEMBER_URL).send(MOCK_POST_REMOVE_MEMBER);
 
-            expect(log.info).toBeCalledWith(MOCK_POST_REMOVE_MEMBER_RESPONSE);
+            const mockLog = log.info as jest.Mock;
+
+            expect(mockLog).toBeCalledWith(MOCK_POST_REMOVE_MEMBER_RESPONSE);
             expect(res.text).toContain(MOCK_REDIRECT_MESSSAGE);
             expect(mockedLogger).toHaveBeenCalledTimes(1);
         });

test/unit/controller/remove-member.controller.spec.ts

@@ -21,7 +21,6 @@ const req = {
 const mockResponse = () => {
     const res = {} as Response;
     res.render = jest.fn().mockReturnValue(res) as any;
-    res.send = jest.fn().mockReturnValue(res) as any;
     res.redirect = jest.fn().mockReturnValue(res) as any;
     return res;
 };
@@ -54,11 +53,12 @@ describe('Remove-member controller test suites', () => {
         test('should log GitHub handle and More Details on POST request', () => {
             const res = mockResponse();
 
-            const infoSpy = jest.spyOn(log, 'info');
+            const mockLogInfo = log.info as jest.Mock;
 
             post(req, res);
 
-            expect(infoSpy).toHaveBeenCalledWith(MOCK_POST_REMOVE_MEMBER_RESPONSE);
+            expect(mockLogInfo).toHaveBeenCalledWith(MOCK_POST_REMOVE_MEMBER_RESPONSE);
+
         });
     });
 });