From 3f549fdd5a13d05a229dd2d8a415032e42e267e9 Mon Sep 17 00:00:00 2001 From: andrej romanov <50377758+auumgn@users.noreply.github.com> Date: Tue, 6 Feb 2024 22:53:53 +0200 Subject: [PATCH 1/3] improve tests --- ui/src/app/user/user-update.component.spec.ts | 238 ++++++++++-------- 1 file changed, 128 insertions(+), 110 deletions(-) diff --git a/ui/src/app/user/user-update.component.spec.ts b/ui/src/app/user/user-update.component.spec.ts index 41e8af1e3..30a4f7922 100644 --- a/ui/src/app/user/user-update.component.spec.ts +++ b/ui/src/app/user/user-update.component.spec.ts @@ -1,24 +1,31 @@ -import { ComponentFixture, TestBed, fakeAsync, tick } from '@angular/core/testing'; -import { FormBuilder, ReactiveFormsModule } from '@angular/forms'; -import { ActivatedRoute, Router } from '@angular/router'; -import { of } from 'rxjs'; -import { UserUpdateComponent } from './user-update.component'; -import { UserService } from './service/user.service'; -import { AccountService } from '../account'; -import { MemberService } from '../member/service/member.service'; -import { AlertService } from '../shared/service/alert.service'; -import { ErrorService } from '../error/service/error.service'; -import { IUser, User } from './model/user.model'; -import { Member } from '../member/model/member.model'; -import { UserValidation } from './model/user-validation.model'; +import { ComponentFixture, TestBed, fakeAsync, tick } from '@angular/core/testing' +import { FormBuilder, ReactiveFormsModule } from '@angular/forms' +import { ActivatedRoute, Router } from '@angular/router' +import { BehaviorSubject, Observable, of } from 'rxjs' +import { UserUpdateComponent } from './user-update.component' +import { UserService } from './service/user.service' +import { AccountService } from '../account' +import { MemberService } from '../member/service/member.service' +import { AlertService } from '../shared/service/alert.service' +import { ErrorService } from '../error/service/error.service' +import { IUser, User } from './model/user.model' +import { Member } from '../member/model/member.model' +import { UserValidation } from './model/user-validation.model' +import { RouterTestingModule } from '@angular/router/testing' +import { FontAwesomeModule } from '@fortawesome/angular-fontawesome' +import { CUSTOM_ELEMENTS_SCHEMA, NO_ERRORS_SCHEMA } from '@angular/core' +import { AppComponent } from '../app.component' +import { NgbPagination } from '@ng-bootstrap/ng-bootstrap' describe('UserUpdateComponent', () => { - let component: UserUpdateComponent; - let fixture: ComponentFixture; - let userService: jasmine.SpyObj; - let accountService: jasmine.SpyObj; - let alertService: jasmine.SpyObj; - let memberService: jasmine.SpyObj; + let component: UserUpdateComponent + let fixture: ComponentFixture + let userService: jasmine.SpyObj + let accountService: jasmine.SpyObj + let alertService: jasmine.SpyObj + let memberService: jasmine.SpyObj + let router: jasmine.SpyObj + let activatedRoute: jasmine.SpyObj beforeEach(() => { const userServiceSpy = jasmine.createSpyObj('UserService', [ @@ -27,119 +34,130 @@ describe('UserUpdateComponent', () => { 'sendActivate', 'hasOwner', 'create', - 'update' - ]); + 'update', + ]) const accountServiceSpy = jasmine.createSpyObj('AccountService', [ 'getAccountData', 'hasAnyAuthority', - 'getSalesforceId' - ]); - const alertServiceSpy = jasmine.createSpyObj('AlertService', ['broadcast']); - const memberServiceSpy = jasmine.createSpyObj('MemberService', [ - 'find', - ]); + 'getSalesforceId', + ]) + const alertServiceSpy = jasmine.createSpyObj('AlertService', ['broadcast']) + const memberServiceSpy = jasmine.createSpyObj('MemberService', ['find']) + const routerSpy = jasmine.createSpyObj('Router', ['navigate']) TestBed.configureTestingModule({ declarations: [UserUpdateComponent], - imports: [ReactiveFormsModule], + imports: [ReactiveFormsModule, RouterTestingModule.withRoutes([]), FontAwesomeModule], providers: [ FormBuilder, - { provide: ActivatedRoute, useValue: { data: of({ user: {salesforceId: 'test'} as IUser }) } }, - // rewrite in the same way as other services - // eslint-disable-next-line - { provide: Router, useValue: { navigate: () => {} } }, { provide: UserService, useValue: userServiceSpy }, { provide: AccountService, useValue: accountServiceSpy }, { provide: MemberService, useValue: memberServiceSpy }, { provide: AlertService, useValue: alertServiceSpy }, { provide: ErrorService, useValue: {} }, - ] - }).compileComponents(); - - fixture = TestBed.createComponent(UserUpdateComponent); - component = fixture.componentInstance; - userService = TestBed.inject(UserService) as jasmine.SpyObj; - accountService = TestBed.inject(AccountService) as jasmine.SpyObj; - alertService = TestBed.inject(AlertService) as jasmine.SpyObj; - memberService = TestBed.inject(MemberService) as jasmine.SpyObj; - - accountService.getAccountData.and.returnValue(of({ - activated: true, - authorities: ['ROLE_USER'], - email: 'email@email.com', - firstName: 'name', - langKey: 'en', - lastName: 'surname', - imageUrl: 'url', - salesforceId: 'sfid', - loggedAs: false, - loginAs: 'sfid', - mainContact: false, - mfaEnabled: false, - })); - userService.hasOwner.and.returnValue(of(true)); - userService.validate.and.returnValue(of(new UserValidation(true, null))); - userService.update.and.returnValue(of({})); - userService.sendActivate.and.returnValue(of(new User())); - fixture.detectChanges(); - }); + ], + schemas: [NO_ERRORS_SCHEMA], + }).compileComponents() + + fixture = TestBed.createComponent(UserUpdateComponent) + component = fixture.componentInstance + userService = TestBed.inject(UserService) as jasmine.SpyObj + accountService = TestBed.inject(AccountService) as jasmine.SpyObj + alertService = TestBed.inject(AlertService) as jasmine.SpyObj + memberService = TestBed.inject(MemberService) as jasmine.SpyObj + activatedRoute = TestBed.inject(ActivatedRoute) as jasmine.SpyObj + router = TestBed.inject(Router) as jasmine.SpyObj + + accountService.getAccountData.and.returnValue( + of({ + activated: true, + authorities: ['ROLE_USER'], + email: 'email@email.com', + firstName: 'name', + langKey: 'en', + lastName: 'surname', + imageUrl: 'url', + salesforceId: 'sfid', + loggedAs: false, + loginAs: 'sfid', + mainContact: false, + mfaEnabled: false, + }) + ) + userService.validate.and.returnValue(of(new UserValidation(true, null))) + userService.update.and.returnValue(of({})) + spyOn(router, 'navigate').and.returnValue(Promise.resolve(true)) + }) it('should create', () => { - expect(component).toBeTruthy(); - }); + expect(component).toBeTruthy() + }) it('should navigate to users list', () => { - const router = TestBed.inject(Router); - const navigateSpy = spyOn(router, 'navigate'); - component.navigateToUsersList(); - expect(navigateSpy).toHaveBeenCalledWith(['/users']); - }); - - it('should disable salesforceId dropdown for non-admin users', fakeAsync(() => { - accountService.hasAnyAuthority.and.returnValue(false); + component.navigateToUsersList() + expect(router.navigate).toHaveBeenCalledWith(['/users']) + }) + + 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())) - component.isExistentMember = true; + fixture.detectChanges() - tick(); - fixture.detectChanges(); - expect(component.disableSalesForceIdDD()).toBe(true); - })); + expect(component.disableSalesForceIdDD()).toBe(true) + }) it('should enable salesforceId dropdown for admin users', () => { - accountService.hasAnyAuthority.and.returnValue(true); - expect(component.disableSalesForceIdDD()).toBe(false); - }); + accountService.hasAnyAuthority.and.returnValue(true) + fixture.detectChanges() + + expect(component.disableSalesForceIdDD()).toBe(false) + }) it('should validate org owners', () => { - component.editForm.patchValue({ salesforceId: '123', mainContact: true }); - component.validateOrgOwners(); - expect(component.hasOwner).toBe(true); - expect(component.editForm.get('salesforceId')?.disabled).toBe(true); - }); - -/* fit('should create new user', fakeAsync(() => { - component.isExistentMember = false; - - component.editForm.patchValue({salesforceId: 'sfid', email: "test@test.com", firstName: "firstName", lastName: "lastName", activated: false, }) - console.log(component.editForm.value); - - userService.create.and.returnValue(of(new User())) - component.save(); - tick(); - expect(userService.validate).toHaveBeenCalled(); - expect(userService.create).toHaveBeenCalled(); - })); */ - - it('should send activation email for existing user', fakeAsync(() => { - component.existentUser = { email: 'test@example.com', activated: false } as IUser; - component.sendActivate(); - tick(); - expect(userService.sendActivate).toHaveBeenCalled(); - })); + 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) - it('should display send activation option for existing user with unactivated email', () => { - component.existentUser = { email: 'test@example.com', activated: false } as IUser; - expect(component.displaySendActivate()).toBe(true); - }); + component.editForm.patchValue({ mainContact: true }) + component.validateOrgOwners() + expect(component.hasOwner).toBe(true) + expect(component.editForm.get('salesforceId')?.disabled).toBe(true) + }) -}); + it('should create new user', () => { + component.editForm.patchValue({ + salesforceId: 'sfid', + email: 'test@test.com', + firstName: 'firstName', + lastName: 'lastName', + activated: false, + mainContact: false, + }) + + userService.create.and.returnValue(of(new User())) + + component.save() + + expect(userService.validate).toHaveBeenCalled() + expect(userService.create).toHaveBeenCalled() + expect(userService.update).toHaveBeenCalledTimes(0) + }) + + it('should send activation email for existing user', () => { + activatedRoute.data = of({ user: { salesforceId: 'test', id: 'id' } as IUser }) + userService.sendActivate.and.returnValue(of(new User())) + fixture.detectChanges() + + component.sendActivate() + expect(userService.sendActivate).toHaveBeenCalled() + }) + + it('should display send activation option for existing user with unactivated email', () => { + component.existentUser = { email: 'test@example.com', activated: false } as IUser + expect(component.displaySendActivate()).toBe(true) + }) +}) From 4d293f3b6d28bd96f77fa0dae2d3d71c5ccd8e75 Mon Sep 17 00:00:00 2001 From: andrej romanov <50377758+auumgn@users.noreply.github.com> Date: Tue, 6 Feb 2024 22:54:17 +0200 Subject: [PATCH 2/3] prettier --- ui/src/app/user/user-update.component.ts | 276 ++++++++++++----------- 1 file changed, 141 insertions(+), 135 deletions(-) diff --git a/ui/src/app/user/user-update.component.ts b/ui/src/app/user/user-update.component.ts index 044b3ddcf..cd67943c1 100644 --- a/ui/src/app/user/user-update.component.ts +++ b/ui/src/app/user/user-update.component.ts @@ -1,39 +1,36 @@ -import { ChangeDetectorRef, Component, OnInit } from '@angular/core'; -import { HttpResponse } from '@angular/common/http'; -import { FormBuilder, Validators } from '@angular/forms'; -import { ActivatedRoute, Router } from '@angular/router'; -import { EMPTY, Observable } from 'rxjs'; -import * as moment from 'moment'; -import { faBan, faCheckCircle, faSave } from '@fortawesome/free-solid-svg-icons'; - - -import { map } from 'rxjs/operators'; -import { AlertService } from '../shared/service/alert.service'; -import { UserService } from './service/user.service'; -import { MemberService } from '../member/service/member.service'; -import { AccountService } from '../account'; -import { IUser, User } from './model/user.model'; -import { IMember } from '../member/model/member.model'; -import { ErrorService } from '../error/service/error.service'; -import { DATE_TIME_FORMAT, emailValidator } from '../app.constants'; +import { ChangeDetectorRef, Component, OnInit } from '@angular/core' +import { HttpResponse } from '@angular/common/http' +import { FormBuilder, Validators } from '@angular/forms' +import { ActivatedRoute, Router } from '@angular/router' +import { EMPTY, Observable } from 'rxjs' +import * as moment from 'moment' +import { faBan, faCheckCircle, faSave } from '@fortawesome/free-solid-svg-icons' + +import { map } from 'rxjs/operators' +import { AlertService } from '../shared/service/alert.service' +import { UserService } from './service/user.service' +import { MemberService } from '../member/service/member.service' +import { AccountService } from '../account' +import { IUser, User } from './model/user.model' +import { IMember } from '../member/model/member.model' +import { ErrorService } from '../error/service/error.service' +import { DATE_TIME_FORMAT, emailValidator } from '../app.constants' @Component({ selector: 'app-user-update', templateUrl: './user-update.component.html', - styleUrls: ['./user-update.component.scss'] + styleUrls: ['./user-update.component.scss'], }) export class UserUpdateComponent { - - - isSaving = false; - isExistentMember = false; - existentUser: IUser | null = null; - faCheckCircle = faCheckCircle; - faBan = faBan; - faSave = faSave; - showIsAdminCheckbox = false; - currentAccount: any; - validation: any; + isSaving = false + isExistentMember = false + existentUser: IUser | null = null + faCheckCircle = faCheckCircle + faBan = faBan + faSave = faSave + showIsAdminCheckbox = false + currentAccount: any + validation: any editForm = this.fb.group({ id: [''], @@ -41,18 +38,18 @@ export class UserUpdateComponent { firstName: ['', Validators.required], lastName: ['', Validators.required], mainContact: [false], - assertionServiceEnabled: [], + assertionServiceEnabled: [false], salesforceId: ['', Validators.required], activated: [false], isAdmin: [false], createdBy: [''], createdDate: [''], lastModifiedBy: [''], - lastModifiedDate: [''] - }); + lastModifiedDate: [''], + }) - memberList = [] as IMember[]; - hasOwner = false; + memberList = [] as IMember[] + hasOwner = false constructor( protected alertService: AlertService, @@ -65,46 +62,46 @@ export class UserUpdateComponent { private fb: FormBuilder, private cdref: ChangeDetectorRef ) { - this.validation = {}; + this.validation = {} } ngOnInit() { - this.isSaving = false; - this.isExistentMember = false; - this.existentUser = null; + this.isSaving = false + this.isExistentMember = false + this.existentUser = null this.activatedRoute.data.subscribe(({ user }) => { - this.existentUser = user; - }); - this.editForm.disable(); - this.accountService.getAccountData().subscribe(account => { - this.currentAccount = account; + this.existentUser = user + }) + this.editForm.disable() + this.accountService.getAccountData().subscribe((account) => { + this.currentAccount = account this.getMemberList().subscribe((list: IMember[]) => { list.forEach((msMember: IMember) => { - this.memberList.push(msMember); - }); - this.editForm.enable(); + this.memberList.push(msMember) + }) + this.editForm.enable() if (this.existentUser) { - this.updateForm(this.existentUser); + this.updateForm(this.existentUser) } - }); - }); - this.cdref.detectChanges(); - this.onChanges(); + }) + }) + this.cdref.detectChanges() + this.onChanges() } onChanges(): void { - this.editForm.get('salesforceId')?.valueChanges.subscribe(val => { - const selectedOrg = this.memberList.find(cm => cm.salesforceId === this.editForm.get(['salesforceId'])?.value); + this.editForm.get('salesforceId')?.valueChanges.subscribe((val) => { + const selectedOrg = this.memberList.find((cm) => cm.salesforceId === this.editForm.get(['salesforceId'])?.value) if (this.hasRoleAdmin()) { if (selectedOrg) { - this.showIsAdminCheckbox = selectedOrg.superadminEnabled || false; + this.showIsAdminCheckbox = selectedOrg.superadminEnabled || false } else { - this.showIsAdminCheckbox = false; + this.showIsAdminCheckbox = false } } else { - this.showIsAdminCheckbox = false; + this.showIsAdminCheckbox = false } - }); + }) } updateForm(user: IUser) { @@ -120,158 +117,153 @@ export class UserUpdateComponent { createdBy: user.createdBy, createdDate: user.createdDate != null ? user.createdDate.format(DATE_TIME_FORMAT) : null, lastModifiedBy: user.lastModifiedBy, - lastModifiedDate: user.lastModifiedDate != null ? user.lastModifiedDate.format(DATE_TIME_FORMAT) : null - }); + lastModifiedDate: user.lastModifiedDate != null ? user.lastModifiedDate.format(DATE_TIME_FORMAT) : null, + }) if (user.mainContact) { - this.editForm.get('mainContact')?.disable(); - this.editForm.get('salesforceId')?.disable(); + this.editForm.get('mainContact')?.disable() + this.editForm.get('salesforceId')?.disable() } if (user.salesforceId) { - this.isExistentMember = true; + this.isExistentMember = true } if (user.email) { - this.editForm.get('email')?.disable(); + this.editForm.get('email')?.disable() } } getMemberList(): Observable { if (this.hasRoleAdmin()) { return this.memberService.getAllMembers().pipe( - map(res => { + map((res) => { if (res.body) { - return res.body; + return res.body } return [] }) - ); + ) } else { return this.memberService.find(this.currentAccount.salesforceId).pipe( - map(res => { + map((res) => { if (res) { - return [res]; + return [res] } return [] }) - ); + ) } } navigateToUsersList() { - this.router.navigate(['/users']); + this.router.navigate(['/users']) } disableSalesForceIdDD() { if (this.hasRoleAdmin()) { - return false; + return false } else if (this.hasRoleOrgOwner() || this.hasRoleConsortiumLead()) { this.editForm.patchValue({ - salesforceId: this.getSalesForceId() - }); - return true; + salesforceId: this.getSalesForceId(), + }) + return true } - return this.isExistentMember; + return this.isExistentMember } getSalesForceId() { - return this.accountService.getSalesforceId(); + return this.accountService.getSalesforceId() } hasRoleAdmin() { - return this.accountService.hasAnyAuthority(['ROLE_ADMIN']); + return this.accountService.hasAnyAuthority(['ROLE_ADMIN']) } hasRoleOrgOwner() { - return this.accountService.hasAnyAuthority(['ROLE_ORG_OWNER']); + return this.accountService.hasAnyAuthority(['ROLE_ORG_OWNER']) } hasRoleConsortiumLead() { - return this.accountService.hasAnyAuthority(['ROLE_CONSORTIUM_LEAD']); + return this.accountService.hasAnyAuthority(['ROLE_CONSORTIUM_LEAD']) } validateOrgOwners() { - this.isSaving = true; + this.isSaving = true const sfId = this.editForm.get('salesforceId')?.value if (sfId) { - this.userService.hasOwner(sfId).subscribe(value => { - this.isSaving = false; + this.userService.hasOwner(sfId).subscribe((value) => { + this.isSaving = false if (!this.editForm.get('mainContact')?.value) { - this.hasOwner = false; + this.hasOwner = false } else { - this.hasOwner = value; + this.hasOwner = value } - }); - + }) + if (this.editForm.get('mainContact')?.value) { - this.editForm.get('salesforceId')?.disable(); + this.editForm.get('salesforceId')?.disable() } else { - this.editForm.get('salesforceId')?.enable(); + this.editForm.get('salesforceId')?.enable() } - } } save() { if (this.editForm.valid) { - this.isSaving = true; - const msUser = this.createFromForm(); + this.isSaving = true + const msUser = this.createFromForm() - this.userService.validate(msUser).subscribe(response => { - const data = response; + this.userService.validate(msUser).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)); + this.subscribeToUpdateResponseWithOwnershipChange(this.userService.update(msUser)) } else { - this.subscribeToUpdateResponse(this.userService.update(msUser)); + this.subscribeToUpdateResponse(this.userService.update(msUser)) } } else if (msUser.mainContact && !this.hasRoleAdmin()) { - this.subscribeToUpdateResponseWithOwnershipChange(this.userService.update(msUser)); + this.subscribeToUpdateResponseWithOwnershipChange(this.userService.update(msUser)) } else { - this.subscribeToUpdateResponse(this.userService.update(msUser)); + this.subscribeToUpdateResponse(this.userService.update(msUser)) } } else { if (msUser.mainContact && !this.hasRoleAdmin()) { - this.subscribeToSaveResponseWithOwnershipChange(this.userService.create(msUser)); + this.subscribeToSaveResponseWithOwnershipChange(this.userService.create(msUser)) } else { - this.subscribeToSaveResponse(this.userService.create(msUser)); + this.subscribeToSaveResponse(this.userService.create(msUser)) } } } else { - this.isSaving = false; - this.validation = data; + this.isSaving = false + this.validation = data } - }); + }) } } sendActivate() { - if (this.existentUser) { - this.userService.sendActivate(this.existentUser).subscribe(res => { + if (this.existentUser?.id) { + this.userService.sendActivate(this.existentUser).subscribe((res) => { if (res) { - this.alertService.broadcast('gatewayApp.msUserServiceMSUser.sendActivate.success.string'); + this.alertService.broadcast('gatewayApp.msUserServiceMSUser.sendActivate.success.string') } else { - this.alertService.broadcast('gatewayApp.msUserServiceMSUser.sendActivate.error.string'); + this.alertService.broadcast('gatewayApp.msUserServiceMSUser.sendActivate.error.string') } - this.navigateToUsersList(); - }); + this.navigateToUsersList() + }) } } displaySendActivate() { if (this.existentUser && this.existentUser.email && !this.existentUser.activated) { - console.log('this.existentMSUser: ', this.existentUser); - console.log('this.existentMSUser.activated', this.existentUser.activated); - return true; + return true } - return false; + return false } private createFromForm(): IUser { - return { ...new User(), id: this.editForm.get(['id'])?.value !== '' ? this.editForm.get(['id'])?.value : undefined, @@ -283,57 +275,71 @@ export class UserUpdateComponent { salesforceId: this.editForm.get(['salesforceId'])?.value, createdBy: this.editForm.get(['createdBy'])?.value, createdDate: - this.editForm.get(['createdDate'])?.value != null ? moment(this.editForm.get(['createdDate'])?.value, DATE_TIME_FORMAT) : undefined, + this.editForm.get(['createdDate'])?.value != null + ? moment(this.editForm.get(['createdDate'])?.value, DATE_TIME_FORMAT) + : undefined, lastModifiedBy: this.editForm.get(['lastModifiedBy'])?.value, lastModifiedDate: this.editForm.get(['lastModifiedDate'])?.value != null ? moment(this.editForm.get(['lastModifiedDate'])?.value, DATE_TIME_FORMAT) - : undefined - }; + : undefined, + } } protected subscribeToSaveResponse(result: Observable) { - result.subscribe(() => this.onSaveSuccess(), () => this.onSaveError()); + result.subscribe( + () => this.onSaveSuccess(), + () => this.onSaveError() + ) } protected subscribeToUpdateResponse(result: Observable) { - result.subscribe(() => this.onUpdateSuccess(), () => this.onSaveError()); + result.subscribe( + () => this.onUpdateSuccess(), + () => this.onSaveError() + ) } protected subscribeToSaveResponseWithOwnershipChange(result: Observable) { - result.subscribe(() => this.onSaveSuccessOwnershipChange(), () => this.onSaveError()); + result.subscribe( + () => this.onSaveSuccessOwnershipChange(), + () => this.onSaveError() + ) } protected subscribeToUpdateResponseWithOwnershipChange(result: Observable) { - result.subscribe(() => this.onUpdateSuccessOwnershipChange(), () => this.onSaveError()); + result.subscribe( + () => this.onUpdateSuccessOwnershipChange(), + () => this.onSaveError() + ) } protected onSaveSuccess() { - this.isSaving = false; - this.navigateToUsersList(); - this.alertService.broadcast('userServiceApp.user.created.string'); + this.isSaving = false + this.navigateToUsersList() + this.alertService.broadcast('userServiceApp.user.created.string') } protected onUpdateSuccess() { - this.isSaving = false; - this.navigateToUsersList(); - this.alertService.broadcast('userServiceApp.user.updated.string'); + this.isSaving = false + this.navigateToUsersList() + this.alertService.broadcast('userServiceApp.user.updated.string') } protected onSaveSuccessOwnershipChange() { - this.isSaving = false; + this.isSaving = false // TODO: confirm this actually works, previously it was set to SERVER_API_URL - window.location.href = '/'; - this.alertService.broadcast('userServiceApp.user.created.string'); + window.location.href = '/' + this.alertService.broadcast('userServiceApp.user.created.string') } protected onUpdateSuccessOwnershipChange() { - this.isSaving = false; - window.location.href = '/'; - this.alertService.broadcast('userServiceApp.user.updated.string'); + this.isSaving = false + window.location.href = '/' + this.alertService.broadcast('userServiceApp.user.updated.string') } protected onSaveError() { - this.isSaving = false; + this.isSaving = false } } From 0f689708824bffd531850dc372e78f8b5c1001c0 Mon Sep 17 00:00:00 2001 From: andrej romanov <50377758+auumgn@users.noreply.github.com> Date: Wed, 7 Feb 2024 18:29:24 +0200 Subject: [PATCH 3/3] add tests --- ui/src/app/user/user-update.component.spec.ts | 108 +++++++++++++++++- ui/src/app/user/user-update.component.ts | 68 ++++++----- 2 files changed, 141 insertions(+), 35 deletions(-) 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') }