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: Extract delete logic endpoint #1586

Merged
merged 30 commits into from
Apr 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
0c28dae
refactor: v3 delete logic api
tshuli Apr 8, 2021
d200512
test: integration tests
tshuli Apr 8, 2021
0b9af1a
chore: use general form model
tshuli Apr 12, 2021
ab36dbb
refactor: use mongoose $pull operator instead of manual filtering
tshuli Apr 13, 2021
4b878c8
chore: return transformMongoError
tshuli Apr 13, 2021
b514f6f
refactor: use ObjectId()
tshuli Apr 13, 2021
5183a65
chore: add unit tests
tshuli Apr 14, 2021
b099515
refactor: default message for LogicNotFoundError
tshuli Apr 14, 2021
d47a9cb
refactor: shift logic to separate router
tshuli Apr 14, 2021
2cf4291
chore: add test case for logicId cannot be found
tshuli Apr 15, 2021
380bd1f
chore: reset mock forms in beforeEach
tshuli Apr 15, 2021
fdffeb2
refactor: FormModel
tshuli Apr 15, 2021
5b57f98
refactor: shift tests to admin-forms.logic.routes.spec
tshuli Apr 15, 2021
7717d43
refactor: simplify return of deleteFormLogic
tshuli Apr 15, 2021
6079d49
chore: frontend changes for delete logic api
tshuli Apr 19, 2021
bf73b36
chore: simplify
tshuli Apr 19, 2021
cbf974f
refactor: use model static method
tshuli Apr 19, 2021
867e846
chore: use $q to splice only upon success
tshuli Apr 19, 2021
bc3767d
refactor: pass in formId string instead of form object
tshuli Apr 20, 2021
a713fda
test: add unit tests for static method
tshuli Apr 20, 2021
7b19b61
chore: return form instead of true
tshuli Apr 20, 2021
54b7861
nit: typo
tshuli Apr 21, 2021
f67c652
chore: add details to error msg
tshuli Apr 21, 2021
6e02df5
chore: add test for multiple logic case
tshuli Apr 21, 2021
cc2d7bc
build: merge conflict
tshuli Apr 21, 2021
1f681e7
chore: not send message on logic delete
tshuli Apr 21, 2021
35a78f8
chore: add return true for axios
tshuli Apr 21, 2021
122edf1
chore: splice upon promise success
tshuli Apr 21, 2021
9aad91c
chore: cast form._id to string
tshuli Apr 21, 2021
c108ff8
chore: update tests
tshuli Apr 21, 2021
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
100 changes: 99 additions & 1 deletion src/app/models/__tests__/form.server.model.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
IEncryptedForm,
IFieldSchema,
IFormSchema,
ILogicSchema,
IPopulatedUser,
Permission,
ResponseMode,
Expand Down Expand Up @@ -384,7 +385,7 @@ describe('Form Model', () => {
expect(actualSavedObject).toEqual(expectedObject)

// Remove indeterministic id from actual permission list
const actualPermissionList = (saved.toObject() as IEncryptedForm).permissionList?.map(
const actualPermissionList = ((saved.toObject() as unknown) as IEncryptedForm).permissionList?.map(
(permission) => omit(permission, '_id'),
)
expect(actualPermissionList).toEqual(permissionList)
Expand Down Expand Up @@ -1053,6 +1054,103 @@ describe('Form Model', () => {
expect(actual).toEqual(expected)
})
})

describe('deleteFormLogic', () => {
const logicId = new ObjectId().toHexString()
const mockFormLogic = {
form_logics: [
{
_id: logicId,
id: logicId,
} as ILogicSchema,
],
}

it('should return form upon successful delete', async () => {
// arrange
const formParams = merge({}, MOCK_EMAIL_FORM_PARAMS, {
admin: populatedAdmin,
status: Status.Public,
responseMode: ResponseMode.Email,
...mockFormLogic,
})
const form = await Form.create(formParams)

// act
const modifiedForm = await Form.deleteFormLogic(form._id, logicId)

// assert
// Form should be returned
expect(modifiedForm).not.toBeNull()

// Form should have correct status, responsemode
expect(modifiedForm?.responseMode).not.toBeNull()
expect(modifiedForm?.responseMode).toEqual(ResponseMode.Email)
expect(modifiedForm?.status).not.toBeNull()
expect(modifiedForm?.status).toEqual(Status.Public)

// Check that form logic has been deleted
expect(modifiedForm?.form_logics).toBeEmpty()
})

it('should return form with remaining logic upon successful delete of one logic', async () => {
// arrange

const logicId2 = new ObjectId().toHexString()
const mockFormLogicMultiple = {
form_logics: [
{
_id: logicId,
id: logicId,
} as ILogicSchema,
{
_id: logicId2,
id: logicId2,
} as ILogicSchema,
],
}

const formParams = merge({}, MOCK_EMAIL_FORM_PARAMS, {
admin: populatedAdmin,
status: Status.Public,
responseMode: ResponseMode.Email,
...mockFormLogicMultiple,
})
const form = await Form.create(formParams)

// act
const modifiedForm = await Form.deleteFormLogic(form._id, logicId)

// assert
// Form should be returned
expect(modifiedForm).not.toBeNull()

// Form should have correct status, responsemode
expect(modifiedForm?.responseMode).not.toBeNull()
expect(modifiedForm?.responseMode).toEqual(ResponseMode.Email)
expect(modifiedForm?.status).not.toBeNull()
expect(modifiedForm?.status).toEqual(Status.Public)

// Check that correct form logic has been deleted
expect(modifiedForm?.form_logics).toBeDefined()
expect(modifiedForm?.form_logics).toHaveLength(1)
const logic = modifiedForm?.form_logics || ['some logic']
expect((logic[0] as any)['_id'].toString()).toEqual(logicId2)
tshuli marked this conversation as resolved.
Show resolved Hide resolved
})

it('should return null if formId is invalid', async () => {
// arrange

const invalidFormId = new ObjectId().toHexString()

// act
const modifiedForm = await Form.deleteFormLogic(invalidFormId, logicId)

// assert
// should return null
expect(modifiedForm).toBeNull()
})
})
})

describe('Methods', () => {
Expand Down
18 changes: 18 additions & 0 deletions src/app/models/form.server.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,24 @@ const compileFormModel = (db: Mongoose): IFormModel => {
)
}

// Deletes specified form logic.
FormSchema.statics.deleteFormLogic = async function (
tshuli marked this conversation as resolved.
Show resolved Hide resolved
this: IFormModel,
formId: string,
logicId: string,
): Promise<IFormSchema | null> {
return this.findByIdAndUpdate(
mongoose.Types.ObjectId(formId),
Copy link
Contributor

Choose a reason for hiding this comment

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

do not need to cast, mongoose will automatically cast for you; string type is fine

{
$pull: { form_logics: { _id: logicId } },
},
{
new: true,
runValidators: true,
},
).exec()
}

// Hooks
FormSchema.pre<IFormSchema>('validate', function (next) {
// Reject save if form document is too large
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import {
IFieldSchema,
IForm,
IFormSchema,
ILogicSchema,
IPopulatedEmailForm,
IPopulatedEncryptedForm,
IPopulatedForm,
Expand Down Expand Up @@ -76,6 +77,7 @@ import {
ForbiddenFormError,
FormDeletedError,
FormNotFoundError,
LogicNotFoundError,
PrivateFormError,
TransferOwnershipError,
} from '../../form.errors'
Expand Down Expand Up @@ -6777,6 +6779,7 @@ describe('admin-form.controller', () => {
MockAuthService.getFormAfterPermissionChecks.mockReturnValue(
okAsync(MOCK_FORM),
)

MockAdminFormService.updateFormField.mockReturnValue(
okAsync(MOCK_UPDATED_FIELD as IFieldSchema),
)
Expand Down Expand Up @@ -7017,6 +7020,7 @@ describe('admin-form.controller', () => {
_id: MOCK_USER_ID,
email: '[email protected]',
} as IPopulatedUser

const MOCK_FORM = {
admin: MOCK_USER,
_id: MOCK_FORM_ID,
Expand All @@ -7039,7 +7043,6 @@ describe('admin-form.controller', () => {
},
body: MOCK_CREATE_FIELD_BODY,
})

beforeEach(() => {
MockUserService.getPopulatedUserById.mockReturnValue(okAsync(MOCK_USER))
MockAuthService.getFormAfterPermissionChecks.mockReturnValue(
Expand Down Expand Up @@ -7296,4 +7299,201 @@ describe('admin-form.controller', () => {
)
})
})

describe('handleDeleteLogic', () => {
const MOCK_USER_ID = new ObjectId().toHexString()
const MOCK_FORM_ID = new ObjectId().toHexString()
const MOCK_USER = {
_id: MOCK_USER_ID,
email: '[email protected]',
} as IPopulatedUser

const logicId = new ObjectId().toHexString()
const mockFormLogic = {
form_logics: [
{
_id: logicId,
id: logicId,
} as ILogicSchema,
],
}

const MOCK_FORM = {
admin: MOCK_USER,
_id: MOCK_FORM_ID,
title: 'mock title',
...mockFormLogic,
} as IPopulatedForm

const mockReq = expressHandler.mockRequest({
params: {
formId: MOCK_FORM_ID,
logicId,
},
session: {
user: {
_id: MOCK_USER_ID,
},
},
})
const mockRes = expressHandler.mockResponse()
beforeEach(() => {
MockUserService.getPopulatedUserById.mockReturnValue(okAsync(MOCK_USER))
MockAuthService.getFormAfterPermissionChecks.mockReturnValue(
okAsync(MOCK_FORM),
)
MockAdminFormService.deleteFormLogic.mockReturnValue(okAsync(true))
})

it('should call all services correctly when request is valid', async () => {
await AdminFormController.handleDeleteLogic(mockReq, mockRes, jest.fn())

expect(MockUserService.getPopulatedUserById).toHaveBeenCalledWith(
MOCK_USER_ID,
)
expect(MockAuthService.getFormAfterPermissionChecks).toHaveBeenCalledWith(
{
user: MOCK_USER,
formId: MOCK_FORM_ID,
level: PermissionLevel.Write,
},
)
expect(MockAdminFormService.deleteFormLogic).toHaveBeenCalledWith(
MOCK_FORM,
logicId,
)

expect(mockRes.sendStatus).toHaveBeenCalledWith(200)
})

it('should return 403 when user does not have permissions to delete logic', async () => {
MockAuthService.getFormAfterPermissionChecks.mockReturnValue(
errAsync(
new ForbiddenFormError('not authorized to perform write operation'),
),
)

await AdminFormController.handleDeleteLogic(mockReq, mockRes, jest.fn())

expect(MockUserService.getPopulatedUserById).toHaveBeenCalledWith(
MOCK_USER_ID,
)
expect(MockAuthService.getFormAfterPermissionChecks).toHaveBeenCalledWith(
{
user: MOCK_USER,
formId: MOCK_FORM_ID,
level: PermissionLevel.Write,
},
)
expect(MockAdminFormService.deleteFormLogic).not.toHaveBeenCalled()

expect(mockRes.status).toHaveBeenCalledWith(403)

expect(mockRes.json).toHaveBeenCalledWith({
message: 'not authorized to perform write operation',
tshuli marked this conversation as resolved.
Show resolved Hide resolved
})
})

it('should return 404 when logicId cannot be found', async () => {
MockAdminFormService.deleteFormLogic.mockReturnValue(
errAsync(new LogicNotFoundError()),
)

await AdminFormController.handleDeleteLogic(mockReq, mockRes, jest.fn())

expect(MockUserService.getPopulatedUserById).toHaveBeenCalledWith(
MOCK_USER_ID,
)
expect(MockAuthService.getFormAfterPermissionChecks).toHaveBeenCalledWith(
{
user: MOCK_USER,
formId: MOCK_FORM_ID,
level: PermissionLevel.Write,
},
)

expect(MockAdminFormService.deleteFormLogic).toHaveBeenCalledWith(
MOCK_FORM,
logicId,
)

expect(mockRes.status).toHaveBeenCalledWith(404)

expect(mockRes.json).toHaveBeenCalledWith({
message: 'logicId does not exist on form',
})
})

it('should return 404 when form cannot be found', async () => {
tshuli marked this conversation as resolved.
Show resolved Hide resolved
MockAuthService.getFormAfterPermissionChecks.mockReturnValue(
errAsync(new FormNotFoundError()),
)

await AdminFormController.handleDeleteLogic(mockReq, mockRes, jest.fn())

expect(MockUserService.getPopulatedUserById).toHaveBeenCalledWith(
MOCK_USER_ID,
)
expect(MockAuthService.getFormAfterPermissionChecks).toHaveBeenCalledWith(
{
user: MOCK_USER,
formId: MOCK_FORM_ID,
level: PermissionLevel.Write,
},
)
expect(MockAdminFormService.deleteFormLogic).not.toHaveBeenCalled()

expect(mockRes.status).toHaveBeenCalledWith(404)

expect(mockRes.json).toHaveBeenCalledWith({
message: 'Form not found',
})
})

it('should return 422 when user in session cannot be retrieved from the database', async () => {
MockUserService.getPopulatedUserById.mockReturnValue(
errAsync(new MissingUserError()),
)

await AdminFormController.handleDeleteLogic(mockReq, mockRes, jest.fn())

expect(MockUserService.getPopulatedUserById).toHaveBeenCalledWith(
MOCK_USER_ID,
)
expect(
MockAuthService.getFormAfterPermissionChecks,
).not.toHaveBeenCalled()

expect(MockAdminFormService.deleteFormLogic).not.toHaveBeenCalled()

expect(mockRes.status).toHaveBeenCalledWith(422)

expect(mockRes.json).toHaveBeenCalledWith({
message: 'User not found',
})
})

it('should return 500 when database error occurs', async () => {
MockUserService.getPopulatedUserById.mockReturnValue(
errAsync(new DatabaseError()),
)

await AdminFormController.handleDeleteLogic(mockReq, mockRes, jest.fn())

expect(MockUserService.getPopulatedUserById).toHaveBeenCalledWith(
MOCK_USER_ID,
)
expect(
MockAuthService.getFormAfterPermissionChecks,
).not.toHaveBeenCalled()

expect(MockAdminFormService.deleteFormLogic).not.toHaveBeenCalled()

expect(mockRes.status).toHaveBeenCalledWith(500)

expect(mockRes.json).toHaveBeenCalledWith({
message: 'Something went wrong. Please try again.',
})
})
})
})
Loading