From afdd200e84f721c159cc6b32236bcb7652f8d848 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Wed, 10 Jul 2024 12:08:19 +0100 Subject: [PATCH] form submission: error handling and logging for email notifications --- src/controllers/CheckAnswersController.js | 92 ++++++++++++++--------- test/unit/check-answers.test.js | 2 +- test/unit/checkAnswersController.test.js | 88 ++++++++++++---------- 3 files changed, 108 insertions(+), 74 deletions(-) diff --git a/src/controllers/CheckAnswersController.js b/src/controllers/CheckAnswersController.js index f47b7658..22766cbe 100644 --- a/src/controllers/CheckAnswersController.js +++ b/src/controllers/CheckAnswersController.js @@ -14,14 +14,25 @@ class CheckAnswersController extends PageController { * @param {Object} req - The HTTP request object. * @param {Object} res - The HTTP response object. * @param {Function} next - The next middleware function. - */ - post (req, res, next) { - this.sendEmails(req, res, next) + */ + async post (req, res, next) { + const result = await this.sendEmails(req, res, next) + for (const err of (result.errors ?? [])) { + console.error(err) + } super.post(req, res, next) } - sendEmails (req, res, next) { + /** + * Attempts to send email notifications. + * + * @param {*} req + * @param {*} res + * @param {*} next + * @returns {Promise<{} | {errors: string[]}>} + */ + async sendEmails (req, res, next) { const name = req.sessionModel.get('name') const email = req.sessionModel.get('email') const organisation = req.sessionModel.get('lpa') @@ -29,39 +40,52 @@ class CheckAnswersController extends PageController { const documentationUrl = req.sessionModel.get('documentation-url') const endpoint = req.sessionModel.get('endpoint-url') - const { RequestTemplateId, AcknowledgementTemplateId } = config.email.templates - - // ToDo: handle errors when sending emails - notifyClient.sendEmail( - RequestTemplateId, - dataManagementEmail, - { - personalisation: { - name, - email, - organisation, - endpoint, - 'documentation-url': documentationUrl, - dataset - } - } - ) + const { RequestTemplateId, AcknowledgementTemplateId } = + config.email.templates - notifyClient.sendEmail( - AcknowledgementTemplateId, + const personalisation = { + name, email, - { - personalisation: { - name, - email, - organisation, - endpoint, - 'documentation-url': documentationUrl, - dataset - } - } - ) + organisation, + endpoint, + 'documentation-url': documentationUrl, + dataset + } + + const [reqResult, ackResult] = await Promise.allSettled([ + notifyClient.sendEmail(RequestTemplateId, dataManagementEmail, { + personalisation + }), + notifyClient.sendEmail(AcknowledgementTemplateId, email, { + personalisation + }) + ]) + + const errors = [] + if (reqResult.status === 'rejected') { + const msg = emailFailureMessage(RequestTemplateId, personalisation) + errors.push(msg) + } + if (ackResult.status === 'rejected') { + const msg = emailFailureMessage(AcknowledgementTemplateId, personalisation) + errors.push(msg) + } + + if (errors.length !== 0) { + return { errors } + } + return {} } } +/** + * + * @param {string} templateId + * @param {{organisation: string, name: string, email: string}} metadata + * @returns + */ +function emailFailureMessage (templateId, { organisation, name, email }) { + return `Failed to send email template=${templateId} to (org: ${organisation}, name: ${name}, email: ${email}):` +} + export default CheckAnswersController diff --git a/test/unit/check-answers.test.js b/test/unit/check-answers.test.js index 75978127..69f505dc 100644 --- a/test/unit/check-answers.test.js +++ b/test/unit/check-answers.test.js @@ -7,7 +7,7 @@ import config from '../../config/index.js' import { stripWhitespace } from '../utils/stripWhiteSpace.js' import { mockDataSubjects } from './data.js' -describe('check-answers View', () => { +describe('check-answers View', async () => { const params = { values: { lpa: 'mockLpa', diff --git a/test/unit/checkAnswersController.test.js b/test/unit/checkAnswersController.test.js index 9a6ae6b1..f0988183 100644 --- a/test/unit/checkAnswersController.test.js +++ b/test/unit/checkAnswersController.test.js @@ -6,7 +6,41 @@ import config from '../../config/index.js' vi.mock('../../src/utils/mailClient.js') -describe('Check answers controller', () => { +function makeRequest () { + return { + sessionModel: { + get: vi.fn().mockImplementation((key) => { + const values = { + name: 'John Doe', + email: 'JohnDoe@mail.com', + lpa: 'LPA', + dataset: 'Dataset', + 'documentation-url': 'Documentation URL', + 'endpoint-url': 'Endpoint URL' + } + return values[key] + }) + } + } +} + +describe('Handle email notification handlers', async () => { + const CheckAnswersController = await vi.importActual('../../src/controllers/CheckAnswersController.js') + const sendEmailMock = vi.fn(() => Promise.reject(new Error('something went wrong'))) + notifyClient.sendEmail = sendEmailMock + + const controller = new CheckAnswersController.default({ route: '/dataset' }) + const req = makeRequest() + const res = {} + const next = vi.fn() + + it('should reuturn list of errors when failed to send email', async () => { + const result = await controller.sendEmails(req, res, next) + expect(result.errors.length).toBe(2) + }) +}) + +describe('Check answers controller', async () => { let CheckAnswersController let checkAnswersController let sendEmailMock @@ -22,57 +56,33 @@ describe('Check answers controller', () => { }) describe('send emails', () => { - it('should get the values from the session model and then send the request and acknowledgement emails', () => { + it('should get the values from the session model and then send the request and acknowledgement emails', async () => { // Mock req, res, next - const req = { - sessionModel: { - get: vi.fn().mockImplementation((key) => { - const values = { - name: 'John Doe', - email: 'JohnDoe@mail.com', - lpa: 'LPA', - dataset: 'Dataset', - 'documentation-url': 'Documentation URL', - 'endpoint-url': 'Endpoint URL' - } - return values[key] - }) - } - } - + const req = makeRequest() const res = {} const next = vi.fn() - checkAnswersController.sendEmails(req, res, next) + await checkAnswersController.sendEmails(req, res, next) + + const personalisation = { + name: 'John Doe', + email: 'JohnDoe@mail.com', + organisation: 'LPA', + dataset: 'Dataset', + 'documentation-url': 'Documentation URL', + endpoint: 'Endpoint URL' + } expect(sendEmailMock).toHaveBeenCalledWith( config.email.templates.RequestTemplateId, config.email.dataManagementEmail, - { - personalisation: { - name: 'John Doe', - email: 'JohnDoe@mail.com', - organisation: 'LPA', - dataset: 'Dataset', - 'documentation-url': 'Documentation URL', - endpoint: 'Endpoint URL' - } - } + { personalisation } ) expect(sendEmailMock).toHaveBeenCalledWith( config.email.templates.AcknowledgementTemplateId, 'JohnDoe@mail.com', - { - personalisation: { - name: 'John Doe', - email: 'JohnDoe@mail.com', - organisation: 'LPA', - endpoint: 'Endpoint URL', - 'documentation-url': 'Documentation URL', - dataset: 'Dataset' - } - } + { personalisation } ) }) })