From e1d5c8a80facc69bae457d20f9f63dc3c8840f1c Mon Sep 17 00:00:00 2001 From: kurilova Date: Fri, 17 May 2024 11:33:39 +0000 Subject: [PATCH 1/4] Adds validation on certificate upload --- modules/ui/src/app/mocks/certificate.mock.ts | 23 +++++- modules/ui/src/app/model/certificate.ts | 1 + .../certificate-item.component.html | 12 ++- .../certificate-item.component.scss | 20 ++++- .../certificate-item.component.spec.ts | 39 +++++++++- .../certificate-item.component.ts | 15 +++- .../certificate-upload-button.component.html | 1 + .../certificate-upload-button.component.ts | 6 +- .../certificates/certificate.validator.ts | 29 ++++++++ .../certificates.component.spec.ts | 21 +++++- .../certificates/certificates.component.ts | 16 +++- .../certificates/certificates.store.spec.ts | 74 ++++++++++++++----- .../pages/certificates/certificates.store.ts | 43 +++++++---- 13 files changed, 252 insertions(+), 48 deletions(-) create mode 100644 modules/ui/src/app/pages/certificates/certificate.validator.ts diff --git a/modules/ui/src/app/mocks/certificate.mock.ts b/modules/ui/src/app/mocks/certificate.mock.ts index 65d0be109..1723b48b4 100644 --- a/modules/ui/src/app/mocks/certificate.mock.ts +++ b/modules/ui/src/app/mocks/certificate.mock.ts @@ -22,8 +22,19 @@ export const certificate = { } as Certificate; export const certificate_uploading = { - name: 'test', + name: 'name', uploading: true, + errors: [], +} as Certificate; + +export const certificate_uploading_with_errors = { + name: 'some very long strange name with symbols!?', + uploading: true, + errors: [ + 'The file name should be alphanumeric, symbols -_. are allowed.', + 'Max name length is 24 characters.', + 'File size should be a max of 4KB', + ], } as Certificate; export const certificate2 = { @@ -31,3 +42,13 @@ export const certificate2 = { organisation: 'Google, Inc.', expires: '2024-09-01T09:00:12Z', } as Certificate; + +export const INVALID_FILE = { + name: 'some very long strange name with symbols!?', + size: 7000, +} as File; + +export const FILE = { + name: 'name', + size: 3000, +} as File; diff --git a/modules/ui/src/app/model/certificate.ts b/modules/ui/src/app/model/certificate.ts index 384120cf7..1589b8edb 100644 --- a/modules/ui/src/app/model/certificate.ts +++ b/modules/ui/src/app/model/certificate.ts @@ -18,4 +18,5 @@ export interface Certificate { organisation?: string; expires?: string; uploading?: boolean; + errors?: string[]; } diff --git a/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.html b/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.html index 28bc1768a..8afc95c64 100644 --- a/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.html +++ b/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.html @@ -11,16 +11,22 @@ {{ certificate.expires | date: 'dd MMM yyyy' }}

- + + {{ + error + }} + diff --git a/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.scss b/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.scss index 3f09e90e2..dc89959cf 100644 --- a/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.scss +++ b/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.scss @@ -21,15 +21,33 @@ --mdc-linear-progress-active-indicator-color: #1967d2; } } + :host:first-child .certificate-item-container { border-top: 1px solid $lighter-grey; } + +:host app-callout { + grid-column: span 3; + ::ng-deep .callout-container { + background-color: $red-50; + } + ::ng-deep .callout-container.error_outline .callout-context { + font-weight: normal; + } + ::ng-deep .callout-container.error_outline .callout-icon { + color: $red-700; + } +} + +.error-message-text { + display: inline-block; +} + .certificate-item-container { display: grid; grid-template-columns: 24px minmax(200px, 1fr) 24px; gap: 16px; box-sizing: border-box; - height: 88px; padding: 12px 0; border-bottom: 1px solid $lighter-grey; } diff --git a/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.spec.ts b/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.spec.ts index b0395e254..6ef255664 100644 --- a/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.spec.ts +++ b/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.spec.ts @@ -4,6 +4,7 @@ import { CertificateItemComponent } from './certificate-item.component'; import { certificate, certificate_uploading, + certificate_uploading_with_errors, } from '../../../mocks/certificate.mock'; describe('CertificateItemComponent', () => { @@ -69,9 +70,15 @@ describe('CertificateItemComponent', () => { const deleteSpy = spyOn(component.deleteButtonClicked, 'emit'); deleteButton.click(); - expect(deleteSpy).toHaveBeenCalledWith('iot.bms.google.com'); + expect(deleteSpy).toHaveBeenCalledWith(certificate); }); }); + + it('should not display errors', () => { + const errors = fixture.nativeElement.querySelectorAll('app-callout'); + + expect(errors.length).toEqual(0); + }); }); describe('uploading certificate', () => { @@ -83,7 +90,7 @@ describe('CertificateItemComponent', () => { it('should have loader', () => { const loader = compiled.querySelector('mat-progress-bar'); - expect(loader).toBeDefined(); + expect(loader).not.toBeNull(); }); it('should have disabled delete button', () => { @@ -94,5 +101,33 @@ describe('CertificateItemComponent', () => { expect(deleteButton.getAttribute('disabled')).toBeTruthy(); }); }); + + describe('uploading certificate with errors', () => { + beforeEach(() => { + component.certificate = certificate_uploading_with_errors; + fixture.detectChanges(); + }); + + it('should not have loader', () => { + const loader = compiled.querySelector('mat-progress-bar'); + + expect(loader).toBeNull(); + }); + + it('should have disabled delete button', () => { + const deleteButton = fixture.nativeElement.querySelector( + '.certificate-item-delete' + ) as HTMLButtonElement; + + expect(deleteButton.getAttribute('disabled')).toBeFalsy(); + }); + + it('should have errors', () => { + const errors = + fixture.nativeElement.querySelectorAll('app-callout span'); + + expect(errors.length).toEqual(3); + }); + }); }); }); diff --git a/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.ts b/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.ts index 87e3a8160..605779ff1 100644 --- a/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.ts +++ b/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.ts @@ -5,16 +5,27 @@ import { CommonModule } from '@angular/common'; import { MatButtonModule } from '@angular/material/button'; import { MatProgressBarModule } from '@angular/material/progress-bar'; import { provideAnimations } from '@angular/platform-browser/animations'; +import { CalloutComponent } from '../../../components/callout/callout.component'; +import { MatError } from '@angular/material/form-field'; +import { CalloutType } from '../../../model/callout-type'; @Component({ selector: 'app-certificate-item', standalone: true, - imports: [MatIcon, MatButtonModule, MatProgressBarModule, CommonModule], + imports: [ + MatIcon, + MatButtonModule, + MatProgressBarModule, + CommonModule, + CalloutComponent, + MatError, + ], providers: [provideAnimations()], templateUrl: './certificate-item.component.html', styleUrl: './certificate-item.component.scss', }) export class CertificateItemComponent { + public readonly CalloutType = CalloutType; @Input() certificate!: Certificate; - @Output() deleteButtonClicked = new EventEmitter(); + @Output() deleteButtonClicked = new EventEmitter(); } diff --git a/modules/ui/src/app/pages/certificates/certificate-upload-button/certificate-upload-button.component.html b/modules/ui/src/app/pages/certificates/certificate-upload-button/certificate-upload-button.component.html index cea3fb1b4..0c1724e55 100644 --- a/modules/ui/src/app/pages/certificates/certificate-upload-button/certificate-upload-button.component.html +++ b/modules/ui/src/app/pages/certificates/certificate-upload-button/certificate-upload-button.component.html @@ -10,4 +10,5 @@ #file_input id="default-file-input" type="file" + accept=".cert,.crt,.pem,.cer" (change)="fileChange($event)" /> diff --git a/modules/ui/src/app/pages/certificates/certificate-upload-button/certificate-upload-button.component.ts b/modules/ui/src/app/pages/certificates/certificate-upload-button/certificate-upload-button.component.ts index 5b06163b4..88cf522b4 100644 --- a/modules/ui/src/app/pages/certificates/certificate-upload-button/certificate-upload-button.component.ts +++ b/modules/ui/src/app/pages/certificates/certificate-upload-button/certificate-upload-button.component.ts @@ -11,8 +11,8 @@ import { MatButtonModule } from '@angular/material/button'; export class CertificateUploadButtonComponent { @Output() fileChanged = new EventEmitter(); fileChange(event: Event) { - const fileList = (event.target as HTMLInputElement).files; - + const input = event.target as HTMLInputElement; + const fileList = input.files; if (fileList && fileList.length < 1) { return; } @@ -21,5 +21,7 @@ export class CertificateUploadButtonComponent { const file: File = fileList[0]; this.fileChanged.emit(file); + input.value = ''; + input.dispatchEvent(new Event('change')); } } diff --git a/modules/ui/src/app/pages/certificates/certificate.validator.ts b/modules/ui/src/app/pages/certificates/certificate.validator.ts new file mode 100644 index 000000000..aaaa7b85d --- /dev/null +++ b/modules/ui/src/app/pages/certificates/certificate.validator.ts @@ -0,0 +1,29 @@ +const FILE_SIZE = 4; +export const FILE_NAME_LENGTH = 24; +const FILE_NAME_REGEXP = new RegExp('^[\\w.-]{1,24}$', 'u'); + +export const getValidationErrors = (file: File): string[] => { + const errors = []; + errors.push(validateFileName(file.name)); + errors.push(validateFileNameLength(file.name)); + errors.push(validateSize(file.size)); + // @ts-expect-error null values are filtered + return errors.filter(error => error != null); +}; +const validateFileName = (name: string): string | null => { + const result = FILE_NAME_REGEXP.test(name); + return !result + ? 'The file name should be alphanumeric, symbols -_. are allowed.' + : null; +}; + +const validateFileNameLength = (name: string): string | null => { + return name.length > FILE_NAME_LENGTH + ? `Max name length is ${FILE_NAME_LENGTH} characters.` + : null; +}; + +const validateSize = (size: number): string | null => { + const result = size > FILE_SIZE * 1000; + return result ? `File size should be a max of ${FILE_SIZE}KB` : null; +}; diff --git a/modules/ui/src/app/pages/certificates/certificates.component.spec.ts b/modules/ui/src/app/pages/certificates/certificates.component.spec.ts index 0a50f6b9d..b73dd05f7 100644 --- a/modules/ui/src/app/pages/certificates/certificates.component.spec.ts +++ b/modules/ui/src/app/pages/certificates/certificates.component.spec.ts @@ -32,6 +32,7 @@ import { MatDialogRef } from '@angular/material/dialog'; import { DeleteFormComponent } from '../../components/delete-form/delete-form.component'; import { TestRunService } from '../../services/test-run.service'; import { NotificationService } from '../../services/notification.service'; +import { FILE, INVALID_FILE } from '../../mocks/certificate.mock'; describe('CertificatesComponent', () => { let component: CertificatesComponent; @@ -43,11 +44,12 @@ describe('CertificatesComponent', () => { jasmine.createSpyObj(['notify']); beforeEach(async () => { - mockService = jasmine.createSpyObj([ + mockService = jasmine.createSpyObj('mockService', [ 'fetchCertificates', 'deleteCertificate', 'uploadCertificate', ]); + mockService.uploadCertificate.and.returnValue(of(true)); mockService.deleteCertificate.and.returnValue(of(true)); mockService.fetchCertificates.and.returnValue( of([certificate, certificate]) @@ -65,6 +67,7 @@ describe('CertificatesComponent', () => { fixture = TestBed.createComponent(CertificatesComponent); component = fixture.componentInstance; fixture.detectChanges(); + mockService.uploadCertificate.calls.reset(); }); it('should create', () => { @@ -127,7 +130,7 @@ describe('CertificatesComponent', () => { } as MatDialogRef); tick(); - component.deleteCertificate(certificate.name); + component.deleteCertificate(certificate); tick(); expect(openSpy).toHaveBeenCalledWith(DeleteFormComponent, { @@ -175,5 +178,19 @@ describe('CertificatesComponent', () => { flush(); })); }); + + describe('#uploadFile', () => { + it('should not call uploadCertificate if file has errors', () => { + component.uploadFile(INVALID_FILE); + + expect(mockService.uploadCertificate).not.toHaveBeenCalled(); + }); + + it('should call uploadCertificate if there is no errors', () => { + component.uploadFile(FILE); + + expect(mockService.uploadCertificate).toHaveBeenCalled(); + }); + }); }); }); diff --git a/modules/ui/src/app/pages/certificates/certificates.component.ts b/modules/ui/src/app/pages/certificates/certificates.component.ts index 0e0b805b1..33623b9fc 100644 --- a/modules/ui/src/app/pages/certificates/certificates.component.ts +++ b/modules/ui/src/app/pages/certificates/certificates.component.ts @@ -24,6 +24,8 @@ import { CertificatesStore } from './certificates.store'; import { DeleteFormComponent } from '../../components/delete-form/delete-form.component'; import { Subject, takeUntil } from 'rxjs'; import { MatDialog } from '@angular/material/dialog'; +import { Certificate } from '../../model/certificate'; +import { FILE_NAME_LENGTH } from './certificate.validator'; @Component({ selector: 'app-certificates', @@ -67,14 +69,14 @@ export class CertificatesComponent implements OnDestroy { this.store.uploadCertificate(file); } - deleteCertificate(name: string) { - this.store.selectCertificate(name); + deleteCertificate(certificate: Certificate) { + this.store.selectCertificate(certificate.name); const dialogRef = this.dialog.open(DeleteFormComponent, { ariaLabel: 'Delete certificate', data: { title: 'Delete certificate', - content: `You are about to delete a certificate ${name}. Are you sure?`, + content: `You are about to delete a certificate ${this.getShortCertificateName(certificate.name)}. Are you sure?`, }, autoFocus: true, hasBackdrop: true, @@ -87,7 +89,7 @@ export class CertificatesComponent implements OnDestroy { .pipe(takeUntil(this.destroy$)) .subscribe(deleteCertificate => { if (deleteCertificate) { - this.store.deleteCertificate(name); + this.store.deleteCertificate(certificate); this.focusNextButton(); } }); @@ -108,4 +110,10 @@ export class CertificatesComponent implements OnDestroy { menuButton?.focus(); } } + + private getShortCertificateName(name: string) { + return name.length <= FILE_NAME_LENGTH + ? name + : `${name.substring(0, FILE_NAME_LENGTH)}...`; + } } diff --git a/modules/ui/src/app/pages/certificates/certificates.store.spec.ts b/modules/ui/src/app/pages/certificates/certificates.store.spec.ts index f42b9e928..42744178f 100644 --- a/modules/ui/src/app/pages/certificates/certificates.store.spec.ts +++ b/modules/ui/src/app/pages/certificates/certificates.store.spec.ts @@ -22,6 +22,9 @@ import { certificate, certificate2, certificate_uploading, + certificate_uploading_with_errors, + FILE, + INVALID_FILE, } from '../../mocks/certificate.mock'; import { CertificatesStore } from './certificates.store'; import { NoopAnimationsModule } from '@angular/platform-browser/animations'; @@ -111,33 +114,54 @@ describe('CertificatesStore', () => { }); describe('uploadCertificate', () => { - const uploadingCertificate = certificate_uploading; - beforeEach(() => { mockService.uploadCertificate.and.returnValue(of(true)); mockService.fetchCertificates.and.returnValue(of([certificate])); }); - it('should update certificates', done => { - certificateStore.viewModel$.pipe(skip(1), take(1)).subscribe(store => { - expect(store.certificates).toContain(uploadingCertificate); - }); + describe('with valid certificate file', () => { + it('should update certificates', done => { + const uploadingCertificate = certificate_uploading; - certificateStore.viewModel$.pipe(skip(2), take(1)).subscribe(store => { - expect(store.certificates).toEqual([certificate]); - done(); + certificateStore.viewModel$ + .pipe(skip(1), take(1)) + .subscribe(store => { + expect(store.certificates).toContain(uploadingCertificate); + }); + + certificateStore.viewModel$ + .pipe(skip(2), take(1)) + .subscribe(store => { + expect(store.certificates).toEqual([certificate]); + done(); + }); + + certificateStore.uploadCertificate(FILE); }); - certificateStore.uploadCertificate(new File([], 'test')); + it('should notify', () => { + certificateStore.uploadCertificate(FILE); + expect(notificationServiceMock.notify).toHaveBeenCalledWith( + 'Certificate successfully added.\niot.bms.google.com by Google, Inc. valid until 01 Sep 2024', + 0, + 'certificate-notification' + ); + }); }); - it('should notify', () => { - certificateStore.uploadCertificate(new File([], 'test')); - expect(notificationServiceMock.notify).toHaveBeenCalledWith( - 'Certificate successfully added.\niot.bms.google.com by Google, Inc. valid until 01 Sep 2024', - 0, - 'certificate-notification' - ); + describe('with invalid certificate file', () => { + it('should update certificates', done => { + const uploadingCertificate = certificate_uploading_with_errors; + + certificateStore.viewModel$ + .pipe(skip(1), take(1)) + .subscribe(store => { + expect(store.certificates).toContain(uploadingCertificate); + done(); + }); + + certificateStore.uploadCertificate(INVALID_FILE); + }); }); }); @@ -152,7 +176,21 @@ describe('CertificatesStore', () => { done(); }); - certificateStore.deleteCertificate('iot.bms.google.com'); + certificateStore.deleteCertificate(certificate); + }); + + it('should update store with no request to server when certificate is uploading', done => { + certificateStore.updateCertificates([ + certificate_uploading, + certificate2, + ]); + + certificateStore.viewModel$.pipe(skip(1), take(1)).subscribe(store => { + expect(store.certificates).toEqual([certificate2]); + done(); + }); + + certificateStore.deleteCertificate(certificate_uploading); }); }); }); diff --git a/modules/ui/src/app/pages/certificates/certificates.store.ts b/modules/ui/src/app/pages/certificates/certificates.store.ts index 1def48617..14ea7c7c4 100644 --- a/modules/ui/src/app/pages/certificates/certificates.store.ts +++ b/modules/ui/src/app/pages/certificates/certificates.store.ts @@ -16,12 +16,13 @@ import { Injectable } from '@angular/core'; import { ComponentStore } from '@ngrx/component-store'; -import { tap, withLatestFrom } from 'rxjs/operators'; -import { catchError, EMPTY, exhaustMap, throwError } from 'rxjs'; +import { switchMap, tap, withLatestFrom } from 'rxjs/operators'; +import { catchError, EMPTY, exhaustMap, of, throwError } from 'rxjs'; import { Certificate } from '../../model/certificate'; import { TestRunService } from '../../services/test-run.service'; import { NotificationService } from '../../services/notification.service'; import { DatePipe } from '@angular/common'; +import { getValidationErrors } from './certificate.validator'; export interface AppComponentState { certificates: Certificate[]; @@ -64,8 +65,14 @@ export class CertificatesStore extends ComponentStore { uploadCertificate = this.effect(trigger$ => { return trigger$.pipe( withLatestFrom(this.certificates$), - tap(([file, certificates]) => { - this.addCertificate(file.name, certificates); + switchMap(res => { + const [file, certificates] = res; + const errors = getValidationErrors(file); + this.addCertificate(file.name, certificates, errors); + if (errors.length > 0) { + return EMPTY; + } + return of(res); }), exhaustMap(([file, certificates]) => { return this.testRunService.uploadCertificate(file).pipe( @@ -88,7 +95,6 @@ export class CertificatesStore extends ComponentStore { ); }), catchError(() => { - this.removeCertificate(file.name, certificates); return EMPTY; }) ); @@ -96,20 +102,31 @@ export class CertificatesStore extends ComponentStore { ); }); - addCertificate(name: string, certificates: Certificate[]) { - const certificate = { name, uploading: true } as Certificate; + addCertificate( + name: string, + certificates: Certificate[], + errors: string[] = [] + ) { + const certificate = { name, uploading: true, errors } as Certificate; this.updateCertificates([certificate, ...certificates]); } - deleteCertificate = this.effect(trigger$ => { + deleteCertificate = this.effect(trigger$ => { return trigger$.pipe( - exhaustMap((name: string) => { - return this.testRunService.deleteCertificate(name).pipe( - withLatestFrom(this.certificates$), - tap(([remove, current]) => { + withLatestFrom(this.certificates$), + exhaustMap(([certificate, current]) => { + if (certificate.uploading) { + this.removeCertificate(certificate.name, current); + return EMPTY; + } + return this.testRunService.deleteCertificate(certificate.name).pipe( + tap(remove => { if (remove) { - this.removeCertificate(name, current); + this.removeCertificate(certificate.name, current); } + }), + catchError(() => { + return EMPTY; }) ); }) From eb2adad8e6465d92cf467ef5e9d661b1d0d02f57 Mon Sep 17 00:00:00 2001 From: kurilova Date: Fri, 17 May 2024 12:56:21 +0000 Subject: [PATCH 2/4] Fix issues --- .../certificate-item/certificate-item.component.spec.ts | 2 +- modules/ui/src/app/pages/certificates/certificate.validator.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.spec.ts b/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.spec.ts index 6ef255664..754bca694 100644 --- a/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.spec.ts +++ b/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.spec.ts @@ -114,7 +114,7 @@ describe('CertificateItemComponent', () => { expect(loader).toBeNull(); }); - it('should have disabled delete button', () => { + it('should not have disabled delete button', () => { const deleteButton = fixture.nativeElement.querySelector( '.certificate-item-delete' ) as HTMLButtonElement; diff --git a/modules/ui/src/app/pages/certificates/certificate.validator.ts b/modules/ui/src/app/pages/certificates/certificate.validator.ts index aaaa7b85d..5cb260e09 100644 --- a/modules/ui/src/app/pages/certificates/certificate.validator.ts +++ b/modules/ui/src/app/pages/certificates/certificate.validator.ts @@ -8,7 +8,7 @@ export const getValidationErrors = (file: File): string[] => { errors.push(validateFileNameLength(file.name)); errors.push(validateSize(file.size)); // @ts-expect-error null values are filtered - return errors.filter(error => error != null); + return errors.filter(error => error !== null); }; const validateFileName = (name: string): string | null => { const result = FILE_NAME_REGEXP.test(name); From 121f4c45ca1dbde2720f0991cb7ce7e469f75ce4 Mon Sep 17 00:00:00 2001 From: kurilova Date: Tue, 21 May 2024 12:14:43 +0000 Subject: [PATCH 3/4] Change error display - show notification --- modules/ui/src/app/mocks/certificate.mock.ts | 19 +++------- modules/ui/src/app/model/certificate.ts | 1 - .../certificate-item.component.html | 11 ++---- .../certificate-item.component.spec.ts | 31 +--------------- .../certificate-item.component.ts | 4 +-- .../certificates/certificate.validator.ts | 7 ++++ .../certificates.component.spec.ts | 2 +- .../certificates/certificates.component.ts | 7 ++-- .../certificates/certificates.store.spec.ts | 34 +++++------------- .../pages/certificates/certificates.store.ts | 36 +++++++++---------- 10 files changed, 44 insertions(+), 108 deletions(-) diff --git a/modules/ui/src/app/mocks/certificate.mock.ts b/modules/ui/src/app/mocks/certificate.mock.ts index 1723b48b4..278dc8ec0 100644 --- a/modules/ui/src/app/mocks/certificate.mock.ts +++ b/modules/ui/src/app/mocks/certificate.mock.ts @@ -22,33 +22,22 @@ export const certificate = { } as Certificate; export const certificate_uploading = { - name: 'name', + name: 'name.cert', uploading: true, - errors: [], -} as Certificate; - -export const certificate_uploading_with_errors = { - name: 'some very long strange name with symbols!?', - uploading: true, - errors: [ - 'The file name should be alphanumeric, symbols -_. are allowed.', - 'Max name length is 24 characters.', - 'File size should be a max of 4KB', - ], } as Certificate; export const certificate2 = { - name: 'sensor.bms.google.com', + name: 'sensor.bms.google.com.cert', organisation: 'Google, Inc.', expires: '2024-09-01T09:00:12Z', } as Certificate; export const INVALID_FILE = { - name: 'some very long strange name with symbols!?', + name: 'some very long strange name with symbols!?.jpg', size: 7000, } as File; export const FILE = { - name: 'name', + name: 'name.cert', size: 3000, } as File; diff --git a/modules/ui/src/app/model/certificate.ts b/modules/ui/src/app/model/certificate.ts index 1589b8edb..384120cf7 100644 --- a/modules/ui/src/app/model/certificate.ts +++ b/modules/ui/src/app/model/certificate.ts @@ -18,5 +18,4 @@ export interface Certificate { organisation?: string; expires?: string; uploading?: boolean; - errors?: string[]; } diff --git a/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.html b/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.html index 8afc95c64..eeb4fb6c8 100644 --- a/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.html +++ b/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.html @@ -11,22 +11,15 @@ {{ certificate.expires | date: 'dd MMM yyyy' }}

- - {{ - error - }} - diff --git a/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.spec.ts b/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.spec.ts index 754bca694..0cb73f90f 100644 --- a/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.spec.ts +++ b/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.spec.ts @@ -4,7 +4,6 @@ import { CertificateItemComponent } from './certificate-item.component'; import { certificate, certificate_uploading, - certificate_uploading_with_errors, } from '../../../mocks/certificate.mock'; describe('CertificateItemComponent', () => { @@ -70,7 +69,7 @@ describe('CertificateItemComponent', () => { const deleteSpy = spyOn(component.deleteButtonClicked, 'emit'); deleteButton.click(); - expect(deleteSpy).toHaveBeenCalledWith(certificate); + expect(deleteSpy).toHaveBeenCalledWith(certificate.name); }); }); @@ -101,33 +100,5 @@ describe('CertificateItemComponent', () => { expect(deleteButton.getAttribute('disabled')).toBeTruthy(); }); }); - - describe('uploading certificate with errors', () => { - beforeEach(() => { - component.certificate = certificate_uploading_with_errors; - fixture.detectChanges(); - }); - - it('should not have loader', () => { - const loader = compiled.querySelector('mat-progress-bar'); - - expect(loader).toBeNull(); - }); - - it('should not have disabled delete button', () => { - const deleteButton = fixture.nativeElement.querySelector( - '.certificate-item-delete' - ) as HTMLButtonElement; - - expect(deleteButton.getAttribute('disabled')).toBeFalsy(); - }); - - it('should have errors', () => { - const errors = - fixture.nativeElement.querySelectorAll('app-callout span'); - - expect(errors.length).toEqual(3); - }); - }); }); }); diff --git a/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.ts b/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.ts index 605779ff1..fc9eac300 100644 --- a/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.ts +++ b/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.ts @@ -7,7 +7,6 @@ import { MatProgressBarModule } from '@angular/material/progress-bar'; import { provideAnimations } from '@angular/platform-browser/animations'; import { CalloutComponent } from '../../../components/callout/callout.component'; import { MatError } from '@angular/material/form-field'; -import { CalloutType } from '../../../model/callout-type'; @Component({ selector: 'app-certificate-item', @@ -25,7 +24,6 @@ import { CalloutType } from '../../../model/callout-type'; styleUrl: './certificate-item.component.scss', }) export class CertificateItemComponent { - public readonly CalloutType = CalloutType; @Input() certificate!: Certificate; - @Output() deleteButtonClicked = new EventEmitter(); + @Output() deleteButtonClicked = new EventEmitter(); } diff --git a/modules/ui/src/app/pages/certificates/certificate.validator.ts b/modules/ui/src/app/pages/certificates/certificate.validator.ts index 5cb260e09..c9445f16d 100644 --- a/modules/ui/src/app/pages/certificates/certificate.validator.ts +++ b/modules/ui/src/app/pages/certificates/certificate.validator.ts @@ -1,10 +1,12 @@ const FILE_SIZE = 4; export const FILE_NAME_LENGTH = 24; const FILE_NAME_REGEXP = new RegExp('^[\\w.-]{1,24}$', 'u'); +const FILE_EXT_REGEXP = new RegExp('(\\.cert|\\.crt|\\.pem|\\.cer)$', 'i'); export const getValidationErrors = (file: File): string[] => { const errors = []; errors.push(validateFileName(file.name)); + errors.push(validateExtension(file.name)); errors.push(validateFileNameLength(file.name)); errors.push(validateSize(file.size)); // @ts-expect-error null values are filtered @@ -17,6 +19,11 @@ const validateFileName = (name: string): string | null => { : null; }; +const validateExtension = (name: string): string | null => { + const result = FILE_EXT_REGEXP.test(name); + return !result ? 'File extension must be .cert, .crt, .pem, .cer.' : null; +}; + const validateFileNameLength = (name: string): string | null => { return name.length > FILE_NAME_LENGTH ? `Max name length is ${FILE_NAME_LENGTH} characters.` diff --git a/modules/ui/src/app/pages/certificates/certificates.component.spec.ts b/modules/ui/src/app/pages/certificates/certificates.component.spec.ts index b73dd05f7..3f611d962 100644 --- a/modules/ui/src/app/pages/certificates/certificates.component.spec.ts +++ b/modules/ui/src/app/pages/certificates/certificates.component.spec.ts @@ -130,7 +130,7 @@ describe('CertificatesComponent', () => { } as MatDialogRef); tick(); - component.deleteCertificate(certificate); + component.deleteCertificate(certificate.name); tick(); expect(openSpy).toHaveBeenCalledWith(DeleteFormComponent, { diff --git a/modules/ui/src/app/pages/certificates/certificates.component.ts b/modules/ui/src/app/pages/certificates/certificates.component.ts index 33623b9fc..3a12c4c14 100644 --- a/modules/ui/src/app/pages/certificates/certificates.component.ts +++ b/modules/ui/src/app/pages/certificates/certificates.component.ts @@ -24,7 +24,6 @@ import { CertificatesStore } from './certificates.store'; import { DeleteFormComponent } from '../../components/delete-form/delete-form.component'; import { Subject, takeUntil } from 'rxjs'; import { MatDialog } from '@angular/material/dialog'; -import { Certificate } from '../../model/certificate'; import { FILE_NAME_LENGTH } from './certificate.validator'; @Component({ @@ -69,14 +68,14 @@ export class CertificatesComponent implements OnDestroy { this.store.uploadCertificate(file); } - deleteCertificate(certificate: Certificate) { - this.store.selectCertificate(certificate.name); + deleteCertificate(certificate: string) { + this.store.selectCertificate(certificate); const dialogRef = this.dialog.open(DeleteFormComponent, { ariaLabel: 'Delete certificate', data: { title: 'Delete certificate', - content: `You are about to delete a certificate ${this.getShortCertificateName(certificate.name)}. Are you sure?`, + content: `You are about to delete a certificate ${this.getShortCertificateName(certificate)}. Are you sure?`, }, autoFocus: true, hasBackdrop: true, diff --git a/modules/ui/src/app/pages/certificates/certificates.store.spec.ts b/modules/ui/src/app/pages/certificates/certificates.store.spec.ts index 42744178f..624f50638 100644 --- a/modules/ui/src/app/pages/certificates/certificates.store.spec.ts +++ b/modules/ui/src/app/pages/certificates/certificates.store.spec.ts @@ -22,7 +22,6 @@ import { certificate, certificate2, certificate_uploading, - certificate_uploading_with_errors, FILE, INVALID_FILE, } from '../../mocks/certificate.mock'; @@ -150,17 +149,14 @@ describe('CertificatesStore', () => { }); describe('with invalid certificate file', () => { - it('should update certificates', done => { - const uploadingCertificate = certificate_uploading_with_errors; - - certificateStore.viewModel$ - .pipe(skip(1), take(1)) - .subscribe(store => { - expect(store.certificates).toContain(uploadingCertificate); - done(); - }); - + it('should notify about errors', () => { certificateStore.uploadCertificate(INVALID_FILE); + + expect(notificationServiceMock.notify).toHaveBeenCalledWith( + 'The file name should be alphanumeric, symbols -_. are allowed.\nFile extension must be .cert, .crt, .pem, .cer.\nMax name length is 24 characters.\nFile size should be a max of 4KB', + 0, + 'certificate-notification' + ); }); }); }); @@ -176,21 +172,7 @@ describe('CertificatesStore', () => { done(); }); - certificateStore.deleteCertificate(certificate); - }); - - it('should update store with no request to server when certificate is uploading', done => { - certificateStore.updateCertificates([ - certificate_uploading, - certificate2, - ]); - - certificateStore.viewModel$.pipe(skip(1), take(1)).subscribe(store => { - expect(store.certificates).toEqual([certificate2]); - done(); - }); - - certificateStore.deleteCertificate(certificate_uploading); + certificateStore.deleteCertificate(certificate.name); }); }); }); diff --git a/modules/ui/src/app/pages/certificates/certificates.store.ts b/modules/ui/src/app/pages/certificates/certificates.store.ts index 14ea7c7c4..762671eb5 100644 --- a/modules/ui/src/app/pages/certificates/certificates.store.ts +++ b/modules/ui/src/app/pages/certificates/certificates.store.ts @@ -66,14 +66,18 @@ export class CertificatesStore extends ComponentStore { return trigger$.pipe( withLatestFrom(this.certificates$), switchMap(res => { - const [file, certificates] = res; + const [file] = res; const errors = getValidationErrors(file); - this.addCertificate(file.name, certificates, errors); if (errors.length > 0) { + this.notify(errors.join('\n')); return EMPTY; } return of(res); }), + tap(res => { + const [file, certificates] = res; + this.addCertificate(file.name, certificates); + }), exhaustMap(([file, certificates]) => { return this.testRunService.uploadCertificate(file).pipe( exhaustMap(uploaded => { @@ -88,10 +92,8 @@ export class CertificatesStore extends ComponentStore { !certificates.some(cert => cert.name === certificate.name) )[0]; this.updateCertificates(newCertificates); - this.notificationService.notify( - `Certificate successfully added.\n${uploadedCertificate.name} by ${uploadedCertificate.organisation} valid until ${this.datePipe.transform(uploadedCertificate.expires, 'dd MMM yyyy')}`, - 0, - 'certificate-notification' + this.notify( + `Certificate successfully added.\n${uploadedCertificate.name} by ${uploadedCertificate.organisation} valid until ${this.datePipe.transform(uploadedCertificate.expires, 'dd MMM yyyy')}` ); }), catchError(() => { @@ -102,27 +104,19 @@ export class CertificatesStore extends ComponentStore { ); }); - addCertificate( - name: string, - certificates: Certificate[], - errors: string[] = [] - ) { - const certificate = { name, uploading: true, errors } as Certificate; + addCertificate(name: string, certificates: Certificate[]) { + const certificate = { name, uploading: true } as Certificate; this.updateCertificates([certificate, ...certificates]); } - deleteCertificate = this.effect(trigger$ => { + deleteCertificate = this.effect(trigger$ => { return trigger$.pipe( withLatestFrom(this.certificates$), exhaustMap(([certificate, current]) => { - if (certificate.uploading) { - this.removeCertificate(certificate.name, current); - return EMPTY; - } - return this.testRunService.deleteCertificate(certificate.name).pipe( + return this.testRunService.deleteCertificate(certificate).pipe( tap(remove => { if (remove) { - this.removeCertificate(certificate.name, current); + this.removeCertificate(certificate, current); } }), catchError(() => { @@ -133,6 +127,10 @@ export class CertificatesStore extends ComponentStore { ); }); + private notify(message: string) { + this.notificationService.notify(message, 0, 'certificate-notification'); + } + private removeCertificate(name: string, current: Certificate[]) { const certificates = current.filter( certificate => certificate.name !== name From 5d8af236e418bc28d06a2460b8abbb3f2160291f Mon Sep 17 00:00:00 2001 From: kurilova Date: Tue, 21 May 2024 12:42:39 +0000 Subject: [PATCH 4/4] Remove unused code --- .../certificate-item.component.scss | 17 ----------------- .../certificate-item.component.spec.ts | 6 ------ .../certificate-item.component.ts | 2 -- 3 files changed, 25 deletions(-) diff --git a/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.scss b/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.scss index dc89959cf..db2a2fb9c 100644 --- a/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.scss +++ b/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.scss @@ -26,23 +26,6 @@ border-top: 1px solid $lighter-grey; } -:host app-callout { - grid-column: span 3; - ::ng-deep .callout-container { - background-color: $red-50; - } - ::ng-deep .callout-container.error_outline .callout-context { - font-weight: normal; - } - ::ng-deep .callout-container.error_outline .callout-icon { - color: $red-700; - } -} - -.error-message-text { - display: inline-block; -} - .certificate-item-container { display: grid; grid-template-columns: 24px minmax(200px, 1fr) 24px; diff --git a/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.spec.ts b/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.spec.ts index 0cb73f90f..5840aa2bb 100644 --- a/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.spec.ts +++ b/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.spec.ts @@ -72,12 +72,6 @@ describe('CertificateItemComponent', () => { expect(deleteSpy).toHaveBeenCalledWith(certificate.name); }); }); - - it('should not display errors', () => { - const errors = fixture.nativeElement.querySelectorAll('app-callout'); - - expect(errors.length).toEqual(0); - }); }); describe('uploading certificate', () => { diff --git a/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.ts b/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.ts index fc9eac300..eb6019b98 100644 --- a/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.ts +++ b/modules/ui/src/app/pages/certificates/certificate-item/certificate-item.component.ts @@ -5,7 +5,6 @@ import { CommonModule } from '@angular/common'; import { MatButtonModule } from '@angular/material/button'; import { MatProgressBarModule } from '@angular/material/progress-bar'; import { provideAnimations } from '@angular/platform-browser/animations'; -import { CalloutComponent } from '../../../components/callout/callout.component'; import { MatError } from '@angular/material/form-field'; @Component({ @@ -16,7 +15,6 @@ import { MatError } from '@angular/material/form-field'; MatButtonModule, MatProgressBarModule, CommonModule, - CalloutComponent, MatError, ], providers: [provideAnimations()],