From 6deee4a577ac4708f22da8489613700c2e0bcfa4 Mon Sep 17 00:00:00 2001 From: Aaron Elkiss Date: Wed, 7 Jun 2023 13:18:25 -0400 Subject: [PATCH 1/3] Use nock for mocking jira API - parameterize service desk ID and JIRA endpoint - test simplest end-to-end workflow for API --- .env.example | 3 ++- customers.js | 15 ++++++++---- package-lock.json | 60 +++++++++++++++++++++++++++++++++++++++++++++++ package.json | 1 + routes/index.js | 10 ++++---- test/index.js | 44 +++++++++++++++++++++++++++++++++- 6 files changed, 122 insertions(+), 11 deletions(-) diff --git a/.env.example b/.env.example index fa88f91..0fddf5a 100644 --- a/.env.example +++ b/.env.example @@ -1,7 +1,8 @@ +JIRA_ENDPOINT='https://whatever.atlassian.net' JIRA_USERNAME = '' JIRA_KEY = '' HT_ACCOUNT_ID = '' # General Support queue GS_SERVICE_DESK_ID = '' -GS_REQUEST_TYPE_ID = '' \ No newline at end of file +GS_REQUEST_TYPE_ID = '' diff --git a/customers.js b/customers.js index 247e239..1a08f2c 100644 --- a/customers.js +++ b/customers.js @@ -1,8 +1,10 @@ const needle = require("needle"); -const JIRA_USERNAME = process.env.JIRA_USERNAME; -const JIRA_KEY = process.env.JIRA_KEY; +const GS_SERVICE_DESK_ID = process.env.GS_SERVICE_DESK_ID; const HT_ACCOUNT_ID = process.env.HT_ACCOUNT_ID; +const JIRA_ENDPOINT = process.env.JIRA_ENDPOINT; +const JIRA_KEY = process.env.JIRA_KEY; +const JIRA_USERNAME = process.env.JIRA_USERNAME; const headerOptions = { username: JIRA_USERNAME, @@ -22,7 +24,7 @@ createNewCustomer = async (email, name) => { try { let createCustomer = await needle( "post", - "https://hathitrust.atlassian.net/rest/servicedeskapi/customer", + `${JIRA_ENDPOINT}/rest/servicedeskapi/customer`, createCustomerData, headerOptions ); @@ -55,7 +57,7 @@ addCustomerToServiceDesk = async (account) => { try { let addCustomer = await needle( "post", - "https://hathitrust.atlassian.net/rest/servicedeskapi/servicedesk/8/customer", + `${JIRA_ENDPOINT}/rest/servicedeskapi/servicedesk/${GS_SERVICE_DESK_ID}/customer`, customerAccountID, headerOptions ); @@ -78,6 +80,9 @@ addCustomerToServiceDesk = async (account) => { //returns account ID of customer exports.getCustomerRecord = async (email, name) => { + // TODO: make sure we have something that looks like a complete email address? + // TODO: What if the email address matches multiple things? + //encode symbols in email address before passing to Jira const encodedEmail = encodeURIComponent(email); @@ -85,7 +90,7 @@ exports.getCustomerRecord = async (email, name) => { //send GET request to general system /user endpoint let getCustomerData = await needle( "get", - `https://hathitrust.atlassian.net/rest/api/latest/user/search?query=${encodedEmail}`, + `${JIRA_ENDPOINT}/rest/api/latest/user/search?query=${encodedEmail}`, { headers: { "X-ExperimentalApi": "opt-in" }, username: JIRA_USERNAME, diff --git a/package-lock.json b/package-lock.json index f9ccf3a..38da82d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -18,6 +18,7 @@ "devDependencies": { "chai": "^4.3.7", "mocha": "^10.2.0", + "nock": "^13.3.1", "nodemon": "^2.0.20", "supertest": "^6.3.3" } @@ -981,6 +982,12 @@ "js-yaml": "bin/js-yaml.js" } }, + "node_modules/json-stringify-safe": { + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/json-stringify-safe/-/json-stringify-safe-5.0.1.tgz", + "integrity": "sha512-ZClg6AaYvamvYEE82d3Iyd3vSSIjQ+odgjaTzRuO3s7toCdFKczob2i0zCh7JE8kWn17yvAWhUVxvqGwUalsRA==", + "dev": true + }, "node_modules/locate-path": { "version": "6.0.0", "resolved": "https://registry.npmjs.org/locate-path/-/locate-path-6.0.0.tgz", @@ -996,6 +1003,12 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/lodash": { + "version": "4.17.21", + "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz", + "integrity": "sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==", + "dev": true + }, "node_modules/log-symbols": { "version": "4.1.0", "resolved": "https://registry.npmjs.org/log-symbols/-/log-symbols-4.1.0.tgz", @@ -1275,6 +1288,44 @@ "node": ">= 0.6" } }, + "node_modules/nock": { + "version": "13.3.1", + "resolved": "https://registry.npmjs.org/nock/-/nock-13.3.1.tgz", + "integrity": "sha512-vHnopocZuI93p2ccivFyGuUfzjq2fxNyNurp7816mlT5V5HF4SzXu8lvLrVzBbNqzs+ODooZ6OksuSUNM7Njkw==", + "dev": true, + "dependencies": { + "debug": "^4.1.0", + "json-stringify-safe": "^5.0.1", + "lodash": "^4.17.21", + "propagate": "^2.0.0" + }, + "engines": { + "node": ">= 10.13" + } + }, + "node_modules/nock/node_modules/debug": { + "version": "4.3.4", + "resolved": "https://registry.npmjs.org/debug/-/debug-4.3.4.tgz", + "integrity": "sha512-PRWFHuSU3eDtQJPvnNY7Jcket1j0t5OuOsFzPPzsekD52Zl8qUfFIPEiswXqIvHWGVHOgX+7G/vCNNhehwxfkQ==", + "dev": true, + "dependencies": { + "ms": "2.1.2" + }, + "engines": { + "node": ">=6.0" + }, + "peerDependenciesMeta": { + "supports-color": { + "optional": true + } + } + }, + "node_modules/nock/node_modules/ms": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz", + "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==", + "dev": true + }, "node_modules/nodemon": { "version": "2.0.20", "resolved": "https://registry.npmjs.org/nodemon/-/nodemon-2.0.20.tgz", @@ -1460,6 +1511,15 @@ "url": "https://github.com/sponsors/jonschlinkert" } }, + "node_modules/propagate": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/propagate/-/propagate-2.0.1.tgz", + "integrity": "sha512-vGrhOavPSTz4QVNuBNdcNXePNdNMaO1xj9yBeH1ScQPjk/rhg9sSlCXPhMkFuaNNW/syTvYqsnbIJxMBfRbbag==", + "dev": true, + "engines": { + "node": ">= 8" + } + }, "node_modules/proxy-addr": { "version": "2.0.7", "resolved": "https://registry.npmjs.org/proxy-addr/-/proxy-addr-2.0.7.tgz", diff --git a/package.json b/package.json index f424d0c..0ff25e3 100644 --- a/package.json +++ b/package.json @@ -21,6 +21,7 @@ "devDependencies": { "chai": "^4.3.7", "mocha": "^10.2.0", + "nock": "^13.3.1", "nodemon": "^2.0.20", "supertest": "^6.3.3" } diff --git a/routes/index.js b/routes/index.js index b188ca9..ebb804c 100644 --- a/routes/index.js +++ b/routes/index.js @@ -6,10 +6,11 @@ const needle = require('needle'); const { getCustomerRecord } = require('../customers'); //Env vars -const JIRA_USERNAME = process.env.JIRA_USERNAME; -const JIRA_KEY = process.env.JIRA_KEY; -const GS_SERVICE_DESK_ID = process.env.GS_SERVICE_DESK_ID; const GS_REQUEST_TYPE_ID = process.env.GS_REQUEST_TYPE_ID; +const GS_SERVICE_DESK_ID = process.env.GS_SERVICE_DESK_ID; +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 = ''; @@ -32,6 +33,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'; @@ -75,7 +77,7 @@ router.post( // do the dang posting of the service desk request const createIssue = await needle( 'post', - 'https://hathitrust.atlassian.net/rest/servicedeskapi/request', + `${JIRA_ENDPOINT}/rest/servicedeskapi/request`, gsRequestBody, headerOptions ); diff --git a/test/index.js b/test/index.js index c7cd4a2..53ad20d 100644 --- a/test/index.js +++ b/test/index.js @@ -1,11 +1,33 @@ +// configuration for tests - would come from the environment for the real app. Must +// be defined before loading the application. defines consts here only for the +// ones that tests use. +const JIRA_ENDPOINT = process.env.JIRA_ENDPOINT = 'https://jira-endpoint.default.test' +process.env.JIRA_USERNAME = 'example_username' +process.env.JIRA_KEY = 'example_key' +process.env.HT_ACCOUNT_ID = 'fake_account_id' +const GS_SERVICE_DESK_ID = process.env.GS_SERVICE_DESK_ID = '99' +process.env.GS_REQUEST_TYPE_ID = '999' + // index.js const expect = require('chai').expect; const request = require('supertest'); const app = require('../app') +const nock = require('nock') + describe('application', function() { + beforeEach(function() { + nock.disableNetConnect() + nock.enableNetConnect('127.0.0.1') + }) + + afterEach(function() { + nock.cleanAll() + nock.enableNetConnect() + }) + it('should exist', function() { expect(app).to.not.be.null }); @@ -25,7 +47,27 @@ describe('application', function() { expect(response.status).to.equal(404) }); - it('with mocked JIRA API, POST /api should give a 200') + it('with mocked JIRA API, POST /api should give a 200 and pass through what JIRA returns', async function() { + const scope = 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) + + .post("/rest/servicedeskapi/request") + .reply(201, { response: "fake-response" }) + + 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" }) + + }); }); From 3988f1802fa9b2762f8294536ef8f3c2cee2ef0c Mon Sep 17 00:00:00 2001 From: Aaron Elkiss Date: Wed, 7 Jun 2023 15:23:41 -0400 Subject: [PATCH 2/3] DEV-726: Log requests using morgan middleware - Adds sinon to spy on stdout to ensure logging happens as expected - Doesn't do anything about the existing console.log calls - Removes TODOs for email address validation (based on conversation in PR) --- app.js | 4 + customers.js | 2 - package-lock.json | 193 ++++++++++++++++++++++++++++++++++++++++++++++ package.json | 2 + test/index.js | 23 +++++- 5 files changed, 219 insertions(+), 5 deletions(-) diff --git a/app.js b/app.js index df97f55..2d8b8e4 100644 --- a/app.js +++ b/app.js @@ -1,5 +1,6 @@ const express = require("express"); const cors = require("cors"); +const morgan = require("morgan"); const rateLimit = require("express-rate-limit"); const app = express(); @@ -12,6 +13,9 @@ app.use("/api", express.json()); // Enable cors app.use("/api", cors()); +// enable logging +app.use(morgan('combined')) + // Rate limiting // returns 429 'Too many requests' error if limit is hit const limiter = rateLimit({ diff --git a/customers.js b/customers.js index 1a08f2c..73ac6f9 100644 --- a/customers.js +++ b/customers.js @@ -80,8 +80,6 @@ addCustomerToServiceDesk = async (account) => { //returns account ID of customer exports.getCustomerRecord = async (email, name) => { - // TODO: make sure we have something that looks like a complete email address? - // TODO: What if the email address matches multiple things? //encode symbols in email address before passing to Jira const encodedEmail = encodeURIComponent(email); diff --git a/package-lock.json b/package-lock.json index 38da82d..086af70 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,6 +13,7 @@ "dotenv": "^16.0.3", "express": "^4.18.2", "express-rate-limit": "^6.7.0", + "morgan": "^1.10.0", "needle": "^3.2.0" }, "devDependencies": { @@ -20,9 +21,54 @@ "mocha": "^10.2.0", "nock": "^13.3.1", "nodemon": "^2.0.20", + "sinon": "^15.1.0", "supertest": "^6.3.3" } }, + "node_modules/@sinonjs/commons": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/@sinonjs/commons/-/commons-3.0.0.tgz", + "integrity": "sha512-jXBtWAF4vmdNmZgD5FoKsVLv3rPgDnLgPbU84LIJ3otV44vJlDRokVng5v8NFJdCf/da9legHcKaRuZs4L7faA==", + "dev": true, + "dependencies": { + "type-detect": "4.0.8" + } + }, + "node_modules/@sinonjs/fake-timers": { + "version": "10.2.0", + "resolved": "https://registry.npmjs.org/@sinonjs/fake-timers/-/fake-timers-10.2.0.tgz", + "integrity": "sha512-OPwQlEdg40HAj5KNF8WW6q2KG4Z+cBCZb3m4ninfTZKaBmbIJodviQsDBoYMPHkOyJJMHnOJo5j2+LKDOhOACg==", + "dev": true, + "dependencies": { + "@sinonjs/commons": "^3.0.0" + } + }, + "node_modules/@sinonjs/samsam": { + "version": "8.0.0", + "resolved": "https://registry.npmjs.org/@sinonjs/samsam/-/samsam-8.0.0.tgz", + "integrity": "sha512-Bp8KUVlLp8ibJZrnvq2foVhP0IVX2CIprMJPK0vqGqgrDa0OHVKeZyBykqskkrdxV6yKBPmGasO8LVjAKR3Gew==", + "dev": true, + "dependencies": { + "@sinonjs/commons": "^2.0.0", + "lodash.get": "^4.4.2", + "type-detect": "^4.0.8" + } + }, + "node_modules/@sinonjs/samsam/node_modules/@sinonjs/commons": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/@sinonjs/commons/-/commons-2.0.0.tgz", + "integrity": "sha512-uLa0j859mMrg2slwQYdO/AkrOfmH+X6LTVmNTS9CqexuE2IvVORIkSpJLqePAbEnKJ77aMmCwr1NUZ57120Xcg==", + "dev": true, + "dependencies": { + "type-detect": "4.0.8" + } + }, + "node_modules/@sinonjs/text-encoding": { + "version": "0.7.2", + "resolved": "https://registry.npmjs.org/@sinonjs/text-encoding/-/text-encoding-0.7.2.tgz", + "integrity": "sha512-sXXKG+uL9IrKqViTtao2Ws6dy0znu9sOaP1di/jKGW1M6VssO8vlpXCQcpZ+jisQ1tTFAC5Jo/EOzFbggBagFQ==", + "dev": true + }, "node_modules/abbrev": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz", @@ -125,6 +171,22 @@ "integrity": "sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==", "dev": true }, + "node_modules/basic-auth": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/basic-auth/-/basic-auth-2.0.1.tgz", + "integrity": "sha512-NF+epuEdnUYVlGuhaxbbq+dvJttwLnGY+YixlXlME5KpQ5W3CnXA5cVTneY3SPbPDRkcjMbifrwmFYcClgOZeg==", + "dependencies": { + "safe-buffer": "5.1.2" + }, + "engines": { + "node": ">= 0.8" + } + }, + "node_modules/basic-auth/node_modules/safe-buffer": { + "version": "5.1.2", + "resolved": "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.1.2.tgz", + "integrity": "sha512-Gd2UZBJDkXlY7GbJxfsE8/nvKkUEU1G38c1siN6QP6a9PT9MmHB8GnpscSmMJSoF8LOIrt8ud/wPtojys4G6+g==" + }, "node_modules/binary-extensions": { "version": "2.2.0", "resolved": "https://registry.npmjs.org/binary-extensions/-/binary-extensions-2.2.0.tgz", @@ -970,6 +1032,12 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/isarray": { + "version": "0.0.1", + "resolved": "https://registry.npmjs.org/isarray/-/isarray-0.0.1.tgz", + "integrity": "sha512-D2S+3GLxWH+uhrNEcoh/fnmYeP8E8/zHl644d/jdA0g2uyXvy3sb0qxotE+ne0LtccHknQzWwZEzhak7oJ0COQ==", + "dev": true + }, "node_modules/js-yaml": { "version": "4.1.0", "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.1.0.tgz", @@ -988,6 +1056,12 @@ "integrity": "sha512-ZClg6AaYvamvYEE82d3Iyd3vSSIjQ+odgjaTzRuO3s7toCdFKczob2i0zCh7JE8kWn17yvAWhUVxvqGwUalsRA==", "dev": true }, + "node_modules/just-extend": { + "version": "4.2.1", + "resolved": "https://registry.npmjs.org/just-extend/-/just-extend-4.2.1.tgz", + "integrity": "sha512-g3UB796vUFIY90VIv/WX3L2c8CS2MdWUww3CNrYmqza1Fg0DURc2K/O4YrnklBdQarSJ/y8JnJYDGc+1iumQjg==", + "dev": true + }, "node_modules/locate-path": { "version": "6.0.0", "resolved": "https://registry.npmjs.org/locate-path/-/locate-path-6.0.0.tgz", @@ -1009,6 +1083,12 @@ "integrity": "sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==", "dev": true }, + "node_modules/lodash.get": { + "version": "4.4.2", + "resolved": "https://registry.npmjs.org/lodash.get/-/lodash.get-4.4.2.tgz", + "integrity": "sha512-z+Uw/vLuy6gQe8cfaFWD7p0wVv8fJl3mbzXh33RS+0oW2wvUqiRXiQ69gLWSLpgB5/6sU+r6BlQR0MBILadqTQ==", + "dev": true + }, "node_modules/log-symbols": { "version": "4.1.0", "resolved": "https://registry.npmjs.org/log-symbols/-/log-symbols-4.1.0.tgz", @@ -1223,6 +1303,32 @@ "url": "https://github.com/chalk/supports-color?sponsor=1" } }, + "node_modules/morgan": { + "version": "1.10.0", + "resolved": "https://registry.npmjs.org/morgan/-/morgan-1.10.0.tgz", + "integrity": "sha512-AbegBVI4sh6El+1gNwvD5YIck7nSA36weD7xvIxG4in80j/UoK8AEGaWnnz8v1GxonMCltmlNs5ZKbGvl9b1XQ==", + "dependencies": { + "basic-auth": "~2.0.1", + "debug": "2.6.9", + "depd": "~2.0.0", + "on-finished": "~2.3.0", + "on-headers": "~1.0.2" + }, + "engines": { + "node": ">= 0.8.0" + } + }, + "node_modules/morgan/node_modules/on-finished": { + "version": "2.3.0", + "resolved": "https://registry.npmjs.org/on-finished/-/on-finished-2.3.0.tgz", + "integrity": "sha512-ikqdkGAAyf/X/gPhXGvfgAytDZtDbr+bkNUJ0N9h5MI/dmdgCs3l6hoHrcUv41sRKew3jIwrp4qQDXiK99Utww==", + "dependencies": { + "ee-first": "1.1.1" + }, + "engines": { + "node": ">= 0.8" + } + }, "node_modules/ms": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/ms/-/ms-2.0.0.tgz", @@ -1288,6 +1394,37 @@ "node": ">= 0.6" } }, + "node_modules/nise": { + "version": "5.1.4", + "resolved": "https://registry.npmjs.org/nise/-/nise-5.1.4.tgz", + "integrity": "sha512-8+Ib8rRJ4L0o3kfmyVCL7gzrohyDe0cMFTBa2d364yIrEGMEoetznKJx899YxjybU6bL9SQkYPSBBs1gyYs8Xg==", + "dev": true, + "dependencies": { + "@sinonjs/commons": "^2.0.0", + "@sinonjs/fake-timers": "^10.0.2", + "@sinonjs/text-encoding": "^0.7.1", + "just-extend": "^4.0.2", + "path-to-regexp": "^1.7.0" + } + }, + "node_modules/nise/node_modules/@sinonjs/commons": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/@sinonjs/commons/-/commons-2.0.0.tgz", + "integrity": "sha512-uLa0j859mMrg2slwQYdO/AkrOfmH+X6LTVmNTS9CqexuE2IvVORIkSpJLqePAbEnKJ77aMmCwr1NUZ57120Xcg==", + "dev": true, + "dependencies": { + "type-detect": "4.0.8" + } + }, + "node_modules/nise/node_modules/path-to-regexp": { + "version": "1.8.0", + "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-1.8.0.tgz", + "integrity": "sha512-n43JRhlUKUAlibEJhPeir1ncUID16QnEjNpwzNdO3Lm4ywrBpBZ5oLD0I6br9evr1Y9JTqwRtAh7JLoOzAQdVA==", + "dev": true, + "dependencies": { + "isarray": "0.0.1" + } + }, "node_modules/nock": { "version": "13.3.1", "resolved": "https://registry.npmjs.org/nock/-/nock-13.3.1.tgz", @@ -1420,6 +1557,14 @@ "node": ">= 0.8" } }, + "node_modules/on-headers": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/on-headers/-/on-headers-1.0.2.tgz", + "integrity": "sha512-pZAE+FJLoyITytdqK0U5s+FIpjN0JP3OzFi/u8Rx+EV5/W+JTWGXG8xFzevE7AjBfDqHv/8vL8qQsIhHnqRkrA==", + "engines": { + "node": ">= 0.8" + } + }, "node_modules/once": { "version": "1.4.0", "resolved": "https://registry.npmjs.org/once/-/once-1.4.0.tgz", @@ -1732,6 +1877,54 @@ "semver": "bin/semver.js" } }, + "node_modules/sinon": { + "version": "15.1.0", + "resolved": "https://registry.npmjs.org/sinon/-/sinon-15.1.0.tgz", + "integrity": "sha512-cS5FgpDdE9/zx7no8bxROHymSlPLZzq0ChbbLk1DrxBfc+eTeBK3y8nIL+nu/0QeYydhhbLIr7ecHJpywjQaoQ==", + "dev": true, + "dependencies": { + "@sinonjs/commons": "^3.0.0", + "@sinonjs/fake-timers": "^10.2.0", + "@sinonjs/samsam": "^8.0.0", + "diff": "^5.1.0", + "nise": "^5.1.4", + "supports-color": "^7.2.0" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/sinon" + } + }, + "node_modules/sinon/node_modules/diff": { + "version": "5.1.0", + "resolved": "https://registry.npmjs.org/diff/-/diff-5.1.0.tgz", + "integrity": "sha512-D+mk+qE8VC/PAUrlAU34N+VfXev0ghe5ywmpqrawphmVZc1bEfn56uo9qpyGp1p4xpzOHkSW4ztBd6L7Xx4ACw==", + "dev": true, + "engines": { + "node": ">=0.3.1" + } + }, + "node_modules/sinon/node_modules/has-flag": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/has-flag/-/has-flag-4.0.0.tgz", + "integrity": "sha512-EykJT/Q1KjTWctppgIAgfSO0tKVuZUjhgMr17kqTumMl6Afv3EISleU7qZUzoXDFTAHTDC4NOoG/ZxU3EvlMPQ==", + "dev": true, + "engines": { + "node": ">=8" + } + }, + "node_modules/sinon/node_modules/supports-color": { + "version": "7.2.0", + "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-7.2.0.tgz", + "integrity": "sha512-qpCAvRl9stuOHveKsn7HncJRvv501qIacKzQlO/+Lwxc9+0q2wLyv4Dfvt80/DPn2pqOBsJdDiogXGR9+OvwRw==", + "dev": true, + "dependencies": { + "has-flag": "^4.0.0" + }, + "engines": { + "node": ">=8" + } + }, "node_modules/statuses": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/statuses/-/statuses-2.0.1.tgz", diff --git a/package.json b/package.json index 0ff25e3..cf8a670 100644 --- a/package.json +++ b/package.json @@ -16,6 +16,7 @@ "dotenv": "^16.0.3", "express": "^4.18.2", "express-rate-limit": "^6.7.0", + "morgan": "^1.10.0", "needle": "^3.2.0" }, "devDependencies": { @@ -23,6 +24,7 @@ "mocha": "^10.2.0", "nock": "^13.3.1", "nodemon": "^2.0.20", + "sinon": "^15.1.0", "supertest": "^6.3.3" } } diff --git a/test/index.js b/test/index.js index 53ad20d..4ca0df8 100644 --- a/test/index.js +++ b/test/index.js @@ -12,9 +12,9 @@ process.env.GS_REQUEST_TYPE_ID = '999' const expect = require('chai').expect; const request = require('supertest'); -const app = require('../app') -const nock = require('nock') - +const app = require('../app'); +const nock = require('nock'); +const sinon = require('sinon'); describe('application', function() { @@ -69,5 +69,22 @@ describe('application', function() { }); + context("with spied logger", function() { + const sandbox = sinon.createSandbox(); + + beforeEach( function() { + sandbox.spy(process.stdout,"write") + }) + + afterEach( function() { + sandbox.restore() + }) + + it('logs output', async function() { + response = await request(app) + .get('/nonexistent') + expect(process.stdout.write.getCall(0).args[0]).to.include('"GET /nonexistent HTTP/1.1" 404'); + }); + }); }); From 188f29f7122bbc7441d8f9afc37001a76b3f7d4c Mon Sep 17 00:00:00 2001 From: Aaron Elkiss Date: Wed, 7 Jun 2023 16:31:57 -0400 Subject: [PATCH 3/3] 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 --- customers.js | 3 +++ routes/index.js | 57 ++++++++++++++++++++++------------------------ test/index.js | 60 +++++++++++++++++++++++++++++++++++++++++-------- 3 files changed, 81 insertions(+), 39 deletions(-) diff --git a/customers.js b/customers.js index 73ac6f9..3766301 100644 --- 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 index ebb804c..8e524a9 100644 --- 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 index 4ca0df8..14b7e15 100644 --- 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();