From 89dcc25b9e601f2ab933a9ea4be48757c93d9f29 Mon Sep 17 00:00:00 2001 From: kurilova Date: Tue, 30 Jul 2024 12:18:18 +0000 Subject: [PATCH 01/19] Use mqtt service instead of calling GET /status every 5 seconds. --- modules/ui/src/app/model/topic.ts | 1 + .../services/test-run-mqtt.service.spec.ts | 23 ++++++++ .../src/app/services/test-run-mqtt.service.ts | 5 ++ modules/ui/src/app/store/effects.spec.ts | 35 ++++++++---- modules/ui/src/app/store/effects.ts | 56 +++++++++---------- 5 files changed, 77 insertions(+), 43 deletions(-) diff --git a/modules/ui/src/app/model/topic.ts b/modules/ui/src/app/model/topic.ts index 31cd8b1a7..8cd74a097 100644 --- a/modules/ui/src/app/model/topic.ts +++ b/modules/ui/src/app/model/topic.ts @@ -1,3 +1,4 @@ export enum Topic { NetworkAdapters = 'events/adapter', + Status = 'status', } diff --git a/modules/ui/src/app/services/test-run-mqtt.service.spec.ts b/modules/ui/src/app/services/test-run-mqtt.service.spec.ts index 637c441a4..bc8ba383a 100644 --- a/modules/ui/src/app/services/test-run-mqtt.service.spec.ts +++ b/modules/ui/src/app/services/test-run-mqtt.service.spec.ts @@ -7,6 +7,7 @@ import SpyObj = jasmine.SpyObj; import { of } from 'rxjs'; import { MOCK_ADAPTERS } from '../mocks/settings.mock'; import { Topic } from '../model/topic'; +import { MOCK_PROGRESS_DATA_IN_PROGRESS } from '../mocks/testrun.mock'; describe('TestRunMqttService', () => { let service: TestRunMqttService; @@ -46,6 +47,28 @@ describe('TestRunMqttService', () => { }); }); + describe('getStatus', () => { + beforeEach(() => { + mockService.observe.and.returnValue( + of(getResponse(MOCK_PROGRESS_DATA_IN_PROGRESS)) + ); + }); + + it('should subscribe the topic', done => { + service.getStatus().subscribe(() => { + expect(mockService.observe).toHaveBeenCalledWith(Topic.Status); + done(); + }); + }); + + it('should return object of type', done => { + service.getStatus().subscribe(res => { + expect(res).toEqual(MOCK_PROGRESS_DATA_IN_PROGRESS); + done(); + }); + }); + }); + function getResponse(response: Type): IMqttMessage { const enc = new TextEncoder(); const message = enc.encode(JSON.stringify(response)); diff --git a/modules/ui/src/app/services/test-run-mqtt.service.ts b/modules/ui/src/app/services/test-run-mqtt.service.ts index c362f11e8..83b366f7d 100644 --- a/modules/ui/src/app/services/test-run-mqtt.service.ts +++ b/modules/ui/src/app/services/test-run-mqtt.service.ts @@ -4,6 +4,7 @@ import { catchError, Observable, of } from 'rxjs'; import { map } from 'rxjs/operators'; import { Adapters } from '../model/setting'; import { Topic } from '../model/topic'; +import { TestrunStatus } from '../model/testrun-status'; @Injectable({ providedIn: 'root', @@ -15,6 +16,10 @@ export class TestRunMqttService { return this.topic(Topic.NetworkAdapters); } + getStatus(): Observable { + return this.topic(Topic.Status); + } + private topic(topicName: string): Observable { return this.mqttService.observe(topicName).pipe( map( diff --git a/modules/ui/src/app/store/effects.spec.ts b/modules/ui/src/app/store/effects.spec.ts index 6f6fae967..9d7984dd7 100644 --- a/modules/ui/src/app/store/effects.spec.ts +++ b/modules/ui/src/app/store/effects.spec.ts @@ -42,6 +42,7 @@ import { } from '../mocks/testrun.mock'; import { fetchSystemStatus, + fetchSystemStatusSuccess, setReports, setStatus, setTestrunStatus, @@ -52,6 +53,7 @@ import { throwError } from 'rxjs/internal/observable/throwError'; import { HttpErrorResponse } from '@angular/common/http'; import { IDLE_STATUS } from '../model/testrun-status'; import { HISTORY } from '../mocks/reports.mock'; +import { TestRunMqttService } from '../services/test-run-mqtt.service'; describe('Effects', () => { let actions$ = new Observable(); @@ -64,6 +66,8 @@ describe('Effects', () => { 'dismissWithTimout', 'openSnackBar', ]); + const mockMqttService: jasmine.SpyObj = + jasmine.createSpyObj('mockMqttService', ['getStatus']); beforeEach(() => { testRunServiceMock = jasmine.createSpyObj('testRunServiceMock', [ @@ -85,11 +89,16 @@ describe('Effects', () => { testRunServiceMock.fetchProfiles.and.returnValue(of([])); testRunServiceMock.getHistory.and.returnValue(of([])); + mockMqttService.getStatus.and.returnValue( + of(MOCK_PROGRESS_DATA_IN_PROGRESS) + ); + TestBed.configureTestingModule({ providers: [ AppEffects, { provide: TestRunService, useValue: testRunServiceMock }, { provide: NotificationService, useValue: notificationServiceMock }, + { provide: TestRunMqttService, useValue: mockMqttService }, provideMockActions(() => actions$), provideMockStore({}), ], @@ -399,14 +408,15 @@ describe('Effects', () => { ); }); - it('should call fetchSystemStatus for status "in progress"', fakeAsync(() => { + it('should call fetchSystemStatus for status "in progress"', () => { effects.onFetchSystemStatusSuccess$.subscribe(() => { - tick(5000); - - expect(dispatchSpy).toHaveBeenCalledWith(fetchSystemStatus()); - discardPeriodicTasks(); + expect(dispatchSpy).toHaveBeenCalledWith( + fetchSystemStatusSuccess({ + systemStatus: MOCK_PROGRESS_DATA_IN_PROGRESS, + }) + ); }); - })); + }); it('should dispatch status and systemStatus', done => { effects.onFetchSystemStatusSuccess$.subscribe(() => { @@ -451,14 +461,15 @@ describe('Effects', () => { ); }); - it('should call fetchSystemStatus for status "waiting for device"', fakeAsync(() => { + it('should call fetchSystemStatus for status "waiting for device"', () => { effects.onFetchSystemStatusSuccess$.subscribe(() => { - tick(5000); - - expect(dispatchSpy).toHaveBeenCalledWith(fetchSystemStatus()); - discardPeriodicTasks(); + expect(dispatchSpy).toHaveBeenCalledWith( + fetchSystemStatusSuccess({ + systemStatus: MOCK_PROGRESS_DATA_IN_PROGRESS, + }) + ); }); - })); + }); it('should open snackbar when waiting for device is too long', fakeAsync(() => { effects.onFetchSystemStatusSuccess$.subscribe(() => { diff --git a/modules/ui/src/app/store/effects.ts b/modules/ui/src/app/store/effects.ts index b94eb55b1..c5528c695 100644 --- a/modules/ui/src/app/store/effects.ts +++ b/modules/ui/src/app/store/effects.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { Injectable, NgZone } from '@angular/core'; +import { Injectable } from '@angular/core'; import { Actions, createEffect, ofType } from '@ngrx/effects'; import { Store } from '@ngrx/store'; import { map, switchMap, tap, withLatestFrom } from 'rxjs/operators'; @@ -25,12 +25,12 @@ import { TestRunService } from '../services/test-run.service'; import { filter, combineLatest, - interval, Subject, timer, take, catchError, EMPTY, + Subscription, } from 'rxjs'; import { selectIsOpenWaitSnackBar, @@ -46,6 +46,7 @@ import { } from '../model/testrun-status'; import { fetchSystemStatus, + fetchSystemStatusSuccess, setReports, setStatus, setTestrunStatus, @@ -54,13 +55,13 @@ import { import { takeUntil } from 'rxjs/internal/operators/takeUntil'; import { NotificationService } from '../services/notification.service'; import { Profile } from '../model/profile'; +import { TestRunMqttService } from '../services/test-run-mqtt.service'; const WAIT_TO_OPEN_SNACKBAR_MS = 60 * 1000; @Injectable() export class AppEffects { - private startInterval = false; - private destroyInterval$: Subject = new Subject(); + private statusSubscription: Subscription | undefined; private destroyWaitDeviceInterval$: Subject = new Subject(); checkInterfacesInConfig$ = createEffect(() => @@ -206,8 +207,7 @@ export class AppEffects { return this.actions$.pipe( ofType(AppActions.stopInterval), tap(() => { - this.startInterval = false; - this.destroyInterval$.next(true); + this.statusSubscription?.unsubscribe(); }) ); }, @@ -219,10 +219,7 @@ export class AppEffects { return this.actions$.pipe( ofType(AppActions.fetchSystemStatusSuccess), tap(({ systemStatus }) => { - if ( - this.testrunService.testrunInProgress(systemStatus.status) && - !this.startInterval - ) { + if (this.testrunService.testrunInProgress(systemStatus.status)) { this.pullingSystemStatusData(); } else if ( !this.testrunService.testrunInProgress(systemStatus.status) @@ -251,12 +248,10 @@ export class AppEffects { tap(([{ systemStatus }, , status]) => { // for app - requires only status if (systemStatus.status !== status?.status) { - this.ngZone.run(() => { - this.store.dispatch(setStatus({ status: systemStatus.status })); - this.store.dispatch( - setTestrunStatus({ systemStatus: systemStatus }) - ); - }); + this.store.dispatch(setStatus({ status: systemStatus.status })); + this.store.dispatch( + setTestrunStatus({ systemStatus: systemStatus }) + ); } else if ( systemStatus.finished !== status?.finished || (systemStatus.tests as TestsData)?.results?.length !== @@ -264,11 +259,9 @@ export class AppEffects { (systemStatus.tests as IResult[])?.length !== (status?.tests as IResult[])?.length ) { - this.ngZone.run(() => { - this.store.dispatch( - setTestrunStatus({ systemStatus: systemStatus }) - ); - }); + this.store.dispatch( + setTestrunStatus({ systemStatus: systemStatus }) + ); } }) ); @@ -353,22 +346,23 @@ export class AppEffects { } private pullingSystemStatusData(): void { - this.ngZone.runOutsideAngular(() => { - this.startInterval = true; - interval(5000) - .pipe( - takeUntil(this.destroyInterval$), - tap(() => this.store.dispatch(fetchSystemStatus())) - ) - .subscribe(); - }); + if ( + this.statusSubscription === undefined || + this.statusSubscription?.closed + ) { + this.statusSubscription = this.testrunMqttService + .getStatus() + .subscribe(systemStatus => { + this.store.dispatch(fetchSystemStatusSuccess({ systemStatus })); + }); + } } constructor( private actions$: Actions, private testrunService: TestRunService, + private testrunMqttService: TestRunMqttService, private store: Store, - private ngZone: NgZone, private notificationService: NotificationService ) {} } From c74ec7743687dbb345df93b66e3f16c6e830de2f Mon Sep 17 00:00:00 2001 From: Sofia Kurilova Date: Fri, 26 Jul 2024 15:29:27 +0200 Subject: [PATCH 02/19] Adds tooltip (#638) Adds tooltip --- modules/ui/src/app/app.component.html | 1 + modules/ui/src/app/app.component.spec.ts | 2 ++ 2 files changed, 3 insertions(+) diff --git a/modules/ui/src/app/app.component.html b/modules/ui/src/app/app.component.html index d9d1c59cf..a5e14e9bd 100644 --- a/modules/ui/src/app/app.component.html +++ b/modules/ui/src/app/app.component.html @@ -266,6 +266,7 @@

Testrun

mat-button routerLink="{{ route }}" routerLinkActive="app-sidebar-button-active" + [matTooltip]="label" (keydown.enter)="onNavigationClick()"> {{ icon }} diff --git a/modules/ui/src/app/app.component.spec.ts b/modules/ui/src/app/app.component.spec.ts index e7e0c4f7c..2cfb60aed 100644 --- a/modules/ui/src/app/app.component.spec.ts +++ b/modules/ui/src/app/app.component.spec.ts @@ -71,6 +71,7 @@ import { LiveAnnouncer } from '@angular/cdk/a11y'; import { HISTORY } from './mocks/reports.mock'; import { TestRunMqttService } from './services/test-run-mqtt.service'; import { MOCK_ADAPTERS } from './mocks/settings.mock'; +import { MatTooltipModule } from '@angular/material/tooltip'; const windowMock = { location: { @@ -138,6 +139,7 @@ describe('AppComponent', () => { CalloutComponent, MatIconTestingModule, CertificatesComponent, + MatTooltipModule, ], providers: [ { provide: TestRunService, useValue: mockService }, From 4c25a91310e5b6735e3b563ed94ec40ae65d71e6 Mon Sep 17 00:00:00 2001 From: Sofia Kurilova Date: Tue, 30 Jul 2024 15:20:52 +0200 Subject: [PATCH 03/19] Fix focus after profile delete - track by name (#640) Fix focus after profile delete - track by name --- .../pages/risk-assessment/risk-assessment.component.html | 6 +----- .../app/pages/risk-assessment/risk-assessment.component.ts | 4 ++-- 2 files changed, 3 insertions(+), 7 deletions(-) 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 af1bbd8a4..0a72e6f70 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 @@ -42,11 +42,7 @@

Saved profiles

{ - return index; + trackByName = (index: number, item: Profile): string => { + return item.name; }; private closeFormAfterDelete(name: string, selectedProfile: Profile | null) { From 904cf95cf33feae3c678d782f7e389323b50183a Mon Sep 17 00:00:00 2001 From: J Boddey Date: Tue, 30 Jul 2024 16:00:35 +0100 Subject: [PATCH 04/19] Update the requests dependency (#643) * Update requests dependency * Update requests dependency * Update dependency in TLS test * Update docker dependency --------- Signed-off-by: J Boddey --- framework/requirements.txt | 4 ++-- modules/test/tls/python/requirements.txt | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/framework/requirements.txt b/framework/requirements.txt index e1412f754..402009ef9 100644 --- a/framework/requirements.txt +++ b/framework/requirements.txt @@ -1,8 +1,8 @@ # Requirements for the core module -requests<2.32.0 +requests==2.32.3 # Requirements for the net_orc module -docker==7.0.0 +docker==7.1.0 ipaddress==1.0.23 netifaces==0.11.0 scapy==2.5.0 diff --git a/modules/test/tls/python/requirements.txt b/modules/test/tls/python/requirements.txt index 714b95ecd..846a224f3 100644 --- a/modules/test/tls/python/requirements.txt +++ b/modules/test/tls/python/requirements.txt @@ -2,4 +2,4 @@ cryptography==38.0.0 # Do not upgrade until TLS module can be fixed to account f pyOpenSSL==23.0.0 lxml==5.1.0 # Requirement of pyshark but if upgraded automatically above 5.1 will cause a python crash pyshark==0.6 -requests==2.32.0 +requests==2.32.3 From e58c3bda120ce0061965175053b9c7f0133c0cc9 Mon Sep 17 00:00:00 2001 From: Aliaksandr Nikitsin Date: Fri, 2 Aug 2024 11:01:43 +0000 Subject: [PATCH 05/19] remove unused output --- framework/python/src/common/session.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/python/src/common/session.py b/framework/python/src/common/session.py index e3328dcb8..e4489b026 100644 --- a/framework/python/src/common/session.py +++ b/framework/python/src/common/session.py @@ -345,7 +345,7 @@ def get_report_tests(self): return {'total': self.get_total_tests(), 'results': test_results} def add_test_result(self, result): - LOGGER.info('------adding resul----t') + updated = False # Check if test has already been added From ae15fc114a4a024344ac09cd6344304be2bba7ae Mon Sep 17 00:00:00 2001 From: Aliaksandr Nikitsin Date: Fri, 2 Aug 2024 15:23:08 +0200 Subject: [PATCH 06/19] encode mqtt message to json --- framework/python/src/common/mqtt.py | 3 +++ framework/python/src/common/session.py | 10 +++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/framework/python/src/common/mqtt.py b/framework/python/src/common/mqtt.py index e63af52ce..32cb421eb 100644 --- a/framework/python/src/common/mqtt.py +++ b/framework/python/src/common/mqtt.py @@ -13,6 +13,7 @@ # limitations under the License. """MQTT client""" +import json import typing as t import paho.mqtt.client as mqtt_client from common import logger @@ -55,4 +56,6 @@ def send_message(self, topic: str, message: t.Union[str, dict]) -> None: message (t.Union[str, dict]): message """ self._connect() + if isinstance(message, dict): + message = json.dumps(message) self._client.publish(topic, str(message)) diff --git a/framework/python/src/common/session.py b/framework/python/src/common/session.py index e4489b026..ecbe10784 100644 --- a/framework/python/src/common/session.py +++ b/framework/python/src/common/session.py @@ -17,6 +17,7 @@ import pytz import json import os +from fastapi.encoders import jsonable_encoder from common import util, logger, mqtt from common.risk_profile import RiskProfile from net_orc.ip_control import IPControl @@ -53,7 +54,10 @@ def wrapper(self, *args, **kwargs): result = method(self, *args, **kwargs) if self.get_status() != 'Idle': - self.get_mqtt_client().send_message(STATUS_TOPIC, self.to_json()) + self.get_mqtt_client().send_message( + STATUS_TOPIC, + jsonable_encoder(self.to_json()) + ) return result return wrapper @@ -285,7 +289,7 @@ def set_config(self, config_json): self._save_config() # Update log level - LOGGER.debug(f'Setting log level to {config_json["log_level"]}') + LOGGER.debug(f'Setting log level to {config_json['log_level']}') logger.set_log_level(config_json['log_level']) def set_target_device(self, device): @@ -345,7 +349,7 @@ def get_report_tests(self): return {'total': self.get_total_tests(), 'results': test_results} def add_test_result(self, result): - + updated = False # Check if test has already been added From f8a0c4e4f0b34ebeb6d4d2f218c03b6f08e0288e Mon Sep 17 00:00:00 2001 From: Sofia Kurilova Date: Wed, 31 Jul 2024 14:46:58 +0200 Subject: [PATCH 07/19] Revert "Expired profile (#619)" (#645) Prevent opening of Expired risk profile --- modules/ui/src/app/mocks/profile.mock.ts | 66 +- modules/ui/src/app/model/profile.ts | 8 +- .../profile-form/profile-form.component.html | 2 +- .../profile-form.component.spec.ts | 598 ++++++++---------- .../profile-form/profile-form.component.ts | 45 +- .../risk-assessment.component.html | 2 +- .../risk-assessment.component.spec.ts | 4 +- .../risk-assessment.component.ts | 6 + .../risk-assessment/risk-assessment.store.ts | 16 +- .../ui/src/app/services/test-run.service.ts | 6 +- 10 files changed, 318 insertions(+), 435 deletions(-) diff --git a/modules/ui/src/app/mocks/profile.mock.ts b/modules/ui/src/app/mocks/profile.mock.ts index 614b6e752..f2dbe82c8 100644 --- a/modules/ui/src/app/mocks/profile.mock.ts +++ b/modules/ui/src/app/mocks/profile.mock.ts @@ -17,7 +17,7 @@ import { FormControlType, Profile, - Question, + ProfileFormat, ProfileStatus, } from '../model/profile'; @@ -29,51 +29,22 @@ export const PROFILE_MOCK: Profile = { { question: 'What is the email of the device owner(s)?', answer: 'boddey@google.com, cmeredith@google.com', - type: FormControlType.EMAIL_MULTIPLE, - validation: { - required: true, - max: '30', - }, }, { question: 'What type of device do you need reviewed?', - answer: 'Type', - type: FormControlType.TEXTAREA, - validation: { - required: true, - max: '28', - }, - description: 'This tells us about the device', + answer: 'IoT Sensor', }, { question: 'Are any of the following statements true about your device?', answer: 'First', - type: FormControlType.SELECT, - options: ['First', 'Second'], - validation: { - required: true, - }, }, { question: 'What features does the device have?', - description: - 'This tells us about the data your device will collectThis tells us about the data your device will collect', - type: FormControlType.SELECT_MULTIPLE, answer: [0, 1, 2], - options: ['Wi-fi', 'Bluetooth', 'ZigBee / Z-Wave / Thread / Matter'], - validation: { - required: true, - }, }, { question: 'Comments', answer: 'Yes', - type: FormControlType.TEXT, - description: 'Please enter any comments here', - validation: { - max: '28', - required: true, - }, }, ], }; @@ -90,7 +61,7 @@ export const PROFILE_MOCK_3: Profile = { questions: [], }; -export const PROFILE_FORM: Question[] = [ +export const PROFILE_FORM: ProfileFormat[] = [ { question: 'Email', type: FormControlType.EMAIL_MULTIPLE, @@ -192,51 +163,22 @@ export const COPY_PROFILE_MOCK: Profile = { { question: 'What is the email of the device owner(s)?', answer: 'boddey@google.com, cmeredith@google.com', - type: FormControlType.EMAIL_MULTIPLE, - validation: { - required: true, - max: '30', - }, }, { question: 'What type of device do you need reviewed?', - answer: 'Type', - type: FormControlType.TEXTAREA, - validation: { - required: true, - max: '28', - }, - description: 'This tells us about the device', + answer: 'IoT Sensor', }, { question: 'Are any of the following statements true about your device?', answer: 'First', - type: FormControlType.SELECT, - options: ['First', 'Second'], - validation: { - required: true, - }, }, { question: 'What features does the device have?', - description: - 'This tells us about the data your device will collectThis tells us about the data your device will collect', - type: FormControlType.SELECT_MULTIPLE, answer: [0, 1, 2], - options: ['Wi-fi', 'Bluetooth', 'ZigBee / Z-Wave / Thread / Matter'], - validation: { - required: true, - }, }, { question: 'Comments', answer: 'Yes', - type: FormControlType.TEXT, - description: 'Please enter any comments here', - validation: { - max: '28', - required: true, - }, }, ], }; diff --git a/modules/ui/src/app/model/profile.ts b/modules/ui/src/app/model/profile.ts index fef15f101..059b3cafe 100644 --- a/modules/ui/src/app/model/profile.ts +++ b/modules/ui/src/app/model/profile.ts @@ -35,13 +35,17 @@ export interface Validation { max?: string; } -export interface Question { +export interface ProfileFormat { question: string; - type?: FormControlType; + type: FormControlType; description?: string; options?: string[]; default?: string; validation?: Validation; +} + +export interface Question { + question?: string; answer?: string | number[]; } diff --git a/modules/ui/src/app/pages/risk-assessment/profile-form/profile-form.component.html b/modules/ui/src/app/pages/risk-assessment/profile-form/profile-form.component.html index af31e4d6d..50f0bf3c7 100644 --- a/modules/ui/src/app/pages/risk-assessment/profile-form/profile-form.component.html +++ b/modules/ui/src/app/pages/risk-assessment/profile-form/profile-form.component.html @@ -43,7 +43,7 @@ - + { - [ - 'very long value very long value very long value very long value very long value very long value very long value', - 'as&@3$', - ].forEach(value => { - const name: HTMLInputElement = compiled.querySelector( - '.form-name' - ) as HTMLInputElement; - name.value = value; - name.dispatchEvent(new Event('input')); - component.nameControl.markAsTouched(); - fixture.detectChanges(); + beforeEach(() => { + component.selectedProfile = null; + fixture.detectChanges(); + }); - const nameError = compiled.querySelector('mat-error')?.innerHTML; - const error = component.nameControl.hasError('invalid_format'); + describe('Profile name input', () => { + it('should be present', () => { + const name: HTMLInputElement = compiled.querySelector( + '.form-name' + ) as HTMLInputElement; - expect(error).toBeTruthy(); - expect(nameError).toContain( - 'Please, check. The Profile name must be a maximum of 28 characters. Only letters, numbers, and accented letters are permitted.' - ); - }); - }); + expect(name).toBeTruthy(); + }); - it('should have "required" error when field is not filled', () => { - const name: HTMLInputElement = compiled.querySelector( - '.form-name' - ) as HTMLInputElement; - ['', ' '].forEach(value => { - name.value = value; - name.dispatchEvent(new Event('input')); - component.nameControl.markAsTouched(); - fixture.detectChanges(); + it('should not contain errors when input is correct', () => { + const name: HTMLInputElement = compiled.querySelector( + '.form-name' + ) as HTMLInputElement; + ['name', 'Gebäude', 'jardín'].forEach(value => { + name.value = value; + name.dispatchEvent(new Event('input')); - const nameError = compiled.querySelector('mat-error')?.innerHTML; - const error = component.nameControl.hasError('required'); + const errors = component.nameControl.errors; + const uiValue = name.value; + const formValue = component.nameControl.value; - expect(error).toBeTruthy(); - expect(nameError).toContain('The Profile name is required'); - }); + expect(uiValue).toEqual(formValue); + expect(errors).toBeNull(); }); + }); - it('should have different profile name error when profile with name is exist', () => { + it('should have "invalid_format" error when field does not satisfy validation rules', () => { + [ + 'very long value very long value very long value very long value very long value very long value very long value', + 'as&@3$', + ].forEach(value => { const name: HTMLInputElement = compiled.querySelector( '.form-name' ) as HTMLInputElement; - name.value = 'Primary profile'; + name.value = value; name.dispatchEvent(new Event('input')); component.nameControl.markAsTouched(); - fixture.detectChanges(); const nameError = compiled.querySelector('mat-error')?.innerHTML; - const error = component.nameControl.hasError('has_same_profile_name'); + const error = component.nameControl.hasError('invalid_format'); expect(error).toBeTruthy(); expect(nameError).toContain( - 'This Profile name is already used for another Risk Assessment profile' + 'Please, check. The Profile name must be a maximum of 28 characters. Only letters, numbers, and accented letters are permitted.' ); }); }); - PROFILE_FORM.forEach((item, index) => { - const uiIndex = index + 1; // as Profile name is at 0 position, the json items start from 1 i + it('should have "required" error when field is not filled', () => { + const name: HTMLInputElement = compiled.querySelector( + '.form-name' + ) as HTMLInputElement; + ['', ' '].forEach(value => { + name.value = value; + name.dispatchEvent(new Event('input')); + component.nameControl.markAsTouched(); + fixture.detectChanges(); - it(`should have form field with specific type"`, () => { - const fields = compiled.querySelectorAll('.profile-form-field'); + const nameError = compiled.querySelector('mat-error')?.innerHTML; + const error = component.nameControl.hasError('required'); - if (item.type === FormControlType.SELECT) { - const select = fields[uiIndex].querySelector('mat-select'); - expect(select).toBeTruthy(); - } else if (item.type === FormControlType.SELECT_MULTIPLE) { - const select = fields[uiIndex].querySelector('mat-checkbox'); - expect(select).toBeTruthy(); - } else if (item.type === FormControlType.TEXTAREA) { - const input = fields[uiIndex]?.querySelector('textarea'); - expect(input).toBeTruthy(); - } else { - const input = fields[uiIndex]?.querySelector('input'); - expect(input).toBeTruthy(); - } + expect(error).toBeTruthy(); + expect(nameError).toContain('The Profile name is required'); }); + }); - it('should have label', () => { - const labels = compiled.querySelectorAll('.field-label'); - const uiIndex = index + 1; // as Profile name is at 0 position, the json items start from 1 i + it('should have different profile name error when profile with name is exist', () => { + const name: HTMLInputElement = compiled.querySelector( + '.form-name' + ) as HTMLInputElement; + name.value = 'Primary profile'; + name.dispatchEvent(new Event('input')); + component.nameControl.markAsTouched(); - const label = item?.validation?.required - ? item.question + ' *' - : item.question; - expect(labels[uiIndex].textContent?.trim()).toEqual(label); - }); + fixture.detectChanges(); - it('should have hint', () => { - const fields = compiled.querySelectorAll('.profile-form-field'); - const uiIndex = index + 1; // as Profile name is at 0 position, the json items start from 1 i - const hint = fields[uiIndex].querySelector('mat-hint'); + const nameError = compiled.querySelector('mat-error')?.innerHTML; + const error = component.nameControl.hasError('has_same_profile_name'); - if (item.description) { - expect(hint?.textContent?.trim()).toEqual(item.description); - } else { - expect(hint).toBeNull(); - } - }); + expect(error).toBeTruthy(); + expect(nameError).toContain( + 'This Profile name is already used for another Risk Assessment profile' + ); + }); + }); + + PROFILE_FORM.forEach((item, index) => { + const uiIndex = index + 1; // as Profile name is at 0 position, the json items start from 1 i + + it(`should have form field with specific type"`, () => { + const fields = compiled.querySelectorAll('.profile-form-field'); if (item.type === FormControlType.SELECT) { - describe('select', () => { - it(`should have default value if provided`, () => { - const fields = compiled.querySelectorAll('.profile-form-field'); - const select = fields[uiIndex].querySelector('mat-select'); - expect(select?.textContent?.trim()).toEqual(item.default || ''); - }); + const select = fields[uiIndex].querySelector('mat-select'); + expect(select).toBeTruthy(); + } else if (item.type === FormControlType.SELECT_MULTIPLE) { + const select = fields[uiIndex].querySelector('mat-checkbox'); + expect(select).toBeTruthy(); + } else if (item.type === FormControlType.TEXTAREA) { + const input = fields[uiIndex]?.querySelector('textarea'); + expect(input).toBeTruthy(); + } else { + const input = fields[uiIndex]?.querySelector('input'); + expect(input).toBeTruthy(); + } + }); - it('should have "required" error when field is not filled', () => { - const fields = compiled.querySelectorAll('.profile-form-field'); + it('should have label', () => { + const labels = compiled.querySelectorAll('.field-label'); + const uiIndex = index + 1; // as Profile name is at 0 position, the json items start from 1 i - component.getControl(index).setValue(''); - component.getControl(index).markAsTouched(); + const label = item?.validation?.required + ? item.question + ' *' + : item.question; + expect(labels[uiIndex].textContent?.trim()).toEqual(label); + }); - fixture.detectChanges(); + it('should have hint', () => { + const fields = compiled.querySelectorAll('.profile-form-field'); + const uiIndex = index + 1; // as Profile name is at 0 position, the json items start from 1 i + const hint = fields[uiIndex].querySelector('mat-hint'); - const error = - fields[uiIndex].querySelector('mat-error')?.innerHTML; + if (item.description) { + expect(hint?.textContent?.trim()).toEqual(item.description); + } else { + expect(hint).toBeNull(); + } + }); - expect(error).toContain('The field is required'); - }); + if (item.type === FormControlType.SELECT) { + describe('select', () => { + it(`should have default value if provided`, () => { + const fields = compiled.querySelectorAll('.profile-form-field'); + const select = fields[uiIndex].querySelector('mat-select'); + expect(select?.textContent?.trim()).toEqual(item.default || ''); }); - } - if (item.type === FormControlType.SELECT_MULTIPLE) { - describe('select multiple', () => { - it(`should mark form group as dirty while tab navigation`, () => { - const fields = compiled.querySelectorAll('.profile-form-field'); - const checkbox = fields[uiIndex].querySelector( - '.field-select-checkbox:last-of-type mat-checkbox' - ); - checkbox?.dispatchEvent( - new KeyboardEvent('keydown', { key: 'Tab' }) - ); - fixture.detectChanges(); + it('should have "required" error when field is not filled', () => { + const fields = compiled.querySelectorAll('.profile-form-field'); - expect(component.getControl(index).dirty).toBeTrue(); - }); + component.getControl(index).setValue(''); + component.getControl(index).markAsTouched(); + + fixture.detectChanges(); + + const error = fields[uiIndex].querySelector('mat-error')?.innerHTML; + + expect(error).toContain('The field is required'); }); - } + }); + } + + if (item.type === FormControlType.SELECT_MULTIPLE) { + describe('select multiple', () => { + it(`should mark form group as dirty while tab navigation`, () => { + const fields = compiled.querySelectorAll('.profile-form-field'); + const checkbox = fields[uiIndex].querySelector( + '.field-select-checkbox:last-of-type mat-checkbox' + ); + checkbox?.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Tab' }) + ); + fixture.detectChanges(); - if ( - item.type === FormControlType.TEXT || - item.type === FormControlType.TEXTAREA || - item.type === FormControlType.EMAIL_MULTIPLE - ) { - describe('text or text-long or email-multiple', () => { - if (item.validation?.required) { - it('should have "required" error when field is not filled', () => { - const fields = compiled.querySelectorAll('.profile-form-field'); - const uiIndex = index + 1; // as Profile name is at 0 position, the json items start from 1 i - const input = fields[uiIndex].querySelector( - '.mat-mdc-input-element' - ) as HTMLInputElement; - ['', ' '].forEach(value => { - input.value = value; - input.dispatchEvent(new Event('input')); - component.getControl(index).markAsTouched(); - fixture.detectChanges(); - const errors = fields[uiIndex].querySelectorAll('mat-error'); - let hasError = false; - errors.forEach(error => { - if (error.textContent === 'The field is required') { - hasError = true; - } - }); - - expect(hasError).toBeTrue(); + expect(component.getControl(index).dirty).toBeTrue(); + }); + }); + } + + if ( + item.type === FormControlType.TEXT || + item.type === FormControlType.TEXTAREA || + item.type === FormControlType.EMAIL_MULTIPLE + ) { + describe('text or text-long or email-multiple', () => { + if (item.validation?.required) { + it('should have "required" error when field is not filled', () => { + const fields = compiled.querySelectorAll('.profile-form-field'); + const uiIndex = index + 1; // as Profile name is at 0 position, the json items start from 1 i + const input = fields[uiIndex].querySelector( + '.mat-mdc-input-element' + ) as HTMLInputElement; + ['', ' '].forEach(value => { + input.value = value; + input.dispatchEvent(new Event('input')); + component.getControl(index).markAsTouched(); + fixture.detectChanges(); + const errors = fields[uiIndex].querySelectorAll('mat-error'); + let hasError = false; + errors.forEach(error => { + if (error.textContent === 'The field is required') { + hasError = true; + } }); + + expect(hasError).toBeTrue(); }); - } + }); + } + + it('should have "invalid_format" error when field does not satisfy validation rules', () => { + const fields = compiled.querySelectorAll('.profile-form-field'); + const uiIndex = index + 1; // as Profile name is at 0 position, the json items start from 1 i + const input: HTMLInputElement = fields[uiIndex].querySelector( + '.mat-mdc-input-element' + ) as HTMLInputElement; + input.value = 'as\\\\\\\\\\""""""""'; + input.dispatchEvent(new Event('input')); + component.getControl(index).markAsTouched(); + fixture.detectChanges(); + const result = + item.type === FormControlType.EMAIL_MULTIPLE + ? 'Please, check the email address. Valid e-mail can contain only latin letters, numbers, @ and . (dot).' + : 'Please, check. “ and \\ are not allowed.'; + const errors = fields[uiIndex].querySelectorAll('mat-error'); + let hasError = false; + errors.forEach(error => { + if (error.textContent === result) { + hasError = true; + } + }); - it('should have "invalid_format" error when field does not satisfy validation rules', () => { + expect(hasError).toBeTrue(); + }); + + if (item.validation?.max) { + it('should have "maxlength" error when field is exceeding max length', () => { const fields = compiled.querySelectorAll('.profile-form-field'); const uiIndex = index + 1; // as Profile name is at 0 position, the json items start from 1 i const input: HTMLInputElement = fields[uiIndex].querySelector( '.mat-mdc-input-element' ) as HTMLInputElement; - input.value = 'as\\\\\\\\\\""""""""'; + input.value = + 'very long value very long value very long value very long value very long value very long value very long value very long value very long value very long value'; input.dispatchEvent(new Event('input')); component.getControl(index).markAsTouched(); fixture.detectChanges(); - const result = - item.type === FormControlType.EMAIL_MULTIPLE - ? 'Please, check the email address. Valid e-mail can contain only latin letters, numbers, @ and . (dot).' - : 'Please, check. “ and \\ are not allowed.'; + const errors = fields[uiIndex].querySelectorAll('mat-error'); let hasError = false; errors.forEach(error => { - if (error.textContent === result) { + if ( + error.textContent === + `The field must be a maximum of ${item.validation?.max} characters.` + ) { hasError = true; } }); - expect(hasError).toBeTrue(); }); - - if (item.validation?.max) { - it('should have "maxlength" error when field is exceeding max length', () => { - const fields = compiled.querySelectorAll('.profile-form-field'); - const uiIndex = index + 1; // as Profile name is at 0 position, the json items start from 1 i - const input: HTMLInputElement = fields[uiIndex].querySelector( - '.mat-mdc-input-element' - ) as HTMLInputElement; - input.value = - 'very long value very long value very long value very long value very long value very long value very long value very long value very long value very long value'; - input.dispatchEvent(new Event('input')); - component.getControl(index).markAsTouched(); - fixture.detectChanges(); - - const errors = fields[uiIndex].querySelectorAll('mat-error'); - let hasError = false; - errors.forEach(error => { - if ( - error.textContent === - `The field must be a maximum of ${item.validation?.max} characters.` - ) { - hasError = true; - } - }); - expect(hasError).toBeTrue(); - }); - } - }); - } - }); - - describe('Draft button', () => { - it('should be disabled when profile name is empty', () => { - component.nameControl.setValue(''); - fixture.detectChanges(); - const draftButton = compiled.querySelector( - '.save-draft-button' - ) as HTMLButtonElement; - - expect(draftButton.disabled).toBeTrue(); + } }); + } + }); - it('should be disabled when profile name is not empty but other fields in wrong format', () => { - component.nameControl.setValue('New profile'); - component.getControl('0').setValue('test'); - fixture.detectChanges(); - const draftButton = compiled.querySelector( - '.save-draft-button' - ) as HTMLButtonElement; - - expect(draftButton.disabled).toBeTrue(); - }); + describe('Draft button', () => { + it('should be disabled when profile name is empty', () => { + component.nameControl.setValue(''); + fixture.detectChanges(); + const draftButton = compiled.querySelector( + '.save-draft-button' + ) as HTMLButtonElement; - it('should be enabled when profile name is not empty; other fields are empty or in correct format', () => { - component.nameControl.setValue('New profile'); - component.getControl('0').setValue('a@test.te;b@test.te, c@test.te'); - fixture.detectChanges(); - const draftButton = compiled.querySelector( - '.save-draft-button' - ) as HTMLButtonElement; + expect(draftButton.disabled).toBeTrue(); + }); - expect(draftButton.disabled).toBeFalse(); - }); + it('should be disabled when profile name is not empty but other fields in wrong format', () => { + component.nameControl.setValue('New profile'); + component.getControl('0').setValue('test'); + fixture.detectChanges(); + const draftButton = compiled.querySelector( + '.save-draft-button' + ) as HTMLButtonElement; - it('should emit new profile in draft status', () => { - component.nameControl.setValue('New profile'); - fixture.detectChanges(); - const emitSpy = spyOn(component.saveProfile, 'emit'); - const draftButton = compiled.querySelector( - '.save-draft-button' - ) as HTMLButtonElement; - draftButton.click(); - - expect(emitSpy).toHaveBeenCalledWith({ - ...NEW_PROFILE_MOCK_DRAFT, - }); - }); + expect(draftButton.disabled).toBeTrue(); }); - describe('Save button', () => { - beforeEach(() => { - fillForm(component); - fixture.detectChanges(); - }); - - it('should be enabled when required fields are present', () => { - const saveButton = compiled.querySelector( - '.save-profile-button' - ) as HTMLButtonElement; + it('should be enabled when profile name is not empty; other fields are empty or in correct format', () => { + component.nameControl.setValue('New profile'); + component.getControl('0').setValue('a@test.te;b@test.te, c@test.te'); + fixture.detectChanges(); + const draftButton = compiled.querySelector( + '.save-draft-button' + ) as HTMLButtonElement; - expect(saveButton.disabled).toBeFalse(); - }); + expect(draftButton.disabled).toBeFalse(); + }); - it('should emit new profile', () => { - const emitSpy = spyOn(component.saveProfile, 'emit'); - const saveButton = compiled.querySelector( - '.save-profile-button' - ) as HTMLButtonElement; - saveButton.click(); + it('should emit new profile in draft status', () => { + component.nameControl.setValue('New profile'); + fixture.detectChanges(); + const emitSpy = spyOn(component.saveProfile, 'emit'); + const draftButton = compiled.querySelector( + '.save-draft-button' + ) as HTMLButtonElement; + draftButton.click(); - expect(emitSpy).toHaveBeenCalledWith({ - ...NEW_PROFILE_MOCK, - }); + expect(emitSpy).toHaveBeenCalledWith({ + ...NEW_PROFILE_MOCK_DRAFT, }); }); + }); - describe('Discard button', () => { - beforeEach(() => { - fillForm(component); - fixture.detectChanges(); - }); - - it('should has Discard text', () => { - const discardButton = compiled.querySelector( - '.discard-button' - ) as HTMLButtonElement; - - expect(discardButton.textContent?.trim()).toEqual('Discard'); - }); + describe('Save button', () => { + beforeEach(() => { + fillForm(component); + fixture.detectChanges(); + }); - it('should be enabled when form is filled', () => { - const discardButton = compiled.querySelector( - '.discard-button' - ) as HTMLButtonElement; + it('should be enabled when required fields are present', () => { + const saveButton = compiled.querySelector( + '.save-profile-button' + ) as HTMLButtonElement; - expect(discardButton.disabled).toBeFalse(); - }); + expect(saveButton.disabled).toBeFalse(); + }); - it('should emit discard', () => { - const emitSpy = spyOn(component.discard, 'emit'); - const discardButton = compiled.querySelector( - '.discard-button' - ) as HTMLButtonElement; - discardButton.click(); + it('should emit new profile', () => { + const emitSpy = spyOn(component.saveProfile, 'emit'); + const saveButton = compiled.querySelector( + '.save-profile-button' + ) as HTMLButtonElement; + saveButton.click(); - expect(emitSpy).toHaveBeenCalled(); + expect(emitSpy).toHaveBeenCalledWith({ + ...NEW_PROFILE_MOCK, }); }); }); - describe('with expire profile', () => { + describe('Discard button', () => { beforeEach(() => { - component.selectedProfile = Object.assign({}, PROFILE_MOCK, { - status: ProfileStatus.EXPIRED, - }); + fillForm(component); fixture.detectChanges(); }); - it('should have disabled fields', () => { - const fields = compiled.querySelectorAll('mat-form-field'); - fields.forEach(field => { - expect( - field.classList.contains('mat-form-field-disabled') - ).toBeTrue(); - }); - }); - - describe('Close button', () => { - it('should has Discard text', () => { - const discardButton = compiled.querySelector( - '.discard-button' - ) as HTMLButtonElement; - - expect(discardButton.textContent?.trim()).toEqual('Close'); - }); + it('should be enabled when form is filled', () => { + const discardButton = compiled.querySelector( + '.discard-button' + ) as HTMLButtonElement; - it('should be enabled', () => { - const discardButton = compiled.querySelector( - '.discard-button' - ) as HTMLButtonElement; - - expect(discardButton.disabled).toBeFalse(); - }); + expect(discardButton.disabled).toBeFalse(); + }); - it('should emit discard', () => { - const emitSpy = spyOn(component.discard, 'emit'); - const discardButton = compiled.querySelector( - '.discard-button' - ) as HTMLButtonElement; - discardButton.click(); + it('should emit discard', () => { + const emitSpy = spyOn(component.discard, 'emit'); + const discardButton = compiled.querySelector( + '.discard-button' + ) as HTMLButtonElement; + discardButton.click(); - expect(emitSpy).toHaveBeenCalled(); - }); + expect(emitSpy).toHaveBeenCalled(); }); }); }); @@ -482,7 +425,6 @@ describe('ProfileFormComponent', () => { it('save profile should have rename field', () => { const emitSpy = spyOn(component.saveProfile, 'emit'); fillForm(component); - fixture.detectChanges(); component.onSaveClick(ProfileStatus.VALID); expect(emitSpy).toHaveBeenCalledWith(RENAME_PROFILE_MOCK); 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 7ac96fd18..684bc67da 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 @@ -46,6 +46,7 @@ import { DeviceValidators } from '../../devices/components/device-form/device.va import { FormControlType, Profile, + ProfileFormat, ProfileStatus, Question, Validation, @@ -80,8 +81,7 @@ export class ProfileFormComponent implements OnInit { profileForm: FormGroup = this.fb.group({}); @ViewChildren(CdkTextareaAutosize) autosize!: QueryList; - questionnaire!: Question[]; - @Input() profileFormat!: Question[]; + @Input() profileFormat!: ProfileFormat[]; @Input() set profiles(profiles: Profile[]) { this.profileList = profiles; @@ -95,8 +95,9 @@ export class ProfileFormComponent implements OnInit { @Input() set selectedProfile(profile: Profile | null) { this.profile = profile; - if (profile && this.questionnaire) { - this.updateForm(profile); + if (profile && this.nameControl) { + this.updateNameValidator(); + this.fillProfileForm(this.profileFormat, profile); } } get selectedProfile() { @@ -111,22 +112,9 @@ export class ProfileFormComponent implements OnInit { private fb: FormBuilder ) {} ngOnInit() { + this.profileForm = this.createProfileForm(this.profileFormat); if (this.selectedProfile) { - this.updateForm(this.selectedProfile); - } else { - this.questionnaire = this.profileFormat; - this.profileForm = this.createProfileForm(this.questionnaire); - } - } - - updateForm(profile: Profile) { - this.questionnaire = profile.questions; - this.profileForm = this.createProfileForm(this.questionnaire); - this.fillProfileForm(profile); - if (profile.status === ProfileStatus.EXPIRED) { - this.profileForm.disable(); - } else { - this.profileForm.enable(); + this.fillProfileForm(this.profileFormat, this.selectedProfile); } } @@ -135,7 +123,7 @@ export class ProfileFormComponent implements OnInit { } private get fieldsHasError(): boolean { - return this.questionnaire.some((field, index) => { + return this.profileFormat.some((field, index) => { return ( this.getControl(index).hasError('invalid_format') || this.getControl(index).hasError('maxlength') @@ -151,7 +139,7 @@ export class ProfileFormComponent implements OnInit { return this.profileForm.get(name.toString()) as AbstractControl; } - createProfileForm(questions: Question[]): FormGroup { + createProfileForm(questions: ProfileFormat[]): FormGroup { // eslint-disable-next-line @typescript-eslint/no-explicit-any const group: any = {}; @@ -171,7 +159,7 @@ export class ProfileFormComponent implements OnInit { group[index] = this.getMultiSelectGroup(question); } else { const validators = this.getValidators( - question.type!, + question.type, question.validation ); group[index] = new FormControl(question.default || '', validators); @@ -199,7 +187,7 @@ export class ProfileFormComponent implements OnInit { return validators; } - getMultiSelectGroup(question: Question): FormGroup { + getMultiSelectGroup(question: ProfileFormat): FormGroup { // eslint-disable-next-line @typescript-eslint/no-explicit-any const group: any = {}; question.options?.forEach((option, index) => { @@ -216,9 +204,9 @@ export class ProfileFormComponent implements OnInit { return this.profileForm?.controls[name] as FormGroup; } - fillProfileForm(profile: Profile): void { + fillProfileForm(profileFormat: ProfileFormat[], profile: Profile): void { this.nameControl.setValue(profile.name); - profile.questions.forEach((question, index) => { + profileFormat.forEach((question, index) => { if (question.type === FormControlType.SELECT_MULTIPLE) { question.options?.forEach((item, idx) => { if ((profile.questions[index].answer as number[])?.includes(idx)) { @@ -260,7 +248,7 @@ export class ProfileFormComponent implements OnInit { } private buildResponseFromForm( - initialQuestions: Question[], + initialQuestions: ProfileFormat[], profileForm: FormGroup, status: ProfileStatus, profile: Profile | null @@ -278,9 +266,8 @@ export class ProfileFormComponent implements OnInit { const questions: Question[] = []; initialQuestions.forEach((initialQuestion, index) => { - const question: Question = { - question: initialQuestion.question, - }; + const question: Question = {}; + question.question = initialQuestion.question; if (initialQuestion.type === FormControlType.SELECT_MULTIPLE) { const answer: number[] = []; 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 0a72e6f70..c5e38e360 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 @@ -47,7 +47,7 @@

Saved profiles

class="profile-item-{{ i }}" [ngClass]="{ selected: profile.name === vm.selectedProfile?.name }" (deleteButtonClicked)="deleteProfile($event, i, vm.selectedProfile)" - (profileClicked)="openForm($event)" + (profileClicked)="profileClicked($event)" (copyProfileClicked)="copyProfileAndOpenForm($event)">
diff --git a/modules/ui/src/app/pages/risk-assessment/risk-assessment.component.spec.ts b/modules/ui/src/app/pages/risk-assessment/risk-assessment.component.spec.ts index dbb359121..8e792ff83 100644 --- a/modules/ui/src/app/pages/risk-assessment/risk-assessment.component.spec.ts +++ b/modules/ui/src/app/pages/risk-assessment/risk-assessment.component.spec.ts @@ -34,7 +34,7 @@ import { } from '../../mocks/profile.mock'; import { of } from 'rxjs'; import { Component, Input } from '@angular/core'; -import { Profile, Question } from '../../model/profile'; +import { Profile, ProfileFormat } from '../../model/profile'; import { MatDialogRef } from '@angular/material/dialog'; import { SimpleDialogComponent } from '../../components/simple-dialog/simple-dialog.component'; import { RiskAssessmentStore } from './risk-assessment.store'; @@ -372,5 +372,5 @@ class FakeProfileItemComponent { class FakeProfileFormComponent { @Input() profiles!: Profile[]; @Input() selectedProfile!: Profile; - @Input() profileFormat!: Question[]; + @Input() profileFormat!: ProfileFormat[]; } 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 0b895f693..dd3d33d9d 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 @@ -54,6 +54,12 @@ export class RiskAssessmentComponent implements OnInit, OnDestroy { this.destroy$.unsubscribe(); } + async profileClicked(profile: Profile | null = null) { + if (profile === null || profile.status !== ProfileStatus.EXPIRED) { + await this.openForm(profile); + } + } + async openForm(profile: Profile | null = null) { this.isOpenProfileForm = true; this.store.updateSelectedProfile(profile); diff --git a/modules/ui/src/app/pages/risk-assessment/risk-assessment.store.ts b/modules/ui/src/app/pages/risk-assessment/risk-assessment.store.ts index 8dc9cce04..93e89b434 100644 --- a/modules/ui/src/app/pages/risk-assessment/risk-assessment.store.ts +++ b/modules/ui/src/app/pages/risk-assessment/risk-assessment.store.ts @@ -19,7 +19,7 @@ import { ComponentStore } from '@ngrx/component-store'; import { tap, withLatestFrom } from 'rxjs/operators'; import { delay, exhaustMap } from 'rxjs'; import { TestRunService } from '../../services/test-run.service'; -import { Profile, Question } from '../../model/profile'; +import { Profile, ProfileFormat } from '../../model/profile'; import { FocusManagerService } from '../../services/focus-manager.service'; import { Store } from '@ngrx/store'; import { AppState } from '../../store/state'; @@ -29,7 +29,7 @@ import { fetchRiskProfiles, setRiskProfiles } from '../../store/actions'; export interface AppComponentState { selectedProfile: Profile | null; profiles: Profile[]; - profileFormat: Question[]; + profileFormat: ProfileFormat[]; } @Injectable() export class RiskAssessmentStore extends ComponentStore { @@ -43,10 +43,12 @@ export class RiskAssessmentStore extends ComponentStore { selectedProfile: this.selectedProfile$, }); - updateProfileFormat = this.updater((state, profileFormat: Question[]) => ({ - ...state, - profileFormat, - })); + updateProfileFormat = this.updater( + (state, profileFormat: ProfileFormat[]) => ({ + ...state, + profileFormat, + }) + ); updateSelectedProfile = this.updater( (state, selectedProfile: Profile | null) => ({ ...state, @@ -121,7 +123,7 @@ export class RiskAssessmentStore extends ComponentStore { return trigger$.pipe( exhaustMap(() => { return this.testRunService.fetchProfilesFormat().pipe( - tap((profileFormat: Question[]) => { + tap((profileFormat: ProfileFormat[]) => { this.updateProfileFormat(profileFormat); }) ); diff --git a/modules/ui/src/app/services/test-run.service.ts b/modules/ui/src/app/services/test-run.service.ts index 9eaf7444b..0ac2b1510 100644 --- a/modules/ui/src/app/services/test-run.service.ts +++ b/modules/ui/src/app/services/test-run.service.ts @@ -30,7 +30,7 @@ import { Version } from '../model/version'; import { Certificate } from '../model/certificate'; import { Profile, - Question, + ProfileFormat, ProfileRisk, RiskResultClassName, } from '../model/profile'; @@ -303,8 +303,8 @@ export class TestRunService { }); } - fetchProfilesFormat(): Observable { - return this.http.get(`${API_URL}/profiles/format`); + fetchProfilesFormat(): Observable { + return this.http.get(`${API_URL}/profiles/format`); } saveProfile(profile: Profile): Observable { From ec04f6687ede299c1d06488172b6baf019e02f13 Mon Sep 17 00:00:00 2001 From: J Boddey Date: Wed, 31 Jul 2024 16:17:39 +0100 Subject: [PATCH 08/19] Improve documentation (#639) * Improve docs * Remove paragraph --- README.md | 4 ++-- docs/README.md | 3 +++ docs/dev/README.md | 25 ++++++++++++++++++++++++ docs/dev/code_quality.md | 16 +++++++++++++++ docs/network/README.md | 3 +-- docs/network/add_new_service.md | 2 +- docs/test/README.md | 1 - docs/test/modules.md | 2 +- modules/test/base/README.md | 7 +++++++ modules/test/dns/README.md | 3 ++- modules/test/dns/conf/module_config.json | 2 +- 11 files changed, 59 insertions(+), 9 deletions(-) create mode 100644 docs/dev/README.md create mode 100644 docs/dev/code_quality.md diff --git a/README.md b/README.md index 4a04e8885..23fd843ca 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,7 @@ When manual testing or configuration changes are required, Testrun will provide - DHCP client - The device must be able to obtain an IP address via DHCP ## Get started ▶️ -Once you have met the hardware and software requirements, you can get started with Testrun by following the [Get started guide](docs/get_started.md). +Once you have met the hardware and software requirements, you can get started with Testrun by following the [Get started guide](docs/get_started.md). Further docs are available in the [docs directory](docs) ## Roadmap :chart_with_upwards_trend: Testrun will constantly evolve to further support end-users by automating device network behaviour against industry standards. For further information on upcoming features, check out the [Roadmap](docs/roadmap.pdf). @@ -59,7 +59,7 @@ We are proud of our tool and strive to provide an enjoyable experience for all o If the application has come across a problem at any point during setup or use, please raise an issue under the [issues tab](https://github.com/google/testrun/issues). Issue templates exist for both bug reports and feature requests. If neither of these are appropriate for your issue, raise a blank issue instead. ## Contributing :keyboard: -The contributing requirements can be found in [CONTRIBUTING.md](CONTRIBUTING.md). In short, checkout the [Google CLA](https://cla.developers.google.com/) site to get started. +The contributing requirements can be found in [CONTRIBUTING.md](CONTRIBUTING.md). In short, checkout the [Google CLA](https://cla.developers.google.com/) site to get started. After that, check out our [developer documentation](docs/dev/README.md). ## FAQ :raising_hand: 1) I have an issue whilst installing/upgrading Testrun, what do I do? diff --git a/docs/README.md b/docs/README.md index 96eb32223..5f055dbb9 100644 --- a/docs/README.md +++ b/docs/README.md @@ -16,3 +16,6 @@ - [Running on a virtual machine](virtual_machine.md) - [Accessibility](ui/accessibility.mp4) - [Roadmap](roadmap.pdf) + +## Something missing? +If you feel there is some documentation that you would find useful, or have found an issue with existing documentation, please raise an issue on GitHub by navigating [here](https://github.com/google/testrun/issues/new/choose) \ No newline at end of file diff --git a/docs/dev/README.md b/docs/dev/README.md new file mode 100644 index 000000000..f11b1b092 --- /dev/null +++ b/docs/dev/README.md @@ -0,0 +1,25 @@ +Testrun logo + +## Developer docs + +## Table of Contents +1) General guidelines (this page) +2) [Code quality](code_quality.md) + +## General guidelines +As an open source project, we absolutely encourage contributions from the community to help Testrun remain an expanding but stable product. However, before contributing there are a number of things to take into consideration. + +1) [Sign the Google CLA](https://cla.developers.google.com/): Whether you are an individual or contributing on behalf of your organisation, you must be covered by a Google CLA. + +2) Determine the scope of your contribution + + - Your contribution is more likely to be accepted if fewer files are changed (keep it simple) + - Are you going to be fixing a bug, dependency issue or a new framework capability? Whatever it is, ensure your pull request fixes or changes just one thing. + +3) Get in touch to discuss whether your proposed changes are likely to be accepted + + - It is best to get the opinion from the core maintainers whether your proposed changes meet our objectives and align with Testrun principles. + +4) Fork Testrun and get developing + + - We aim to provide thorough and easy to ready developer documentation to help you contribute successfully. \ No newline at end of file diff --git a/docs/dev/code_quality.md b/docs/dev/code_quality.md new file mode 100644 index 000000000..47eabcf95 --- /dev/null +++ b/docs/dev/code_quality.md @@ -0,0 +1,16 @@ +Testrun logo + +## Code quality + +Whilst developing code for Testrun, there are some style guides that you should follow. + + - Python: https://google.github.io/styleguide/pyguide.html + - Angular: https://google.github.io/styleguide/angularjs-google-style.html + - Shell: https://google.github.io/styleguide/shellguide.html + - HTML/CSS: https://google.github.io/styleguide/htmlcssguide.html + - JSON: https://google.github.io/styleguide/jsoncstyleguide.xml + - Markdown: https://google.github.io/styleguide/docguide/style.html + +### Automated actions + +The current code base has been able to achieve 0 code lint issues. To maintain this, all lint checks are enforced on pull requests to dev and main. Please ensure that these lint checks are passing before marking your pull requests as 'Ready for review'. \ No newline at end of file diff --git a/docs/network/README.md b/docs/network/README.md index b5536c30c..0f97ecd7b 100644 --- a/docs/network/README.md +++ b/docs/network/README.md @@ -1,10 +1,9 @@ Testrun logo - ## Network Overview ## Table of Contents -1) Network Overview (this page) +1) Network overview (this page) 2) [How to identify network interfaces](identify_interfaces.md) 3) [Addresses](addresses.md) 4) [Add a new network service](add_new_service.md) diff --git a/docs/network/add_new_service.md b/docs/network/add_new_service.md index 7a07e43be..b3fa22514 100644 --- a/docs/network/add_new_service.md +++ b/docs/network/add_new_service.md @@ -65,7 +65,7 @@ COPY $MODULE_DIR/bin /testrun/bin # Copy over all python files COPY $MODULE_DIR/python /testrun/python -# Do not specify a CMD or Entrypoint as Test Run will automatically start your service as required +# Do not specify a CMD or Entrypoint as Testrun will automatically start your service as required ``` ### Example of start_network_service script diff --git a/docs/test/README.md b/docs/test/README.md index 19aa691d8..3163b4c84 100644 --- a/docs/test/README.md +++ b/docs/test/README.md @@ -2,7 +2,6 @@ ## Testing - The test requirements that are investigated by Testrun can be found in the [test modules documentation](/docs/test/modules.md). To understand the testing results, various definitions of test results and requirements are specified in the [statuses documentation](/docs/test/statuses.md). \ No newline at end of file diff --git a/docs/test/modules.md b/docs/test/modules.md index 7c5851ba4..2fe5983b1 100644 --- a/docs/test/modules.md +++ b/docs/test/modules.md @@ -10,7 +10,7 @@ Testrun provides some pre-built test modules for you to use when testing your ow | Baseline | A sample test module | [Baseline module](/modules/test/baseline/README.md) | | Connection | Verify IP and DHCP based behavior | [Connection module](/modules/test/conn/README.md) | | DNS | Verify DNS functionality | [DNS module](/modules/test/dns/README.md) | -| NMAP | Ensure unsecure services are disabled | [NMAP module](/modules/test/nmap/README.md) | +| Services | Ensure unsecure services are disabled | [Services module](/modules/test/services/README.md) | | NTP | Verify NTP functionality | [NTP module](/modules/test/ntp/README.md) | | Protocol | Inspect BMS protocol implementation | [Protocol Module](/modules/test/protocol/README.md) | | TLS | Determine TLS client and server behavior | [TLS module](/modules/test/tls/README.md) | diff --git a/modules/test/base/README.md b/modules/test/base/README.md index e7f05d80e..24a725607 100644 --- a/modules/test/base/README.md +++ b/modules/test/base/README.md @@ -14,6 +14,13 @@ The ```config/module_config.json``` provides the name and description of the mod Within the ```python/src``` directory, basic logging and environment variables are provided to the test module. +Within the ```usr/local/etc``` directory there is a local copy of the MAC OUI database. This is just in case a new copy is unable to be downloaded during the install or update process. + +## GRPC server +Within the python directory, GRPC client code is provided to allow test modules to programmatically modify the various network services provided by Testrun. + +These currently include obtaining information about and controlling the DHCP servers in failover configuration. + ## Tests covered No tests are run by this module \ No newline at end of file diff --git a/modules/test/dns/README.md b/modules/test/dns/README.md index 13f0df5fd..79bce57f7 100644 --- a/modules/test/dns/README.md +++ b/modules/test/dns/README.md @@ -15,4 +15,5 @@ Within the ```python/src``` directory, the below tests are executed. | ID | Description | Expected behavior | Required result |---|---|---|---| | dns.network.hostname_resolution | Verifies that the device resolves hostnames | The device sends DNS requests | Required | -| dns.network.from_dhcp | Verifies that the device allows for a DNS server to be provided by the DHCP server | The device sends DNS requests to the DNS server provided by the DHCP server | Roadmap | \ No newline at end of file +| dns.network.from_dhcp | Verifies that the device allows for a DNS server to be provided by the DHCP server | The device sends DNS requests to the DNS server provided by the DHCP server | Roadmap | +| dns.mdns | Does the device has MDNS (or any kind of IP multicast) | Device may send MDNS requests | Informational | \ No newline at end of file diff --git a/modules/test/dns/conf/module_config.json b/modules/test/dns/conf/module_config.json index 38ec3bcb9..f048d5deb 100644 --- a/modules/test/dns/conf/module_config.json +++ b/modules/test/dns/conf/module_config.json @@ -34,7 +34,7 @@ }, { "name": "dns.mdns", - "test_description": "If the device has MDNS (or any kind of IP multicast), can it be disabled", + "test_description": "Does the device has MDNS (or any kind of IP multicast)", "expected_behavior": "Device may send MDNS requests", "required_result": "Informational" } From 39bc41e42816bf7e72ebd84c0f261cabdb1e049f Mon Sep 17 00:00:00 2001 From: kurilova Date: Mon, 29 Jul 2024 11:33:23 +0000 Subject: [PATCH 09/19] Text changes --- modules/ui/src/app/app.component.html | 8 +++++--- modules/ui/src/app/app.component.spec.ts | 10 +++++----- .../app/components/snack-bar/snack-bar.component.html | 5 +++-- .../ui/src/app/interceptors/error.interceptor.spec.ts | 10 +++++++--- modules/ui/src/app/interceptors/error.interceptor.ts | 8 +++++--- .../ui/src/app/pages/settings/settings.component.html | 2 +- 6 files changed, 26 insertions(+), 17 deletions(-) diff --git a/modules/ui/src/app/app.component.html b/modules/ui/src/app/app.component.html index a5e14e9bd..122e8b129 100644 --- a/modules/ui/src/app/app.component.html +++ b/modules/ui/src/app/app.component.html @@ -127,7 +127,8 @@

Testrun

error.devicePortMissed && error.internetPortMissed; else onePortMissed "> - No ports are detected. Please define a valid ones using + No ports detected. Please connect and configure network and device + connections in the Selected port is missing! Please define a valid one using @@ -213,7 +214,8 @@

Testrun

vm.systemStatus === StatusOfTestrun.InProgress && isRiskAssessmentRoute === false "> - Congratulations, the device is under test now! Do not forget to fill + The device is now being tested. Why not take the time to complete the + device Testrun role="link" class="message-link" >Risk Assessment questionnaire. It is required to complete verification process. + >? diff --git a/modules/ui/src/app/app.component.spec.ts b/modules/ui/src/app/app.component.spec.ts index 2cfb60aed..b626123d7 100644 --- a/modules/ui/src/app/app.component.spec.ts +++ b/modules/ui/src/app/app.component.spec.ts @@ -522,12 +522,12 @@ describe('AppComponent', () => { fixture.detectChanges(); }); - it('should have callout component with "Congratulations" text', () => { + it('should have callout component with "The device is now being tested" text', () => { const callout = compiled.querySelector('app-callout'); const calloutContent = callout?.innerHTML.trim(); expect(callout).toBeTruthy(); - expect(calloutContent).toContain('Congratulations'); + expect(calloutContent).toContain('The device is now being tested'); }); it('should have callout component with "Risk Assessment" link', () => { @@ -554,12 +554,12 @@ describe('AppComponent', () => { fixture.detectChanges(); }); - it('should have callout component with "Congratulations" text', () => { + it('should have callout component with "The device is now being tested" text', () => { const callout = compiled.querySelector('app-callout'); const calloutContent = callout?.innerHTML.trim(); expect(callout).toBeTruthy(); - expect(calloutContent).toContain('Congratulations'); + expect(calloutContent).toContain('The device is now being tested'); }); it('should have callout component with "Risk Assessment" link', () => { @@ -740,7 +740,7 @@ describe('AppComponent', () => { const calloutContent = callout?.innerHTML.trim(); expect(callout).toBeTruthy(); - expect(calloutContent).toContain('No ports are detected.'); + expect(calloutContent).toContain('No ports detected.'); }); }); diff --git a/modules/ui/src/app/components/snack-bar/snack-bar.component.html b/modules/ui/src/app/components/snack-bar/snack-bar.component.html index 716198299..539623d4b 100644 --- a/modules/ui/src/app/components/snack-bar/snack-bar.component.html +++ b/modules/ui/src/app/components/snack-bar/snack-bar.component.html @@ -15,9 +15,10 @@ -->
-

The Waiting for Device stage is taking more than one minute.

+

It is taking longer than expected to find your device on the network.

- Please check device connection or stop and update system configuration. + Please check the connection to the device or stop and update your system + configuration.

diff --git a/modules/ui/src/app/interceptors/error.interceptor.spec.ts b/modules/ui/src/app/interceptors/error.interceptor.spec.ts index 9fff32863..5df8ca2ac 100644 --- a/modules/ui/src/app/interceptors/error.interceptor.spec.ts +++ b/modules/ui/src/app/interceptors/error.interceptor.spec.ts @@ -42,6 +42,10 @@ describe('ErrorInterceptor', () => { interceptor = TestBed.inject(ErrorInterceptor); }); + afterEach(() => { + notificationServiceMock.notify.calls.reset(); + }); + it('should be created', () => { expect(interceptor).toBeTruthy(); }); @@ -60,7 +64,7 @@ describe('ErrorInterceptor', () => { interceptor.intercept(requestMock, next).subscribe( () => ({}), () => { - expect(notificationServiceMock.notify).toHaveBeenCalledWith('error'); + expect(notificationServiceMock.notify).toHaveBeenCalledWith('Something went wrong. Check the logs for details here /usr/local/testrun/testrun.log'); done(); } ); @@ -79,7 +83,7 @@ describe('ErrorInterceptor', () => { () => ({}), () => { expect(notificationServiceMock.notify).toHaveBeenCalledWith( - 'Back End is not responding. Please, try again later.' + 'Testrun is not responding. Please try again in a moment.' ); done(); } @@ -99,7 +103,7 @@ describe('ErrorInterceptor', () => { () => ({}), () => { expect(notificationServiceMock.notify).toHaveBeenCalledWith( - 'Back End is not responding. Please, try again later.' + 'Testrun is not responding. Please try again in a moment.' ); done(); } diff --git a/modules/ui/src/app/interceptors/error.interceptor.ts b/modules/ui/src/app/interceptors/error.interceptor.ts index 924cbde02..82f0cba2b 100644 --- a/modules/ui/src/app/interceptors/error.interceptor.ts +++ b/modules/ui/src/app/interceptors/error.interceptor.ts @@ -35,6 +35,7 @@ import { finalize } from 'rxjs/operators'; const DEFAULT_TIMEOUT_MS = 5000; const SYSTEM_STOP_TIMEOUT_MS = 60 * 1000; +const LOGS_FOLDER = '/usr/local/testrun/testrun.log'; @Injectable() export class ErrorInterceptor implements HttpInterceptor { @@ -57,17 +58,18 @@ export class ErrorInterceptor implements HttpInterceptor { catchError((error: HttpErrorResponse | TimeoutError) => { if (error instanceof TimeoutError) { this.notificationService.notify( - 'Back End is not responding. Please, try again later.' + 'Testrun is not responding. Please try again in a moment.' ); } else { if (error.status === 0) { this.notificationService.notify( - 'Back End is not responding. Please, try again later.' + 'Testrun is not responding. Please try again in a moment.' ); } else { this.notificationService.notify( - error.error?.error || error.message + `Something went wrong. Check the logs for details here ${LOGS_FOLDER}` ); + console.error(error.error?.error || error.message); } } return throwError(error); diff --git a/modules/ui/src/app/pages/settings/settings.component.html b/modules/ui/src/app/pages/settings/settings.component.html index 36849b42e..089ebd5eb 100644 --- a/modules/ui/src/app/pages/settings/settings.component.html +++ b/modules/ui/src/app/pages/settings/settings.component.html @@ -116,7 +116,7 @@

System settings

- Warning! No ports is detected. + Warning! No ports detected.
From c26bcb095692306901ffba088fe330f3c1f39f63 Mon Sep 17 00:00:00 2001 From: kurilova Date: Wed, 31 Jul 2024 09:48:18 +0000 Subject: [PATCH 10/19] Fix text for the BE error --- modules/ui/src/app/interceptors/error.interceptor.spec.ts | 4 +++- modules/ui/src/app/interceptors/error.interceptor.ts | 3 +-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/modules/ui/src/app/interceptors/error.interceptor.spec.ts b/modules/ui/src/app/interceptors/error.interceptor.spec.ts index 5df8ca2ac..bd8950993 100644 --- a/modules/ui/src/app/interceptors/error.interceptor.spec.ts +++ b/modules/ui/src/app/interceptors/error.interceptor.spec.ts @@ -64,7 +64,9 @@ describe('ErrorInterceptor', () => { interceptor.intercept(requestMock, next).subscribe( () => ({}), () => { - expect(notificationServiceMock.notify).toHaveBeenCalledWith('Something went wrong. Check the logs for details here /usr/local/testrun/testrun.log'); + expect(notificationServiceMock.notify).toHaveBeenCalledWith( + 'Something went wrong. Check the Terminal for details.' + ); done(); } ); diff --git a/modules/ui/src/app/interceptors/error.interceptor.ts b/modules/ui/src/app/interceptors/error.interceptor.ts index 82f0cba2b..cfa512a85 100644 --- a/modules/ui/src/app/interceptors/error.interceptor.ts +++ b/modules/ui/src/app/interceptors/error.interceptor.ts @@ -35,7 +35,6 @@ import { finalize } from 'rxjs/operators'; const DEFAULT_TIMEOUT_MS = 5000; const SYSTEM_STOP_TIMEOUT_MS = 60 * 1000; -const LOGS_FOLDER = '/usr/local/testrun/testrun.log'; @Injectable() export class ErrorInterceptor implements HttpInterceptor { @@ -67,7 +66,7 @@ export class ErrorInterceptor implements HttpInterceptor { ); } else { this.notificationService.notify( - `Something went wrong. Check the logs for details here ${LOGS_FOLDER}` + 'Something went wrong. Check the Terminal for details.' ); console.error(error.error?.error || error.message); } From c86093c2382308c01af5297ebac11fbd943c5179 Mon Sep 17 00:00:00 2001 From: Sofia Kurilova Date: Thu, 1 Aug 2024 15:57:13 +0200 Subject: [PATCH 11/19] Change tooltip (#650) * Change tooltip --- .../profile-item/profile-item.component.html | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/ui/src/app/pages/risk-assessment/profile-item/profile-item.component.html b/modules/ui/src/app/pages/risk-assessment/profile-item/profile-item.component.html index 5d1b86514..1384226fc 100644 --- a/modules/ui/src/app/pages/risk-assessment/profile-item/profile-item.component.html +++ b/modules/ui/src/app/pages/risk-assessment/profile-item/profile-item.component.html @@ -22,7 +22,11 @@ class="profile-item-info" role="button" tabindex="0" - matTooltip="{{ profile.status }}" + matTooltip="{{ + profile.status === ProfileStatus.EXPIRED + ? 'Expired. Please, create a new Risk profile.' + : profile.status + }}" (click)="profileClicked.emit(profile)" (keydown.enter)="profileClicked.emit(profile)"> Date: Thu, 1 Aug 2024 15:10:37 +0100 Subject: [PATCH 12/19] Allows draft profiles to become expired (#636) * Allow draft profiles to expire * Move status method into risk profile class * Use existing method * Check for expiry in validate method * Remove unused variable --- framework/python/src/common/risk_profile.py | 8 +++---- framework/python/src/common/session.py | 25 +++++++++------------ testing/unit/services/output/services.log | 6 ----- 3 files changed, 14 insertions(+), 25 deletions(-) delete mode 100644 testing/unit/services/output/services.log diff --git a/framework/python/src/common/risk_profile.py b/framework/python/src/common/risk_profile.py index 6afb229ac..760f73d72 100644 --- a/framework/python/src/common/risk_profile.py +++ b/framework/python/src/common/risk_profile.py @@ -96,11 +96,11 @@ def get_file_path(self): self.name + '.json') def _validate(self, profile_json, profile_format): - if self._valid(profile_json, profile_format): - if self._expired(): - self.status = 'Expired' + if self._expired(): + self.status = 'Expired' + elif self._valid(profile_json, profile_format): # User only wants to save a draft - elif 'status' in profile_json and profile_json['status'] == 'Draft': + if 'status' in profile_json and profile_json['status'] == 'Draft': self.status = 'Draft' else: self.status = 'Valid' diff --git a/framework/python/src/common/session.py b/framework/python/src/common/session.py index ecbe10784..26824dd8a 100644 --- a/framework/python/src/common/session.py +++ b/framework/python/src/common/session.py @@ -440,17 +440,26 @@ def _load_profiles(self): try: for risk_profile_file in os.listdir( os.path.join(self._root_dir, PROFILES_DIR)): + LOGGER.debug(f'Discovered profile {risk_profile_file}') + # Open the risk profile file with open(os.path.join(self._root_dir, PROFILES_DIR, risk_profile_file), encoding='utf-8') as f: + + # Parse risk profile json json_data = json.load(f) + + # Instantiate a new risk profile risk_profile = RiskProfile() + + # Pass JSON to populate risk profile risk_profile.load( profile_json=json_data, profile_format=self._profile_format ) - risk_profile.status = self.check_profile_status(risk_profile) + + # Add risk profile to session self._profiles.append(risk_profile) except Exception as e: @@ -565,20 +574,6 @@ def update_profile(self, profile_json): return risk_profile - def check_profile_status(self, profile): - - if profile.status == 'Valid': - - # Check expiry - created_date = profile.created.timestamp() - - today = datetime.datetime.now().timestamp() - - if created_date < (today - SECONDS_IN_YEAR): - profile.status = 'Expired' - - return profile.status - def delete_profile(self, profile): try: diff --git a/testing/unit/services/output/services.log b/testing/unit/services/output/services.log deleted file mode 100644 index 7df3f745b..000000000 --- a/testing/unit/services/output/services.log +++ /dev/null @@ -1,6 +0,0 @@ -Jun 17 09:23:01 test_services INFO Module report generated at: testing/unit/services/output/services_report.html -Jun 17 09:23:01 test_services INFO Module report generated at: testing/unit/services/output/services_report.html -Jun 17 09:23:01 test_services INFO Module report generated at: testing/unit/services/output/services_report.html -Jun 17 09:32:48 test_services INFO Module report generated at: testing/unit/services/output/services_report.html -Jun 17 09:32:48 test_services INFO Module report generated at: testing/unit/services/output/services_report.html -Jun 17 09:32:48 test_services INFO Module report generated at: testing/unit/services/output/services_report.html From cb4a559a915aa128787a96dbe1e52cc55b0a9665 Mon Sep 17 00:00:00 2001 From: J Boddey Date: Fri, 2 Aug 2024 10:48:07 +0100 Subject: [PATCH 13/19] Build UI during package instead of install (#621) * Build UI during package * Fix local build * Install npm * Remove duplicate build message * Fix ESLint * Fix script * Modify scripts * Improve scripts * Fix copy command * Try installing package * Depend on package job * Add sudo * Add sudo * Troubleshoot * Fix workflow * Checkout source for prepare command * Built ui within a container * Mount src files for build instead of static copy in build image * Attempt to fix actions * Remove manual build container cleanup methods * undo failed attempts to fix actions * Fix path * Remove -it flag --------- Signed-off-by: J Boddey Co-authored-by: kurilova Co-authored-by: jhughesbiot --- .github/workflows/package.yml | 23 ++++++++++++++++- cmd/build | 10 +++++--- cmd/build_ui | 37 ++++++++++++++++++++++++++++ cmd/install | 18 ++++++++++++-- cmd/package | 13 ++++++++-- framework/python/src/core/testrun.py | 2 +- modules/ui/angular.json | 5 ++-- modules/ui/build.Dockerfile | 20 +++++++++++++++ modules/ui/ui.Dockerfile | 11 ++------- 9 files changed, 119 insertions(+), 20 deletions(-) create mode 100755 cmd/build_ui create mode 100644 modules/ui/build.Dockerfile diff --git a/.github/workflows/package.yml b/.github/workflows/package.yml index e923735aa..598ec8a8d 100644 --- a/.github/workflows/package.yml +++ b/.github/workflows/package.yml @@ -7,12 +7,13 @@ on: push: branches: - 'dev' + - 'release/*' permissions: contents: read jobs: - testrun_package: + create_package: permissions: {} name: Package runs-on: ubuntu-22.04 @@ -28,3 +29,23 @@ jobs: with: name: testrun_package path: testrun*.deb + + install_package: + permissions: {} + needs: create_package + name: Install + runs-on: ubuntu-22.04 + timeout-minutes: 15 + steps: + - name: Checkout source + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 + - name: Download package + uses: actions/download-artifact@v4 + with: + name: testrun_package + - name: Install dependencies + shell: bash {0} + run: sudo cmd/prepare + - name: Install package + shell: bash {0} + run: sudo apt install ./testrun*.deb diff --git a/cmd/build b/cmd/build index 9be35dcf3..d3294a681 100755 --- a/cmd/build +++ b/cmd/build @@ -36,9 +36,13 @@ fi # Builds all docker images echo Building docker images -# Build user interface -echo Building user interface -if docker build -t test-run/ui -f modules/ui/ui.Dockerfile . ; then +# Check if UI has already been built (if -l was used during install) +if [ ! -d "modules/ui/dist" ]; then + cmd/build_ui +fi + +# Build UI image +if docker build -t testrun/ui -f modules/ui/ui.Dockerfile . ; then echo Successully built the user interface else echo An error occured whilst building the user interface diff --git a/cmd/build_ui b/cmd/build_ui new file mode 100755 index 000000000..afb0d8827 --- /dev/null +++ b/cmd/build_ui @@ -0,0 +1,37 @@ +#!/bin/bash -e + +# Copyright 2023 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Build the UI +echo Building the ui builder + +# Build UI builder image +if docker build -t testrun/build-ui -f modules/ui/build.Dockerfile . ; then + echo Successully built the ui builder +else + echo An error occured whilst building the ui builder + exit 1 +fi + +# Check that the container is not already running +docker kill tr-ui-build 2> /dev/null || true + +echo "Building the user interface" + +# Start build container and build the ui dist +docker run --rm -v $PWD/modules/ui:/modules/ui testrun/build-ui /bin/sh -c "npm install && npm run build" + +# Kill the container (Should not be running anymore) +docker kill tr-ui-build 2> /dev/null || true diff --git a/cmd/install b/cmd/install index 53d12b324..c350a969f 100755 --- a/cmd/install +++ b/cmd/install @@ -20,15 +20,29 @@ echo Installing application dependencies while getopts ":l" option; do case $option in l) # Install Testrun in local directory - TESTRUN_DIR=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")"/.. && pwd) + TESTRUN_DIR=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")"/.. && pwd) esac done # Check if TESTRUN_DIR has been set, otherwise install in /usr/local/testrun if [[ -z "${TESTRUN_DIR}" ]]; then TESTRUN_DIR=/usr/local/testrun + + # Check that user is sudo + if [[ "$EUID" -ne 0 ]]; then + echo "Installing Testrun in the default location requires sudo. Run using sudo cmd/install" + exit 1 + fi + else TESTRUN_DIR="${TESTRUN_DIR}" + + # Check that user is in docker group + if ! (id -nGz "$USER" | grep -qzxF "docker"); then + echo User is not in docker group. Follow https://docs.docker.com/engine/install/linux-postinstall/ to finish setting up docker. + exit 1 + fi + fi echo Installing Testrun at $TESTRUN_DIR @@ -51,7 +65,7 @@ cp -n local/system.json.example local/system.json deactivate # Build docker images -sudo cmd/build +cmd/build # Create local folders mkdir -p local/devices diff --git a/cmd/package b/cmd/package index fc418ab05..719258a83 100755 --- a/cmd/package +++ b/cmd/package @@ -16,6 +16,12 @@ # Creates a package for Testrun +# Check that user is not root +if [[ "$EUID" == 0 ]]; then + echo "Must not run as root. Use cmd/package as regular user" + exit 1 +fi + MAKE_SRC_DIR=make MAKE_CONTROL_DIR=make/DEBIAN/control @@ -25,10 +31,10 @@ version=$(grep -R "Version: " $MAKE_CONTROL_DIR | awk '{print $2}') # Replace invalid characters version="${version//./_}" -# Delete existing make files -rm -rf $MAKE_SRC_DIR/usr +echo Building package for testrun v${version} # Delete existing make files +echo Cleaning up previous build files rm -rf $MAKE_SRC_DIR/usr # Copy testrun script to /bin @@ -60,6 +66,9 @@ mkdir -p $MAKE_SRC_DIR/usr/local/testrun/local/risk_profiles mkdir -p local/root_certs cp -r local/root_certs $MAKE_SRC_DIR/usr/local/testrun/local/ +# Build the UI +cmd/build_ui + # Copy framework and modules into testrun folder cp -r {framework,modules} $MAKE_SRC_DIR/usr/local/testrun diff --git a/framework/python/src/core/testrun.py b/framework/python/src/core/testrun.py index 95011c58b..c8138fb99 100644 --- a/framework/python/src/core/testrun.py +++ b/framework/python/src/core/testrun.py @@ -482,7 +482,7 @@ def start_ui(self): try: client.containers.run( - image='test-run/ui', + image='testrun/ui', auto_remove=True, name='tr-ui', hostname='testrun.io', diff --git a/modules/ui/angular.json b/modules/ui/angular.json index 135a9c031..0bf42377f 100644 --- a/modules/ui/angular.json +++ b/modules/ui/angular.json @@ -33,7 +33,7 @@ "budgets": [ { "type": "initial", - "maximumWarning": "1000kb", + "maximumWarning": "1500kb", "maximumError": "3000kb" }, { @@ -94,6 +94,7 @@ } }, "cli": { - "schematicCollections": ["@angular-eslint/schematics"] + "schematicCollections": ["@angular-eslint/schematics"], + "analytics": false } } diff --git a/modules/ui/build.Dockerfile b/modules/ui/build.Dockerfile new file mode 100644 index 000000000..180ad9747 --- /dev/null +++ b/modules/ui/build.Dockerfile @@ -0,0 +1,20 @@ +# Copyright 2023 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Image name: testrun/build-ui +FROM node@sha256:ffebb4405810c92d267a764b21975fb2d96772e41877248a37bf3abaa0d3b590 as build + +# Set the working directory +WORKDIR /modules/ui + diff --git a/modules/ui/ui.Dockerfile b/modules/ui/ui.Dockerfile index da56be93e..7ecb32dbd 100644 --- a/modules/ui/ui.Dockerfile +++ b/modules/ui/ui.Dockerfile @@ -12,17 +12,10 @@ # See the License for the specific language governing permissions and # limitations under the License. -# Image name: test-run/ui -FROM node@sha256:ffebb4405810c92d267a764b21975fb2d96772e41877248a37bf3abaa0d3b590 as build - -WORKDIR /modules/ui -COPY modules/ui/ /modules/ui -RUN npm install -RUN npm run build - +# Image name: testrun/ui FROM nginx@sha256:4c0fdaa8b6341bfdeca5f18f7837462c80cff90527ee35ef185571e1c327beac -COPY --from=build /modules/ui/dist/ /usr/share/nginx/html +COPY modules/ui/dist/ /usr/share/nginx/html EXPOSE 8080 From 5916a18f635bcb506bf96dfd6e061b1b9862b3ea Mon Sep 17 00:00:00 2001 From: Sofia Kurilova Date: Fri, 2 Aug 2024 14:41:57 +0200 Subject: [PATCH 14/19] Feature/risk in selected (#654) * Adds risk to selected value * Adds risk to selected value --------- Co-authored-by: J Boddey --- .../download-zip-modal.component.html | 12 +++++++++--- .../download-zip-modal.component.scss | 12 ++++++++++++ .../download-zip-modal.component.spec.ts | 6 +----- .../download-zip-modal.component.ts | 16 ++++++++++------ 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/modules/ui/src/app/components/download-zip-modal/download-zip-modal.component.html b/modules/ui/src/app/components/download-zip-modal/download-zip-modal.component.html index 072ab9c19..2e9446cfa 100644 --- a/modules/ui/src/app/components/download-zip-modal/download-zip-modal.component.html +++ b/modules/ui/src/app/components/download-zip-modal/download-zip-modal.component.html @@ -55,11 +55,17 @@ [(value)]="selectedProfile" aria-label="Please choose a Risk Profile from the list"> - {{ selectedProfile }} + {{ selectedProfile.name }} + + {{ selectedProfile.risk }} risk +
''' index += 1 @@ -635,6 +643,33 @@ def _generate_css(self): ul { margin-top: 0; } + + .risk-label{ + position: absolute; + top: 0px; + right: 0px; + width: 52px; + height: 16px; + font-family: 'Google Sans', sans-serif; + font-size: 8px; + font-weight: 500; + line-height: 16px; + letter-spacing: 0.64px; + text-align: center; + font-weight: bold; + border-radius: 3px; + } + + .risk-label-high{ + background-color: #FCE8E6; + color: #C5221F; + } + + .risk-label-limited{ + width: 65px; + background-color:#E4F7FB; + color: #007B83; + } ''' def to_pdf(self, device): From 2e4bfe66f1a72fe6a1b6c3fb70f73cd2e0ccd738 Mon Sep 17 00:00:00 2001 From: kurilova Date: Tue, 30 Jul 2024 12:18:18 +0000 Subject: [PATCH 16/19] Use mqtt service instead of calling GET /status every 5 seconds. --- modules/ui/src/app/model/topic.ts | 5 ++++ .../services/test-run-mqtt.service.spec.ts | 25 ++++++++++++++++++- .../src/app/services/test-run-mqtt.service.ts | 6 ++++- 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/modules/ui/src/app/model/topic.ts b/modules/ui/src/app/model/topic.ts index 8cd74a097..d330dbb82 100644 --- a/modules/ui/src/app/model/topic.ts +++ b/modules/ui/src/app/model/topic.ts @@ -1,4 +1,9 @@ export enum Topic { NetworkAdapters = 'events/adapter', + InternetConnection = 'events/internet', Status = 'status', } + +export interface InternetConnection { + connection: boolean | null; +} diff --git a/modules/ui/src/app/services/test-run-mqtt.service.spec.ts b/modules/ui/src/app/services/test-run-mqtt.service.spec.ts index bc8ba383a..19bda437a 100644 --- a/modules/ui/src/app/services/test-run-mqtt.service.spec.ts +++ b/modules/ui/src/app/services/test-run-mqtt.service.spec.ts @@ -7,6 +7,7 @@ import SpyObj = jasmine.SpyObj; import { of } from 'rxjs'; import { MOCK_ADAPTERS } from '../mocks/settings.mock'; import { Topic } from '../model/topic'; +import { MOCK_INTERNET } from '../mocks/topic.mock'; import { MOCK_PROGRESS_DATA_IN_PROGRESS } from '../mocks/testrun.mock'; describe('TestRunMqttService', () => { @@ -27,7 +28,7 @@ describe('TestRunMqttService', () => { expect(service).toBeTruthy(); }); - describe('', () => { + describe('getNetworkAdapters', () => { beforeEach(() => { mockService.observe.and.returnValue(of(getResponse(MOCK_ADAPTERS))); }); @@ -47,6 +48,28 @@ describe('TestRunMqttService', () => { }); }); + describe('getInternetConnection', () => { + beforeEach(() => { + mockService.observe.and.returnValue(of(getResponse(MOCK_INTERNET))); + }); + + it('should subscribe the topic', done => { + service.getInternetConnection().subscribe(() => { + expect(mockService.observe).toHaveBeenCalledWith( + Topic.InternetConnection + ); + done(); + }); + }); + + it('should return object of type', done => { + service.getInternetConnection().subscribe(res => { + expect(res).toEqual(MOCK_INTERNET); + done(); + }); + }); + }); + describe('getStatus', () => { beforeEach(() => { mockService.observe.and.returnValue( diff --git a/modules/ui/src/app/services/test-run-mqtt.service.ts b/modules/ui/src/app/services/test-run-mqtt.service.ts index 83b366f7d..d5e805da6 100644 --- a/modules/ui/src/app/services/test-run-mqtt.service.ts +++ b/modules/ui/src/app/services/test-run-mqtt.service.ts @@ -3,8 +3,8 @@ import { IMqttMessage, MqttService } from 'ngx-mqtt'; import { catchError, Observable, of } from 'rxjs'; import { map } from 'rxjs/operators'; import { Adapters } from '../model/setting'; -import { Topic } from '../model/topic'; import { TestrunStatus } from '../model/testrun-status'; +import { InternetConnection, Topic } from '../model/topic'; @Injectable({ providedIn: 'root', @@ -16,6 +16,10 @@ export class TestRunMqttService { return this.topic(Topic.NetworkAdapters); } + getInternetConnection(): Observable { + return this.topic(Topic.InternetConnection); + } + getStatus(): Observable { return this.topic(Topic.Status); } From ee6969d33c9a7a99a6e1071a86144d6351ff7b7c Mon Sep 17 00:00:00 2001 From: kurilova Date: Tue, 30 Jul 2024 12:18:18 +0000 Subject: [PATCH 17/19] Use mqtt service instead of calling GET /status every 5 seconds. --- modules/ui/src/app/services/test-run-mqtt.service.spec.ts | 1 + modules/ui/src/app/services/test-run-mqtt.service.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/modules/ui/src/app/services/test-run-mqtt.service.spec.ts b/modules/ui/src/app/services/test-run-mqtt.service.spec.ts index 19bda437a..9ec3f0a7b 100644 --- a/modules/ui/src/app/services/test-run-mqtt.service.spec.ts +++ b/modules/ui/src/app/services/test-run-mqtt.service.spec.ts @@ -7,6 +7,7 @@ import SpyObj = jasmine.SpyObj; import { of } from 'rxjs'; import { MOCK_ADAPTERS } from '../mocks/settings.mock'; import { Topic } from '../model/topic'; +import { MOCK_PROGRESS_DATA_IN_PROGRESS } from '../mocks/testrun.mock'; import { MOCK_INTERNET } from '../mocks/topic.mock'; import { MOCK_PROGRESS_DATA_IN_PROGRESS } from '../mocks/testrun.mock'; diff --git a/modules/ui/src/app/services/test-run-mqtt.service.ts b/modules/ui/src/app/services/test-run-mqtt.service.ts index d5e805da6..bac488e03 100644 --- a/modules/ui/src/app/services/test-run-mqtt.service.ts +++ b/modules/ui/src/app/services/test-run-mqtt.service.ts @@ -5,6 +5,7 @@ import { map } from 'rxjs/operators'; import { Adapters } from '../model/setting'; import { TestrunStatus } from '../model/testrun-status'; import { InternetConnection, Topic } from '../model/topic'; +import { TestrunStatus } from '../model/testrun-status'; @Injectable({ providedIn: 'root', From e41db23063cc2e8229c6683c9b62d3d8059f01bd Mon Sep 17 00:00:00 2001 From: kurilova Date: Tue, 30 Jul 2024 12:18:18 +0000 Subject: [PATCH 18/19] Use mqtt service instead of calling GET /status every 5 seconds. --- modules/ui/src/app/services/test-run-mqtt.service.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/ui/src/app/services/test-run-mqtt.service.spec.ts b/modules/ui/src/app/services/test-run-mqtt.service.spec.ts index 9ec3f0a7b..b3ddef581 100644 --- a/modules/ui/src/app/services/test-run-mqtt.service.spec.ts +++ b/modules/ui/src/app/services/test-run-mqtt.service.spec.ts @@ -7,6 +7,7 @@ import SpyObj = jasmine.SpyObj; import { of } from 'rxjs'; import { MOCK_ADAPTERS } from '../mocks/settings.mock'; import { Topic } from '../model/topic'; +import { MOCK_INTERNET } from '../mocks/topic.mock'; import { MOCK_PROGRESS_DATA_IN_PROGRESS } from '../mocks/testrun.mock'; import { MOCK_INTERNET } from '../mocks/topic.mock'; import { MOCK_PROGRESS_DATA_IN_PROGRESS } from '../mocks/testrun.mock'; From de7706fcd0e0fd9fcbd82ccda1a997447c611421 Mon Sep 17 00:00:00 2001 From: Aliaksandr Nikitsin Date: Fri, 2 Aug 2024 14:21:59 +0000 Subject: [PATCH 19/19] pylint --- framework/python/src/common/session.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/python/src/common/session.py b/framework/python/src/common/session.py index bc4cdb739..d5eaac37b 100644 --- a/framework/python/src/common/session.py +++ b/framework/python/src/common/session.py @@ -288,7 +288,7 @@ def set_config(self, config_json): self._save_config() # Update log level - LOGGER.debug(f'Setting log level to {config_json['log_level']}') + LOGGER.debug(f'Setting log level to {config_json["log_level"]}') logger.set_log_level(config_json['log_level']) def set_target_device(self, device):