diff --git a/ui/src/app/user/user-update.component.spec.ts b/ui/src/app/user/user-update.component.spec.ts index 30a4f7922..71c705882 100644 --- a/ui/src/app/user/user-update.component.spec.ts +++ b/ui/src/app/user/user-update.component.spec.ts @@ -70,6 +70,7 @@ describe('UserUpdateComponent', () => { accountService.getAccountData.and.returnValue( of({ + id: 'test', activated: true, authorities: ['ROLE_USER'], email: 'email@email.com', @@ -84,6 +85,7 @@ describe('UserUpdateComponent', () => { mfaEnabled: false, }) ) + memberService.find.and.returnValue(of(new Member())) userService.validate.and.returnValue(of(new UserValidation(true, null))) userService.update.and.returnValue(of({})) spyOn(router, 'navigate').and.returnValue(Promise.resolve(true)) @@ -101,7 +103,6 @@ describe('UserUpdateComponent', () => { it('should disable salesforceId dropdown on init for non-admin users', () => { activatedRoute.data = of({ user: { salesforceId: 'test', id: 'id' } as IUser }) accountService.hasAnyAuthority.and.returnValue(false) - memberService.find.and.returnValue(of(new Member())) fixture.detectChanges() expect(component.disableSalesForceIdDD()).toBe(true) @@ -114,15 +115,21 @@ describe('UserUpdateComponent', () => { expect(component.disableSalesForceIdDD()).toBe(false) }) - it('should validate org owners', () => { + it('should validate non-owners', () => { userService.hasOwner.and.returnValue(of(true)) fixture.detectChanges() + component.editForm.patchValue({ salesforceId: '123', mainContact: false }) component.validateOrgOwners() expect(component.hasOwner).toBe(false) expect(component.editForm.get('salesforceId')?.disabled).toBe(false) + }) - component.editForm.patchValue({ mainContact: true }) + it('should validate org owners', () => { + userService.hasOwner.and.returnValue(of(true)) + fixture.detectChanges() + + component.editForm.patchValue({ salesforceId: '123', mainContact: true }) component.validateOrgOwners() expect(component.hasOwner).toBe(true) expect(component.editForm.get('salesforceId')?.disabled).toBe(true) @@ -145,6 +152,94 @@ describe('UserUpdateComponent', () => { expect(userService.validate).toHaveBeenCalled() expect(userService.create).toHaveBeenCalled() expect(userService.update).toHaveBeenCalledTimes(0) + expect(router.navigate).toHaveBeenCalledWith(['/users']) + }) + + it('should create new user as org owner', () => { + component.editForm.patchValue({ + salesforceId: 'sfid', + email: 'test@test.com', + firstName: 'firstName', + lastName: 'lastName', + activated: false, + mainContact: true, + }) + + userService.create.and.returnValue(of(new User())) + + component.save() + + expect(userService.validate).toHaveBeenCalled() + expect(userService.create).toHaveBeenCalled() + expect(userService.update).toHaveBeenCalledTimes(0) + expect(router.navigate).toHaveBeenCalledWith(['/']) + }) + + it('should update existing user', () => { + activatedRoute.data = of({ + user: { + salesforceId: 'test', + id: 'test', + email: 'test@test.com', + firstName: 'hello', + lastName: 'hello', + mainContact: false, + } as IUser, + }) + fixture.detectChanges() + userService.update.and.returnValue(of(new User())) + + component.save() + + expect(userService.validate).toHaveBeenCalled() + expect(userService.update).toHaveBeenCalled() + expect(userService.create).toHaveBeenCalledTimes(0) + expect(router.navigate).toHaveBeenCalledWith(['/users']) + }) + + it('should update user to org owner and redirect to homepage', () => { + activatedRoute.data = of({ + user: { + salesforceId: 'test', + id: 'test', + email: 'test@test.com', + firstName: 'hello', + lastName: 'hello', + mainContact: true, + } as IUser, + }) + fixture.detectChanges() + userService.update.and.returnValue(of(new User())) + + component.save() + + expect(userService.validate).toHaveBeenCalled() + expect(userService.update).toHaveBeenCalled() + expect(userService.create).toHaveBeenCalledTimes(0) + expect(router.navigate).toHaveBeenCalledWith(['/']) + }) + + it('should update user to org owner and redirect to users list', () => { + activatedRoute.data = of({ + user: { + salesforceId: 'test', + id: 'testing', + email: 'test@test.com', + firstName: 'hello', + lastName: 'hello', + mainContact: true, + } as IUser, + }) + fixture.detectChanges() + accountService.hasAnyAuthority.and.returnValue(true) + userService.update.and.returnValue(of(new User())) + + component.save() + + expect(userService.validate).toHaveBeenCalled() + expect(userService.update).toHaveBeenCalled() + expect(userService.create).toHaveBeenCalledTimes(0) + expect(router.navigate).toHaveBeenCalledWith(['/users']) }) it('should send activation email for existing user', () => { @@ -156,8 +251,13 @@ describe('UserUpdateComponent', () => { expect(userService.sendActivate).toHaveBeenCalled() }) - it('should display send activation option for existing user with unactivated email', () => { + it('should display send activation option for existing user with inactive account', () => { component.existentUser = { email: 'test@example.com', activated: false } as IUser expect(component.displaySendActivate()).toBe(true) }) + + it('should not display send activation option for existing user with active account', () => { + component.existentUser = { email: 'test@example.com', activated: true } as IUser + expect(component.displaySendActivate()).toBe(false) + }) }) diff --git a/ui/src/app/user/user-update.component.ts b/ui/src/app/user/user-update.component.ts index cd67943c1..116c72943 100644 --- a/ui/src/app/user/user-update.component.ts +++ b/ui/src/app/user/user-update.component.ts @@ -159,6 +159,10 @@ export class UserUpdateComponent { this.router.navigate(['/users']) } + navigateToHomePage() { + this.router.navigate(['/']) + } + disableSalesForceIdDD() { if (this.hasRoleAdmin()) { return false @@ -211,28 +215,30 @@ export class UserUpdateComponent { save() { if (this.editForm.valid) { this.isSaving = true - const msUser = this.createFromForm() + const userFromForm = this.createFromForm() - this.userService.validate(msUser).subscribe((response) => { + this.userService.validate(userFromForm).subscribe((response) => { const data = response if (data.valid) { - if (msUser.id !== undefined) { - if (this.currentAccount.id === msUser.id) { - if (this.currentAccount.mainContact !== msUser.mainContact) { - this.subscribeToUpdateResponseWithOwnershipChange(this.userService.update(msUser)) + if (userFromForm.id !== undefined) { + if (this.currentAccount.id === userFromForm.id) { + // ownership change functions redirect to homepage instead of redirecting to users list + // as users who lose org owner status shouldn't have access to the users list + if (this.currentAccount.mainContact !== userFromForm.mainContact) { + this.subscribeToUpdateResponseWithOwnershipChange(this.userService.update(userFromForm)) } else { - this.subscribeToUpdateResponse(this.userService.update(msUser)) + this.subscribeToUpdateResponse(this.userService.update(userFromForm)) } - } else if (msUser.mainContact && !this.hasRoleAdmin()) { - this.subscribeToUpdateResponseWithOwnershipChange(this.userService.update(msUser)) + } else if (userFromForm.mainContact && !this.hasRoleAdmin()) { + this.subscribeToUpdateResponseWithOwnershipChange(this.userService.update(userFromForm)) } else { - this.subscribeToUpdateResponse(this.userService.update(msUser)) + this.subscribeToUpdateResponse(this.userService.update(userFromForm)) } } else { - if (msUser.mainContact && !this.hasRoleAdmin()) { - this.subscribeToSaveResponseWithOwnershipChange(this.userService.create(msUser)) + if (userFromForm.mainContact && !this.hasRoleAdmin()) { + this.subscribeToSaveResponseWithOwnershipChange(this.userService.create(userFromForm)) } else { - this.subscribeToSaveResponse(this.userService.create(msUser)) + this.subscribeToSaveResponse(this.userService.create(userFromForm)) } } } else { @@ -287,31 +293,31 @@ export class UserUpdateComponent { } protected subscribeToSaveResponse(result: Observable) { - result.subscribe( - () => this.onSaveSuccess(), - () => this.onSaveError() - ) + result.subscribe({ + next: () => this.onSaveSuccess(), + error: () => this.onSaveError(), + }) } protected subscribeToUpdateResponse(result: Observable) { - result.subscribe( - () => this.onUpdateSuccess(), - () => this.onSaveError() - ) + result.subscribe({ + next: () => this.onUpdateSuccess(), + error: () => this.onSaveError(), + }) } protected subscribeToSaveResponseWithOwnershipChange(result: Observable) { - result.subscribe( - () => this.onSaveSuccessOwnershipChange(), - () => this.onSaveError() - ) + result.subscribe({ + next: () => this.onSaveSuccessOwnershipChange(), + error: () => this.onSaveError(), + }) } protected subscribeToUpdateResponseWithOwnershipChange(result: Observable) { - result.subscribe( - () => this.onUpdateSuccessOwnershipChange(), - () => this.onSaveError() - ) + result.subscribe({ + next: () => this.onUpdateSuccessOwnershipChange(), + error: () => this.onSaveError(), + }) } protected onSaveSuccess() { @@ -329,13 +335,13 @@ export class UserUpdateComponent { protected onSaveSuccessOwnershipChange() { this.isSaving = false // TODO: confirm this actually works, previously it was set to SERVER_API_URL - window.location.href = '/' + this.navigateToHomePage() this.alertService.broadcast('userServiceApp.user.created.string') } protected onUpdateSuccessOwnershipChange() { this.isSaving = false - window.location.href = '/' + this.navigateToHomePage() this.alertService.broadcast('userServiceApp.user.updated.string') }