Skip to content

Commit

Permalink
general: fix error 400 bad request
Browse files Browse the repository at this point in the history
* Closes rero/rero-ils#2705.
* Removes localeStorage user data.
* Updates guards that used the user localeStorage.
* Improves somes guards.
* Adds some tests for guards.

Co-Authored-by: Bertrand Zuchuat <bertrand.zuchuat@rero.ch>
  • Loading branch information
Garfield-fr committed Jun 1, 2022
1 parent 972884d commit 33dcdf1
Show file tree
Hide file tree
Showing 18 changed files with 648 additions and 281 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
/*
* RERO ILS UI
* Copyright (C) 2022 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 { HttpClientTestingModule } from '@angular/common/http/testing';
import { fakeAsync, TestBed, tick } from '@angular/core/testing';
import { Router } from '@angular/router';
import { RouterTestingModule } from '@angular/router/testing';
import { TranslateModule } from '@ngx-translate/core';
import { CoreModule } from '@rero/ng-core';
import { UserService } from '@rero/shared';
import { cloneDeep } from 'lodash-es';
import { userTestingService } from 'projects/admin/tests/utils';
import { of } from 'rxjs';
import { ErrorPageComponent } from '../../../error/error-page/error-page.component';
import { AcqOrderApiService } from '../../api/acq-order-api.service';
import { AcqReceiptApiService } from '../../api/acq-receipt-api.service';
import { AcqOrderStatus, AcqOrderType } from '../../classes/order';
import { CanOrderReceiptGuard } from './can-order-receipt.guard';

describe('CanOrderReceiptGuard', () => {
let guard: CanOrderReceiptGuard;
let acqOrderApiService: AcqOrderApiService;
let acqReceiptApiService: AcqReceiptApiService;
let router: Router;

const routes = [
{
path: 'errors/400',
component: ErrorPageComponent
},
{
path: 'errors/403',
component: ErrorPageComponent
}
];

const order = {
reference: 'foo',
priority: 1,
type: AcqOrderType.MONOGRAPH,
currency: 'CHF',
order_date: new Date(),
account_statement: {
provisional: {
total_amount: 100,
quantity: 10
},
expenditure: {
total_amount: 100,
quantity: 10
}
},
vendor: {
$ref: 'http://localhost/api/vendors/1'
},
organisation: {
$ref: 'https://localhost/api/organisations/1'
},
library: {
$ref: 'https://localhost/api/libraries/1'
},
status: AcqOrderStatus.ORDERED,
notes: []
};

const receipt = {
acq_order: {
pid: '1',
},
exchange_rate: 1,
amount_adjustments: [{
label: 'amount',
amount: 10,
acq_account: {
$ref: 'https://localhost/api/acq_accounts/1'
}
}],
organisation: {
$ref: 'https://localhost/api/organisations/1'
},
notes: []
};


const activatedRouteSnapshotSpy = jasmine.createSpyObj('ActivatedRouteSnapshot', ['']);
activatedRouteSnapshotSpy.params = { pid: '1' };
activatedRouteSnapshotSpy.queryParams = { };

beforeEach(() => {
TestBed.configureTestingModule({
imports: [
RouterTestingModule.withRoutes(routes),
HttpClientTestingModule,
TranslateModule.forRoot(),
CoreModule
],
providers: [
{ provide: UserService, useValue: userTestingService }
]
});
guard = TestBed.inject(CanOrderReceiptGuard);
acqOrderApiService = TestBed.inject(AcqOrderApiService);
acqReceiptApiService = TestBed.inject(AcqReceiptApiService);
router = TestBed.inject(Router);
});

it('should be created', () => {
expect(guard).toBeTruthy();
});

it('should return a 400 error if any parameters are missing', fakeAsync(() => {
const activatedRoute = cloneDeep(activatedRouteSnapshotSpy);
activatedRoute.params = {};
guard.canActivate(activatedRoute).subscribe(() => {
tick();
expect(router.url).toBe('/errors/400');
});
}));

it('should return true if the order in status ordered (new receipt)', () => {
spyOn(acqOrderApiService, 'getOrder').and.returnValue(of(order));
guard.canActivate(activatedRouteSnapshotSpy).subscribe((access: boolean) => {
expect(access).toBeTruthy();
});
});

it('should return a 403 error if the order in status received (new receipt)', fakeAsync(() => {
const orderReceived = cloneDeep(order);
orderReceived.status = AcqOrderStatus.RECEIVED;
spyOn(acqOrderApiService, 'getOrder').and.returnValue(of(orderReceived));
guard.canActivate(activatedRouteSnapshotSpy).subscribe(() => {
tick();
expect(router.url).toBe('/errors/403');
});
}));

it('should return a 403 error if the order and receipt in not the same order pid (update receipt)', fakeAsync(() => {
const activatedRoute = cloneDeep(activatedRouteSnapshotSpy);
activatedRoute.queryParams.receipt = '1';
activatedRoute.params.pid = '2';
spyOn(acqOrderApiService, 'getOrder').and.returnValue(of(order));
spyOn(acqReceiptApiService, 'getReceipt').and.returnValue(of(receipt));
guard.canActivate(activatedRoute).subscribe(() => {
tick();
expect(router.url).toBe('/errors/403');
});
}));

it('should return true if the order and receipt in the same order pid (update receipt)', () => {
const activatedRoute = cloneDeep(activatedRouteSnapshotSpy);
activatedRoute.queryParams.receipt = '1';
spyOn(acqOrderApiService, 'getOrder').and.returnValue(of(order));
spyOn(acqReceiptApiService, 'getReceipt').and.returnValue(of(receipt));
guard.canActivate(activatedRoute).subscribe((access: boolean) => {
expect(access).toBeTruthy();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import { Injectable } from '@angular/core';
import { ActivatedRouteSnapshot, CanActivate, Router, RouterStateSnapshot, UrlTree } from '@angular/router';
import { ActivatedRouteSnapshot, CanActivate, Router } from '@angular/router';
import { extractIdOnRef } from '@rero/ng-core';
import { IUserLocaleStorage, UserService } from '@rero/shared';
import { UserService } from '@rero/shared';
import { combineLatest, Observable, of } from 'rxjs';
import { catchError, map } from 'rxjs/operators';
import { AcqOrderApiService } from '../../api/acq-order-api.service';
import { AcqReceiptApiService } from '../../api/acq-receipt-api.service';
import { IAcqOrder, AcqOrderStatus } from '../../classes/order';
import { AcqOrderStatus, IAcqOrder } from '../../classes/order';
import { IAcqReceipt } from '../../classes/receipt';

@Injectable({
Expand All @@ -47,46 +47,54 @@ export class CanOrderReceiptGuard implements CanActivate {
/**
* Can activate
* @param route - ActivatedRouteSnapshot
* @param state - RouterStateSnapshot
* @returns Observable
* @returns True if the conditions is satisfied
* @throws redirect to error 403 or 400
*/
canActivate(
route: ActivatedRouteSnapshot,
state: RouterStateSnapshot): Observable<boolean | UrlTree> | Promise<boolean | UrlTree> | boolean | UrlTree {
const orderPid = route.paramMap.get('pid');
canActivate(route: ActivatedRouteSnapshot): Observable<boolean> {
const orderPid = route.params.pid;
const receiptPid = route.queryParams.receipt;
if (!receiptPid) {
this._orderQuery(orderPid).subscribe((validate: boolean) => {
if (!validate) {
this._router.navigate(['/errors/403'], { skipLocationChange: true });
}
});
if (orderPid) {
if (!receiptPid) {
return this._orderQuery(orderPid).pipe(
map((validate: boolean) => {
if (!validate) {
this._router.navigate(['/errors/403'], { skipLocationChange: true });
return false;
}
return true;
})
);
} else {
return combineLatest([
this._orderQuery(orderPid),
this._receiptQuery(receiptPid, orderPid)
]).pipe(
map(([order, receipt]) => {
if (!order || !receipt) {
this._router.navigate(['/errors/403'], { skipLocationChange: true });
return false;
}
return true;
})
);
}
} else {
combineLatest([this._orderQuery(orderPid), this._receiptQuery(receiptPid, orderPid)])
.subscribe(([order, receipt]) => {
if (!order || !receipt) {
this._router.navigate(['/errors/403'], { skipLocationChange: true });
}
});
this._router.navigate(['/errors/400'], { skipLocationChange: true });
return of(false);
}
return true;
}

/**
* Check if an order has requirement to manage a receipt anymore.
*
* To manage a receipt, the related order must be related to the current user library and
* the order status should be ORDERED or PARTIALLY RECEIVED
* @param orderPid - the order pid
* @returns Observable<boolean>: True if a receipt could be managed, False otherwise
*/
private _orderQuery(orderPid: string): Observable<boolean> {
return this._acqOrderApiService
.getOrder(orderPid)
.pipe(
return this._acqOrderApiService.getOrder(orderPid).pipe(
map((order: IAcqOrder) => {
const userLocale: IUserLocaleStorage = this._userService.getOnLocaleStorage();
if (userLocale.currentLibrary !== extractIdOnRef(order.library.$ref)) {
if (this._userService.user.currentLibrary !== extractIdOnRef(order.library.$ref)) {
return false;
}
const validStatuses = [AcqOrderStatus.ORDERED, AcqOrderStatus.PARTIALLY_RECEIVED];
Expand All @@ -103,9 +111,7 @@ export class CanOrderReceiptGuard implements CanActivate {
* @returns Observable<boolean>: True if the receipt is well related to this order
*/
private _receiptQuery(receiptPid: string, orderPid: string): Observable<boolean> {
return this._acqReceiptApiService
.getReceipt(receiptPid)
.pipe(
return this._acqReceiptApiService.getReceipt(receiptPid).pipe(
map((receipt: IAcqReceipt) => orderPid === receipt.acq_order.pid),
catchError(() => of(false))
);
Expand Down
14 changes: 7 additions & 7 deletions projects/admin/src/app/guard/acq-order-line.guard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,20 @@ import { LibraryGuard } from './library.guard';
})
export class AcqOrderLineGuard extends LibraryGuard {

/** Return the library linked to an acquisition order number.
* @param route: the current URL route
* @return: the library pid linked to the resource from the 'order' query parameters
/**
* Return the library linked to an acquisition order number.
* @param route: the current URL route
* @return: the library pid linked to the resource from the 'order' query parameters
*/
getOwningLibrary$(route: ActivatedRouteSnapshot): Observable<string> {
let orderPid = route.queryParams.order;
if (orderPid === undefined) {
orderPid = route.params.pid;
}
return this._recordService.getRecord('acq_orders', orderPid).pipe(
map( data => data.metadata || {} ),
map( metadata => metadata.library || {} ),
map( library => extractIdOnRef(library.$ref) )
map(data => data.metadata || {}),
map(metadata => metadata.library || {}),
map(library => extractIdOnRef(library.$ref))
);
}

}
Loading

0 comments on commit 33dcdf1

Please sign in to comment.