diff --git a/modules/ui/src/app/app-routing.module.ts b/modules/ui/src/app/app-routing.module.ts index a3c05b296..12510a13f 100644 --- a/modules/ui/src/app/app-routing.module.ts +++ b/modules/ui/src/app/app-routing.module.ts @@ -65,6 +65,7 @@ export const routes: Routes = [ { path: 'risk-assessment', component: RiskAssessmentComponent, + canDeactivate: [CanDeactivateGuard], title: 'Testrun - Risk Assessment', }, { diff --git a/modules/ui/src/app/mocks/profile.mock.ts b/modules/ui/src/app/mocks/profile.mock.ts index b68d022aa..7e6322bbe 100644 --- a/modules/ui/src/app/mocks/profile.mock.ts +++ b/modules/ui/src/app/mocks/profile.mock.ts @@ -59,7 +59,7 @@ export const PROFILE_MOCK_3: Profile = { export const PROFILE_FORM: ProfileFormat[] = [ { - question: 'Email', + question: 'What is the email of the device owner(s)?', type: FormControlType.EMAIL_MULTIPLE, validation: { required: true, @@ -108,7 +108,10 @@ export const NEW_PROFILE_MOCK = { status: ProfileStatus.VALID, name: 'New profile', questions: [ - { question: 'Email', answer: 'a@test.te;b@test.te, c@test.te' }, + { + question: 'What is the email of the device owner(s)?', + answer: 'a@test.te;b@test.te, c@test.te', + }, { question: 'What type of device do you need reviewed?', answer: 'test', @@ -129,7 +132,7 @@ export const NEW_PROFILE_MOCK_DRAFT = { status: ProfileStatus.DRAFT, name: 'New profile', questions: [ - { question: 'Email', answer: '' }, + { question: 'What is the email of the device owner(s)?', answer: '' }, { question: 'What type of device do you need reviewed?', answer: '', @@ -217,7 +220,7 @@ export const OUTDATED_DRAFT_PROFILE_MOCK: Profile = { }, { question: 'What is the email of the device owner(s)?', - answer: 'boddey@google.com, cmeredith@google.com', + answer: '', }, { question: 'What type of device do you need reviewed?', diff --git a/modules/ui/src/app/pages/risk-assessment/profile-form/profile-form.component.spec.ts b/modules/ui/src/app/pages/risk-assessment/profile-form/profile-form.component.spec.ts index 48e24a425..8691cbcf9 100644 --- a/modules/ui/src/app/pages/risk-assessment/profile-form/profile-form.component.spec.ts +++ b/modules/ui/src/app/pages/risk-assessment/profile-form/profile-form.component.spec.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { ComponentFixture, fakeAsync, TestBed } from '@angular/core/testing'; import { ProfileFormComponent } from './profile-form.component'; import { BrowserAnimationsModule } from '@angular/platform-browser/animations'; @@ -30,15 +30,31 @@ import { RENAME_PROFILE_MOCK, } from '../../../mocks/profile.mock'; import { ProfileStatus } from '../../../model/profile'; +import { RiskAssessmentStore } from '../risk-assessment.store'; +import { TestRunService } from '../../../services/test-run.service'; +import { provideMockStore } from '@ngrx/store/testing'; +import { of } from 'rxjs'; +import { MatDialogRef } from '@angular/material/dialog'; +import { SimpleDialogComponent } from '../../../components/simple-dialog/simple-dialog.component'; describe('ProfileFormComponent', () => { let component: ProfileFormComponent; let fixture: ComponentFixture; let compiled: HTMLElement; + const testrunServiceMock: jasmine.SpyObj = + jasmine.createSpyObj('testrunServiceMock', [ + 'fetchQuestionnaireFormat', + 'saveDevice', + ]); beforeEach(async () => { await TestBed.configureTestingModule({ imports: [ProfileFormComponent, BrowserAnimationsModule], + providers: [ + RiskAssessmentStore, + { provide: TestRunService, useValue: testrunServiceMock }, + provideMockStore({}), + ], }).compileComponents(); fixture = TestBed.createComponent(ProfileFormComponent); @@ -312,14 +328,15 @@ describe('ProfileFormComponent', () => { ).toBeTrue(); }); - it('should have an error when uses the name of copy profile', () => { + it('should have an error when uses the name of copy profile', fakeAsync(() => { component.selectedProfile = DRAFT_COPY_PROFILE_MOCK; component.profiles = [PROFILE_MOCK, PROFILE_MOCK_2, COPY_PROFILE_MOCK]; + fixture.detectChanges(); expect( component.nameControl.hasError('has_same_profile_name') ).toBeTrue(); - }); + })); }); describe('with no profile', () => { @@ -336,6 +353,31 @@ describe('ProfileFormComponent', () => { expect(emitSpy).toHaveBeenCalledWith(NEW_PROFILE_MOCK); }); }); + + describe('openCloseDialog', () => { + it('should open discard modal', fakeAsync(() => { + const openSpy = spyOn(component.dialog, 'open').and.returnValue({ + afterClosed: () => of(true), + } as MatDialogRef); + + component.openCloseDialog(); + + expect(openSpy).toHaveBeenCalledWith(SimpleDialogComponent, { + ariaLabel: 'Discard the Risk Assessment changes', + data: { + title: 'Discard changes?', + content: `You have unsaved changes that would be permanently lost.`, + confirmName: 'Discard', + }, + autoFocus: true, + hasBackdrop: true, + disableClose: true, + panelClass: ['simple-dialog', 'discard-dialog'], + }); + + openSpy.calls.reset(); + })); + }); }); function fillForm(component: ProfileFormComponent) { diff --git a/modules/ui/src/app/pages/risk-assessment/profile-form/profile-form.component.ts b/modules/ui/src/app/pages/risk-assessment/profile-form/profile-form.component.ts index 5a8be53be..763645cc1 100644 --- a/modules/ui/src/app/pages/risk-assessment/profile-form/profile-form.component.ts +++ b/modules/ui/src/app/pages/risk-assessment/profile-form/profile-form.component.ts @@ -51,6 +51,12 @@ import { FormControlType } from '../../../model/question'; import { ProfileValidators } from './profile.validators'; import { DynamicFormComponent } from '../../../components/dynamic-form/dynamic-form.component'; import { CdkTrapFocus } from '@angular/cdk/a11y'; +import { Observable } from 'rxjs/internal/Observable'; +import { of } from 'rxjs/internal/observable/of'; +import { map } from 'rxjs/internal/operators/map'; +import { SimpleDialogComponent } from '../../../components/simple-dialog/simple-dialog.component'; +import { MatDialog } from '@angular/material/dialog'; +import { RiskAssessmentStore } from '../risk-assessment.store'; @Component({ selector: 'app-profile-form', @@ -75,21 +81,23 @@ import { CdkTrapFocus } from '@angular/cdk/a11y'; export class ProfileFormComponent implements OnInit, AfterViewInit { private profileValidators = inject(ProfileValidators); private fb = inject(FormBuilder); - + private store = inject(RiskAssessmentStore); private profile: Profile | null = null; private profileList!: Profile[]; private injector = inject(Injector); private nameValidator!: ValidatorFn; + private changeProfile = true; public readonly ProfileStatus = ProfileStatus; profileForm: FormGroup = this.fb.group({}); + dialog = inject(MatDialog); readonly autosize = viewChildren(CdkTextareaAutosize); @Input() profileFormat!: ProfileFormat[]; @Input() isCopyProfile!: boolean; @Input() set profiles(profiles: Profile[]) { this.profileList = profiles; - if (this.nameControl) { - this.updateNameValidator(); + if (this.nameControl && this.profile) { + this.updateNameValidator(this.profile); } } get profiles() { @@ -100,12 +108,20 @@ export class ProfileFormComponent implements OnInit, AfterViewInit { if (this.isCopyProfile && this.profile) { this.deleteCopy.emit(this.profile); } - this.profile = profile; - if (profile && this.nameControl) { - this.updateNameValidator(); - this.fillProfileForm(this.profileFormat, profile); + if (this.changeProfile || this.profileHasNoChanges()) { + this.changeProfile = false; + this.profile = profile; + if (profile && this.nameControl) { + this.updateNameValidator(profile); + this.fillProfileForm(this.profileFormat, profile); + } + } else if (this.profile != profile) { + // prevent select profile before user confirmation + this.store.updateSelectedProfile(this.profile); + this.openCloseDialogToChangeProfile(profile); } } + get selectedProfile() { return this.profile; } @@ -277,6 +293,7 @@ export class ProfileFormComponent implements OnInit, AfterViewInit { onSaveClick(status: ProfileStatus) { const response = this.buildResponseFromForm(status, this.selectedProfile); this.saveProfile.emit(response); + this.changeProfile = true; } onDiscardClick() { @@ -291,6 +308,39 @@ export class ProfileFormComponent implements OnInit, AfterViewInit { this.copyProfile.emit(this.selectedProfile!); } + close(): Observable { + if (this.profileHasNoChanges()) { + return of(true); + } + return this.openCloseDialog().pipe(map(res => !!res)); + } + + openCloseDialog() { + const dialogRef = this.dialog.open(SimpleDialogComponent, { + ariaLabel: 'Discard the Risk Assessment changes', + data: { + title: 'Discard changes?', + content: `You have unsaved changes that would be permanently lost.`, + confirmName: 'Discard', + }, + autoFocus: true, + hasBackdrop: true, + disableClose: true, + panelClass: ['simple-dialog', 'discard-dialog'], + }); + + return dialogRef?.afterClosed(); + } + + private openCloseDialogToChangeProfile(profile: Profile | null) { + this.openCloseDialog().subscribe(close => { + if (close) { + this.changeProfile = true; + this.store.updateSelectedProfile(profile); + } + }); + } + private buildResponseFromForm( status: ProfileStatus | '', profile: Profile | null @@ -301,13 +351,13 @@ export class ProfileFormComponent implements OnInit, AfterViewInit { }; if (profile && !this.isCopyProfile) { request.name = profile.name; - request.rename = this.nameControl.value?.trim(); + request.rename = this.nameControl?.value?.trim(); } else { - request.name = this.nameControl.value?.trim(); + request.name = this.nameControl?.value?.trim(); } const questions: Question[] = []; - this.profileFormat.forEach((initialQuestion, index) => { + this.profileFormat?.forEach((initialQuestion, index) => { const question: Question = {}; question.question = initialQuestion.question; @@ -342,11 +392,11 @@ export class ProfileFormComponent implements OnInit, AfterViewInit { ); } - private updateNameValidator() { + private updateNameValidator(profile: Profile) { this.nameControl.removeValidators([this.nameValidator]); this.nameValidator = this.profileValidators.differentProfileName( this.profileList, - this.profile + profile ); this.nameControl.addValidators(this.nameValidator); this.nameControl.updateValueAndValidity(); diff --git a/modules/ui/src/app/pages/risk-assessment/risk-assessment.component.html b/modules/ui/src/app/pages/risk-assessment/risk-assessment.component.html index e353615a4..49c44420f 100644 --- a/modules/ui/src/app/pages/risk-assessment/risk-assessment.component.html +++ b/modules/ui/src/app/pages/risk-assessment/risk-assessment.component.html @@ -37,6 +37,7 @@ message="Select a profile from the list on the left to view or edit the profile."> { let component: RiskAssessmentComponent; @@ -76,13 +79,19 @@ describe('RiskAssessmentComponent', () => { MatToolbarModule, MatSidenavModule, BrowserAnimationsModule, + MatIconTestingModule, + MatIcon, ], providers: [ { provide: TestRunService, useValue: mockService }, { provide: RiskAssessmentStore, useValue: mockRiskAssessmentStore }, { provide: LiveAnnouncer, useValue: mockLiveAnnouncer }, ], - }).compileComponents(); + }) + .overrideComponent(RiskAssessmentComponent, { + set: { encapsulation: ViewEncapsulation.None }, + }) + .compileComponents(); TestBed.overrideProvider(RiskAssessmentStore, { useValue: mockRiskAssessmentStore, @@ -388,35 +397,29 @@ describe('RiskAssessmentComponent', () => { }); describe('#discard', () => { - it('should open discard modal', fakeAsync(() => { - const openSpy = spyOn(component.dialog, 'open').and.returnValue({ - afterClosed: () => of(true), - } as MatDialogRef); + beforeEach(async () => { + await component.openForm(); + }); + + it('should call openCloseDialog', () => { + const openCloseDialogSpy = spyOn( + component.form(), + 'openCloseDialog' + ).and.returnValue(of(true)); component.discard(null); - expect(openSpy).toHaveBeenCalledWith(SimpleDialogComponent, { - ariaLabel: 'Discard the Risk Assessment changes', - data: { - title: 'Discard changes?', - content: `You have unsaved changes that would be permanently lost.`, - confirmName: 'Discard', - }, - autoFocus: true, - hasBackdrop: true, - disableClose: true, - panelClass: ['simple-dialog', 'discard-dialog'], - }); + expect(openCloseDialogSpy).toHaveBeenCalled(); - openSpy.calls.reset(); - })); + openCloseDialogSpy.calls.reset(); + }); describe('with no selected profile', () => { beforeEach(() => { - spyOn(component.dialog, 'open').and.returnValue({ - afterClosed: () => of(true), - } as MatDialogRef); - + spyOn( + component.form(), + 'openCloseDialog' + ).and.returnValue(of(true)); component.discard(null); }); @@ -433,10 +436,10 @@ describe('RiskAssessmentComponent', () => { describe('with selected profile', () => { beforeEach(fakeAsync(() => { - spyOn(component.dialog, 'open').and.returnValue({ - afterClosed: () => of(true), - } as MatDialogRef); - + spyOn( + component.form(), + 'openCloseDialog' + ).and.returnValue(of(true)); component.discard(PROFILE_MOCK); tick(100); })); diff --git a/modules/ui/src/app/pages/risk-assessment/risk-assessment.component.ts b/modules/ui/src/app/pages/risk-assessment/risk-assessment.component.ts index fa2d9b7c5..f85a4bd80 100644 --- a/modules/ui/src/app/pages/risk-assessment/risk-assessment.component.ts +++ b/modules/ui/src/app/pages/risk-assessment/risk-assessment.component.ts @@ -20,6 +20,7 @@ import { OnInit, ViewContainerRef, inject, + viewChild, ChangeDetectorRef, } from '@angular/core'; import { RiskAssessmentStore } from './risk-assessment.store'; @@ -49,6 +50,8 @@ import { ListLayoutComponent } from '../../components/list-layout/list-layout.co import { LayoutType } from '../../model/layout-type'; import { NoEntitySelectedComponent } from '../../components/no-entity-selected/no-entity-selected.component'; import { EntityAction, EntityActionResult } from '../../model/entity-action'; +import { of } from 'rxjs/internal/observable/of'; +import { CanComponentDeactivate } from '../../guards/can-deactivate.guard'; const matFormFieldDefaultOptions: MatFormFieldDefaultOptions = { hideRequiredMarker: true, @@ -81,12 +84,15 @@ const matFormFieldDefaultOptions: MatFormFieldDefaultOptions = { ], changeDetection: ChangeDetectionStrategy.OnPush, }) -export class RiskAssessmentComponent implements OnInit, OnDestroy { +export class RiskAssessmentComponent + implements OnInit, OnDestroy, CanComponentDeactivate +{ readonly LayoutType = LayoutType; readonly ProfileStatus = ProfileStatus; + readonly form = viewChild('profileFormComponent'); private store = inject(RiskAssessmentStore); private liveAnnouncer = inject(LiveAnnouncer); - private cd = inject(ChangeDetectorRef); + cd = inject(ChangeDetectorRef); private destroy$: Subject = new Subject(); dialog = inject(MatDialog); element = inject(ViewContainerRef); @@ -95,6 +101,15 @@ export class RiskAssessmentComponent implements OnInit, OnDestroy { isOpenProfileForm = false; isCopyProfile = false; + canDeactivate(): Observable { + const form = this.form(); + if (form) { + return form.close(); + } else { + return of(true); + } + } + ngOnInit() { this.store.getProfilesFormat(); } @@ -115,6 +130,7 @@ export class RiskAssessmentComponent implements OnInit, OnDestroy { this.store.updateSelectedProfile(profile); await this.liveAnnouncer.announce('Risk assessment questionnaire'); this.store.setFocusOnProfileForm(); + this.cd.detectChanges(); } async copyProfileAndOpenForm(profile: Profile, profiles: Profile[]) { @@ -197,21 +213,8 @@ export class RiskAssessmentComponent implements OnInit, OnDestroy { } private openCloseDialog(selectedProfile: Profile | null) { - const dialogRef = this.dialog.open(SimpleDialogComponent, { - ariaLabel: 'Discard the Risk Assessment changes', - data: { - title: 'Discard changes?', - content: `You have unsaved changes that would be permanently lost.`, - confirmName: 'Discard', - }, - autoFocus: true, - hasBackdrop: true, - disableClose: true, - panelClass: ['simple-dialog', 'discard-dialog'], - }); - - dialogRef - ?.afterClosed() + this.form() + ?.openCloseDialog() .pipe(takeUntil(this.destroy$)) .subscribe(close => { if (close) {