-
Notifications
You must be signed in to change notification settings - Fork 87
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(adminform): update form collab #1744
Merged
Merged
Changes from 7 commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
48c2673
feat(form/server/model): adds method to updateFormCollaborators
seaerchin d161b8a
feat(form/types; admin-form/service): adds service method for updatin…
seaerchin 9734865
feat(admin-forms/settings/routes): adds controller and handler for up…
seaerchin 2edf96f
feat(public/service): adds method on AdminFormService to update colla…
seaerchin 9824906
refactor(collaborator-modal.client.controller): replaces old call wit…
seaerchin 02f7e64
docs(admin-forms): fixed typos in handleUpdateCollaborators
seaerchin d42744c
test(tests): adds unit tests for form model, admin form service and a…
seaerchin cf0bff1
refactor(admin-form/controller): changed unknown on joi vaildator to …
seaerchin 9ebfb68
refactor(types): adds _id to the type of Permission
seaerchin 9271b89
refactor(admin-form/collaborators): updated dto to be just an array; …
seaerchin 9ce023e
fix(collaborator-modal/client/controller): fixed fe callsites showing…
seaerchin 3a5b5cf
fix(collaborator-modal/client/controller): fixed error caused by acci…
seaerchin bf7a6b4
Merge branch 'develop' into refactor/update-form-collab
seaerchin dba5f22
Merge branch 'develop' into refactor/update-form-collab
seaerchin 32a6fa3
refactor(admin-form/controller): changed to use .json and formCollabo…
seaerchin 45bd0f0
test(form/server/model/spec): changed test to use .rejects()
seaerchin f50f43e
style(collaborator-modal/client/controller): updated error message
seaerchin bf2efda
test(admin-forms): integration tests for updating of collaborators (#…
seaerchin 87119cc
refactor(adminform): extract get collaborators (#1762)
seaerchin 4188ab9
Merge branch 'develop' into refactor/update-form-collab
seaerchin 77e6433
chore(admin-forms/settings/routes/spec): removed double import
seaerchin File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
/* eslint-disable @typescript-eslint/ban-ts-comment */ | ||
import { ObjectId } from 'bson-ext' | ||
import { cloneDeep, merge, omit, orderBy, pick } from 'lodash' | ||
import { cloneDeep, map, merge, omit, orderBy, pick } from 'lodash' | ||
import mongoose, { Types } from 'mongoose' | ||
|
||
import getFormModel, { | ||
|
@@ -1625,5 +1625,45 @@ describe('Form Model', () => { | |
expect(updatedForm).toBeNull() | ||
}) | ||
}) | ||
|
||
describe('updateFormCollaborators', () => { | ||
it('should return the form with an updated list of collaborators', async () => { | ||
// Arrange | ||
const newCollaborators = [ | ||
{ | ||
email: `fakeuser@${MOCK_ADMIN_DOMAIN}`, | ||
write: false, | ||
}, | ||
] | ||
|
||
// Act | ||
const actual = await validForm.updateFormCollaborators(newCollaborators) | ||
|
||
// Assert | ||
const actualPermissionsWithoutId = map( | ||
actual.permissionList, | ||
(collaborator) => pick(collaborator, ['email', 'write']), | ||
) | ||
expect(actualPermissionsWithoutId).toEqual(newCollaborators) | ||
}) | ||
|
||
it('should return an error if validation fails', async () => { | ||
// Arrange | ||
const newCollaborators = [ | ||
{ | ||
email: `[email protected]`, | ||
write: false, | ||
}, | ||
] | ||
|
||
// Act | ||
const actual = await validForm | ||
.updateFormCollaborators(newCollaborators) | ||
.catch((error) => error) | ||
|
||
// Assert | ||
expect(actual).toBeInstanceOf(mongoose.Error.ValidationError) | ||
seaerchin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}) | ||
}) | ||
}) | ||
}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import { PresignedPost } from 'aws-sdk/clients/s3' | ||
import { ObjectId } from 'bson-ext' | ||
import { StatusCodes } from 'http-status-codes' | ||
import { assignIn, cloneDeep, merge, pick } from 'lodash' | ||
import { err, errAsync, ok, okAsync, Result } from 'neverthrow' | ||
import { PassThrough } from 'stream' | ||
|
@@ -7818,4 +7819,181 @@ describe('admin-form.controller', () => { | |
}) | ||
}) | ||
}) | ||
|
||
describe('_handleUpdateCollaborators', () => { | ||
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 MOCK_FORM = { | ||
admin: MOCK_USER, | ||
_id: MOCK_FORM_ID, | ||
} as IPopulatedForm | ||
const MOCK_COLLABORATORS = [ | ||
{ | ||
email: `[email protected]`, | ||
write: false, | ||
}, | ||
] | ||
const MOCK_REQ = expressHandler.mockRequest({ | ||
params: { | ||
formId: MOCK_FORM_ID, | ||
}, | ||
body: { permissionList: MOCK_COLLABORATORS }, | ||
session: { | ||
user: { | ||
_id: MOCK_USER_ID, | ||
}, | ||
}, | ||
}) | ||
|
||
beforeEach(() => { | ||
// Mock various services to return expected results. | ||
MockUserService.getPopulatedUserById.mockReturnValue(okAsync(MOCK_USER)) | ||
MockAuthService.getFormAfterPermissionChecks.mockReturnValue( | ||
okAsync(MOCK_FORM), | ||
) | ||
}) | ||
it('should return 200 when collaborators are updated successfully', async () => { | ||
// Arrange | ||
MockAdminFormService.updateFormCollaborators.mockReturnValueOnce( | ||
okAsync(MOCK_COLLABORATORS), | ||
) | ||
const mockRes = expressHandler.mockResponse() | ||
const expectedResponse = { permissionList: MOCK_COLLABORATORS } | ||
|
||
// Act | ||
await AdminFormController._handleUpdateCollaborators( | ||
MOCK_REQ, | ||
mockRes, | ||
jest.fn(), | ||
) | ||
|
||
// Assert | ||
expect(mockRes.status).toBeCalledWith(StatusCodes.OK) | ||
expect(mockRes.json).toBeCalledWith(expectedResponse) | ||
}) | ||
|
||
it('should return 403 when the user does not have sufficient permissions to update the form', async () => { | ||
// Arrange | ||
const ERROR_MESSAGE = 'all your base are belong to us' | ||
MockAuthService.getFormAfterPermissionChecks.mockReturnValueOnce( | ||
errAsync(new ForbiddenFormError(ERROR_MESSAGE)), | ||
) | ||
const mockRes = expressHandler.mockResponse() | ||
const expectedResponse = { message: ERROR_MESSAGE } | ||
|
||
// Act | ||
await AdminFormController._handleUpdateCollaborators( | ||
MOCK_REQ, | ||
mockRes, | ||
jest.fn(), | ||
) | ||
|
||
// Assert | ||
expect(mockRes.status).toBeCalledWith(StatusCodes.FORBIDDEN) | ||
expect(mockRes.json).toBeCalledWith(expectedResponse) | ||
expect( | ||
MockAdminFormService.updateFormCollaborators, | ||
).not.toHaveBeenCalled() | ||
}) | ||
|
||
it('should return 404 when the form could not be found', async () => { | ||
// Arrange | ||
const ERROR_MESSAGE = 'all your base are belong to us' | ||
MockAuthService.getFormAfterPermissionChecks.mockReturnValueOnce( | ||
errAsync(new FormNotFoundError(ERROR_MESSAGE)), | ||
) | ||
const mockRes = expressHandler.mockResponse() | ||
const expectedResponse = { message: ERROR_MESSAGE } | ||
|
||
// Act | ||
await AdminFormController._handleUpdateCollaborators( | ||
MOCK_REQ, | ||
mockRes, | ||
jest.fn(), | ||
) | ||
|
||
// Assert | ||
expect(mockRes.status).toBeCalledWith(StatusCodes.NOT_FOUND) | ||
expect(mockRes.json).toBeCalledWith(expectedResponse) | ||
expect( | ||
MockAdminFormService.updateFormCollaborators, | ||
).not.toHaveBeenCalled() | ||
}) | ||
|
||
it('should return 410 when the form has been archived', async () => { | ||
// Arrange | ||
const ERROR_MESSAGE = 'all your base are belong to us' | ||
MockAuthService.getFormAfterPermissionChecks.mockReturnValueOnce( | ||
errAsync(new FormDeletedError(ERROR_MESSAGE)), | ||
) | ||
const mockRes = expressHandler.mockResponse() | ||
const expectedResponse = { message: ERROR_MESSAGE } | ||
|
||
// Act | ||
await AdminFormController._handleUpdateCollaborators( | ||
MOCK_REQ, | ||
mockRes, | ||
jest.fn(), | ||
) | ||
|
||
// Assert | ||
expect(mockRes.status).toBeCalledWith(StatusCodes.GONE) | ||
expect(mockRes.json).toBeCalledWith(expectedResponse) | ||
expect( | ||
MockAdminFormService.updateFormCollaborators, | ||
).not.toHaveBeenCalled() | ||
}) | ||
|
||
it('should return 422 when the session user could not be retrieved from the database', async () => { | ||
// Arrange | ||
const ERROR_MESSAGE = 'all your base are belong to us' | ||
MockUserService.getPopulatedUserById.mockReturnValueOnce( | ||
errAsync(new MissingUserError(ERROR_MESSAGE)), | ||
) | ||
const expectedResponse = { message: ERROR_MESSAGE } | ||
const mockRes = expressHandler.mockResponse() | ||
|
||
// Act | ||
await AdminFormController._handleUpdateCollaborators( | ||
MOCK_REQ, | ||
mockRes, | ||
jest.fn(), | ||
) | ||
|
||
// Assert | ||
expect(mockRes.status).toBeCalledWith(StatusCodes.UNPROCESSABLE_ENTITY) | ||
expect(mockRes.json).toBeCalledWith(expectedResponse) | ||
expect( | ||
MockAdminFormService.updateFormCollaborators, | ||
).not.toHaveBeenCalled() | ||
}) | ||
|
||
it('should return 500 when a database error occurs', async () => { | ||
// Arrange | ||
const ERROR_MESSAGE = 'all your base are belong to us' | ||
MockUserService.getPopulatedUserById.mockReturnValueOnce( | ||
errAsync(new DatabaseError(ERROR_MESSAGE)), | ||
) | ||
const expectedResponse = { message: ERROR_MESSAGE } | ||
const mockRes = expressHandler.mockResponse() | ||
|
||
// Act | ||
await AdminFormController._handleUpdateCollaborators( | ||
MOCK_REQ, | ||
mockRes, | ||
jest.fn(), | ||
) | ||
|
||
// Assert | ||
expect(mockRes.status).toBeCalledWith(StatusCodes.INTERNAL_SERVER_ERROR) | ||
expect(mockRes.json).toBeCalledWith(expectedResponse) | ||
expect( | ||
MockAdminFormService.updateFormCollaborators, | ||
).not.toHaveBeenCalled() | ||
}) | ||
}) | ||
}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ import getFormModel, { | |
getEncryptedFormModel, | ||
} from 'src/app/models/form.server.model' | ||
import { | ||
ApplicationError, | ||
DatabaseConflictError, | ||
DatabaseError, | ||
DatabasePayloadSizeError, | ||
|
@@ -67,6 +68,7 @@ import { | |
reorderFormField, | ||
transferFormOwnership, | ||
updateForm, | ||
updateFormCollaborators, | ||
updateFormField, | ||
updateFormSettings, | ||
} from '../admin-form.service' | ||
|
@@ -1543,4 +1545,56 @@ describe('admin-form.service', () => { | |
) | ||
}) | ||
}) | ||
|
||
describe('updateFormCollaborators', () => { | ||
it('should return the list of collaborators when update is successful', async () => { | ||
// Arrange | ||
const newCollaborators = [ | ||
{ | ||
email: `[email protected]`, | ||
write: false, | ||
}, | ||
] | ||
const mockForm = ({ | ||
title: 'some mock form', | ||
updateFormCollaborators: jest | ||
.fn() | ||
.mockResolvedValue({ permissionList: newCollaborators }), | ||
} as unknown) as IPopulatedForm | ||
|
||
// Act | ||
const actual = await updateFormCollaborators(mockForm, newCollaborators) | ||
|
||
// Assert | ||
expect(mockForm.updateFormCollaborators).toHaveBeenCalledWith( | ||
newCollaborators, | ||
) | ||
expect(actual._unsafeUnwrap()).toEqual(newCollaborators) | ||
}) | ||
|
||
it('should return an application error when updating the form model fails', async () => { | ||
// Arrange | ||
const newCollaborators = [ | ||
{ | ||
email: `[email protected]`, | ||
write: false, | ||
}, | ||
] | ||
const mockForm = ({ | ||
title: 'some mock form', | ||
updateFormCollaborators: jest | ||
.fn() | ||
.mockRejectedValue(new DatabaseError()), | ||
} as unknown) as IPopulatedForm | ||
|
||
// Act | ||
const actual = await updateFormCollaborators(mockForm, newCollaborators) | ||
|
||
// Assert | ||
expect(mockForm.updateFormCollaborators).toHaveBeenCalledWith( | ||
newCollaborators, | ||
) | ||
expect(actual._unsafeUnwrapErr()).toBeInstanceOf(ApplicationError) | ||
}) | ||
}) | ||
}) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hahahahahaha @ using
_.map
instead of native mapThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
y u laff