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

refactor: migrate SmsFactory to Typescript #387

Merged
merged 14 commits into from
Oct 5, 2020
Merged
Show file tree
Hide file tree
Changes from 6 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
48 changes: 0 additions & 48 deletions src/app/factories/sms.factory.js

This file was deleted.

2 changes: 1 addition & 1 deletion src/app/modules/user/user.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { StatusCodes } from 'http-status-codes'

import { createLoggerWithLabel } from '../../../config/logger'
import { IPopulatedUser } from '../../../types'
import SmsFactory from '../../factories/sms.factory'
import { SmsFactory } from '../../services/sms/sms.factory'
import { ApplicationError } from '../core/core.errors'

import {
Expand Down
3 changes: 1 addition & 2 deletions src/app/modules/verification/verification.factory.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { RequestHandler } from 'express'
import { StatusCodes } from 'http-status-codes'

import featureManager from '../../../config/feature-manager'
import { FeatureNames } from '../../../config/feature-manager/types'
import featureManager, { FeatureNames } from '../../../config/feature-manager'

import * as verification from './verification.controller'

Expand Down
10 changes: 4 additions & 6 deletions src/app/modules/verification/verification.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ import {
IVerificationFieldSchema,
IVerificationSchema,
} from '../../../types'
import smsFactory from '../../factories/sms.factory'
import getFormModel from '../../models/form.server.model'
import getVerificationModel from '../../models/verification.server.model'
import MailService from '../../services/mail.service'
import { SmsFactory } from '../../services/sms/sms.factory'
import { generateOtp } from '../../utils/otp'

import { ITransaction } from './verification.types'
Expand Down Expand Up @@ -257,17 +257,15 @@ const sendOTPForField = async (
field: IVerificationFieldSchema,
recipient: string,
otp: string,
): Promise<void> => {
): Promise<unknown> => {
karrui marked this conversation as resolved.
Show resolved Hide resolved
const { fieldType } = field
switch (fieldType) {
case 'mobile':
// call sms - it should validate the recipient
await smsFactory.sendVerificationOtp(recipient, otp, formId)
break
return SmsFactory.sendVerificationOtp(recipient, otp, formId)
case 'email':
// call email - it should validate the recipient
await MailService.sendVerificationOtp(recipient, otp)
break
return MailService.sendVerificationOtp(recipient, otp)
default:
throw new Error(`sendOTPForField: ${fieldType} is unsupported`)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
/* eslint-disable @typescript-eslint/ban-ts-comment */
import { ObjectId } from 'bson'
import { cloneDeep, merge, omit } from 'lodash'
import mongoose from 'mongoose'
import dbHandler from 'tests/unit/backend/helpers/jest-db'

import getSmsCountModel from 'src/app/models/sms_count.server.model'
import { IVerificationSmsCount, LogType, SmsType } from 'src/types'

import dbHandler from '../helpers/jest-db'
import getSmsCountModel from '../sms_count.server.model'
import { IVerificationSmsCount, LogType, SmsType } from '../sms.types'

const SmsCount = getSmsCountModel(mongoose)

Expand Down Expand Up @@ -194,7 +194,7 @@ describe('SmsCount', () => {
form: MOCK_FORM_ID,
}).lean()

expect(actualLog!._id).toBeDefined()
expect(actualLog?._id).toBeDefined()
// Retrieve object and compare to params, remove indeterministic keys
const actualSavedObject = omit(actualLog, ['_id', 'createdAt', '__v'])
expect(actualSavedObject).toEqual(expectedLog)
Expand All @@ -220,7 +220,7 @@ describe('SmsCount', () => {
form: MOCK_FORM_ID,
}).lean()

expect(actualLog!._id).toBeDefined()
expect(actualLog?._id).toBeDefined()
// Retrieve object and compare to params, remove indeterministic keys
const actualSavedObject = omit(actualLog, ['_id', 'createdAt', '__v'])
expect(actualSavedObject).toEqual(expectedLog)
Expand Down
16 changes: 16 additions & 0 deletions src/app/services/sms/sms.errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { ApplicationError } from '../../modules/core/core.errors'

export class SmsSendError extends ApplicationError {
code?: number
sendStatus: unknown
karrui marked this conversation as resolved.
Show resolved Hide resolved

constructor(
message = 'Error sending OTP. Please try again later and if the problem persists, contact us.',
code?: number,
sendStatus?: unknown,
karrui marked this conversation as resolved.
Show resolved Hide resolved
) {
super(message)
this.code = code
this.sendStatus = sendStatus
}
}
59 changes: 59 additions & 0 deletions src/app/services/sms/sms.factory.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import Twilio from 'twilio'

import FeatureManager, { FeatureNames } from '../../../config/feature-manager'

import { sendAdminContactOtp, sendVerificationOtp } from './sms.service'
import { TwilioConfig } from './sms.types'

interface ISmsFactory {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move these to the types folder?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use typeof to get the types leh, I scared circular dependency :X

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment for future developers?

sendVerificationOtp: (
recipient: string,
otp: string,
formId: string,
) => ReturnType<typeof sendVerificationOtp>
sendAdminContactOtp: (
recipient: string,
otp: string,
userId: string,
) => ReturnType<typeof sendAdminContactOtp>
}

const createSmsFactory = (): ISmsFactory => {
const smsFeature = FeatureManager.get(FeatureNames.Sms)

if (!smsFeature.isEnabled) {
const errorMessage = 'SMS feature must be enabled in Feature Manager first'
return {
sendAdminContactOtp: () => {
throw new Error(`sendAdminContactOtp: ${errorMessage}`)
},
sendVerificationOtp: () => {
throw new Error(`sendVerificationOtp: ${errorMessage}`)
},
}
}

const {
twilioAccountSid,
twilioApiKey,
twilioApiSecret,
twilioMsgSrvcSid,
} = smsFeature.props

const twilioClient = Twilio(twilioApiKey, twilioApiSecret, {
accountSid: twilioAccountSid,
})
const twilioConfig: TwilioConfig = {
msgSrvcSid: twilioMsgSrvcSid,
client: twilioClient,
}

return {
sendVerificationOtp: (recipient, otp, formId) =>
sendVerificationOtp(recipient, otp, formId, twilioConfig),
sendAdminContactOtp: (recipient, otp, userId) =>
arshadali172 marked this conversation as resolved.
Show resolved Hide resolved
sendAdminContactOtp(recipient, otp, userId, twilioConfig),
}
}

export const SmsFactory = createSmsFactory()
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,22 @@ import mongoose from 'mongoose'
import NodeCache from 'node-cache'
import Twilio from 'twilio'

import config from '../../config/config'
import { createLoggerWithLabel } from '../../config/logger'
import { isPhoneNumber } from '../../shared/util/phone-num-validation'
import { VfnErrors } from '../../shared/util/verification'
import config from '../../../config/config'
import { createLoggerWithLabel } from '../../../config/logger'
import { isPhoneNumber } from '../../../shared/util/phone-num-validation'
import { VfnErrors } from '../../../shared/util/verification'
import { AdminContactOtpData, FormOtpData } from '../../../types'
import getFormModel from '../../models/form.server.model'

import getSmsCountModel from './sms_count.server.model'
import { SmsSendError } from './sms.errors'
import {
AdminContactOtpData,
FormOtpData,
LogSmsParams,
LogType,
SmsType,
} from '../../types'
import getFormModel from '../models/form.server.model'
import getSmsCountModel from '../models/sms_count.server.model'
TwilioConfig,
TwilioCredentials,
} from './sms.types'

const logger = createLoggerWithLabel(module)
const SmsCount = getSmsCountModel(mongoose)
Expand All @@ -28,13 +31,6 @@ const secretsManager = new SecretsManager({ region: config.aws.region })
// credentials, or wait 10 seconds before.
const twilioClientCache = new NodeCache({ deleteOnExpire: true, stdTTL: 10 })

type TwilioCredentials = {
accountSid: string
apiKey: string
apiSecret: string
messagingServiceSid: string
}

/**
* Retrieves credentials from secrets manager
* @param msgSrvcName The name of credential stored in the secret manager.
Expand Down Expand Up @@ -71,11 +67,6 @@ const getCredentials = async (
return null
}

type TwilioConfig = {
client: Twilio.Twilio
msgSrvcSid: string
}

/**
*
* @param msgSrvcName The name of credential stored in the secret manager
Expand Down Expand Up @@ -179,7 +170,17 @@ const send = async (
// Sent but with error code.
// Throw error to be caught in catch block.
if (!sid || errorCode) {
throw new TwilioError(errorMessage, errorCode, status)
const smsSendError = new SmsSendError(errorMessage)
logger.error({
message: 'Error sending SMS OTP via Twilio',
meta: {
action: 'send',
errorCode,
status,
},
error: smsSendError,
})
throw smsSendError
}

// Log success
Expand All @@ -200,6 +201,14 @@ const send = async (
})
})

logger.info({
message: 'Successfully sent sms',
meta: {
action: 'send',
otpData,
},
})

return true
})
.catch((err) => {
Expand Down Expand Up @@ -249,7 +258,7 @@ const send = async (
* @param formId Form id for logging.
*
*/
const sendVerificationOtp = async (
export const sendVerificationOtp = async (
recipient: string,
otp: string,
formId: string,
Expand Down Expand Up @@ -282,7 +291,7 @@ const sendVerificationOtp = async (
return send(twilioData, otpData, recipient, message, SmsType.verification)
}

const sendAdminContactOtp = async (
export const sendAdminContactOtp = async (
recipient: string,
otp: string,
userId: string,
Expand All @@ -304,24 +313,3 @@ const sendAdminContactOtp = async (

return send(defaultConfig, otpData, recipient, message, SmsType.adminContact)
}

class TwilioError extends Error {
code: number
status: string

constructor(message: string, code: number, status: string) {
super(message)
this.code = code
this.status = status
this.name = this.constructor.name

// Set the prototype explicitly.
// See https://github.com/facebook/jest/issues/8279
Object.setPrototypeOf(this, TwilioError.prototype)
}
}

module.exports = {
sendVerificationOtp,
sendAdminContactOtp,
}
21 changes: 19 additions & 2 deletions src/types/sms_count.ts → src/app/services/sms/sms.types.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import { Document, Model } from 'mongoose'
import { Twilio } from 'twilio'

import { FormOtpData, IFormSchema } from './form'
import { AdminContactOtpData, IUserSchema } from './user'
import {
AdminContactOtpData,
FormOtpData,
IFormSchema,
IUserSchema,
} from 'src/types'

export enum SmsType {
verification = 'VERIFICATION',
Expand Down Expand Up @@ -50,3 +55,15 @@ export type IAdminContactSmsCountSchema = ISmsCountSchema
export interface ISmsCountModel extends Model<ISmsCountSchema> {
logSms: (logParams: LogSmsParams) => Promise<ISmsCountSchema>
}

export type TwilioCredentials = {
accountSid: string
apiKey: string
apiSecret: string
messagingServiceSid: string
}

export type TwilioConfig = {
client: Twilio
msgSrvcSid: string
}
Loading