Skip to content

Commit

Permalink
feat(sms-limiting): sms counts retrieval (#2276)
Browse files Browse the repository at this point in the history
* feat(sms_count.server.model): adds new isOnboardedAcc key and scripts to add/delete the key

* refactor(sms.types): adds property that checks if a sms is sent using formsg's credentials

* refactor(sms_count.server.model): adds pre-save hook to automatically set property

* feat(sms.types): updated typings of IVerificationsmscount

* test(sms_count.server): fixed tests and added extra tests for isOnboardedAccount

* feat(sms.service): adds new method to retrieve free sms counts of a user

* test(sms.service): adds new unit tests for sms count  retrieval

* test(sms_count.server.model): updated test case for a client using default credentials

* feat(sms.controller): added new sms controller iwth method to retrieve counts and msgsrvcid

fix(sms.controller.ts): added mapRouteError for sms controller

* feat(routes): exposes new route at sms/:userId/:formId

* fix(sms.controller): removed sms count retrieval requiring the userId as it sohuld be for the admin

* refactor(sms): count retrieval no longer required userId

* test(sms.controller): added unit tests for controller method to retrieve counts

* fix(sms.controller): changed to relative imports in spec to avoid e2e tests breaking

* refactor(sms_count): sms service wraps model count documents call

* test(sms.service): reworked tests to account for mocking of db

* refactor(sms.controller): removed extra middleware from sms controller

* test(sms.controller.spec): renamed sms controller's exposed method in tests

* refactor(sms.routes): removed unused sms route

* refactor(sms.controller): changed counts retrieval to be solely focused on counts

* feat(admin-forms.form.routes): adds new route for retrieving free sms ocunts for admin of a form

* refactor(sms_count): shifts default msgsrvcid to be from config, helper method defined in test

* test(sms.controller.spec): updated tests to use mocks

* test(sms_count.server.model.spec): updated db tests to use smsConfig

* fix(v3.routes): removed extra import statement

* refactor(types): renamed smscounts meta and shifted it to api

renamed to smsCountsDto because it is used to transfer data b/w fe and be. against giving it a more
specific name because this is a type signifying a transfer of sms count data. context for what this
sms counts actually means should be given by the caller

* test(sms_count.server.model.spec): updated test to use sms_config

* docs(sms): updated docs for sms counts retrieval in controller and route

* test(sms.service.spec): made tests more explicit

* refactor(admin-form.controller): shifts over handler for count retrieval from sms to admin-form

* test(admin-form.controller.spec): shifts over associated tests

* refactor(sms_count.server.model.spec): changed tests structure

* chore(sms.util): removed unneeded method

* refactor(sms.types): added in docs for retrieveFreeSmsCounts

* fix(admin-forms.form.routes): removed erroneous import

* fix(sms.service): adds error logging
  • Loading branch information
seaerchin authored Jul 14, 2021
1 parent 76e4ade commit 7f730dd
Show file tree
Hide file tree
Showing 8 changed files with 348 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ import {
} from 'tests/unit/backend/helpers/generate-form-data'
import expressHandler from 'tests/unit/backend/helpers/jest-express'

import * as SmsService from '../../../../services/sms/sms.service'
import * as UserService from '../../../user/user.service'
import {
ForbiddenFormError,
Expand Down Expand Up @@ -133,6 +134,8 @@ jest.mock('../../../user/user.service')
const MockUserService = mocked(UserService)
jest.mock('src/app/services/mail/mail.service')
const MockMailService = mocked(MailService)
jest.mock('../../../../services/sms/sms.service')
const MockSmsService = mocked(SmsService)

describe('admin-form.controller', () => {
beforeEach(() => jest.clearAllMocks())
Expand Down Expand Up @@ -9798,4 +9801,139 @@ describe('admin-form.controller', () => {
expect(MockAdminFormService.getFormField).not.toHaveBeenCalled()
})
})

describe('handleGetFreeSmsCountForFormAdmin', () => {
const mockForm = {
admin: new ObjectId().toHexString(),
} as unknown as IFormSchema
const VERIFICATION_SMS_COUNT = 3

beforeAll(() => {
MockFormService.retrieveFormById.mockReturnValue(okAsync(mockForm))
MockSmsService.retrieveFreeSmsCounts.mockReturnValue(
okAsync(VERIFICATION_SMS_COUNT),
)
})

it('should retrieve counts and msgSrvcId when the user and the form exist', async () => {
// Arrange
const MOCK_REQ = expressHandler.mockRequest({
params: {
formId: mockForm._id,
},
session: {
user: {
_id: 'exists',
},
},
})
const mockRes = expressHandler.mockResponse()
const expected = VERIFICATION_SMS_COUNT

// Act
await AdminFormController.handleGetFreeSmsCountForFormAdmin(
MOCK_REQ,
mockRes,
jest.fn(),
)

// Assert
expect(mockRes.status).toBeCalledWith(200)
expect(mockRes.json).toBeCalledWith(expected)
})

it('should return 404 when the form is not found in the database', async () => {
// Arrange
const MOCK_REQ = expressHandler.mockRequest({
params: {
formId: new ObjectId().toHexString(),
},
session: {
user: {
_id: 'exists',
},
},
})
MockFormService.retrieveFormById.mockReturnValueOnce(
errAsync(new FormNotFoundError()),
)
const mockRes = expressHandler.mockResponse()
const expected = {
message: 'Form not found',
}

// Act
await AdminFormController.handleGetFreeSmsCountForFormAdmin(
MOCK_REQ,
mockRes,
jest.fn(),
)

// Assert
expect(mockRes.status).toBeCalledWith(404)
expect(mockRes.json).toBeCalledWith(expected)
})

it('should return 500 when a database error occurs during form retrieval', async () => {
// Arrange
const MOCK_REQ = expressHandler.mockRequest({
params: {
formId: mockForm._id,
},
session: {
user: {
_id: 'exists',
},
},
})
const mockRes = expressHandler.mockResponse()
const retrieveSpy = jest.spyOn(FormService, 'retrieveFormById')
retrieveSpy.mockReturnValueOnce(errAsync(new DatabaseError()))
const expected = {
message: 'Something went wrong. Please try again.',
}

// Act
await AdminFormController.handleGetFreeSmsCountForFormAdmin(
MOCK_REQ,
mockRes,
jest.fn(),
)

// Assert
expect(mockRes.status).toBeCalledWith(500)
expect(mockRes.json).toBeCalledWith(expected)
})

it('should return 500 when a database error occurs during count retrieval', async () => {
// Arrange
const MOCK_REQ = expressHandler.mockRequest({
params: {
formId: mockForm._id,
},
session: {
user: {
_id: 'exists',
},
},
})
const mockRes = expressHandler.mockResponse()
const retrieveSpy = jest.spyOn(SmsService, 'retrieveFreeSmsCounts')
retrieveSpy.mockReturnValueOnce(errAsync(new DatabaseError()))
const expected = {
message: 'Something went wrong. Please try again.',
}

// Act
await AdminFormController.handleGetFreeSmsCountForFormAdmin(
MOCK_REQ,
mockRes,
jest.fn(),
)

// Assert
expect(mockRes.status).toBeCalledWith(500)
expect(mockRes.json).toBeCalledWith(expected)
})
})
})
45 changes: 45 additions & 0 deletions src/app/modules/form/admin-form/admin-form.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import {
} from '../../../../types/api'
import { createLoggerWithLabel } from '../../../config/logger'
import MailService from '../../../services/mail/mail.service'
import * as SmsService from '../../../services/sms/sms.service'
import { createReqMeta } from '../../../utils/request'
import * as AuthService from '../../auth/auth.service'
import {
Expand Down Expand Up @@ -2380,3 +2381,47 @@ export const handleUpdateStartPage = [
}),
_handleUpdateStartPage,
] as ControllerHandler[]

/**
* Handler to retrieve the free sms counts used by a form's administrator
* This is the controller for GET /admin/forms/:formId/verified-sms/count/free
* @param formId The id of the form to retrieve the free sms counts for
* @returns 200 with free sms counts when successful
* @returns 404 when the formId is not found in the database
* @returns 500 when a database error occurs during retrieval
*/
export const handleGetFreeSmsCountForFormAdmin: ControllerHandler<
{
formId: string
},
ErrorDto | number
> = (req, res) => {
const { formId } = req.params
const logMeta = {
action: 'handleGetFreeSmsCountForFormAdmin',
...createReqMeta(req),
formId,
}

// Step 1: Check that the form exists
return (
FormService.retrieveFormById(formId)
// Step 2: Retrieve the free sms count
.andThen(({ admin }) => {
return SmsService.retrieveFreeSmsCounts(String(admin))
})
// Step 3: Map/MapErr accordingly
.map((freeSmsCountForAdmin) =>
res.status(StatusCodes.OK).json(freeSmsCountForAdmin),
)
.mapErr((error) => {
logger.error({
message: 'Error while retrieving sms counts for user',
meta: logMeta,
error,
})
const { statusCode, errorMessage } = mapRouteError(error)
return res.status(statusCode).json({ message: errorMessage })
})
)
}
14 changes: 14 additions & 0 deletions src/app/routes/api/v3/admin/forms/admin-forms.form.routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,3 +197,17 @@ AdminFormsFormRouter.put(
'/:formId([a-fA-F0-9]{24})/start-page',
AdminFormController.handleUpdateStartPage,
)

/**
* Retrieves the free sms counts used by a form's administrator
* @security session
*
* @returns 200 with the free sms counts
* @returns 401 when user does not exist in session
* @returns 404 when the formId is not found in the database
* @returns 500 when a database error occurs during retrieval
*/
AdminFormsFormRouter.get(
'/:formId([a-fA-F0-9]{24})/verified-sms/count/free',
AdminFormController.handleGetFreeSmsCountForFormAdmin,
)
36 changes: 35 additions & 1 deletion src/app/services/sms/__tests__/sms.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@ import { ObjectId } from 'bson'
import mongoose from 'mongoose'

import getFormModel from 'src/app/models/form.server.model'
import { MalformedParametersError } from 'src/app/modules/core/core.errors'
import {
DatabaseError,
MalformedParametersError,
} from 'src/app/modules/core/core.errors'
import { getMongoErrorMessage } from 'src/app/utils/handle-mongo-error'
import { VfnErrors } from 'src/shared/util/verification'
import { FormOtpData, IFormSchema, IUserSchema, ResponseMode } from 'src/types'

Expand Down Expand Up @@ -351,6 +355,36 @@ describe('sms.service', () => {
})
})

describe('retrieveFreeSmsCounts', () => {
const VERIFICATION_SMS_COUNT = 3

it('should retrieve sms counts correctly for a specified user', async () => {
// Arrange
const retrieveSpy = jest.spyOn(SmsCountModel, 'retrieveFreeSmsCounts')
retrieveSpy.mockResolvedValueOnce(VERIFICATION_SMS_COUNT)

// Act
const actual = await SmsService.retrieveFreeSmsCounts(testUser._id)

// Assert
expect(actual._unsafeUnwrap()).toBe(VERIFICATION_SMS_COUNT)
})

it('should return a database error when retrieval fails', async () => {
// Arrange
const retrieveSpy = jest.spyOn(SmsCountModel, 'retrieveFreeSmsCounts')
retrieveSpy.mockRejectedValueOnce('ohno')

// Act
const actual = await SmsService.retrieveFreeSmsCounts(testUser._id)

// Assert
expect(actual._unsafeUnwrapErr()).toEqual(
new DatabaseError(getMongoErrorMessage('ohno')),
)
})
})

it('should log failure and throw error when contact OTP fails to send', async () => {
// Act
const actualResult = await SmsService.sendAdminContactOtp(
Expand Down
56 changes: 53 additions & 3 deletions src/app/services/sms/__tests__/sms_count.server.model.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import mongoose from 'mongoose'

import dbHandler from 'tests/unit/backend/helpers/jest-db'

import { smsConfig } from '../../../config/features/sms.config'
import { IVerificationSmsCount, LogType, SmsType } from '../sms.types'
import getSmsCountModel from '../sms_count.server.model'

Expand Down Expand Up @@ -230,9 +231,22 @@ describe('SmsCount', () => {
})

describe('VerificationCount Schema', () => {
const twilioMsgSrvcSid = smsConfig.twilioMsgSrvcSid

beforeAll(() => {
smsConfig.twilioMsgSrvcSid = MOCK_MSG_SRVC_SID
})

afterAll(() => {
smsConfig.twilioMsgSrvcSid = twilioMsgSrvcSid
})

it('should create and save successfully', async () => {
// Arrange
const smsCountParams = createVerificationSmsCountParams()
const expected = merge(smsCountParams, {
isOnboardedAccount: false,
})

// Act
const validSmsCount = new SmsCount(smsCountParams)
Expand All @@ -249,7 +263,7 @@ describe('SmsCount', () => {
'createdAt',
'__v',
])
expect(actualSavedObject).toEqual(smsCountParams)
expect(actualSavedObject).toEqual(expected)
})

it('should save successfully, but not save fields that is not defined in the schema', async () => {
Expand All @@ -260,6 +274,9 @@ describe('SmsCount', () => {
extra: 'somethingExtra',
},
)
const expected = merge(omit(smsCountParamsWithExtra, 'extra'), {
isOnboardedAccount: false,
})

// Act
const validSmsCount = new SmsCount(smsCountParamsWithExtra)
Expand All @@ -278,7 +295,38 @@ describe('SmsCount', () => {
'createdAt',
'__v',
])
expect(actualSavedObject).toEqual(omit(smsCountParamsWithExtra, 'extra'))
expect(actualSavedObject).toEqual(expected)
})

it('should save successfully and set isOnboarded to true when the credentials are different from default', async () => {
// Arrange
const verificationParams = merge(
createVerificationSmsCountParams({
logType: LogType.success,
smsType: SmsType.Verification,
}),
{ msgSrvcSid: 'i am different' },
)

// Act
const validSmsCount = new SmsCount(verificationParams)
const saved = await validSmsCount.save()

// Assert
// All fields should exist
// Object Id should be defined when successfully saved to MongoDB.
expect(saved._id).toBeDefined()
expect(saved.createdAt).toBeInstanceOf(Date)
// Retrieve object and compare to params, remove indeterministic keys
const actualSavedObject = omit(saved.toObject(), [
'_id',
'createdAt',
'__v',
])
expect(omit(actualSavedObject, 'isOnboardedAccount')).toEqual(
verificationParams,
)
expect(actualSavedObject.isOnboardedAccount).toBe(true)
})

it('should reject if form key is missing', async () => {
Expand Down Expand Up @@ -566,7 +614,6 @@ const createVerificationSmsCountParams = ({
smsCountParams.logType = logType
smsCountParams.smsType = smsType
smsCountParams.msgSrvcSid = MOCK_MSG_SRVC_SID

return smsCountParams
}

Expand All @@ -589,6 +636,9 @@ const logAndReturnExpectedLog = async ({
msgSrvcSid: MOCK_MSG_SRVC_SID,
smsType,
logType,
...(smsType === SmsType.Verification && {
isOnboardedAccount: !(MOCK_MSG_SRVC_SID === smsConfig.twilioMsgSrvcSid),
}),
}

return expectedLog
Expand Down
Loading

0 comments on commit 7f730dd

Please sign in to comment.