Skip to content

Commit

Permalink
records: handle errors
Browse files Browse the repository at this point in the history
* Improves errors handling in search, detail and edit views.
* Adds an error component to display the error.
* Adds an interface for error objects.
* Removes `console.log` when a http error is catched.
* Sends an error object with status and title when a http error occurs.
* Types the parameters of subscribers to `getRecords` observable.
* Exports error component for using it in host's projects.
* Closes rero/sonar-ui#114.

Co-Authored-by: Sébastien Délèze <sebastien.deleze@rero.ch>
  • Loading branch information
Sébastien Délèze committed Aug 17, 2020
1 parent 5a3cce2 commit 85f4cae
Show file tree
Hide file tree
Showing 15 changed files with 235 additions and 58 deletions.
5 changes: 4 additions & 1 deletion projects/rero/ng-core/src/lib/core.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { ModalModule } from 'ngx-bootstrap/modal';
import { NgxSpinnerModule } from 'ngx-spinner';
import { ToastrModule } from 'ngx-toastr';
import { DialogComponent } from './dialog/dialog.component';
import { ErrorComponent } from './error/error.component';
import { CallbackArrayFilterPipe } from './pipe/callback-array-filter.pipe';
import { DefaultPipe } from './pipe/default.pipe';
import { Nl2brPipe } from './pipe/nl2br.pipe';
Expand All @@ -49,7 +50,8 @@ import { MenuComponent } from './widget/menu/menu.component';
MenuComponent,
TranslateLanguagePipe,
TextReadMoreComponent,
SortByKeysPipe
SortByKeysPipe,
ErrorComponent
],
imports: [
CommonModule,
Expand Down Expand Up @@ -80,6 +82,7 @@ import { MenuComponent } from './widget/menu/menu.component';
SearchInputComponent,
MenuComponent,
TextReadMoreComponent,
ErrorComponent,
SortByKeysPipe,
NgxSpinnerModule
],
Expand Down
24 changes: 24 additions & 0 deletions projects/rero/ng-core/src/lib/error/error.component.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<!--
 RERO angular core
 Copyright (C) 2020 RERO

 This program is free software: you can redistribute it and/or modify
 it under the terms of the GNU Affero General Public License as published by
 the Free Software Foundation, version 3 of the License.

 This program is distributed in the hope that it will be useful,
 but WITHOUT ANY WARRANTY; without even the implied warranty of
 MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 GNU Affero General Public License for more details.

 You should have received a copy of the GNU Affero General Public License
 along with this program. If not, see <http://www.gnu.org/licenses/>.
-->
<div class="text-center my-5">
<p class="display-4">
<i class="fa fa-flash mr-3"></i>
<span class="text-muted">{{ error.status }}</span>
{{ error.title }}
</p>
<h5 class="mt-5">{{ error.message | default: ('You cannot access this page.' | translate) }}</h5>
</div>
48 changes: 48 additions & 0 deletions projects/rero/ng-core/src/lib/error/error.component.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* RERO angular core
* Copyright (C) 2020 RERO
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, version 3 of the License.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import { async, ComponentFixture, TestBed } from '@angular/core/testing';
import { TranslateFakeLoader, TranslateLoader, TranslateModule } from '@ngx-translate/core';
import { DefaultPipe } from '../pipe/default.pipe';
import { ErrorComponent } from './error.component';

describe('ErrorComponent', () => {
let component: ErrorComponent;
let fixture: ComponentFixture<ErrorComponent>;

beforeEach(async(() => {
TestBed.configureTestingModule({
declarations: [ ErrorComponent, DefaultPipe ],
imports: [
TranslateModule.forRoot({
loader: { provide: TranslateLoader, useClass: TranslateFakeLoader }
})
]
})
.compileComponents();
}));

beforeEach(() => {
fixture = TestBed.createComponent(ErrorComponent);
component = fixture.componentInstance;
component.error = { title: 'Error', status: 500 };
fixture.detectChanges();
});

it('should create', () => {
expect(component).toBeTruthy();
});
});
31 changes: 31 additions & 0 deletions projects/rero/ng-core/src/lib/error/error.component.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* RERO angular core
* Copyright (C) 2020 RERO
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, version 3 of the License.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import { Component, Input } from '@angular/core';
import { Error } from './error';

/**
* Component for displaying errors.
*/
@Component({
selector: 'ng-core-error',
templateUrl: './error.component.html'
})
export class ErrorComponent {
// Error object containing title, status and optionally a message.
@Input()
error: Error;
}
25 changes: 25 additions & 0 deletions projects/rero/ng-core/src/lib/error/error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* RERO angular core
* Copyright (C) 2020 RERO
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, version 3 of the License.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

/**
* Interface representing an error.
*/
export interface Error {
status: number;
title: string;
message?: string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
 along with this program. If not, see <http://www.gnu.org/licenses/>.
-->
<div class="main-content">
<div class="alert alert-danger" *ngIf="error">{{ error | translate }}</div>
<ng-core-error [error]="error" *ngIf="error"></ng-core-error>
<div class="float-right ml-4 mt-2 mb-4" *ngIf="record && adminMode.can">
<a href class="btn btn-sm btn-primary" [routerLink]="['../../edit', record.metadata.pid]"
*ngIf="updateStatus && updateStatus.can" id="detail-edit-button">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import { ActionStatus } from '@rero/ng-core/public-api';
import { ModalModule } from 'ngx-bootstrap/modal';
import { ToastrModule } from 'ngx-toastr';
import { Observable, of, throwError } from 'rxjs';
import { ErrorComponent } from '../../error/error.component';
import { DefaultPipe } from '../../pipe/default.pipe';
import { RecordService } from '../record.service';
import { DetailComponent } from './detail.component';
import { RecordDetailDirective } from './detail.directive';
Expand Down Expand Up @@ -78,7 +80,9 @@ describe('RecordDetailComponent', () => {
declarations: [
DetailComponent,
JsonComponent,
RecordDetailDirective
RecordDetailDirective,
ErrorComponent,
DefaultPipe
],
imports: [
TranslateModule.forRoot({
Expand Down Expand Up @@ -174,7 +178,6 @@ describe('RecordDetailComponent', () => {
fixture.detectChanges();

expect(component.record).toBe(null);
expect(component.error).toBe('error');
});

it('should use a custom view component for displaying record', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { TranslateService } from '@ngx-translate/core';
import { NgxSpinnerService } from 'ngx-spinner';
import { ToastrService } from 'ngx-toastr';
import { Observable, Subscription } from 'rxjs';
import { Error } from '../../error/error';
import { ActionStatus } from '../action-status';
import { RecordUiService } from '../record-ui.service';
import { RecordService } from '../record.service';
Expand Down Expand Up @@ -61,7 +62,7 @@ export class DetailComponent implements OnInit, OnDestroy {
/**
* Error message
*/
error: string = null;
error: Error;

/**
* Admin mode for CRUD operations
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,4 @@
</form>
</div>
</div>
<ng-core-error [error]="error" *ngIf="error"></ng-core-error>
50 changes: 31 additions & 19 deletions projects/rero/ng-core/src/lib/record/editor/editor.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import { ToastrService } from 'ngx-toastr';
import { combineLatest, Observable, of, Subscription } from 'rxjs';
import { map, switchMap } from 'rxjs/operators';
import { ApiService } from '../../api/api.service';
import { Error } from '../../error/error';
import { Record } from '../record';
import { RecordUiService } from '../record-ui.service';
import { RecordService } from '../record.service';
import { EditorService } from './editor.service';
Expand Down Expand Up @@ -74,6 +76,9 @@ export class EditorComponent implements OnInit, OnChanges, OnDestroy {
// store pid on edit mode
pid = null;

// If an error occurred, it is stored, to display in interface.
error: Error;

// subscribers
private _subscribers: Subscription[] = [];

Expand Down Expand Up @@ -171,26 +176,33 @@ export class EditorComponent implements OnInit, OnChanges, OnDestroy {
);
}

combineLatest([schema$, record$]).subscribe(([schemaform, data]) => {
// Set schema
this.setSchema(schemaform.schema);
combineLatest([schema$, record$]).subscribe(
([schemaform, data]) => {
// Set schema
this.setSchema(schemaform.schema);

// Check permissions and set record
if (data.result && data.result.can === false) {
this._toastrService.error(
this._translateService.instant('You cannot update this record'),
this._translateService.instant(this.recordType)
);
this._location.back();
} else {
this._setModel(data.record);
}

// Check permissions and set record
if (data.result && data.result.can === false) {
this._toastrService.error(
this._translateService.instant('You cannot update this record'),
this._translateService.instant(this.recordType)
);
this._location.back();
} else {
this._setModel(data.record);
// add a small amount of time as the editor needs additionnal time to
// resolve all async tasks
setTimeout(() => this._spinner.hide(), 500);
this.loadingChange.emit(false);
},
(error) => {
this.error = error;
this._spinner.hide();
this.loadingChange.emit(false);
}

// add a small amount of time as the editor needs additionnal time to
// resolve all async tasks
setTimeout(() => this._spinner.hide(), 500);
this.loadingChange.emit(false);
});
);
}
);
}
Expand Down Expand Up @@ -467,7 +479,7 @@ export class EditorComponent implements OnInit, OnChanges, OnDestroy {
f.templateOptions.options = this._recordService
.getRecords(recordType, query, 1, RecordService.MAX_REST_RESULTS_SIZE)
.pipe(
map(data =>
map((data: Record) =>
data.hits.hits.map((record: any) => {
return {
label: formOptions.remoteOptions.labelField && formOptions.remoteOptions.labelField in record.metadata
Expand Down
17 changes: 11 additions & 6 deletions projects/rero/ng-core/src/lib/record/record.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
import { HttpErrorResponse } from '@angular/common/http';
import { HttpClientTestingModule, HttpTestingController } from '@angular/common/http/testing';
import { getTestBed, TestBed } from '@angular/core/testing';
import { TranslateFakeLoader, TranslateLoader, TranslateModule } from '@ngx-translate/core';
import { ApiService } from '../api/api.service';
import { Error } from '../error/error';
import { Record } from './record';
import { RecordService } from './record.service';

Expand All @@ -34,7 +36,10 @@ describe('RecordService', () => {

TestBed.configureTestingModule({
imports: [
HttpClientTestingModule
HttpClientTestingModule,
TranslateModule.forRoot({
loader: { provide: TranslateLoader, useClass: TranslateFakeLoader }
})
],
providers: [
{ provide: ApiService, useValue: spy }
Expand Down Expand Up @@ -68,7 +73,7 @@ describe('RecordService', () => {
links: {}
};

service.getRecords('documents', '', 1, 10, [{ key: 'author', values: ['John doe'] }]).subscribe(data => {
service.getRecords('documents', '', 1, 10, [{ key: 'author', values: ['John doe'] }]).subscribe((data: Record) => {
expect(data.hits.total).toBe(2);
});

Expand All @@ -82,8 +87,8 @@ describe('RecordService', () => {

service.getRecords('documents').subscribe(
() => fail('should have failed with the 404 error'),
(error: HttpErrorResponse) => {
expect(error).toContain('Something bad happened');
(error: Error) => {
expect(error.title).toBe('Not found');
});

const req = httpMock.expectOne(request => request.method === 'GET' && request.url === url + '/');
Expand All @@ -96,8 +101,8 @@ describe('RecordService', () => {

service.getRecords('documents').subscribe(
() => fail('should have failed with the 404 error'),
(error: HttpErrorResponse) => {
expect(error).toContain('Something bad happened');
(error: Error) => {
expect(error.title).toBe('An error occurred');
});

const req = httpMock.expectOne(request => request.method === 'GET' && request.url === url + '/');
Expand Down
Loading

0 comments on commit 85f4cae

Please sign in to comment.