From 03b05f843f62ff7a5eeef2c56d1202db16109797 Mon Sep 17 00:00:00 2001 From: paf <paf@titelfrei.de> Date: Thu, 23 Jul 2020 16:24:17 +0200 Subject: [PATCH] error handling for getBooklet --- src/app/group-monitor/backend.service.ts | 29 ++-- src/app/group-monitor/booklet.service.ts | 28 ++-- .../group-monitor/group-monitor.interfaces.ts | 4 + .../test-view/test-view.component.css | 5 + .../test-view/test-view.component.html | 124 ++++++++++-------- .../test-view/test-view.component.ts | 24 ++-- 6 files changed, 131 insertions(+), 83 deletions(-) diff --git a/src/app/group-monitor/backend.service.ts b/src/app/group-monitor/backend.service.ts index 2b7aed00..a9d135f2 100644 --- a/src/app/group-monitor/backend.service.ts +++ b/src/app/group-monitor/backend.service.ts @@ -1,9 +1,10 @@ import {Injectable} from '@angular/core'; import {Observable, of} from 'rxjs'; import {catchError} from 'rxjs/operators'; -import {GroupData, TestSession} from './group-monitor.interfaces'; +import {BookletError, GroupData, TestSession} from './group-monitor.interfaces'; import {WebsocketBackendService} from './websocket-backend.service'; import {HttpHeaders} from '@angular/common/http'; +import {ApiError} from '../app.interfaces'; @Injectable() export class BackendService extends WebsocketBackendService<TestSession[]> { @@ -20,20 +21,30 @@ export class BackendService extends WebsocketBackendService<TestSession[]> { } - public getBooklet(bookletName: string): Observable<string> { + public getBooklet(bookletName: string): Observable<string|BookletError> { console.log("load booklet for " + bookletName); const headers = new HttpHeaders({ 'Content-Type': 'text/xml' }).set('Accept', 'text/xml'); + const missingFileError: BookletError = {error: 'missing-file'}; + const generalError: BookletError = {error: 'general'}; + return this.http .get(this.serverUrl + `booklet/${bookletName}`, {headers, responseType: 'text'}) - // .pipe( // TODO useful error handling - // catchError((err: ApiError) => { - // console.warn(`getTestData Api-Error: ${err.code} ${err.info}`); - // return of(false) - // }) - // ); + .pipe( // TODO useful error handling + catchError((err: ApiError) => { + console.warn(`getTestData Api-Error: ${err.code} ${err.info}`); + if (err.code === 404) { + // could potentially happen when booklet file was removed since test was started + // TODO interceptor + return of(missingFileError); + } else { + // TODO interceptor should have interfered and moved to error-page https://github.com/iqb-berlin/testcenter-frontend/issues/53 + return of(generalError); + } + }) + ); } @@ -45,7 +56,7 @@ export class BackendService extends WebsocketBackendService<TestSession[]> { return of(<GroupData>{ name: groupName, label: groupName, - }) + }); })); } } diff --git a/src/app/group-monitor/booklet.service.ts b/src/app/group-monitor/booklet.service.ts index 267a60dc..55cf50a1 100644 --- a/src/app/group-monitor/booklet.service.ts +++ b/src/app/group-monitor/booklet.service.ts @@ -4,7 +4,7 @@ import {BackendService} from './backend.service'; import {Observable, of} from 'rxjs'; import {isDefined} from '@angular/compiler/src/util'; import {map, shareReplay} from 'rxjs/operators'; -import {Booklet, BookletMetadata, Restrictions, Testlet, Unit} from './group-monitor.interfaces'; +import {Booklet, BookletError, BookletMetadata, Restrictions, Testlet, Unit} from './group-monitor.interfaces'; import {BookletConfig} from '../config/booklet-config'; @@ -12,7 +12,7 @@ import {BookletConfig} from '../config/booklet-config'; export class BookletService { - public booklets: Observable<Booklet>[] = []; + public booklets: Observable<Booklet|BookletError>[] = []; constructor( @@ -20,7 +20,7 @@ export class BookletService { ) { } - public getBooklet(bookletName: string): Observable<Booklet|boolean> { + public getBooklet(bookletName: string): Observable<Booklet|BookletError> { if (isDefined(this.booklets[bookletName])) { @@ -31,22 +31,26 @@ export class BookletService { if (bookletName == "") { // console.log("EMPTY bookletID"); - this.booklets[bookletName] = of<Booklet|boolean>(false); + this.booklets[bookletName] = of<Booklet|BookletError>({error: 'missing-id'}); } else { // console.log('LOADING testletOrUnit data for ' + bookletName + ' not available. loading'); this.booklets[bookletName] = this.bs.getBooklet(bookletName) - .pipe(map(BookletService.parseBookletXml)) - .pipe(shareReplay(1)); + .pipe( + map((response: string|BookletError) => { + return (typeof response === 'string') ? BookletService.parseBookletXml(response) : response; + }), + shareReplay(1) + ); } return this.booklets[bookletName]; } - private static parseBookletXml(xmlString: string): Booklet|boolean { + private static parseBookletXml(xmlString: string): Booklet|BookletError { try { @@ -54,7 +58,8 @@ export class BookletService { const bookletElement = domParser.parseFromString(xmlString, 'text/xml').documentElement; if (bookletElement.nodeName !== 'Booklet') { - throw new Error("XML is not Booklet"); + console.warn('XML-root is not `Booklet`'); + return {error: 'xml'}; } return { @@ -65,9 +70,8 @@ export class BookletService { } catch (error) { - console.log('error reading booklet XML:'); - console.log(error); - return false; + console.warn('Error reading booklet XML:', error); + return {error: 'xml'}; } } @@ -167,7 +171,7 @@ export class BookletService { const elements = BookletService.xmlGetDirectChildrenByTagName(element, [childName]); if (!elements.length && !isOptional) { - throw new Error(`Missing field: '${childName}'`); // TODO hierauf wird irgendwie gar nicht reagiert + throw new Error(`Missing field: '${childName}'`); } return elements.length ? elements[0] : null; } diff --git a/src/app/group-monitor/group-monitor.interfaces.ts b/src/app/group-monitor/group-monitor.interfaces.ts index 0146f769..d7726f09 100644 --- a/src/app/group-monitor/group-monitor.interfaces.ts +++ b/src/app/group-monitor/group-monitor.interfaces.ts @@ -26,6 +26,10 @@ export interface Booklet { units: Testlet; } +export interface BookletError { + error: 'xml' | 'missing-id' | 'missing-file' | 'general', +} + export interface BookletMetadata { id: string; label: string; diff --git a/src/app/group-monitor/test-view/test-view.component.css b/src/app/group-monitor/test-view/test-view.component.css index 671edf14..90de49c4 100644 --- a/src/app/group-monitor/test-view/test-view.component.css +++ b/src/app/group-monitor/test-view/test-view.component.css @@ -100,3 +100,8 @@ mat-icon + h1 { vertical-align: middle; align-items: center; } + +.warning { + color: red; + font-weight: bold +} diff --git a/src/app/group-monitor/test-view/test-view.component.html b/src/app/group-monitor/test-view/test-view.component.html index 617186c8..0facbcaf 100644 --- a/src/app/group-monitor/test-view/test-view.component.html +++ b/src/app/group-monitor/test-view/test-view.component.html @@ -17,59 +17,77 @@ <td *ngIf="displayOptions.groupColumn === 'show'"><div class="vertical-align-middle">{{testStatus.groupLabel}}</div></td> -<ng-container *ngIf="(booklet$ | async) as booklet; else: noBooklet"> - - <td class="booklet"> - - <div class="vertical-align-middle"> - - <h1>{{booklet.metadata.label}}</h1> - - <mat-icon class="unit-badge" - *ngIf="hasState(testStatus.testState, 'status', 'locked')" - matTooltip="Testheft gesperrt" - >lock - </mat-icon> - - <mat-icon class="unit-badge" - *ngIf="!hasState(testStatus.testState, 'status', 'locked')" - matTooltip="Testheft gesperrt" - >lock_open - </mat-icon> - - <mat-icon class="unit-badge" - *ngIf="hasState(testStatus.testState, 'BOOKLETLOCKEDbyTESTEE')" - matTooltip="Testheft abgeschlossen" - >done_all - </mat-icon> - </div> - </td> - - <td *ngIf="booklet.units as testlet"> - - <div - class="units-container" - [class]="hasState(testStatus.testState, 'status', 'locked') ? 'locked' : ''" - [ngSwitch]="displayOptions.view" - > - <div class="units full" *ngSwitchCase="'full'" > - <ng-container *ngTemplateOutlet="testletTemplate; context: {$implicit: testlet}"></ng-container> - </div> - - <div class="units medium" *ngSwitchCase="'medium'" > - <ng-container *ngTemplateOutlet="bookletTemplateMedium; context: {$implicit: testlet}"></ng-container> - </div> - - <div class="units small" *ngSwitchCase="'small'" > - <ng-container *ngTemplateOutlet="testletBookletSmall; context: {$implicit: testlet}"></ng-container> - </div> - </div> +<ng-container *ngIf="(booklet$ | async) as booklet"> + + <ng-container *ngIf="isBooklet(booklet); else: noBooklet"> + + <td class="booklet" > + + <div class="vertical-align-middle"> + + <h1>{{booklet.metadata.label}}</h1> + + <mat-icon class="unit-badge" + *ngIf="hasState(testStatus.testState, 'status', 'locked')" + matTooltip="Testheft gesperrt" + >lock + </mat-icon> + + <mat-icon class="unit-badge" + *ngIf="!hasState(testStatus.testState, 'status', 'locked')" + matTooltip="Testheft gesperrt" + >lock_open + </mat-icon> + + <mat-icon class="unit-badge" + *ngIf="hasState(testStatus.testState, 'BOOKLETLOCKEDbyTESTEE')" + matTooltip="Testheft abgeschlossen" + >done_all + </mat-icon> + </div> + </td> + + <td *ngIf="booklet.units as testlet"> + + <div + class="units-container" + [class]="hasState(testStatus.testState, 'status', 'locked') ? 'locked' : ''" + [ngSwitch]="displayOptions.view" + > + <div class="units full" *ngSwitchCase="'full'" > + <ng-container *ngTemplateOutlet="testletTemplate; context: {$implicit: testlet}"></ng-container> + </div> + + <div class="units medium" *ngSwitchCase="'medium'" > + <ng-container *ngTemplateOutlet="bookletTemplateMedium; context: {$implicit: testlet}"></ng-container> + </div> + + <div class="units small" *ngSwitchCase="'small'" > + <ng-container *ngTemplateOutlet="testletBookletSmall; context: {$implicit: testlet}"></ng-container> + </div> + </div> + + <ng-container *ngIf="displayOptions.view === 'full'"> + <ng-container *ngTemplateOutlet="featuredUnit"></ng-container> + </ng-container> - <ng-container *ngIf="displayOptions.view === 'full'"> - <ng-container *ngTemplateOutlet="featuredUnit"></ng-container> - </ng-container> + </td> + </ng-container> - </td> + <ng-template #noBooklet> + <td colspan="2" class="booklet"> + <span *ngIf="booklet.error == 'missing-id'">Kein Testheft zugeordnet</span> + <span *ngIf="booklet.error == 'missing-file'" class="warning"> + Kein Zugriff auf Testheft-Datei (`{{testStatus.bookletName}}`)! + </span> + <span *ngIf="booklet.error == 'xml'" class="warning"> + Konnte Testheft-Datei (`{{testStatus.bookletName}}`) lesen! + </span> + <span *ngIf="booklet.error == 'general'" class="warning">> + Fehler beim Zugriff aus Testheft-Datei (`{{testStatus.bookletName}}`)! + </span> + </td> + </ng-template> </ng-container> <ng-template #featuredUnit> @@ -196,6 +214,4 @@ </ng-template> -<ng-template #noBooklet> - <td colspan="2" class="booklet">Noch kein Test gestarted.</td> -</ng-template> + diff --git a/src/app/group-monitor/test-view/test-view.component.ts b/src/app/group-monitor/test-view/test-view.component.ts index 5d0268a9..e5e547a1 100644 --- a/src/app/group-monitor/test-view/test-view.component.ts +++ b/src/app/group-monitor/test-view/test-view.component.ts @@ -1,7 +1,7 @@ import {Component, Input, OnChanges, OnDestroy, OnInit, SimpleChange} from '@angular/core'; import {BookletService} from '../booklet.service'; import {combineLatest, Observable, Subject, Subscription} from 'rxjs'; -import {Booklet, TestSession, Testlet, Unit, TestViewDisplayOptions} from '../group-monitor.interfaces'; +import {Booklet, TestSession, Testlet, Unit, TestViewDisplayOptions, BookletError} from '../group-monitor.interfaces'; import {map} from 'rxjs/operators'; import {TestMode} from '../../config/test-mode'; @@ -11,6 +11,8 @@ function isUnit(testletOrUnit: Testlet|Unit): testletOrUnit is Unit { return !('children' in testletOrUnit); } + + interface UnitContext { unit?: Unit, parent?: Testlet, @@ -34,7 +36,7 @@ export class TestViewComponent implements OnInit, OnDestroy, OnChanges { @Input() displayOptions: TestViewDisplayOptions; private testStatus$: Subject<TestSession>; - public booklet$: Observable<boolean|Booklet>; + public booklet$: Observable<Booklet|BookletError>; public featuredUnit$: Observable<UnitContext|null>; private childrenSubscription: Subscription; @@ -55,21 +57,21 @@ export class TestViewComponent implements OnInit, OnDestroy, OnChanges { this.booklet$ = this.bookletsService.getBooklet(this.testStatus.bookletName || ""); - this.childrenSubscription = this.booklet$.subscribe((booklet: Booklet|boolean) => { + this.childrenSubscription = this.booklet$.subscribe((booklet: Booklet|BookletError) => { // TODO kann/soll das mit in dem unteren stehen? - if ((booklet !== null) && (booklet !== true) && (booklet !== false)) { + if (this.isBooklet(booklet)) { this.units = booklet.units.children; } }); - this.featuredUnit$ = combineLatest<[Booklet|null, TestSession]>([this.booklet$, this.testStatus$]) - .pipe(map((bookletAndStatus: [Booklet|boolean, TestSession]) => { + this.featuredUnit$ = combineLatest<[Booklet|BookletError, TestSession]>([this.booklet$, this.testStatus$]) + .pipe(map((bookletAndStatus: [Booklet|BookletError, TestSession]) => { console.log("MAP"); - const booklet: Booklet|boolean = bookletAndStatus[0]; + const booklet: Booklet|BookletError = bookletAndStatus[0]; - if ((booklet === true) || (booklet === false)) { + if (!this.isBooklet(booklet)) { return null; } @@ -93,6 +95,12 @@ export class TestViewComponent implements OnInit, OnDestroy, OnChanges { } + isBooklet(bookletOrBookletError: Booklet|BookletError): bookletOrBookletError is Booklet { + + return !('error' in bookletOrBookletError); + } + + getTestletType(testletOrUnit: Unit|Testlet): 'testlet'|'unit' { return isUnit(testletOrUnit) ? 'unit': 'testlet'; -- GitLab