diff --git a/shared/src/business/useCaseHelper/caseAssociation/createUserForContact.ts b/shared/src/business/useCaseHelper/caseAssociation/createUserForContact.ts index 9187af75fc4..b1addfb14cd 100644 --- a/shared/src/business/useCaseHelper/caseAssociation/createUserForContact.ts +++ b/shared/src/business/useCaseHelper/caseAssociation/createUserForContact.ts @@ -42,10 +42,28 @@ export const createUserForContact = async ({ const userRaw = userEntity.validate().toRawObject(); - await applicationContext.getPersistenceGateway().createNewPetitionerUser({ - applicationContext, - user: userRaw, - }); + const createUserPromise = applicationContext + .getUserGateway() + .createUser(applicationContext, { + attributesToUpdate: { + email: userRaw.pendingEmail, + name: userRaw.name, + role: ROLES.petitioner, + userId: userRaw.userId, + }, + email: userRaw.pendingEmail!, + resendInvitationEmail: false, + }); + + const createUserRecordsPromise = applicationContext + .getPersistenceGateway() + .createUserRecords({ + applicationContext, + user: userRaw, + userId: userRaw.userId, + }); + + await Promise.all([createUserPromise, createUserRecordsPromise]); const rawCase = caseEntity.toRawObject(); const userCaseEntity = new UserCase(rawCase); diff --git a/web-api/src/business/useCases/practitioner/createPractitionerUserInteractor.ts b/web-api/src/business/useCases/practitioner/createPractitionerUserInteractor.ts index 42bda73d028..553772046c9 100644 --- a/web-api/src/business/useCases/practitioner/createPractitionerUserInteractor.ts +++ b/web-api/src/business/useCases/practitioner/createPractitionerUserInteractor.ts @@ -1,3 +1,4 @@ +import { ROLES, Role } from '@shared/business/entities/EntityConstants'; import { ROLE_PERMISSIONS, isAuthorized, @@ -17,6 +18,17 @@ export const createPractitionerUserInteractor = async ( throw new UnauthorizedError('Unauthorized for creating practitioner user'); } + const practitionerRoleTypes: Role[] = [ + ROLES.privatePractitioner, + ROLES.irsPractitioner, + ROLES.inactivePractitioner, + ]; + if (!practitionerRoleTypes.includes(user.role)) { + throw new Error( + `Role must be ${ROLES.privatePractitioner}, ${ROLES.irsPractitioner}, or ${ROLES.inactivePractitioner}`, + ); + } + user.pendingEmail = user.email; user.email = undefined; @@ -24,12 +36,51 @@ export const createPractitionerUserInteractor = async ( user, }); - const createdUser = await applicationContext - .getPersistenceGateway() - .createOrUpdatePractitionerUser({ + if (!practitioner.pendingEmail) { + return await applicationContext.getPersistenceGateway().createUserRecords({ applicationContext, - user: practitioner, + user, + userId: practitioner.userId, + }); + } + + const existingUser = await applicationContext + .getUserGateway() + .getUserByEmail(applicationContext, { + email: practitioner.pendingEmail, + }); + + if (!existingUser) { + await applicationContext.getUserGateway().createUser(applicationContext, { + attributesToUpdate: { + email: practitioner.pendingEmail, + name: practitioner.name, + role: practitioner.role, + userId: practitioner.userId, + }, + email: practitioner.pendingEmail, + resendInvitationEmail: false, }); - return { barNumber: createdUser.barNumber }; + await applicationContext.getPersistenceGateway().createUserRecords({ + applicationContext, + user, + userId: practitioner.userId, + }); + } else { + await applicationContext.getUserGateway().updateUser(applicationContext, { + attributesToUpdate: { + role: practitioner.role, + }, + email: practitioner.pendingEmail, + }); + + await applicationContext.getPersistenceGateway().createUserRecords({ + applicationContext, + user, + userId: existingUser.userId, + }); + } + + return { barNumber: practitioner.barNumber }; }; diff --git a/web-api/src/getPersistenceGateway.ts b/web-api/src/getPersistenceGateway.ts index 5be6c81d993..ca5d9180bdf 100644 --- a/web-api/src/getPersistenceGateway.ts +++ b/web-api/src/getPersistenceGateway.ts @@ -19,10 +19,11 @@ import { removeLock, } from './persistence/dynamo/locks/acquireLock'; import { createMessage } from './persistence/dynamo/messages/createMessage'; -import { createNewPetitionerUser } from './persistence/dynamo/users/createNewPetitionerUser'; import { createNewPractitionerUser } from './persistence/dynamo/users/createNewPractitionerUser'; -import { createOrUpdatePractitionerUser } from './persistence/dynamo/users/createOrUpdatePractitionerUser'; -import { createOrUpdateUser } from './persistence/dynamo/users/createOrUpdateUser'; +import { + createOrUpdateUser, + createUserRecords, +} from './persistence/dynamo/users/createOrUpdateUser'; import { createPractitionerDocument } from './persistence/dynamo/practitioners/createPractitionerDocument'; import { createTrialSession } from './persistence/dynamo/trialSessions/createTrialSession'; import { createTrialSessionWorkingCopy } from './persistence/dynamo/trialSessions/createTrialSessionWorkingCopy'; @@ -229,13 +230,12 @@ const gatewayMethods = { createCaseTrialSortMappingRecords, createJobStatus, createMessage, - createNewPetitionerUser, createNewPractitionerUser, - createOrUpdatePractitionerUser, createOrUpdateUser, createPractitionerDocument, createTrialSession, createTrialSessionWorkingCopy, + createUserRecords, deleteKeyCount, editPractitionerDocument, fetchPendingItems, diff --git a/web-api/src/persistence/dynamo/users/createNewPetitionerUser.test.ts b/web-api/src/persistence/dynamo/users/createNewPetitionerUser.test.ts deleted file mode 100644 index 3f13522cade..00000000000 --- a/web-api/src/persistence/dynamo/users/createNewPetitionerUser.test.ts +++ /dev/null @@ -1,43 +0,0 @@ -import { ROLES } from '../../../../../shared/src/business/entities/EntityConstants'; -import { applicationContext } from '../../../../../shared/src/business/test/createTestApplicationContext'; -import { createNewPetitionerUser } from './createNewPetitionerUser'; - -describe('createNewPetitionerUser', () => { - const mockUser = { - entityName: 'User', - name: 'Bob Ross', - pendingEmail: 'petitioner@example.com', - role: ROLES.petitioner, - section: 'petitioner', - userId: 'e6df170d-bc7d-428b-b0f2-decb3f9b83a8', - }; - - it('should make a call to create the specified user in persistence', async () => { - await createNewPetitionerUser({ - applicationContext, - user: mockUser, - }); - - expect(applicationContext.getUserGateway().createUser).toHaveBeenCalledWith( - applicationContext, - { - attributesToUpdate: { - email: mockUser.pendingEmail, - name: mockUser.name, - role: mockUser.role, - userId: mockUser.userId, - }, - email: mockUser.pendingEmail, - resendInvitationEmail: false, - }, - ); - expect( - applicationContext.getDocumentClient().put.mock.calls[0][0].Item, - ).toMatchObject({ - ...mockUser, - pk: `user|${mockUser.userId}`, - sk: `user|${mockUser.userId}`, - userId: mockUser.userId, - }); - }); -}); diff --git a/web-api/src/persistence/dynamo/users/createNewPetitionerUser.ts b/web-api/src/persistence/dynamo/users/createNewPetitionerUser.ts deleted file mode 100644 index 4d6d08c8ef1..00000000000 --- a/web-api/src/persistence/dynamo/users/createNewPetitionerUser.ts +++ /dev/null @@ -1,58 +0,0 @@ -import * as client from '../../dynamodbClientService'; -import { ROLES } from '../../../../../shared/src/business/entities/EntityConstants'; -import { RawUser } from '@shared/business/entities/User'; -import { ServerApplicationContext } from '@web-api/applicationContext'; - -export const createUserRecords = async ({ - applicationContext, - newUser, - userId, -}: { - applicationContext: IApplicationContext; - newUser: RawUser; - userId: string; -}) => { - await client.put({ - Item: { - ...newUser, - pk: `user|${userId}`, - sk: `user|${userId}`, - userId, - }, - applicationContext, - }); - - return { - ...newUser, - userId, - }; -}; - -export const createNewPetitionerUser = async ({ - applicationContext, - user, -}: { - applicationContext: ServerApplicationContext; - user: RawUser; -}): Promise => { - const createUserPromise = applicationContext - .getUserGateway() - .createUser(applicationContext, { - attributesToUpdate: { - email: user.pendingEmail, - name: user.name, - role: ROLES.petitioner, - userId: user.userId, - }, - email: user.pendingEmail!, - resendInvitationEmail: false, - }); - - const createUserRecordsPromise = createUserRecords({ - applicationContext, - newUser: user, - userId: user.userId, - }); - - await Promise.all([createUserPromise, createUserRecordsPromise]); -}; diff --git a/web-api/src/persistence/dynamo/users/createOrUpdatePractitionerUser.test.ts b/web-api/src/persistence/dynamo/users/createOrUpdatePractitionerUser.test.ts deleted file mode 100644 index 044aabbfbf9..00000000000 --- a/web-api/src/persistence/dynamo/users/createOrUpdatePractitionerUser.test.ts +++ /dev/null @@ -1,335 +0,0 @@ -import { ROLES } from '../../../../../shared/src/business/entities/EntityConstants'; -import { UserNotFoundException } from '@aws-sdk/client-cognito-identity-provider'; -import { applicationContext } from '../../../../../shared/src/business/test/createTestApplicationContext'; -import { - createOrUpdatePractitionerUser, - createUserRecords, -} from './createOrUpdatePractitionerUser'; - -describe('createOrUpdatePractitionerUser', () => { - const oldEnv = process.env; - - afterAll(() => { - process.env = oldEnv; - }); - - const userId = '9b52c605-edba-41d7-b045-d5f992a499d3'; - const privatePractitionerUser = { - barNumber: 'pt1234', - email: 'test@example.com', - name: 'Test Private Practitioner', - role: ROLES.privatePractitioner, - section: 'privatePractitioner', - userId, - }; - const irsPractitionerUser = { - barNumber: 'pt1234', - email: 'test@example.com', - name: 'Test IRS Practitioner', - role: ROLES.irsPractitioner, - section: 'privatePractitioner', - }; - const inactivePractitionerUser = { - barNumber: 'pt1234', - email: 'test@example.com', - name: 'Test Inactive Practitioner', - role: ROLES.inactivePractitioner, - section: 'privatePractitioner', - }; - const privatePractitionerUserWithSection = { - barNumber: 'pt1234', - email: 'test@example.com', - name: 'Test Private Practitioner', - role: ROLES.privatePractitioner, - section: 'privatePractitioner', - }; - const privatePractitionerUserWithoutBarNumber = { - barNumber: '', - name: 'Test Private Practitioner', - role: ROLES.privatePractitioner, - section: 'privatePractitioner', - }; - const otherUser = { - barNumber: 'pt1234', - email: 'test@example.com', - name: 'Test Other', - role: 'other', - section: 'other', - }; - - const setupNonExistingUserMock = () => { - applicationContext - .getCognito() - .adminGetUser.mockRejectedValue( - new UserNotFoundException({ $metadata: {}, message: '' }), - ); - }; - - beforeAll(() => { - applicationContext.getCognito().adminGetUser.mockResolvedValue({ - Username: 'f7bea269-fa95-424d-aed8-2cb988df2073', - }); - - applicationContext.getCognito().adminCreateUser.mockResolvedValue({ - User: { Username: userId }, - }); - - applicationContext.getDocumentClient().put.mockResolvedValue(null); - }); - - it('persists a private practitioner user with name and barNumber mapping records but does not call cognito adminCreateUser if there is no email address', async () => { - await createUserRecords({ - applicationContext, - user: privatePractitionerUser, - userId, - }); - - expect( - applicationContext.getDocumentClient().put.mock.calls[0][0], - ).toMatchObject({ - Item: { - ...privatePractitionerUser, - pk: `user|${userId}`, - sk: `user|${userId}`, - }, - }); - expect( - applicationContext.getDocumentClient().put.mock.calls[1][0], - ).toMatchObject({ - Item: { - pk: 'privatePractitioner|TEST PRIVATE PRACTITIONER', - sk: `user|${userId}`, - }, - }); - expect( - applicationContext.getDocumentClient().put.mock.calls[2][0], - ).toMatchObject({ - Item: { - pk: 'privatePractitioner|PT1234', - sk: `user|${userId}`, - }, - }); - }); - - it('does not persist mapping records for practitioner without barNumber', async () => { - await createUserRecords({ - applicationContext, - user: privatePractitionerUserWithoutBarNumber, - userId, - }); - - expect( - applicationContext.getDocumentClient().put.mock.calls[0][0], - ).toMatchObject({ - Item: { - ...privatePractitionerUserWithoutBarNumber, - pk: `user|${userId}`, - sk: `user|${userId}`, - }, - }); - - await createOrUpdatePractitionerUser({ - applicationContext, - user: privatePractitionerUserWithoutBarNumber as any, - }); - - expect( - applicationContext.getCognito().adminCreateUser, - ).not.toHaveBeenCalled(); - expect(applicationContext.getCognito().adminGetUser).not.toHaveBeenCalled(); - expect( - applicationContext.getUserGateway().updateUser, - ).not.toHaveBeenCalled(); - }); - - it('should call cognito adminCreateUser for a private practitioner user with email address', async () => { - setupNonExistingUserMock(); - - await createOrUpdatePractitionerUser({ - applicationContext, - user: privatePractitionerUserWithSection as any, - }); - - expect( - applicationContext.getCognito().adminCreateUser.mock.calls[0][0].Username, - ).toBe(privatePractitionerUserWithSection.email); - expect( - applicationContext.getUserGateway().updateUser, - ).not.toHaveBeenCalled(); - }); - - it('should call cognito adminCreateUser for a private practitioner user with pendingEmail when it is defined', async () => { - const mockPendingEmail = 'noone@example.com'; - setupNonExistingUserMock(); - - await createOrUpdatePractitionerUser({ - applicationContext, - user: { - ...privatePractitionerUserWithSection, - email: undefined, - pendingEmail: mockPendingEmail, - } as any, - }); - - expect( - applicationContext.getCognito().adminCreateUser.mock.calls[0][0].Username, - ).toBe(mockPendingEmail); - }); - - it('should call cognito adminCreateUser for an IRS practitioner user with email address', async () => { - setupNonExistingUserMock(); - - await createOrUpdatePractitionerUser({ - applicationContext, - user: irsPractitionerUser as any, - }); - - expect(applicationContext.getCognito().adminCreateUser).toHaveBeenCalled(); - expect( - applicationContext.getUserGateway().updateUser, - ).not.toHaveBeenCalled(); - }); - - it('should call cognito adminCreateUser for an inactive practitioner user with email address', async () => { - setupNonExistingUserMock(); - - await createOrUpdatePractitionerUser({ - applicationContext, - user: inactivePractitionerUser as any, - }); - - expect(applicationContext.getCognito().adminCreateUser).toHaveBeenCalled(); - expect( - applicationContext.getUserGateway().updateUser, - ).not.toHaveBeenCalled(); - }); - - it('should call cognito adminCreateUser for a private practitioner user with email address and use a random uniqueId if the response does not contain a username (for local testing)', async () => { - applicationContext.getCognito().adminCreateUser.mockResolvedValue({}); - applicationContext - .getCognito() - .adminGetUser.mockRejectedValue( - new UserNotFoundException({ $metadata: {}, message: '' }), - ); - - await createOrUpdatePractitionerUser({ - applicationContext, - user: privatePractitionerUserWithSection as any, - }); - - expect(applicationContext.getCognito().adminCreateUser).toHaveBeenCalled(); - expect( - applicationContext.getUserGateway().updateUser, - ).not.toHaveBeenCalled(); - }); - - it('should throw an error when attempting to create a user that is not role private, IRS practitioner or inactive practitioner', async () => { - await expect( - createOrUpdatePractitionerUser({ - applicationContext, - user: otherUser as any, - }), - ).rejects.toThrow( - `Role must be ${ROLES.privatePractitioner}, ${ROLES.irsPractitioner}, or ${ROLES.inactivePractitioner}`, - ); - }); - - it('should call adminCreateUser with the correct UserAttributes', async () => { - applicationContext.getCognito().adminCreateUser.mockReturnValue({ - User: { Username: '123' }, - }); - - setupNonExistingUserMock(); - - await createOrUpdatePractitionerUser({ - applicationContext, - user: privatePractitionerUser as any, - }); - - expect( - applicationContext.getCognito().adminCreateUser, - ).toHaveBeenCalledWith({ - DesiredDeliveryMediums: ['EMAIL'], - UserAttributes: [ - { - Name: 'email_verified', - Value: 'True', - }, - { - Name: 'email', - Value: 'test@example.com', - }, - { - Name: 'custom:role', - Value: 'privatePractitioner', - }, - { - Name: 'name', - Value: 'Test Private Practitioner', - }, - ], - UserPoolId: undefined, - Username: 'test@example.com', - }); - }); - - describe('createUserRecords', () => { - it('attempts to persist a private practitioner user with name and barNumber mapping records', async () => { - await createUserRecords({ - applicationContext, - user: privatePractitionerUser, - userId, - }); - - expect(applicationContext.getDocumentClient().put.mock.calls.length).toBe( - 3, - ); - expect( - applicationContext.getDocumentClient().put.mock.calls[0][0], - ).toMatchObject({ - Item: { - ...privatePractitionerUser, - pk: `user|${userId}`, - sk: `user|${userId}`, - }, - }); - expect( - applicationContext.getDocumentClient().put.mock.calls[1][0], - ).toMatchObject({ - Item: { - pk: 'privatePractitioner|TEST PRIVATE PRACTITIONER', - sk: `user|${userId}`, - }, - }); - expect( - applicationContext.getDocumentClient().put.mock.calls[2][0], - ).toMatchObject({ - Item: { - pk: 'privatePractitioner|PT1234', - sk: `user|${userId}`, - }, - }); - }); - - it('does not persist mapping records for private practitioner without barNumber', async () => { - await createUserRecords({ - applicationContext, - user: privatePractitionerUserWithoutBarNumber, - userId, - }); - - expect(applicationContext.getDocumentClient().put.mock.calls.length).toBe( - 1, - ); - expect( - applicationContext.getDocumentClient().put.mock.calls[0][0], - ).toMatchObject({ - Item: { - ...privatePractitionerUserWithoutBarNumber, - pk: `user|${userId}`, - sk: `user|${userId}`, - }, - }); - }); - }); -}); diff --git a/web-api/src/persistence/dynamo/users/createOrUpdatePractitionerUser.ts b/web-api/src/persistence/dynamo/users/createOrUpdatePractitionerUser.ts deleted file mode 100644 index 997336b5cf7..00000000000 --- a/web-api/src/persistence/dynamo/users/createOrUpdatePractitionerUser.ts +++ /dev/null @@ -1,160 +0,0 @@ -import * as client from '../../dynamodbClientService'; -import { AdminCreateUserCommandInput } from '@aws-sdk/client-cognito-identity-provider'; -import { - ROLES, - Role, -} from '../../../../../shared/src/business/entities/EntityConstants'; -import { RawUser } from '@shared/business/entities/User'; -import { ServerApplicationContext } from '@web-api/applicationContext'; - -export const createUserRecords = async ({ - applicationContext, - user, - userId, -}: { - applicationContext: IApplicationContext; - user: any; - userId: string; -}) => { - delete user.password; - - if (user.barNumber === '') { - delete user.barNumber; - } - - await client.put({ - Item: { - ...user, - pk: `user|${userId}`, - sk: `user|${userId}`, - userId, - }, - applicationContext, - }); - - if (user.name && user.barNumber) { - const upperCaseName = user.name.toUpperCase(); - await client.put({ - Item: { - pk: `${user.role}|${upperCaseName}`, - sk: `user|${userId}`, - }, - applicationContext, - }); - const upperCaseBarNumber = user.barNumber.toUpperCase(); - await client.put({ - Item: { - pk: `${user.role}|${upperCaseBarNumber}`, - sk: `user|${userId}`, - }, - applicationContext, - }); - } - - return { - ...user, - userId, - }; -}; - -export const createOrUpdatePractitionerUser = async ({ - applicationContext, - user, -}: { - applicationContext: ServerApplicationContext; - user: RawUser; -}) => { - let userId = applicationContext.getUniqueId(); - const practitionerRoleTypes: Role[] = [ - ROLES.privatePractitioner, - ROLES.irsPractitioner, - ROLES.inactivePractitioner, - ]; - - if (!practitionerRoleTypes.includes(user.role)) { - throw new Error( - `Role must be ${ROLES.privatePractitioner}, ${ROLES.irsPractitioner}, or ${ROLES.inactivePractitioner}`, - ); - } - - const userEmail = user.email || user.pendingEmail; - - if (!userEmail) { - return await createUserRecords({ - applicationContext, - user, - userId, - }); - } - - const existingUser = await applicationContext - .getUserGateway() - .getUserByEmail(applicationContext, { - email: userEmail, - }); - - if (!existingUser) { - let params: AdminCreateUserCommandInput = { - DesiredDeliveryMediums: ['EMAIL'], - UserAttributes: [ - { - Name: 'email_verified', - Value: 'True', - }, - { - Name: 'email', - Value: userEmail, - }, - { - Name: 'custom:role', - Value: user.role, - }, - { - Name: 'name', - Value: user.name, - }, - ], - UserPoolId: process.env.USER_POOL_ID, - Username: userEmail, - }; - - if (process.env.STAGE !== 'prod') { - params.TemporaryPassword = process.env.DEFAULT_ACCOUNT_PASS; - } - - const response = await applicationContext - .getCognito() - .adminCreateUser(params); - - if (response?.User?.Username) { - const userIdAttribute = - response.User.Attributes?.find(element => { - if (element.Name === 'custom:userId') { - return element; - } - }) || - response.User.Attributes?.find(element => { - if (element.Name === 'sub') { - return element; - } - }); - userId = userIdAttribute?.Value!; - } - } else { - await applicationContext.getUserGateway().updateUser(applicationContext, { - attributesToUpdate: { - role: user.role, - }, - email: userEmail, - }); - - // eslint-disable-next-line prefer-destructuring - userId = existingUser.userId; - } - - return await createUserRecords({ - applicationContext, - user, - userId, - }); -}; diff --git a/web-api/src/persistence/dynamo/users/createOrUpdateUser.ts b/web-api/src/persistence/dynamo/users/createOrUpdateUser.ts index ce1be563296..0a27ad00feb 100644 --- a/web-api/src/persistence/dynamo/users/createOrUpdateUser.ts +++ b/web-api/src/persistence/dynamo/users/createOrUpdateUser.ts @@ -24,9 +24,6 @@ export const createUserRecords = async ({ } if (user.section) { - // we never have a need to query users by these sections, and since there are SO many - // users in these sections, it locks up the migration. We should use elasticsearch if we - // have a future need to query for users by section if ( user.section !== ROLES.privatePractitioner && user.section !== ROLES.petitioner && @@ -81,8 +78,8 @@ export const createUserRecords = async ({ }); if ( - (user.role === ROLES.privatePractitioner || - user.role === ROLES.irsPractitioner) && + // (user.role === ROLES.privatePractitioner || + // user.role === ROLES.irsPractitioner) && user.name && user.barNumber ) { @@ -94,6 +91,7 @@ export const createUserRecords = async ({ }, applicationContext, }); + const upperCaseBarNumber = user.barNumber.toUpperCase(); await client.put({ Item: {