Improve error handling for feedback collector

- Use default account ID if no email is provided - Extract posting to JIRA as a separate method - Throw exception if attempting to create requestBody with no formName
hathitrust · Jun 7, 2023 · 188f29f · 188f29f
1 parent 3988f18
commit 188f29f
Show file tree

Hide file tree

Showing 3 changed files with 81 additions and 39 deletions.
diff --git a/customers.js b/customers.js
@@ -81,6 +81,9 @@ addCustomerToServiceDesk = async (account) => {
 //returns account ID of customer
 exports.getCustomerRecord = async (email, name) => {
 
+  // bail out early if no email was provided
+  if(!email) { return HT_ACCOUNT_ID }
+
   //encode symbols in email address before passing to Jira
   const encodedEmail = encodeURIComponent(email);
 

diff --git a/routes/index.js b/routes/index.js
@@ -12,9 +12,6 @@ const JIRA_ENDPOINT = process.env.JIRA_ENDPOINT;
 const JIRA_KEY = process.env.JIRA_KEY;
 const JIRA_USERNAME = process.env.JIRA_USERNAME;
 
-let descriptionBuilt = true;
-let errorMessage = '';
-
 //headers for general Jira http requests
 const headerOptions = {
   username: JIRA_USERNAME,
@@ -33,11 +30,7 @@ const buildDescription = async (requestBodyObject) => {
   } else if (requestBodyObject.formName == 'content-correction') {
     return `*CONTENT QUALITY CORRECTION* \n\n URL of book with problem: ${requestBodyObject.bookURL} \n Title of book: ${requestBodyObject.itemTitle} \n Overall quality: ${requestBodyObject.imageQuality} \n Specific page image problems: ${requestBodyObject.imageProblems} \n\n Other: ${requestBodyObject.description} \n\n User agent: ${requestBodyObject.userAgent} \n User URL: ${requestBodyObject.userURL} \n User auth: ${requestBodyObject.userAuthStatus}`;
   } else {
-    // TODO - use exception handling; test the error handling
-    descriptionBuilt = false;
-    errorMessage =
-      'Issue description did not build, check formName variable is set on front-end form';
-    return;
+    throw new Error('Issue description did not build, check formName variable is set on front-end form');
   }
 };
 
@@ -56,6 +49,28 @@ const buildGSRequest = async (requestBodyObject, accountID) => {
   return JSON.stringify(bodyObject);
 };
 
+const createJiraIssue = async(gsRequestBody, headerOptions, res) => {
+  const httpResponse = await needle(
+    'post',
+    `${JIRA_ENDPOINT}/rest/servicedeskapi/request`,
+    gsRequestBody,
+    headerOptions
+  );
+  const jiraResponse = httpResponse.body;
+
+  //error handling for the Jira response
+  if (httpResponse.statusCode == 201) {
+    //issue created successfully, send back 200 OK along with response object
+    res.status(200).json(jiraResponse);
+  } else {
+    //something went wrong, send back 500, response object and console error/message
+    res.status(500).json(jiraResponse);
+    console.error(
+      'Jira issue not created, error: ', jiraResponse.errors
+    );
+  }
+}
+
 router.post(
   '/',
   //TODO
@@ -75,29 +90,11 @@ router.post(
       const gsRequestBody = await buildGSRequest(requestBodyObject, customerID);
 
       // do the dang posting of the service desk request
-      const createIssue = await needle(
-        'post',
-        `${JIRA_ENDPOINT}/rest/servicedeskapi/request`,
-        gsRequestBody,
-        headerOptions
-      );
-      const jiraResp = createIssue.body;
-      const jiraStatus = createIssue.statusCode;
-
-      //error handling for the Jira response
-      if (descriptionBuilt && jiraStatus == 201) {
-        //issue created successfully, send back 200 OK along with response object
-        res.status(200).json(jiraResp);
-      } else {
-        //something went wrong, send back 500, response object and console error/message
-        res.status(500).json(jiraResp);
-        console.error(
-          'Jira issue not created, error: ',
-          errorMessage || jiraResp.errors
-        );
-      }
+      await(createJiraIssue(gsRequestBody, headerOptions, res))
+
     } catch (error) {
-      res.status(500).json({ error });
+      res.status(500).json({ error: error.message });
+      console.error(error)
     }
   }
 );

diff --git a/test/index.js b/test/index.js
@@ -16,6 +16,16 @@ const app = require('../app');
 const nock = require('nock');
 const sinon = require('sinon');
 
+function mockJiraCustomer() {
+  return nock(JIRA_ENDPOINT)
+    .get("/rest/api/latest/user/search")
+    .query({ query: /.*/ })
+    .reply(200, [{ accountId: "fake-account-id" }])
+
+    .post(`/rest/servicedeskapi/servicedesk/${GS_SERVICE_DESK_ID}/customer`)
+    .reply(204)
+}
+
 describe('application', function() {
 
   beforeEach(function() {
@@ -48,27 +58,59 @@ describe('application', function() {
   });
 
   it('with mocked JIRA API, POST /api should give a 200 and pass through what JIRA returns', async function() {
-    const scope = nock(JIRA_ENDPOINT)
+    scope = mockJiraCustomer()
+      .post("/rest/servicedeskapi/request")
+      .reply(201, { response: "fake-response" })
+
+    response = await request(app)
+      .post('/api')
+    .send({summary: 'summary', formName: 'basic-form', email: 'somebody@somewhere.test'})
 
-      .get("/rest/api/latest/user/search")
-      .query({ query: /.*/ })
-      .reply(200, [{ accountId: "fake-account-id" }])
+    expect(response.status).to.equal(200)
+    expect(response.body).to.eql({ response: "fake-response" })
+    expect(scope.isDone())
 
-      .post(`/rest/servicedeskapi/servicedesk/${GS_SERVICE_DESK_ID}/customer`)
-      .reply(204)
+  });
 
+  it('when JIRA returns an error, POST /api gives a 500 and passes through what JIRA returns', async function() {
+    scope = mockJiraCustomer()
       .post("/rest/servicedeskapi/request")
-      .reply(201, { response: "fake-response" })
+      .reply(401, { message: "fake-error" })
 
     response = await request(app)
       .post('/api')
     .send({summary: 'summary', formName: 'basic-form'})
 
-    expect(response.status).to.equal(200)
-    expect(response.body).to.eql({ response: "fake-response" })
+    expect(response.status).to.equal(500)
+    expect(response.body).to.eql({ message: "fake-error" })
+    expect(scope.isDone())
+  });
 
+  it('with no parameters, returns a 500 and doesnt call jira', async function() {
+    response = await request(app)
+      .post('/api')
+      .send()
+
+    expect(response.status).to.equal(500)
+    expect(response.body.error).to.include("formName")
+
+    // n.b.: nock would throw an error if something was called unexpectedly
   });
 
+  it('with no email parameter, posts issue with default customer', async function() {
+    scope = nock(JIRA_ENDPOINT)
+      .post("/rest/servicedeskapi/request")
+      .reply(201)
+
+    response = await request(app)
+      .post('/api')
+      .send({summary: 'summary', formName: 'basic-form'})
+
+    expect(response.status).to.equal(200)
+    expect(scope.isDone())
+  });
+
+
   context("with spied logger", function() { 
     const sandbox = sinon.createSandbox();