Refactor to simplify http handler code

nhsuk · May 26, 2021 · 8f20db7 · 8f20db7
1 parent 7bfb4e8
commit 8f20db7
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 55 deletions.
diff --git a/comments/index.js b/comments/index.js
@@ -1,3 +1,3 @@
-const { handleRequest, requestTypes } = require('../lib/index.js');
+const { handleComments } = require('../lib/index.js');
 
-module.exports = (context, req) => handleRequest(context, req, requestTypes.COMMENTS);
+module.exports = (context, req) => handleComments(context, req);
diff --git a/healthcheck/index.js b/healthcheck/index.js
@@ -1,3 +1,3 @@
-const { handleRequest, requestTypes } = require('../lib/index.js');
+const { handleHealthcheck } = require('../lib/index.js');
 
-module.exports = async (context, req) => handleRequest(context, req, requestTypes.HEALTHCHECK);
+module.exports = async (context, req) => handleHealthcheck(context, req);
diff --git a/lib/index.js b/lib/index.js
@@ -2,58 +2,53 @@ const database = require('./database.js');
 const { HttpError } = require('./validation.js');
 const responses = require('./responses.js');
 
-const requestTypes = {
-  COMMENTS: 'COMMENTS',
-  HEALTHCHECK: 'HEALTHCHECK',
-  SATISFIED: 'SATISFIED',
+const handleError = (err) => {
+  // only return error message in response if it is a HttpError
+  if (err instanceof HttpError) {
+    return responses.httpError(err.statusCode, err.message);
+  }
+  console.error('500 ERROR:', err.message); // eslint-disable-line no-console
+  return responses.error500();
 };
 
-module.exports.requestTypes = requestTypes;
+module.exports.handleHealthcheck = async () => {
+  const healthy = await database.healthcheck();
+  if (!healthy) {
+    return responses.httpError(503, 'Service unavailable');
+  }
 
-/**
- * @param {Object} context - See azure function context documentation
- * @param {Object} req - See azure function request documentation
- * @param {string} requestType - requestTypes enum value representing which type of request this is
- * @returns {Object} response object
- */
-module.exports.handleRequest = async (context, req, requestType) => {
-  if (requestType === requestTypes.HEALTHCHECK) {
-    const healthy = await database.healthcheck();
-    if (!healthy) {
-      return responses.httpError(503, 'Service unavailable');
-    }
+  return responses.healthcheckOk();
+};
 
-    return responses.healthcheckOk();
+module.exports.handleSatisfied = async (context, req) => {
+  if (!req.body) {
+    return responses.nodata();
+  }
+  let token = req.body.token || null;
+  try {
+    token = await database.saveInitialResponse({
+      isSatisfied: req.body.isSatisfied,
+      token,
+      url: req.body.url,
+    });
+  } catch (err) {
+    return handleError(err);
   }
+  return responses.ok(token);
+};
 
+module.exports.handleComments = async (context, req) => {
   if (!req.body) {
     return responses.nodata();
   }
-
   let token = req.body.token || null;
-
   try {
-    if (requestType === requestTypes.SATISFIED) {
-      token = await database.saveInitialResponse({
-        isSatisfied: req.body.isSatisfied,
-        token,
-        url: req.body.url,
-      });
-    }
-    if (requestType === requestTypes.COMMENTS) {
-      token = await database.saveTextComments({
-        comments: req.body.comments,
-        token,
-      });
-    }
+    token = await database.saveTextComments({
+      comments: req.body.comments,
+      token,
+    });
   } catch (err) {
-    // only return error message in response if it is a HttpError
-    if (err instanceof HttpError) {
-      return responses.httpError(err.statusCode, err.message);
-    }
-    context.log('500 ERROR:', err.message);
-    return responses.error500();
+    return handleError(err);
   }
-
   return responses.ok(token);
 };
diff --git a/lib/index.test.js b/lib/index.test.js
@@ -1,6 +1,6 @@
 /* global expect jest */
 
-const { handleRequest, requestTypes } = require('./index.js');
+const { handleComments, handleSatisfied, handleHealthcheck } = require('./index.js');
 const { ValidationError } = require('./validation.js');
 const context = require('../tests/unit/defaultContext.js');
 
@@ -16,7 +16,7 @@ describe('http handler for /satisfied/', () => {
     const request = {
       body: { isSatisfied: true },
     };
-    const response = await handleRequest(context, request, requestTypes.SATISFIED);
+    const response = await handleSatisfied(context, request);
     expect(response).toMatchObject({ body: { status: 'ok' } });
     expect(database.saveInitialResponse).toBeCalledWith({ isSatisfied: true, token: null });
   });
@@ -27,7 +27,7 @@ describe('http handler for /satisfied/', () => {
         isSatisfied: false,
       },
     };
-    const response = await handleRequest(context, request, requestTypes.SATISFIED);
+    const response = await handleSatisfied(context, request);
     expect(response).toMatchObject({ body: { status: 'ok' } });
     expect(database.saveInitialResponse).toBeCalledWith({ isSatisfied: false, token: null });
   });
@@ -39,7 +39,7 @@ describe('http handler for /satisfied/', () => {
     const request = {
       body: { isSatisfied: true },
     };
-    const response = await handleRequest(context, request, requestTypes.SATISFIED);
+    const response = await handleSatisfied(context, request);
     expect(response).toMatchObject({ body: { error: 'Something went wrong' }, status: 500 });
   });
 
@@ -50,13 +50,13 @@ describe('http handler for /satisfied/', () => {
     const request = {
       body: { isSatisfied: true },
     };
-    const response = await handleRequest(context, request, requestTypes.SATISFIED);
+    const response = await handleSatisfied(context, request);
     expect(response).toMatchObject({ body: { error: 'some validation message' }, status: 400 });
   });
 
   test('returns error when no body given', async () => {
     const request = {};
-    const response = await handleRequest(context, request, requestTypes.SATISFIED);
+    const response = await handleSatisfied(context, request);
     expect(response).toMatchObject({ body: { error: 'Please pass a JSON request body' }, status: 400 });
   });
 });
@@ -73,7 +73,7 @@ describe('http handler for /comments/', () => {
         token: 'my-token',
       },
     };
-    const response = await handleRequest(context, request, requestTypes.COMMENTS);
+    const response = await handleComments(context, request);
     expect(response).toMatchObject({ body: { status: 'ok' } });
     expect(database.saveTextComments).toBeCalledWith({ comments: 'Needs more cowbell.', token: 'my-token' });
   });
@@ -87,14 +87,14 @@ describe('http handler for /healthcheck/', () => {
   test('returns ok when database is available', async () => {
     database.healthcheck = jest.fn().mockImplementation(async () => true);
     const request = {};
-    const response = await handleRequest(context, request, requestTypes.HEALTHCHECK);
+    const response = await handleHealthcheck(context, request);
     expect(response).toMatchObject({ body: { status: 'ok' } });
   });
 
   test('returns 503 when database is unavailable', async () => {
     database.healthcheck = jest.fn().mockImplementation(async () => false);
     const request = {};
-    const response = await handleRequest(context, request, requestTypes.HEALTHCHECK);
+    const response = await handleHealthcheck(context, request);
     expect(response).toMatchObject({ status: 503 });
   });
 });
diff --git a/satisfied/index.js b/satisfied/index.js
@@ -1,3 +1,3 @@
-const { handleRequest, requestTypes } = require('../lib/index.js');
+const { handleSatisfied } = require('../lib/index.js');
 
-module.exports = async (context, req) => handleRequest(context, req, requestTypes.SATISFIED);
+module.exports = async (context, req) => handleSatisfied(context, req);