Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: bounce notification #4107

Merged
merged 4 commits into from
Jul 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions src/app/modules/bounce/__tests__/bounce.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ describe('handleSns', () => {
expect(MockBounceService.getEditorsWithContactNumbers).toHaveBeenCalledWith(
mockForm,
)
expect(mockBounceDoc.hasNotified).toHaveBeenCalled()
expect(MockBounceService.sendEmailBounceNotification).toHaveBeenCalledWith(
mockBounceDoc,
mockForm,
Expand Down Expand Up @@ -229,6 +230,7 @@ describe('handleSns', () => {
expect(MockBounceService.getEditorsWithContactNumbers).toHaveBeenCalledWith(
mockForm,
)
expect(mockBounceDoc.hasNotified).toHaveBeenCalled()
expect(MockBounceService.sendEmailBounceNotification).toHaveBeenCalledWith(
mockBounceDoc,
mockForm,
Expand Down Expand Up @@ -257,6 +259,54 @@ describe('handleSns', () => {
expect(MOCK_RES.sendStatus).toHaveBeenCalledWith(200)
})

it('should call services correctly when recipients have already been notified of a critical bounce', async () => {
mockBounceDoc.hasNotified.mockReturnValue(true)

await handleSns(MOCK_REQ, MOCK_RES, jest.fn())

expect(MockBounceService.validateSnsRequest).toHaveBeenCalledWith(
MOCK_REQ.body,
)
expect(MockBounceService.logEmailNotification).toHaveBeenCalledWith(
MOCK_NOTIFICATION,
)
expect(MockBounceService.extractEmailType).toHaveBeenCalledWith(
MOCK_NOTIFICATION,
)
expect(MockBounceService.getUpdatedBounceDoc).toHaveBeenCalledWith(
MOCK_NOTIFICATION,
)
expect(MockFormService.retrieveFullFormById).toHaveBeenCalledWith(
mockBounceDoc.formId,
)
expect(mockBounceDoc.isCriticalBounce).toHaveBeenCalled()
expect(MockBounceService.getEditorsWithContactNumbers).toHaveBeenCalledWith(
mockForm,
)
expect(mockBounceDoc.hasNotified).toHaveBeenCalled()
// Notification functions are not called
expect(MockBounceService.sendEmailBounceNotification).not.toHaveBeenCalled()
expect(MockBounceService.sendSmsBounceNotification).not.toHaveBeenCalled()
expect(mockBounceDoc.setNotificationState).not.toHaveBeenCalled()
expect(mockBounceDoc.areAllPermanentBounces).toHaveBeenCalled()
expect(MockFormService.deactivateForm).toHaveBeenCalledWith(
mockBounceDoc.formId,
)
expect(MockBounceService.notifyAdminsOfDeactivation).toHaveBeenCalledWith(
mockForm,
MOCK_CONTACTS,
)
expect(MockBounceService.logCriticalBounce).toHaveBeenCalledWith({
bounceDoc: mockBounceDoc,
notification: MOCK_NOTIFICATION,
autoEmailRecipients: [],
autoSmsRecipients: [],
hasDeactivated: true,
})
expect(MockBounceService.saveBounceDoc).toHaveBeenCalledWith(mockBounceDoc)
expect(MOCK_RES.sendStatus).toHaveBeenCalledWith(200)
})

it('should return 200 even when errors are thrown in getUpdatedBounceDoc', async () => {
MockBounceService.getUpdatedBounceDoc.mockReturnValueOnce(
errAsync(new DatabaseError()),
Expand Down Expand Up @@ -334,6 +384,7 @@ describe('handleSns', () => {
expect(MockBounceService.getEditorsWithContactNumbers).toHaveBeenCalledWith(
mockForm,
)
expect(mockBounceDoc.hasNotified).toHaveBeenCalled()
expect(mockBounceDoc.areAllPermanentBounces).toHaveBeenCalled()
expect(MockFormService.deactivateForm).toHaveBeenCalledWith(
mockBounceDoc.formId,
Expand Down Expand Up @@ -367,6 +418,7 @@ describe('handleSns', () => {
expect(MockBounceService.getEditorsWithContactNumbers).toHaveBeenCalledWith(
mockForm,
)
expect(mockBounceDoc.hasNotified).toHaveBeenCalled()
expect(mockBounceDoc.areAllPermanentBounces).toHaveBeenCalled()
expect(MockFormService.deactivateForm).toHaveBeenCalledWith(
mockBounceDoc.formId,
Expand Down
38 changes: 26 additions & 12 deletions src/app/modules/bounce/bounce.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { ControllerHandler } from '../core/core.types'
import * as FormService from '../form/form.service'

import * as BounceService from './bounce.service'
import { AdminNotificationRecipients } from './bounce.types'

const logger = createLoggerWithLabel(module)

Expand Down Expand Up @@ -78,16 +79,29 @@ export const handleSns: ControllerHandler<
// Send notifications and deactivate form on best-effort basis, ignore errors
const possibleSmsRecipients =
await BounceService.getEditorsWithContactNumbers(form).unwrapOr([])
const emailRecipients = await BounceService.sendEmailBounceNotification(
bounceDoc,
form,
).unwrapOr([])
const smsRecipients = await BounceService.sendSmsBounceNotification(
bounceDoc,
form,
possibleSmsRecipients,
).unwrapOr([])
bounceDoc.setNotificationState(emailRecipients, smsRecipients)

const notificationRecipients: AdminNotificationRecipients = {
emailRecipients: [],
smsRecipients: [],
}
// Check if notifications have been sent to form admins and collaborators
if (!bounceDoc.hasNotified()) {
notificationRecipients.emailRecipients =
await BounceService.sendEmailBounceNotification(
bounceDoc,
form,
).unwrapOr([])
notificationRecipients.smsRecipients =
await BounceService.sendSmsBounceNotification(
bounceDoc,
form,
possibleSmsRecipients,
).unwrapOr([])
bounceDoc.setNotificationState(
notificationRecipients.emailRecipients,
notificationRecipients.smsRecipients,
)
}

const shouldDeactivate = bounceDoc.areAllPermanentBounces()
if (shouldDeactivate) {
Expand All @@ -102,8 +116,8 @@ export const handleSns: ControllerHandler<
BounceService.logCriticalBounce({
bounceDoc,
notification,
autoEmailRecipients: emailRecipients,
autoSmsRecipients: smsRecipients,
autoEmailRecipients: notificationRecipients.emailRecipients,
autoSmsRecipients: notificationRecipients.smsRecipients,
hasDeactivated: shouldDeactivate,
})
}
Expand Down
6 changes: 6 additions & 0 deletions src/app/modules/bounce/bounce.types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { UserWithContactNumber } from '../user/user.types'

export interface AdminNotificationRecipients {
emailRecipients: string[]
smsRecipients: UserWithContactNumber[]
}