From 2fbe101fa940b22d20225846bec07035ae47c0a1 Mon Sep 17 00:00:00 2001 From: Louis-Solirius <148347068+Louis-Solirius@users.noreply.github.com> Date: Fri, 22 Nov 2024 14:39:01 +0000 Subject: [PATCH 1/2] SPL-473 - Add userAgent to feedback email (#290) * Add userAgent to feedback email * Update emailjs-mailer.test.js * Linting * Remove reqHeaders as a param in 'failed email sending' test block * Update userAgent initialisation --- app/emailjs-mailer.js | 7 ++++-- app/router.js | 2 +- test/unit/app/emailjs-mailer.test.js | 33 +++++++++++++++++++++++----- 3 files changed, 34 insertions(+), 8 deletions(-) diff --git a/app/emailjs-mailer.js b/app/emailjs-mailer.js index a500713d..e5949249 100644 --- a/app/emailjs-mailer.js +++ b/app/emailjs-mailer.js @@ -2,7 +2,7 @@ const emailjs = require('@emailjs/nodejs') const logger = require('./logger') const plannerText = 'Planner' -const sendMail = async (experience, moreDetails, emailjsIds, options) => { +const sendMail = async (experience, moreDetails, emailjsIds, options, reqHeaders) => { const currentDateTime = new Date().toLocaleString('en-GB', { timeZone: 'Europe/London', year: 'numeric', @@ -13,11 +13,14 @@ const sendMail = async (experience, moreDetails, emailjsIds, options) => { second: '2-digit' }) + const userAgent = reqHeaders?.['user-agent'] || '' + const templateParams = { experience, moreDetails, dateTime: currentDateTime, - plannerOrEligibility: plannerText + plannerOrEligibility: plannerText, + userAgent } try { diff --git a/app/router.js b/app/router.js index 2d1894f5..a9ade636 100644 --- a/app/router.js +++ b/app/router.js @@ -424,7 +424,7 @@ router } const experience = req.body.feedback const moreDetail = req.body['feedback-more-detail'] - emailJSEmail(experience, moreDetail, emailjsIds, options).then(() => + emailJSEmail(experience, moreDetail, emailjsIds, options, req.headers).then(() => res.redirect('/feedback/confirmation') ) }) diff --git a/test/unit/app/emailjs-mailer.test.js b/test/unit/app/emailjs-mailer.test.js index 61ded69b..5c2ac906 100644 --- a/test/unit/app/emailjs-mailer.test.js +++ b/test/unit/app/emailjs-mailer.test.js @@ -33,9 +33,10 @@ describe('sendMail', function () { const plannerText = 'Planner' const emailjsIds = { serviceID: 'test_service', templateID: 'test_template' } const options = { publicKey: 'test_public', privateKey: 'test_private' } + const userAgent = { 'user-agent': 'test-agent' } it('should call emailjs.send with correct parameters', async function () { - await sendMail(experience, moreDetails, emailjsIds, options) + await sendMail(experience, moreDetails, emailjsIds, options, userAgent) expect(emailjsSendStub.calledOnce).to.equal(true) const args = emailjsSendStub.getCall(0).args @@ -45,12 +46,13 @@ describe('sendMail', function () { expect(templateParams.experience).to.equal(experience) expect(templateParams.moreDetails).to.equal(moreDetails) expect(templateParams.plannerOrEligibility).to.equal(plannerText) + expect(templateParams.userAgent).to.equal(userAgent['user-agent']) expect(templateParams).to.have.property('dateTime') expect(args[3]).to.equal(options) }) it('should log success message when email is sent successfully', async function () { - await sendMail(experience, moreDetails, emailjsIds, options) + await sendMail(experience, moreDetails, emailjsIds, options, userAgent) expect(loggerInfoStub.calledOnce).to.equal(true) const logArgs = loggerInfoStub.getCall(0).args[0] @@ -58,20 +60,41 @@ describe('sendMail', function () { expect(logArgs.eventType).to.equal('MailEvent') expect(logArgs.eventResult).to.equal('Success') }) + + it('should handle undefined userAgent gracefully', async function () { + const experience = 'Great service!' + const moreDetails = 'No additional feedback' + const emailjsIds = { serviceID: 'test_service', templateID: 'test_template' } + const options = { publicKey: 'test_public', privateKey: 'test_private' } + const userAgent = undefined + + await sendMail(experience, moreDetails, emailjsIds, options, userAgent) + + expect(emailjsSendStub.calledOnce).to.equal(true) + const args = emailjsSendStub.getCall(0).args + expect(args[0]).to.equal(emailjsIds.serviceID) + expect(args[1]).to.equal(emailjsIds.templateID) + const templateParams = args[2] + expect(templateParams.experience).to.equal(experience) + expect(templateParams.moreDetails).to.equal(moreDetails) + expect(templateParams.userAgent).to.equal('') + expect(templateParams).to.have.property('dateTime') + expect(args[3]).to.equal(options) + }) }) describe('failed email sending', function () { const experience = 'Great service!' const moreDetails = 'No additional feedback' - const reqHeaders = { 'User-Agent': 'test-agent' } const emailjsIds = { serviceID: 'test_service', templateID: 'test_template' } const options = { publicKey: 'test_public', privateKey: 'test_private' } + const userAgent = { 'user-agent': 'test-agent' } it('should log error message when emailjs.send rejects', async function () { const error = { text: 'Error sending email' } emailjsSendStub.rejects(error) - await sendMail(experience, moreDetails, reqHeaders, emailjsIds, options) + await sendMail(experience, moreDetails, emailjsIds, options, userAgent) expect(loggerErrorStub.calledOnce).to.equal(true) const logArgs = loggerErrorStub.getCall(0).args[0] @@ -85,7 +108,7 @@ describe('sendMail', function () { const error = { text: 'Unexpected error' } emailjsSendStub.throws(error) - await sendMail(experience, moreDetails, reqHeaders, emailjsIds, options) + await sendMail(experience, moreDetails, emailjsIds, options) expect(loggerErrorStub.calledOnce).to.equal(true) const logArgs = loggerErrorStub.getCall(0).args[0] From 8664c94f62823ce46e56b2569816df6cc9cf76a4 Mon Sep 17 00:00:00 2001 From: Devin Rathod <119806596+devind-ra@users.noreply.github.com> Date: Fri, 22 Nov 2024 14:47:54 +0000 Subject: [PATCH 2/2] changing import in server.js to work with govuk-frontend module (#291) --- server.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server.js b/server.js index a504a30f..b4e754a7 100644 --- a/server.js +++ b/server.js @@ -220,7 +220,7 @@ function initialisePublic (app) { app.use('/public', express.static(path.join(__dirname, '/public'))) app.use( '/', - express.static(path.join(__dirname, '/node_modules/govuk-frontend/govuk/')) + express.static(path.join(__dirname, '/node_modules/govuk-frontend/dist/govuk/')) ) }