From 97dc241c5208d949826a6ebd930615adb47c31c7 Mon Sep 17 00:00:00 2001
From: Yana De Pauw <yana@atmire.com>
Date: Thu, 17 Mar 2022 13:00:00 +0100
Subject: [PATCH] 88300: Issue 1379 - Angular: Browse-by Subject does not allow
 to proceed to next pages once filtered

---
 .../shared/browse-by/browse-by.component.html |  23 +++-
 .../browse-by/browse-by.component.spec.ts     |  20 +++-
 .../shared/browse-by/browse-by.component.ts   |  19 ++-
 .../text/starts-with-text.component.html      |  19 ---
 .../text/starts-with-text.component.spec.ts   | 110 ------------------
 src/app/shared/testing/route-service.stub.ts  |   7 +-
 src/assets/i18n/en.json5                      |   6 +-
 7 files changed, 65 insertions(+), 139 deletions(-)

diff --git a/src/app/shared/browse-by/browse-by.component.html b/src/app/shared/browse-by/browse-by.component.html
index c133324681..4837c92db8 100644
--- a/src/app/shared/browse-by/browse-by.component.html
+++ b/src/app/shared/browse-by/browse-by.component.html
@@ -12,6 +12,10 @@
     <div *ngIf="enableArrows">
       <div class="row">
         <div class="col-12">
+          <div *ngIf="shouldDisplayResetButton$ |async" class="d-inline-block float-left reset">
+            <a class="btn btn-secondary" [routerLink]="[]"
+               [queryParams]="{startsWith: null, value: null}" queryParamsHandling="merge"><i class="fas fa-arrow-left"></i> {{'browse.back.all-results' | translate}}</a>
+          </div>
           <div *ngIf="!hideGear" ngbDropdown #paginationControls="ngbDropdown" placement="bottom-right" class="d-inline-block float-right">
             <button class="btn btn-secondary" id="paginationControls" [title]="'pagination.options.description' | translate" [attr.aria-label]="'pagination.options.description' | translate" ngbDropdownToggle><i class="fas fa-cog" aria-hidden="true"></i></button>
             <div id="paginationControlsDropdownMenu" aria-labelledby="paginationControls" ngbDropdownMenu>
@@ -29,14 +33,25 @@
         </li>
       </ul>
       <div>
-        <button id="nav-prev" type="button" class="btn btn-outline-primary float-left" (click)="goPrev()" [disabled]="objects?.payload?.currentPage <= 1"><i class="fas fa-angle-left"></i> {{'browse.previous.button' |translate}}</button>
-        <button id="nav-next" type="button" class="btn btn-outline-primary float-right" (click)="goNext()" [disabled]="objects?.payload?.currentPage >= objects?.payload?.totalPages"><i class="fas fa-angle-right"></i> {{'browse.next.button' |translate}}</button>
+        <button id="nav-prev" type="button" class="btn btn-outline-primary float-left" (click)="goPrev()"  [disabled]="objects?.payload?.currentPage <= 1"><i class="fas fa-angle-left"></i> {{'browse.previous.button' |translate}}</button>
+          <button id="nav-next" type="button" class="btn btn-outline-primary float-right" (click)="goNext()"  [disabled]="objects?.payload?.currentPage >= objects?.payload?.totalPages">
+                    <span [ngbTooltip]="objects?.payload?.currentPage >= objects?.payload?.totalPages ? 'browse.next.button.disabled.tooltip': null">
+            <i class="fas fa-angle-right"></i> {{'browse.next.button' |translate}}
+                    </span>
+          </button>
       </div>
     </div>
   </div>
   <ds-loading *ngIf="!objects || objects?.isLoading" message="{{'loading.browse-by' | translate}}"></ds-loading>
   <ds-error *ngIf="objects?.hasFailed" message="{{'error.browse-by' | translate}}"></ds-error>
-  <div *ngIf="!objects?.isLoading && objects?.payload?.page.length === 0" class="alert alert-info w-100" role="alert">
-    {{'browse.empty' | translate}}
+  <div *ngIf="!objects?.isLoading && objects?.payload?.page.length === 0">
+    <div *ngIf="shouldDisplayResetButton$ |async" class="d-inline-block mb-4 reset">
+      <a class="btn btn-secondary" [routerLink]="[]"
+         [queryParams]="{startsWith: null, value: null}" queryParamsHandling="merge"><i
+              class="fas fa-arrow-left"></i> {{'browse.back.all-results' | translate}}</a>
+    </div>
+    <div class="alert alert-info w-100" role="alert">
+      {{'browse.empty' | translate}}
+    </div>
   </div>
 </ng-container>
diff --git a/src/app/shared/browse-by/browse-by.component.spec.ts b/src/app/shared/browse-by/browse-by.component.spec.ts
index 806f4bdb6f..875ad02dd2 100644
--- a/src/app/shared/browse-by/browse-by.component.spec.ts
+++ b/src/app/shared/browse-by/browse-by.component.spec.ts
@@ -18,9 +18,10 @@ import { PaginationComponentOptions } from '../pagination/pagination-component-o
 import { SortDirection, SortOptions } from '../../core/cache/models/sort-options.model';
 import { createSuccessfulRemoteDataObject$ } from '../remote-data.utils';
 import { storeModuleConfig } from '../../app.reducer';
-import { FindListOptions } from '../../core/data/request.models';
 import { PaginationService } from '../../core/pagination/pagination.service';
 import { PaginationServiceStub } from '../testing/pagination-service.stub';
+import { RouteService } from '../../core/services/route.service';
+import { routeServiceStub } from '../testing/route-service.stub';
 
 describe('BrowseByComponent', () => {
   let comp: BrowseByComponent;
@@ -75,7 +76,8 @@ describe('BrowseByComponent', () => {
       ],
       declarations: [],
       providers: [
-        {provide: PaginationService, useValue: paginationService}
+        {provide: PaginationService, useValue: paginationService},
+        {provide: RouteService, useValue: routeServiceStub}
       ],
       schemas: [NO_ERRORS_SCHEMA]
     }).compileComponents();
@@ -163,4 +165,18 @@ describe('BrowseByComponent', () => {
     });
   });
 
+  describe('reset filters button', () => {
+    it('should not be present when no startsWith or value is present ', () => {
+      const button = fixture.debugElement.query(By.css('reset'));
+      expect(button).toBeNull();
+    });
+    it('should be present when a startsWith or value is present ', () => {
+      comp.shouldDisplayResetButton$ = observableOf(true);
+      fixture.detectChanges();
+
+      const button = fixture.debugElement.query(By.css('reset'));
+      expect(button).toBeDefined();
+    });
+  });
+
 });
diff --git a/src/app/shared/browse-by/browse-by.component.ts b/src/app/shared/browse-by/browse-by.component.ts
index 1f05ad2258..9f4350402a 100644
--- a/src/app/shared/browse-by/browse-by.component.ts
+++ b/src/app/shared/browse-by/browse-by.component.ts
@@ -4,10 +4,13 @@ import { PaginatedList } from '../../core/data/paginated-list.model';
 import { PaginationComponentOptions } from '../pagination/pagination-component-options.model';
 import { SortDirection, SortOptions } from '../../core/cache/models/sort-options.model';
 import { fadeIn, fadeInOut } from '../animations/fade';
-import { Observable } from 'rxjs';
+import { combineLatest as observableCombineLatest, Observable } from 'rxjs';
 import { ListableObject } from '../object-collection/shared/listable-object.model';
 import { getStartsWithComponent, StartsWithType } from '../starts-with/starts-with-decorator';
 import { PaginationService } from '../../core/pagination/pagination.service';
+import { RouteService } from '../../core/services/route.service';
+import { map } from 'rxjs/operators';
+import { hasValue } from '../empty.util';
 
 @Component({
   selector: 'ds-browse-by',
@@ -97,8 +100,14 @@ export class BrowseByComponent implements OnInit {
    */
   public sortDirections = SortDirection;
 
+  /**
+   * Observable that tracks if the back button should be displayed based on the path parameters
+   */
+  shouldDisplayResetButton$: Observable<boolean>;
+
   public constructor(private injector: Injector,
                      protected paginationService: PaginationService,
+                     private routeService: RouteService
   ) {
 
   }
@@ -148,6 +157,14 @@ export class BrowseByComponent implements OnInit {
       ],
       parent: this.injector
     });
+
+    const startsWith$ = this.routeService.getQueryParameterValue('startsWith');
+    const value$ = this.routeService.getQueryParameterValue('value');
+
+    this.shouldDisplayResetButton$ = observableCombineLatest([startsWith$, value$]).pipe(
+      map(([startsWith, value]) => hasValue(startsWith) || hasValue(value))
+    );
+
   }
 
 }
diff --git a/src/app/shared/starts-with/text/starts-with-text.component.html b/src/app/shared/starts-with/text/starts-with-text.component.html
index 7a93adf7ac..3314d9cc4a 100644
--- a/src/app/shared/starts-with/text/starts-with-text.component.html
+++ b/src/app/shared/starts-with/text/starts-with-text.component.html
@@ -1,24 +1,5 @@
 <form class="w-100" [formGroup]="formData" (ngSubmit)="submitForm(formData.value)">
   <div class="row">
-    <div class="col-12 d-none d-sm-inline-block">
-      <ul class="list-inline">
-        <li class="list-inline-item" *ngFor="let option of startsWithOptions">
-          <a [routerLink]="[]" (click)="setStartsWith(option)">{{option}}</a>
-        </li>
-      </ul>
-    </div>
-    <div class="col-4 col-sm-12 d-sm-none">
-      <select class="form-control" (change)="setStartsWithEvent($event)">
-        <option [value]="'-1'" [selected]="!startsWith">
-          {{'browse.startsWith.choose_start' | translate}}
-        </option>
-        <option *ngFor="let option of startsWithOptions"
-                [value]="option"
-                [selected]="isSelectedOption(option) ? 'selected': null">
-          {{option}}
-        </option>
-      </select>
-    </div>
     <div class="form-group input-group col-8 col-sm-12 col-md-6">
       <input class="form-control" placeholder="{{'browse.startsWith.type_text' | translate}}" type="text" name="startsWith" formControlName="startsWith" [value]="getStartsWith()" />
       <span class="input-group-append">
diff --git a/src/app/shared/starts-with/text/starts-with-text.component.spec.ts b/src/app/shared/starts-with/text/starts-with-text.component.spec.ts
index 9f9d9d6d42..ca7e74f7f2 100644
--- a/src/app/shared/starts-with/text/starts-with-text.component.spec.ts
+++ b/src/app/shared/starts-with/text/starts-with-text.component.spec.ts
@@ -51,116 +51,6 @@ describe('StartsWithTextComponent', () => {
     expect(comp.formData.value.startsWith).toBeDefined();
   });
 
-  describe('when selecting the first option in the dropdown', () => {
-    let select;
-
-    beforeEach(() => {
-      select = fixture.debugElement.query(By.css('select')).nativeElement;
-      select.value = select.options[0].value;
-      select.dispatchEvent(new Event('change'));
-      fixture.detectChanges();
-    });
-
-    it('should set startsWith to undefined', () => {
-      expect(comp.startsWith).toBeUndefined();
-    });
-
-    it('should not add a startsWith query parameter', () => {
-      route.queryParams.subscribe((params) => {
-        expect(params.startsWith).toBeUndefined();
-      });
-    });
-  });
-
-  describe('when selecting "0-9" in the dropdown', () => {
-    let select;
-    let input;
-    const expectedValue = '0';
-    const extras: NavigationExtras = {
-      queryParams: Object.assign({ startsWith: expectedValue }),
-      queryParamsHandling: 'merge'
-    };
-
-    beforeEach(() => {
-      select = fixture.debugElement.query(By.css('select')).nativeElement;
-      input = fixture.debugElement.query(By.css('input')).nativeElement;
-      select.value = select.options[1].value;
-      select.dispatchEvent(new Event('change'));
-      fixture.detectChanges();
-    });
-
-    it('should set startsWith to "0"', () => {
-      expect(comp.startsWith).toEqual(expectedValue);
-    });
-
-    it('should add a startsWith query parameter', () => {
-      expect(router.navigate).toHaveBeenCalledWith([], extras);
-    });
-
-    it('should automatically fill in the input field', () => {
-      expect(input.value).toEqual(expectedValue);
-    });
-  });
-
-  describe('when selecting an option in the dropdown', () => {
-    let select;
-    let input;
-    const expectedValue = options[1];
-    const extras: NavigationExtras = {
-      queryParams: Object.assign({ startsWith: expectedValue }),
-      queryParamsHandling: 'merge'
-    };
-
-    beforeEach(() => {
-      select = fixture.debugElement.query(By.css('select')).nativeElement;
-      input = fixture.debugElement.query(By.css('input')).nativeElement;
-      select.value = select.options[2].value;
-      select.dispatchEvent(new Event('change'));
-      fixture.detectChanges();
-    });
-
-    it('should set startsWith to the expected value', () => {
-      expect(comp.startsWith).toEqual(expectedValue);
-    });
-
-    it('should add a startsWith query parameter', () => {
-      expect(router.navigate).toHaveBeenCalledWith([], extras);
-    });
-
-    it('should automatically fill in the input field', () => {
-      expect(input.value).toEqual(expectedValue);
-    });
-  });
-
-  describe('when clicking an option in the list', () => {
-    let optionLink;
-    let input;
-    const expectedValue = options[1];
-    const extras: NavigationExtras = {
-      queryParams: Object.assign({ startsWith: expectedValue }),
-      queryParamsHandling: 'merge'
-    };
-
-    beforeEach(() => {
-      optionLink = fixture.debugElement.query(By.css('.list-inline-item:nth-child(2) > a')).nativeElement;
-      input = fixture.debugElement.query(By.css('input')).nativeElement;
-      optionLink.click();
-      fixture.detectChanges();
-    });
-
-    it('should set startsWith to the expected value', () => {
-      expect(comp.startsWith).toEqual(expectedValue);
-    });
-
-    it('should add a startsWith query parameter', () => {
-      expect(router.navigate).toHaveBeenCalledWith([], extras);
-    });
-
-    it('should automatically fill in the input field', () => {
-      expect(input.value).toEqual(expectedValue);
-    });
-  });
-
   describe('when filling in the input form', () => {
     let form;
     const expectedValue = 'A';
diff --git a/src/app/shared/testing/route-service.stub.ts b/src/app/shared/testing/route-service.stub.ts
index 1962755bbc..3a5b5b16b5 100644
--- a/src/app/shared/testing/route-service.stub.ts
+++ b/src/app/shared/testing/route-service.stub.ts
@@ -1,14 +1,17 @@
-import { of as observableOf } from 'rxjs';
+import { EMPTY, of as observableOf } from 'rxjs';
 
 export const routeServiceStub: any = {
   /* tslint:disable:no-empty */
   hasQueryParamWithValue: (param: string, value: string) => {
+    return EMPTY;
   },
   hasQueryParam: (param: string) => {
+    return EMPTY;
   },
   removeQueryParameterValue: (param: string, value: string) => {
+    return EMPTY;
   },
-  addQueryParameterValue: (param: string, value: string) => {
+  addQueryParameterValue: (param: string, value: string) => { return EMPTY;
   },
   getQueryParameterValues: (param: string) => {
     return observableOf({});
diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5
index d9b833804e..ddd7c8f693 100644
--- a/src/assets/i18n/en.json5
+++ b/src/assets/i18n/en.json5
@@ -576,6 +576,8 @@
 
 
 
+  "browse.back.all-results": "All browse results",
+
   "browse.comcol.by.author": "By Author",
 
   "browse.comcol.by.dateissued": "By Issue Date",
@@ -606,6 +608,8 @@
 
   "browse.next.button": "Next",
 
+  "browse.next.button.disabled.tooltip": "No more pages of results",
+
   "browse.previous.button": "Previous",
 
   "browse.startsWith.choose_start": "(Choose start)",
@@ -650,7 +654,7 @@
 
   "browse.startsWith.type_date.label": "Or type in a date (year-month) and click on the Browse button",
 
-  "browse.startsWith.type_text": "Type the first few letters and click on the Browse button",
+  "browse.startsWith.type_text": "Filter results by typing the first few letters",
 
   "browse.title": "Browsing {{ collection }} by {{ field }} {{ value }}",