diff --git a/src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.html b/src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.html index dcbdd77bffa..b3341df3705 100644 --- a/src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.html +++ b/src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.html @@ -1,5 +1,5 @@
- + {{object.value}} diff --git a/src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.spec.ts b/src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.spec.ts index a4490bd9519..0dbff7ee46f 100644 --- a/src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.spec.ts +++ b/src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.spec.ts @@ -1,12 +1,16 @@ -import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; -import { ChangeDetectionStrategy, NO_ERRORS_SCHEMA } from '@angular/core'; +import { ComponentFixture, fakeAsync, TestBed, tick, waitForAsync } from '@angular/core/testing'; +import { ChangeDetectionStrategy, Component, NO_ERRORS_SCHEMA } from '@angular/core'; import { By } from '@angular/platform-browser'; +import { Params, Router } from '@angular/router'; +import { RouterTestingModule } from '@angular/router/testing'; +import { Store } from '@ngrx/store'; import { TruncatePipe } from '../../utils/truncate.pipe'; import { BrowseEntryListElementComponent } from './browse-entry-list-element.component'; import { BrowseEntry } from '../../../core/shared/browse-entry.model'; import { PaginationService } from '../../../core/pagination/pagination.service'; import { RouteService } from '../../../core/services/route.service'; import { of as observableOf } from 'rxjs'; + let browseEntryListElementComponent: BrowseEntryListElementComponent; let fixture: ComponentFixture; @@ -15,22 +19,21 @@ const mockValue: BrowseEntry = Object.assign(new BrowseEntry(), { value: 'De Langhe Kristof' }); -let paginationService; -let routeService; const pageParam = 'bbm.page'; -function init() { - paginationService = jasmine.createSpyObj('paginationService', { - getPageParam: pageParam - }); - - routeService = jasmine.createSpyObj('routeService', { - getQueryParameterValue: observableOf('1') - }); +@Component({ template: '' }) +class DummyComponent { } + describe('BrowseEntryListElementComponent', () => { beforeEach(waitForAsync(() => { - init(); + const paginationService = jasmine.createSpyObj('paginationService', { + getPageParam: pageParam + }); + const routeService = jasmine.createSpyObj('routeService', { + getQueryParameterValue: observableOf(undefined) + }); + TestBed.configureTestingModule({ declarations: [BrowseEntryListElementComponent, TruncatePipe], providers: [ @@ -62,3 +65,85 @@ describe('BrowseEntryListElementComponent', () => { }); }); }); + +describe('BrowseEntryListElementComponent link', () => { + // The real RouteService is used here on purpose: every parameter below reaches the component + // through the router, the way it does in the browser, so the assertions are on what the URL + // actually produces rather than on what a stub was told to answer. + const scopeUUID = 'a2f2d0a1-3f0e-4d3a-9c1b-5f7e8a9b0c1d'; + let router: Router; + + const hrefFor = (queryParams: Params): string => { + void router.navigate(['/browse/author'], { queryParams }); + tick(); + + fixture = TestBed.createComponent(BrowseEntryListElementComponent); + fixture.componentInstance.object = mockValue; + fixture.detectChanges(); + + return fixture.debugElement.query(By.css('a.lead')).nativeElement.getAttribute('href'); + }; + + beforeEach(waitForAsync(() => { + const paginationService = jasmine.createSpyObj('paginationService', { + getPageParam: pageParam + }); + + TestBed.configureTestingModule({ + imports: [ + RouterTestingModule.withRoutes([ + { path: 'browse/author', component: DummyComponent } + ]) + ], + declarations: [BrowseEntryListElementComponent, DummyComponent, TruncatePipe], + providers: [ + { provide: 'objectElementProvider', useValue: { mockValue } }, + {provide: PaginationService, useValue: paginationService}, + {provide: Store, useValue: jasmine.createSpyObj('store', ['dispatch'])}, + ], + schemas: [NO_ERRORS_SCHEMA] + }).compileComponents(); + })); + + beforeEach(() => { + router = TestBed.inject(Router); + }); + + it('should read its parameters through the real RouteService', () => { + expect(TestBed.inject(RouteService) instanceof RouteService).toBeTruthy(); + }); + + it('should carry over the scope and the pagination settings', fakeAsync(() => { + const href = hrefFor({ + scope: scopeUUID, + 'bbm.rpp': '40', + 'bbm.sf': 'title', + 'bbm.sd': 'DESC' + }); + + expect(href).toContain(`scope=${scopeUUID}`); + expect(href).toContain('bbm.rpp=40'); + expect(href).toContain('bbm.sf=title'); + expect(href).toContain('bbm.sd=DESC'); + })); + + it('should replace the current page with the page to return to', fakeAsync(() => { + const href = hrefFor({ 'bbm.page': '3' }); + + expect(href).toContain('bbm.return=3'); + expect(href).not.toContain('bbm.page='); + })); + + it('should drop a parameter the browse page never asked for', fakeAsync(() => { + const href = hrefFor({ scope: scopeUUID, 'amp;value': 'Some Author' }); + + expect(href).toContain(`scope=${scopeUUID}`); + expect(href).not.toContain('amp'); + })); + + it('should not pass on a scope that is present but empty', fakeAsync(() => { + const href = hrefFor({ scope: '' }); + + expect(href).not.toContain('scope='); + })); +}); diff --git a/src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.ts b/src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.ts index 667da726ed8..984a116de0e 100644 --- a/src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.ts +++ b/src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.ts @@ -8,7 +8,7 @@ import { PaginationService } from '../../../core/pagination/pagination.service'; import { Params } from '@angular/router'; import { BBM_PAGINATION_ID } from '../../../browse-by/browse-by-metadata-page/browse-by-metadata-page.component'; import { RouteService } from 'src/app/core/services/route.service'; -import { Observable } from 'rxjs'; +import { combineLatest, Observable } from 'rxjs'; import { map } from 'rxjs/operators'; @Component({ @@ -37,16 +37,29 @@ export class BrowseEntryListElementComponent extends AbstractListableElementComp /** * Get the query params to access the item page of this browse entry. + * + * Carries over only the parameters a browse page actually uses. Anything else in the current URL + * is dropped, so a malformed parameter cannot be reflected back into the links we generate. */ private getQueryParams(): Observable { const pageParamName = this.paginationService.getPageParam(BBM_PAGINATION_ID); - return this.routeService.getQueryParameterValue(pageParamName).pipe( - map((currentPage) => { + return combineLatest([ + this.routeService.getQueryParameterValue(pageParamName), + this.routeService.getQueryParameterValue('scope'), + this.routeService.getQueryParameterValue(`${BBM_PAGINATION_ID}.rpp`), + this.routeService.getQueryParameterValue(`${BBM_PAGINATION_ID}.sf`), + this.routeService.getQueryParameterValue(`${BBM_PAGINATION_ID}.sd`), + ]).pipe( + map(([currentPage, scope, rpp, sortField, sortDirection]) => { return { value: this.object.value, authority: !!this.object.authority ? this.object.authority : undefined, + scope: scope || undefined, startsWith: undefined, [pageParamName]: null, + [`${BBM_PAGINATION_ID}.rpp`]: rpp || undefined, + [`${BBM_PAGINATION_ID}.sf`]: sortField || undefined, + [`${BBM_PAGINATION_ID}.sd`]: sortDirection || undefined, [BBM_PAGINATION_ID + '.return']: currentPage }; })