diff --git a/shared/src/test/mockUsers.ts b/shared/src/test/mockUsers.ts index 6a0e9512ba1..960bebf95ae 100644 --- a/shared/src/test/mockUsers.ts +++ b/shared/src/test/mockUsers.ts @@ -13,7 +13,10 @@ import { import { RawIrsPractitioner } from '@shared/business/entities/IrsPractitioner'; import { RawPractitioner } from '@shared/business/entities/Practitioner'; import { RawUser } from '@shared/business/entities/User'; -import { getJudgesChambers } from '../../../web-client/src/business/chambers/getJudgesChambers'; +import { + getJudgesChambers, + getJudgesChambersWithLegacy, +} from '../../../web-client/src/business/chambers/getJudgesChambers'; export const adcUser = { name: 'ADC', @@ -79,6 +82,14 @@ export const judgeUser: RawUser = { userId: '43b00e5f-b78c-476c-820e-5d6ed1d58828', }; +export const legacyJudgeUser: RawUser = { + entityName: 'User', + name: 'Legacy Judge Ginsburg', + role: ROLES.legacyJudge, + section: getJudgesChambersWithLegacy().LEGACY_JUDGES_CHAMBERS_SECTION.section, + userId: 'dc67e189-cf3e-4ca3-a33f-91db111ec270', +}; + export const judgeColvin: RawUser = { email: 'judgeColvin@example.com', entityName: 'User', @@ -115,7 +126,7 @@ export const trialClerkUser: RawUser = { }; export const caseServicesSupervisorUser = { - name: 'CaseServicesSupervisor', + name: 'Test Case Services Supervisor', role: ROLES.caseServicesSupervisor, section: CASE_SERVICES_SUPERVISOR_SECTION, userId: '4562df8a-5c98-49a0-9c53-d8e4ff3b76bb', @@ -137,7 +148,9 @@ export const docketClerk1User: RawUser = { userId: 'b7d90c05-f6cd-442c-a168-202db587f16f', }; -export const petitionsClerkUser = { +export const petitionsClerkUser: RawUser = { + email: 'petitionsclerk1@example.com', + entityName: 'User', name: 'Petitionsclerk1', role: ROLES.petitionsClerk, section: PETITIONS_SECTION, diff --git a/web-api/src/persistence/dynamo/users/createOrUpdateUser.test.ts b/web-api/src/persistence/dynamo/users/createOrUpdateUser.test.ts index 4ee3e2094a6..f713a24ab48 100644 --- a/web-api/src/persistence/dynamo/users/createOrUpdateUser.test.ts +++ b/web-api/src/persistence/dynamo/users/createOrUpdateUser.test.ts @@ -1,216 +1,164 @@ -import { - PETITIONS_SECTION, - 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 { + caseServicesSupervisorUser, + judgeColvin, + legacyJudgeUser, + petitionsClerkUser, + privatePractitionerUser, +} from '@shared/test/mockUsers'; import { createOrUpdateUser, createUserRecords } from './createOrUpdateUser'; -const JUDGES_CHAMBERS_WITH_LEGACY = applicationContext - .getPersistenceGateway() - .getJudgesChambersWithLegacy(); - describe('createOrUpdateUser', () => { - const userId = '9b52c605-edba-41d7-b045-d5f992a499d3'; - const petitionsClerkUser = { - email: 'test@example.com', - name: 'Test Petitionsclerk', - password: 'tempPass', - role: ROLES.petitionsClerk, - section: PETITIONS_SECTION, - }; - const privatePractitionerUser = { - barNumber: 'pt1234', //intentionally lower case - should be converted to upper case when persisted - name: 'Test Private Practitioner', - role: ROLES.privatePractitioner, - section: 'privatePractitioner', - }; - const privatePractitionerUserWithoutBarNumber = { - barNumber: '', - name: 'Test Private Practitioner', - role: ROLES.privatePractitioner, - section: 'privatePractitioner', - }; - const caseServicesSupervisorUser = { - name: 'Test Case Services Supervisor', - role: ROLES.caseServicesSupervisor, - section: 'caseServicesSupervisor', - }; - - beforeAll(() => { - applicationContext.getCognito().adminGetUser.mockReturnValue({ - promise: () => - Promise.resolve({ - Username: '562d6260-aa9b-4010-af99-536d3872c752', - }), - }); - - applicationContext.getCognito().adminCreateUser.mockReturnValue({ - promise: () => - Promise.resolve({ - User: { Username: '562d6260-aa9b-4010-af99-536d3872c752' }, - }), - }); + const mockTemporaryPassword = 'tempPass'; - applicationContext.getCognito().adminUpdateUserAttributes.mockReturnValue({ - promise: () => Promise.resolve(), - }); - - applicationContext.getCognito().adminDisableUser.mockReturnValue({ - promise: () => Promise.resolve(), - }); - - applicationContext.getDocumentClient().put.mockResolvedValue(null); - }); - - it('should create a user only if the user does not already exist', async () => { - applicationContext.getCognito().adminGetUser.mockReturnValue({ - promise: () => { - const error = new Error(); - (error as any).code = 'UserNotFoundException'; - return Promise.reject(error); - }, + it('should ONLY create a user when they do not already exist', async () => { + applicationContext + .getCognito() + .adminGetUser.mockRejectedValue( + new UserNotFoundException({ $metadata: {}, message: '' }), + ); + applicationContext.getCognito().adminCreateUser.mockResolvedValue({ + User: { Username: petitionsClerkUser.userId }, }); await createOrUpdateUser({ applicationContext, disableCognitoUser: false, - password: petitionsClerkUser.password, - user: petitionsClerkUser as any, + password: mockTemporaryPassword, + user: petitionsClerkUser, }); - expect( - applicationContext.getCognito().adminDisableUser, - ).not.toHaveBeenCalled(); - expect( - applicationContext.getCognito().adminUpdateUserAttributes, - ).not.toHaveBeenCalled(); - }); - - it('should create a user and cognito record, but disable the cognito user', async () => { - applicationContext.getCognito().adminGetUser.mockReturnValue({ - promise: () => { - const error = new Error(); - (error as any).code = 'UserNotFoundException'; - return Promise.reject(error); - }, - }); - - await createOrUpdateUser({ - applicationContext, - disableCognitoUser: true, - password: petitionsClerkUser.password, - user: petitionsClerkUser as any, - }); - - expect(applicationContext.getCognito().adminCreateUser).toHaveBeenCalled(); - expect(applicationContext.getCognito().adminDisableUser).toHaveBeenCalled(); - expect(applicationContext.getCognito().adminGetUser).toHaveBeenCalled(); - expect( - applicationContext.getCognito().adminUpdateUserAttributes, - ).not.toHaveBeenCalled(); - }); - - it('should call adminCreateUser with the correct UserAttributes', async () => { - await createOrUpdateUser({ - applicationContext, - disableCognitoUser: false, - password: petitionsClerkUser.password, - user: petitionsClerkUser as any, - }); expect( applicationContext.getCognito().adminCreateUser, ).toHaveBeenCalledWith({ + DesiredDeliveryMediums: ['EMAIL'], MessageAction: 'SUPPRESS', + TemporaryPassword: mockTemporaryPassword, UserAttributes: [ { Name: 'email_verified', Value: 'True', }, + { Name: 'email', - Value: 'test@example.com', + Value: petitionsClerkUser.email, }, { Name: 'custom:role', - Value: 'petitionsclerk', + Value: petitionsClerkUser.role, }, { Name: 'name', - Value: 'Test Petitionsclerk', + Value: petitionsClerkUser.name, }, ], UserPoolId: undefined, - Username: 'test@example.com', + Username: petitionsClerkUser.email, }); + expect( + applicationContext.getCognito().adminDisableUser, + ).not.toHaveBeenCalled(); + expect( + applicationContext.getCognito().adminUpdateUserAttributes, + ).not.toHaveBeenCalled(); }); - it('should attempt to update the user if the user already exists', async () => { - applicationContext.getCognito().adminGetUser.mockReturnValue({ - promise: () => - Promise.resolve({ - Username: '562d6260-aa9b-4010-af99-536d3872c752', - }), + it('should create a user and cognito record, but disable the cognito user', async () => { + applicationContext + .getCognito() + .adminGetUser.mockRejectedValue( + new UserNotFoundException({ $metadata: {}, message: '' }), + ); + applicationContext.getCognito().adminCreateUser.mockResolvedValue({ + User: { Username: petitionsClerkUser.userId }, }); await createOrUpdateUser({ applicationContext, - disableCognitoUser: false, - password: petitionsClerkUser.password, - user: petitionsClerkUser as any, + disableCognitoUser: true, + password: mockTemporaryPassword, + user: petitionsClerkUser, }); - expect( - applicationContext.getCognito().adminCreateUser, - ).not.toHaveBeenCalled(); + expect(applicationContext.getCognito().adminCreateUser).toHaveBeenCalled(); + expect(applicationContext.getCognito().adminDisableUser).toHaveBeenCalled(); expect(applicationContext.getCognito().adminGetUser).toHaveBeenCalled(); expect( applicationContext.getCognito().adminUpdateUserAttributes, - ).toHaveBeenCalled(); + ).not.toHaveBeenCalled(); }); - it('attempts to persist a private practitioner user with name and barNumber mapping records', async () => { - await createUserRecords({ - applicationContext, - user: privatePractitionerUser, - userId, + it('should attempt to update the user when the user already exists', async () => { + applicationContext.getCognito().adminGetUser.mockResolvedValue({ + Username: petitionsClerkUser.userId, }); - expect( - applicationContext.getDocumentClient().put.mock.calls[0][0], - ).toMatchObject({ - Item: { - ...privatePractitionerUser, - pk: `user|${userId}`, - sk: `user|${userId}`, - }, + await createOrUpdateUser({ + applicationContext, + disableCognitoUser: false, + password: mockTemporaryPassword, + user: petitionsClerkUser, }); + + expect(applicationContext.getCognito().adminGetUser).toHaveBeenCalled(); expect( - applicationContext.getDocumentClient().put.mock.calls[1][0], - ).toMatchObject({ - Item: { - pk: 'privatePractitioner|TEST PRIVATE PRACTITIONER', - sk: `user|${userId}`, - }, - }); + applicationContext.getCognito().adminCreateUser, + ).not.toHaveBeenCalled(); expect( - applicationContext.getDocumentClient().put.mock.calls[2][0], - ).toMatchObject({ - Item: { - pk: 'privatePractitioner|PT1234', - sk: `user|${userId}`, - }, - }); + applicationContext.getCognito().adminUpdateUserAttributes, + ).toHaveBeenCalled(); }); describe('createUserRecords', () => { - it('attempts to persist a petitionsclerk user with a section mapping record', async () => { + const mockPrivatePractitionerUser = { + ...privatePractitionerUser, + barNumber: 'pt1234', //intentionally lower case - should be converted to upper case when persisted + }; + + it('should persist a private practitioner user with name and barNumber mapping records', async () => { + await createUserRecords({ + applicationContext, + user: mockPrivatePractitionerUser, + userId: mockPrivatePractitionerUser.userId, + }); + + expect( + applicationContext.getDocumentClient().put.mock.calls[0][0], + ).toMatchObject({ + Item: { + ...mockPrivatePractitionerUser, + pk: `user|${mockPrivatePractitionerUser.userId}`, + sk: `user|${mockPrivatePractitionerUser.userId}`, + }, + }); + expect( + applicationContext.getDocumentClient().put.mock.calls[1][0], + ).toMatchObject({ + Item: { + pk: 'privatePractitioner|PRIVATE PRACTITIONER', + sk: `user|${mockPrivatePractitionerUser.userId}`, + }, + }); + expect( + applicationContext.getDocumentClient().put.mock.calls[2][0], + ).toMatchObject({ + Item: { + pk: 'privatePractitioner|PT1234', + sk: `user|${mockPrivatePractitionerUser.userId}`, + }, + }); + }); + + it('should persist a petitions clerk user with a section mapping record', async () => { await createUserRecords({ applicationContext, user: petitionsClerkUser, - userId, + userId: petitionsClerkUser.userId, }); - expect(applicationContext.getDocumentClient().put.mock.calls.length).toBe( + expect(applicationContext.getDocumentClient().put).toHaveBeenCalledTimes( 2, ); expect( @@ -218,7 +166,7 @@ describe('createOrUpdateUser', () => { ).toMatchObject({ Item: { pk: 'section|petitions', - sk: `user|${userId}`, + sk: `user|${petitionsClerkUser.userId}`, }, }); expect( @@ -226,33 +174,28 @@ describe('createOrUpdateUser', () => { ).toMatchObject({ Item: { ...petitionsClerkUser, - pk: `user|${userId}`, - sk: `user|${userId}`, + pk: `user|${petitionsClerkUser.userId}`, + sk: `user|${petitionsClerkUser.userId}`, }, }); }); - it('attempts to persist a judge user with a section mapping record for the chambers and the judge', async () => { - const judgeUser = { - name: 'Judge Adam', - role: ROLES.judge, - section: 'adamsChambers', - }; - + it('should persist a judge user with a section mapping record for the chambers and the judge', async () => { await createUserRecords({ applicationContext, - user: judgeUser, - userId, + user: judgeColvin, + userId: judgeColvin.userId, }); - expect(applicationContext.getDocumentClient().put.mock.calls.length).toBe( + + expect(applicationContext.getDocumentClient().put).toHaveBeenCalledTimes( 3, ); expect( applicationContext.getDocumentClient().put.mock.calls[0][0], ).toMatchObject({ Item: { - pk: 'section|adamsChambers', - sk: `user|${userId}`, + pk: `section|${judgeColvin.section}`, + sk: `user|${judgeColvin.userId}`, }, }); expect( @@ -260,33 +203,27 @@ describe('createOrUpdateUser', () => { ).toMatchObject({ Item: { pk: 'section|judge', - sk: `user|${userId}`, + sk: `user|${judgeColvin.userId}`, }, }); expect( applicationContext.getDocumentClient().put.mock.calls[2][0], ).toMatchObject({ Item: { - ...judgeUser, - pk: `user|${userId}`, - sk: `user|${userId}`, + ...judgeColvin, + pk: `user|${judgeColvin.userId}`, + sk: `user|${judgeColvin.userId}`, }, }); }); - it('attempts to persist a legacy judge user with a section mapping record for the chambers and the judge', async () => { - const judgeUser = { - name: 'Legacy Judge Ginsburg', - role: ROLES.legacyJudge, - section: - JUDGES_CHAMBERS_WITH_LEGACY.LEGACY_JUDGES_CHAMBERS_SECTION.section, - }; - + it('should persist a legacy judge user with a section mapping record for the chambers and the judge', async () => { await createUserRecords({ applicationContext, - user: judgeUser, - userId, + user: legacyJudgeUser, + userId: legacyJudgeUser.userId, }); + expect(applicationContext.getDocumentClient().put.mock.calls.length).toBe( 3, ); @@ -294,8 +231,8 @@ describe('createOrUpdateUser', () => { applicationContext.getDocumentClient().put.mock.calls[0][0], ).toMatchObject({ Item: { - pk: `section|${JUDGES_CHAMBERS_WITH_LEGACY.LEGACY_JUDGES_CHAMBERS_SECTION.section}`, - sk: `user|${userId}`, + pk: `section|${legacyJudgeUser.section}`, + sk: `user|${legacyJudgeUser.userId}`, }, }); expect( @@ -303,25 +240,30 @@ describe('createOrUpdateUser', () => { ).toMatchObject({ Item: { pk: 'section|judge', - sk: `user|${userId}`, + sk: `user|${legacyJudgeUser.userId}`, }, }); expect( applicationContext.getDocumentClient().put.mock.calls[2][0], ).toMatchObject({ Item: { - ...judgeUser, - pk: `user|${userId}`, - sk: `user|${userId}`, + ...legacyJudgeUser, + pk: `user|${legacyJudgeUser.userId}`, + sk: `user|${legacyJudgeUser.userId}`, }, }); }); - it('does not persist mapping records for practitioner without barNumber', async () => { + it('should NOT persist mapping records for practitioner that does not have a barNumber', async () => { + const privatePractitionerUserWithoutBarNumber = { + ...privatePractitionerUser, + barNumber: undefined, + }; + await createUserRecords({ applicationContext, user: privatePractitionerUserWithoutBarNumber, - userId, + userId: privatePractitionerUserWithoutBarNumber.userId, }); expect(applicationContext.getDocumentClient().put.mock.calls.length).toBe( @@ -332,17 +274,17 @@ describe('createOrUpdateUser', () => { ).toMatchObject({ Item: { ...privatePractitionerUserWithoutBarNumber, - pk: `user|${userId}`, - sk: `user|${userId}`, + pk: `user|${privatePractitionerUserWithoutBarNumber.userId}`, + sk: `user|${privatePractitionerUserWithoutBarNumber.userId}`, }, }); }); - it('attempts to persist a private practitioner user with name and barNumber mapping records', async () => { + it('should persist a private practitioner user with name and barNumber mapping records', async () => { await createUserRecords({ applicationContext, - user: privatePractitionerUser, - userId, + user: mockPrivatePractitionerUser, + userId: mockPrivatePractitionerUser.userId, }); expect(applicationContext.getDocumentClient().put.mock.calls.length).toBe( @@ -352,17 +294,17 @@ describe('createOrUpdateUser', () => { applicationContext.getDocumentClient().put.mock.calls[0][0], ).toMatchObject({ Item: { - ...privatePractitionerUser, - pk: `user|${userId}`, - sk: `user|${userId}`, + ...mockPrivatePractitionerUser, + pk: `user|${mockPrivatePractitionerUser.userId}`, + sk: `user|${mockPrivatePractitionerUser.userId}`, }, }); expect( applicationContext.getDocumentClient().put.mock.calls[1][0], ).toMatchObject({ Item: { - pk: 'privatePractitioner|TEST PRIVATE PRACTITIONER', - sk: `user|${userId}`, + pk: 'privatePractitioner|PRIVATE PRACTITIONER', + sk: `user|${mockPrivatePractitionerUser.userId}`, }, }); expect( @@ -370,7 +312,7 @@ describe('createOrUpdateUser', () => { ).toMatchObject({ Item: { pk: 'privatePractitioner|PT1234', - sk: `user|${userId}`, + sk: `user|${mockPrivatePractitionerUser.userId}`, }, }); }); @@ -379,7 +321,7 @@ describe('createOrUpdateUser', () => { await createUserRecords({ applicationContext, user: caseServicesSupervisorUser, - userId, + userId: caseServicesSupervisorUser.userId, }); expect( @@ -387,7 +329,7 @@ describe('createOrUpdateUser', () => { ).toMatchObject({ Item: { pk: `section|${caseServicesSupervisorUser.section}`, - sk: `user|${userId}`, + sk: `user|${caseServicesSupervisorUser.userId}`, }, }); expect( @@ -395,7 +337,7 @@ describe('createOrUpdateUser', () => { ).toMatchObject({ Item: { pk: 'section|docket', - sk: `user|${userId}`, + sk: `user|${caseServicesSupervisorUser.userId}`, }, }); expect( @@ -403,41 +345,40 @@ describe('createOrUpdateUser', () => { ).toMatchObject({ Item: { pk: 'section|petitions', - sk: `user|${userId}`, + sk: `user|${caseServicesSupervisorUser.userId}`, }, }); expect( applicationContext.getDocumentClient().put.mock.calls[3][0], ).toMatchObject({ Item: { - pk: `user|${userId}`, - sk: `user|${userId}`, + pk: `user|${caseServicesSupervisorUser.userId}`, + sk: `user|${caseServicesSupervisorUser.userId}`, }, }); }); - it('does not persist section mapping record if user does not have a section', async () => { - const privatePractitionerUserWithoutSection = { - name: 'Test Private Practitioner', - role: ROLES.privatePractitioner, - }; - + it('should NOT persist section mapping record when the user does not have a section', async () => { await createUserRecords({ applicationContext, - user: privatePractitionerUserWithoutSection, - userId, + user: { + ...petitionsClerkUser, + section: undefined, + }, + userId: petitionsClerkUser.userId, }); - expect(applicationContext.getDocumentClient().put.mock.calls.length).toBe( + expect(applicationContext.getDocumentClient().put).toHaveBeenCalledTimes( 1, ); expect( applicationContext.getDocumentClient().put.mock.calls[0][0], ).toMatchObject({ Item: { - ...privatePractitionerUserWithoutSection, - pk: `user|${userId}`, - sk: `user|${userId}`, + ...petitionsClerkUser, + pk: `user|${petitionsClerkUser.userId}`, + section: undefined, + sk: `user|${petitionsClerkUser.userId}`, }, }); });