TUL/Browse links: carry only the parameters a browse page uses

src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.html

@@ -1,5 +1,5 @@
 <div class="d-flex flex-row">
-    <a *ngIf="linkType != linkTypes.None" [target]="(linkType == linkTypes.ExternalLink) ? '_blank' : '_self'" rel="noopener noreferrer" [routerLink]="[]" [queryParams]="queryParams$ | async" [queryParamsHandling]="'merge'" class="lead">
+    <a *ngIf="linkType != linkTypes.None" [target]="(linkType == linkTypes.ExternalLink) ? '_blank' : '_self'" rel="noopener noreferrer" [routerLink]="[]" [queryParams]="queryParams$ | async" class="lead">
         {{object.value}}
     </a>
     <span *ngIf="linkType == linkTypes.None" class="lead">

src/app/shared/object-list/browse-entry-list-element/browse-entry-list-element.component.spec.ts

@@ -7,6 +7,7 @@ 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';
+import { take } from 'rxjs/operators';
 let browseEntryListElementComponent: BrowseEntryListElementComponent;
 let fixture: ComponentFixture<BrowseEntryListElementComponent>;
 
@@ -18,15 +19,18 @@ const mockValue: BrowseEntry = Object.assign(new BrowseEntry(), {
 let paginationService;
 let routeService;
 const pageParam = 'bbm.page';
+let queryParamsInUrl: { [name: string]: string };
 
 function init() {
   paginationService = jasmine.createSpyObj('paginationService', {
     getPageParam: pageParam
   });
 
-  routeService = jasmine.createSpyObj('routeService', {
-    getQueryParameterValue: observableOf('1')
-  });
+  queryParamsInUrl = { [pageParam]: '1' };
+  routeService = jasmine.createSpyObj('routeService', ['getQueryParameterValue']);
+  routeService.getQueryParameterValue.and.callFake(
+    (name: string) => observableOf(queryParamsInUrl[name])
+  );
 }
 describe('BrowseEntryListElementComponent', () => {
   beforeEach(waitForAsync(() => {
@@ -61,4 +65,41 @@ describe('BrowseEntryListElementComponent', () => {
       expect(browseEntryLink.nativeElement.textContent.trim()).toBe(mockValue.value);
     });
   });
+
+  describe('queryParams', () => {
+    let emitted;
+
+    const buildQueryParams = () => {
+      browseEntryListElementComponent.object = mockValue;
+      fixture.detectChanges();
+      browseEntryListElementComponent.queryParams$.pipe(take(1)).subscribe((p) => emitted = p);
+    };
+
+    it('should keep the scope of the community or collection being browsed', () => {
+      queryParamsInUrl.scope = '0eb1f4d0-fd7c-4c2c-b0d9-32ee18f5e1c1';
+      buildQueryParams();
+
+      expect(emitted.scope).toBe('0eb1f4d0-fd7c-4c2c-b0d9-32ee18f5e1c1');
+    });
+
+    it('should keep the page size and sort chosen by the user', () => {
+      queryParamsInUrl['bbm.rpp'] = '40';
+      queryParamsInUrl['bbm.sf'] = 'title';
+      queryParamsInUrl['bbm.sd'] = 'DESC';
+      buildQueryParams();
+
+      expect(emitted['bbm.rpp']).toBe('40');
+      expect(emitted['bbm.sf']).toBe('title');
+      expect(emitted['bbm.sd']).toBe('DESC');
+    });
+
+    it('should drop parameters it does not recognise', () => {
+      queryParamsInUrl['amp;value'] = 'Some Author';
+      queryParamsInUrl.utm_source = 'newsletter';
+      buildQueryParams();
+
+      expect(Object.keys(emitted)).not.toContain('amp;value');
+      expect(Object.keys(emitted)).not.toContain('utm_source');
+    });
+  });
 });

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<Params> {
     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
         };
       })