Skip to content

Commit

Permalink
feat(btn): frm 1723 internal flow postman (#7343)
Browse files Browse the repository at this point in the history
* add postman sms service

* feat: add user betaflag for postman sms

* add sendBouncedSubmissionSms

* test: add sendBouncedSubmissionSms cases

* add sendFormDeactivatedSms

* add sendAdminContactOtp

* add package-lock

* remove non-mop sms services, point sms.errors to postman-sms.errors

* migrate non-mop smses to use postman sms service

* update tests to remove migrated functions

* fix incorrect api key reference

* fix form.admin not populated before passing into flag field

* update test cases for bounce, sms service

* update test cases for bounce service

* update test cases for user controller

* update sms error import path

* refactor: rename mop sms to use internal api convention, fix typo on logmeta.action

* chore: fix build failures due to merge conflicts

* chore: remove redundant logs
  • Loading branch information
KenLSM authored Jul 5, 2024
1 parent 14f975a commit f8763c5
Show file tree
Hide file tree
Showing 25 changed files with 579 additions and 922 deletions.
8 changes: 7 additions & 1 deletion .template-env
Original file line number Diff line number Diff line change
Expand Up @@ -114,4 +114,10 @@ FORMSG_SDK_MODE=
# Used to check if BE Server is currently running on local development environment
# One of boolean: "true" | "false"
# USE_MOCK_TWILIO=
# USE_MOCK_POSTMAN_SMS=
# USE_MOCK_POSTMAN_SMS=

# POSTMAN_MOP_CAMPAIGN_ID=
# POSTMAN_MOP_CAMPAIGN_API_KEY=
# POSTMAN_INTERNAL_CAMPAIGN_ID=
# POSTMAN_INTERNAL_CAMPAIGN_API_KEY=
# POSTMAN_BASE_URL=https://test.postman.gov.sg/api/v2
2 changes: 2 additions & 0 deletions __tests__/setup/.test-env
Original file line number Diff line number Diff line change
Expand Up @@ -91,4 +91,6 @@ SSM_ENV_SITE_NAME=test
API_KEY_VERSION=v1
POSTMAN_MOP_CAMPAIGN_ID=campaign_tesst
POSTMAN_MOP_CAMPAIGN_API_KEY=key_test_123
POSTMAN_INTERNAL_CAMPAIGN_ID=campaign_tesst
POSTMAN_INTERNAL_CAMPAIGN_API_KEY=key_test_123
POSTMAN_BASE_URL=https://test.postman.gov.sg/api/v2
2 changes: 2 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,8 @@ services:
- WOGAA_FEEDBACK_ENDPOINT
- POSTMAN_MOP_CAMPAIGN_ID=campaign_test
- POSTMAN_MOP_CAMPAIGN_API_KEY=key_test_123
- POSTMAN_INTERNAL_CAMPAIGN_ID=campaign_test_2
- POSTMAN_INTERNAL_CAMPAIGN_API_KEY=key_test_456
- POSTMAN_BASE_URL=https://test.postman.gov.sg/api/v2

mockpass:
Expand Down
20 changes: 18 additions & 2 deletions src/app/config/features/postman-sms.config.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
import convict, { Schema } from 'convict'

export interface ISms {
export interface IPostmanSms {
// MOP SMSes are to be sent using GovSG sender id
mopCampaignId: string
mopCampaignApiKey: string
// Internal SMSes are to be sent using FormSG sender id
internalCampaignId: string
internalCampaignApiKey: string
postmanBaseUrl: string
}

const postmanSmsSchema: Schema<ISms> = {
const postmanSmsSchema: Schema<IPostmanSms> = {
mopCampaignId: {
doc: 'Postman SMS messaging campaign ID',
format: String,
Expand All @@ -19,6 +23,18 @@ const postmanSmsSchema: Schema<ISms> = {
default: null,
env: 'POSTMAN_MOP_CAMPAIGN_API_KEY',
},
internalCampaignId: {
doc: 'Postman SMS messaging internal (non-mop) campaign ID',
format: String,
default: null,
env: 'POSTMAN_INTERNAL_CAMPAIGN_ID',
},
internalCampaignApiKey: {
doc: 'Postman SMS messaging internal (non-mop) campaign ID',
format: String,
default: null,
env: 'POSTMAN_INTERNAL_CAMPAIGN_API_KEY',
},
postmanBaseUrl: {
doc: 'Postman base URL',
format: String,
Expand Down
187 changes: 107 additions & 80 deletions src/app/modules/bounce/__tests__/bounce.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import getFormModel from 'src/app/models/form.server.model'
import * as UserService from 'src/app/modules/user/user.service'
import { EMAIL_HEADERS, EmailType } from 'src/app/services/mail/mail.constants'
import MailService from 'src/app/services/mail/mail.service'
import { SmsFactory } from 'src/app/services/sms/sms.factory'
import PostmanSmsService from 'src/app/services/postman-sms/postman-sms.service'
import {
BounceType,
IPopulatedForm,
Expand All @@ -32,13 +32,12 @@ jest.mock('src/app/config/logger')
const MockLoggerModule = jest.mocked(LoggerModule)
jest.mock('src/app/services/mail/mail.service')
const MockMailService = jest.mocked(MailService)
jest.mock('src/app/services/sms/sms.factory', () => ({
SmsFactory: {
sendFormDeactivatedSms: jest.fn(),
sendBouncedSubmissionSms: jest.fn(),
},
jest.mock('src/app/services/postman-sms/postman-sms.service', () => ({
sendFormDeactivatedSms: jest.fn(),
sendBouncedSubmissionSms: jest.fn(),
}))
const MockSmsFactory = jest.mocked(SmsFactory)
const MockedPostmanSmsService = jest.mocked(PostmanSmsService)

jest.mock('src/app/modules/user/user.service')
const MockUserService = jest.mocked(UserService)

Expand All @@ -53,7 +52,7 @@ import * as BounceService from 'src/app/modules/bounce/bounce.service'
import {
InvalidNumberError,
SmsSendError,
} from 'src/app/services/sms/sms.errors'
} from 'src/app/services/postman-sms/postman-sms.errors'

import {
InvalidNotificationError,
Expand Down Expand Up @@ -399,31 +398,39 @@ describe('BounceService', () => {
formId: form._id,
bounces: [],
})
MockSmsFactory.sendBouncedSubmissionSms.mockReturnValue(okAsync(true))
MockedPostmanSmsService.sendBouncedSubmissionSms.mockReturnValue(
okAsync(true),
)

const notifiedRecipients = await BounceService.sendSmsBounceNotification(
bounceDoc,
form,
[MOCK_CONTACT, MOCK_CONTACT_2],
)

expect(MockSmsFactory.sendBouncedSubmissionSms).toHaveBeenCalledTimes(2)
expect(MockSmsFactory.sendBouncedSubmissionSms).toHaveBeenCalledWith({
adminEmail: testUser.email,
adminId: String(testUser._id),
formId: form._id,
formTitle: form.title,
recipient: MOCK_CONTACT.contact,
recipientEmail: MOCK_CONTACT.email,
})
expect(MockSmsFactory.sendBouncedSubmissionSms).toHaveBeenCalledWith({
adminEmail: testUser.email,
adminId: String(testUser._id),
formId: form._id,
formTitle: form.title,
recipient: MOCK_CONTACT_2.contact,
recipientEmail: MOCK_CONTACT_2.email,
})
expect(
MockedPostmanSmsService.sendBouncedSubmissionSms,
).toHaveBeenCalledTimes(2)
expect(
MockedPostmanSmsService.sendBouncedSubmissionSms,
).toHaveBeenCalledWith(
testUser.email,
String(testUser._id),
form._id,
form.title,
MOCK_CONTACT.contact,
MOCK_CONTACT.email,
)
expect(
MockedPostmanSmsService.sendBouncedSubmissionSms,
).toHaveBeenCalledWith(
testUser.email,
String(testUser._id),
form._id,
form.title,
MOCK_CONTACT_2.contact,
MOCK_CONTACT_2.email,
)
expect(notifiedRecipients._unsafeUnwrap()).toEqual([
MOCK_CONTACT,
MOCK_CONTACT_2,
Expand All @@ -439,7 +446,7 @@ describe('BounceService', () => {
formId: form._id,
bounces: [],
})
MockSmsFactory.sendBouncedSubmissionSms
MockedPostmanSmsService.sendBouncedSubmissionSms
.mockReturnValueOnce(okAsync(true))
.mockReturnValueOnce(errAsync(new InvalidNumberError()))

Expand All @@ -449,23 +456,29 @@ describe('BounceService', () => {
[MOCK_CONTACT, MOCK_CONTACT_2],
)

expect(MockSmsFactory.sendBouncedSubmissionSms).toHaveBeenCalledTimes(2)
expect(MockSmsFactory.sendBouncedSubmissionSms).toHaveBeenCalledWith({
adminEmail: testUser.email,
adminId: String(testUser._id),
formId: form._id,
formTitle: form.title,
recipient: MOCK_CONTACT.contact,
recipientEmail: MOCK_CONTACT.email,
})
expect(MockSmsFactory.sendBouncedSubmissionSms).toHaveBeenCalledWith({
adminEmail: testUser.email,
adminId: String(testUser._id),
formId: form._id,
formTitle: form.title,
recipient: MOCK_CONTACT_2.contact,
recipientEmail: MOCK_CONTACT_2.email,
})
expect(
MockedPostmanSmsService.sendBouncedSubmissionSms,
).toHaveBeenCalledTimes(2)
expect(
MockedPostmanSmsService.sendBouncedSubmissionSms,
).toHaveBeenCalledWith(
testUser.email,
String(testUser._id),
form._id,
form.title,
MOCK_CONTACT.contact,
MOCK_CONTACT.email,
)
expect(
MockedPostmanSmsService.sendBouncedSubmissionSms,
).toHaveBeenCalledWith(
testUser.email,
String(testUser._id),
form._id,
form.title,
MOCK_CONTACT_2.contact,
MOCK_CONTACT_2.email,
)
expect(notifiedRecipients._unsafeUnwrap()).toEqual([MOCK_CONTACT])
})
})
Expand Down Expand Up @@ -838,39 +851,47 @@ describe('BounceService', () => {
admin: testUser._id,
title: MOCK_FORM_TITLE,
}).populate('admin')) as IPopulatedForm
MockSmsFactory.sendFormDeactivatedSms.mockReturnValue(okAsync(true))
MockedPostmanSmsService.sendFormDeactivatedSms.mockReturnValue(
okAsync(true),
)

const result = await BounceService.notifyAdminsOfDeactivation(form, [
MOCK_CONTACT,
MOCK_CONTACT_2,
])

expect(result._unsafeUnwrap()).toEqual(true)
expect(MockSmsFactory.sendFormDeactivatedSms).toHaveBeenCalledTimes(2)
expect(MockSmsFactory.sendFormDeactivatedSms).toHaveBeenCalledWith({
adminEmail: form.admin.email,
adminId: String(form.admin._id),
formId: form._id,
formTitle: form.title,
recipient: MOCK_CONTACT.contact,
recipientEmail: MOCK_CONTACT.email,
})
expect(MockSmsFactory.sendFormDeactivatedSms).toHaveBeenCalledWith({
adminEmail: form.admin.email,
adminId: String(form.admin._id),
formId: form._id,
formTitle: form.title,
recipient: MOCK_CONTACT_2.contact,
recipientEmail: MOCK_CONTACT_2.email,
})
expect(
MockedPostmanSmsService.sendFormDeactivatedSms,
).toHaveBeenCalledTimes(2)
expect(
MockedPostmanSmsService.sendFormDeactivatedSms,
).toHaveBeenCalledWith(
form.admin.email,
String(form.admin._id),
form._id,
form.title,
MOCK_CONTACT.contact,
MOCK_CONTACT.email,
)
expect(
MockedPostmanSmsService.sendFormDeactivatedSms,
).toHaveBeenCalledWith(
form.admin.email,
String(form.admin._id),
form._id,
form.title,
MOCK_CONTACT_2.contact,
MOCK_CONTACT_2.email,
)
})

it('should return true even when some SMSes fail', async () => {
const form = (await new Form({
admin: testUser._id,
title: MOCK_FORM_TITLE,
}).populate('admin')) as IPopulatedForm
MockSmsFactory.sendFormDeactivatedSms
MockedPostmanSmsService.sendFormDeactivatedSms
.mockReturnValueOnce(okAsync(true))
.mockReturnValueOnce(errAsync(new SmsSendError()))

Expand All @@ -880,23 +901,29 @@ describe('BounceService', () => {
])

expect(result._unsafeUnwrap()).toEqual(true)
expect(MockSmsFactory.sendFormDeactivatedSms).toHaveBeenCalledTimes(2)
expect(MockSmsFactory.sendFormDeactivatedSms).toHaveBeenCalledWith({
adminEmail: form.admin.email,
adminId: String(form.admin._id),
formId: form._id,
formTitle: form.title,
recipient: MOCK_CONTACT.contact,
recipientEmail: MOCK_CONTACT.email,
})
expect(MockSmsFactory.sendFormDeactivatedSms).toHaveBeenCalledWith({
adminEmail: form.admin.email,
adminId: String(form.admin._id),
formId: form._id,
formTitle: form.title,
recipient: MOCK_CONTACT_2.contact,
recipientEmail: MOCK_CONTACT_2.email,
})
expect(
MockedPostmanSmsService.sendFormDeactivatedSms,
).toHaveBeenCalledTimes(2)
expect(
MockedPostmanSmsService.sendFormDeactivatedSms,
).toHaveBeenCalledWith(
form.admin.email,
String(form.admin._id),
form._id,
form.title,
MOCK_CONTACT.contact,
MOCK_CONTACT.email,
)
expect(
MockedPostmanSmsService.sendFormDeactivatedSms,
).toHaveBeenCalledWith(
form.admin.email,
String(form.admin._id),
form._id,
form.title,
MOCK_CONTACT_2.contact,
MOCK_CONTACT_2.email,
)
})
})
})
5 changes: 4 additions & 1 deletion src/app/modules/bounce/bounce.errors.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { InvalidNumberError, SmsSendError } from '../../services/sms/sms.errors'
import {
InvalidNumberError,
SmsSendError,
} from '../../services/postman-sms/postman-sms.errors'
import { ApplicationError } from '../core/core.errors'

/**
Expand Down
34 changes: 17 additions & 17 deletions src/app/modules/bounce/bounce.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
} from '../../config/logger'
import { EMAIL_HEADERS, EmailType } from '../../services/mail/mail.constants'
import MailService from '../../services/mail/mail.service'
import { SmsFactory } from '../../services/sms/sms.factory'
import PostmanSmsService from '../../services/postman-sms/postman-sms.service'
import { transformMongoError } from '../../utils/handle-mongo-error'
import { PossibleDatabaseError } from '../core/core.errors'
import { getCollabEmailsWithPermission } from '../form/form.utils'
Expand Down Expand Up @@ -219,14 +219,14 @@ export const sendSmsBounceNotification = (
// empty array as list of recipients.
): ResultAsync<UserWithContactNumber[], never> => {
const smsResults = possibleSmsRecipients.map((recipient) =>
SmsFactory.sendBouncedSubmissionSms({
adminEmail: form.admin.email,
adminId: String(form.admin._id),
formId: form._id,
formTitle: form.title,
recipient: recipient.contact,
recipientEmail: recipient.email,
})
PostmanSmsService.sendBouncedSubmissionSms(
form.admin.email,
String(form.admin._id),
form._id,
form.title,
recipient.contact,
recipient.email,
)
.map(() => recipient)
.mapErr(
(error) => new SendBounceSmsNotificationError(error, recipient.contact),
Expand Down Expand Up @@ -368,14 +368,14 @@ export const notifyAdminsOfDeactivation = (
// Best-effort attempt to send SMSes, don't propagate error upwards
): ResultAsync<true, never> => {
const smsResults = possibleSmsRecipients.map((recipient) =>
SmsFactory.sendFormDeactivatedSms({
adminEmail: form.admin.email,
adminId: String(form.admin._id),
formId: form._id,
formTitle: form.title,
recipient: recipient.contact,
recipientEmail: recipient.email,
}),
PostmanSmsService.sendFormDeactivatedSms(
form.admin.email,
String(form.admin._id),
form._id,
form.title,
recipient.contact,
recipient.email,
),
)
return ResultAsync.combineWithAllErrors(smsResults)
.map(() => true as const)
Expand Down
Loading

0 comments on commit f8763c5

Please sign in to comment.