111731: Fixed search filter input suggestions not resetting the page number by moving that logic to the search configuration service

src/app/core/shared/search/search-configuration.service.spec.ts

@@ -1,4 +1,5 @@
 /* eslint-disable no-empty, @typescript-eslint/no-empty-function */
+import { Params } from '@angular/router';
 import { SearchConfigurationService } from './search-configuration.service';
 import { ActivatedRouteStub } from '../../../shared/testing/active-router.stub';
 import { PaginationComponentOptions } from '../../../shared/pagination/pagination-component-options.model';
@@ -41,9 +42,10 @@ describe('SearchConfigurationService', () => {
     getQueryParamsWithPrefix: observableOf(prefixFilter),
     getRouteParameterValue: observableOf(''),
     getParamsExceptValue: observableOf({}),
+    getParamsWithAdditionalValue: observableOf({}),
   });
 
-  const paginationService = new PaginationServiceStub();
+  let paginationService: PaginationServiceStub;
 
 
   const activatedRoute: ActivatedRouteStub = new ActivatedRouteStub();
@@ -72,6 +74,12 @@ describe('SearchConfigurationService', () => {
     }
   };
   beforeEach(() => {
+    paginationService = new PaginationServiceStub(Object.assign(new PaginationComponentOptions(), {
+      id: defaults.pagination.id,
+      currentPage: 1,
+      pageSize: 20,
+    }));
+
     service = new SearchConfigurationService(routeService, paginationService as any, activatedRoute as any, linkService, halService, requestService, rdb);
   });
 
@@ -305,5 +313,44 @@ describe('SearchConfigurationService', () => {
 
       expect(routeService.getParamsExceptValue).toHaveBeenCalledWith('f.dateIssued.max', '2000');
     });
+
+    it('should reset the page to 1', (done: DoneFn) => {
+      service.unselectAppliedFilterParams('dateIssued.max', '2000').subscribe((params: Params) => {
+        expect(params[`${defaults.pagination.id}.page`]).toBe(1);
+        done();
+      });
+    });
+  });
+
+  describe('selectNewAppliedFilterParams', () => {
+    let appliedFilter: AppliedFilter;
+
+    beforeEach(() => {
+      appliedFilter = Object.assign(new AppliedFilter(), {
+        filter: 'author',
+        operator: 'authority',
+        value: '1282121b-5394-4689-ab93-78d537764052',
+        label: 'Odinson, Thor',
+      });
+    });
+
+    it('should return all params with the applied filter', () => {
+      service.selectNewAppliedFilterParams(appliedFilter.filter, appliedFilter.value, appliedFilter.operator);
+
+      expect(routeService.getParamsWithAdditionalValue).toHaveBeenCalledWith('f.author', '1282121b-5394-4689-ab93-78d537764052,authority');
+    });
+
+    it('should be able to add AppliedFilter without operator', () => {
+      service.selectNewAppliedFilterParams('dateIssued.max', '2000');
+
+      expect(routeService.getParamsWithAdditionalValue).toHaveBeenCalledWith('f.dateIssued.max', '2000');
+    });
+
+    it('should reset the page to 1', (done: DoneFn) => {
+      service.selectNewAppliedFilterParams('dateIssued.max', '2000').subscribe((params: Params) => {
+        expect(params[`${defaults.pagination.id}.page`]).toBe(1);
+        done();
+      });
+    });
   });
 });

src/app/core/shared/search/search-configuration.service.ts

@@ -527,25 +527,33 @@ export class SearchConfigurationService implements OnDestroy {
   }
 
   /**
-   * Calculates the {@link Params} of the search after removing a filter with a certain value
+   * Calculates the {@link Params} of the search after removing a filter with a certain value and resets the page number.
    *
    * @param filterName The {@link AppliedFilter}'s name
    * @param value The {@link AppliedFilter}'s value
    * @param operator The {@link AppliedFilter}'s optional operator
    */
   unselectAppliedFilterParams(filterName: string, value: string, operator?: string): Observable<Params> {
-    return this.routeService.getParamsExceptValue(`f.${filterName}`, hasValue(operator) ? addOperatorToFilterValue(value, operator) : value);
+    return this.routeService.getParamsExceptValue(`f.${filterName}`, hasValue(operator) ? addOperatorToFilterValue(value, operator) : value).pipe(
+      map((params: Params) => Object.assign(params, {
+        [this.paginationService.getPageParam(this.paginationID)]: 1,
+      })),
+    );
   }
 
   /**
-   * Calculates the {@link Params} of the search after removing a filter with a certain value
+   * Calculates the {@link Params} of the search after adding a filter with a certain value and resets the page number.
    *
    * @param filterName The {@link AppliedFilter}'s name
    * @param value The {@link AppliedFilter}'s value
    * @param operator The {@link AppliedFilter}'s optional operator
    */
   selectNewAppliedFilterParams(filterName: string, value: string, operator?: string): Observable<Params> {
-    return this.routeService.getParamsWithAdditionalValue(`f.${filterName}`, hasValue(operator) ? addOperatorToFilterValue(value, operator) : value);
+    return this.routeService.getParamsWithAdditionalValue(`f.${filterName}`, hasValue(operator) ? addOperatorToFilterValue(value, operator) : value).pipe(
+      map((params: Params) => Object.assign(params, {
+        [this.paginationService.getPageParam(this.paginationID)]: 1,
+      })),
+    );
   }
 
   /**

src/app/shared/search/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.spec.ts

@@ -3,9 +3,9 @@ import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
 import { FormsModule } from '@angular/forms';
 import { By } from '@angular/platform-browser';
 import { NoopAnimationsModule } from '@angular/platform-browser/animations';
-import { Router, Params } from '@angular/router';
+import { Router } from '@angular/router';
 import { TranslateModule } from '@ngx-translate/core';
-import { of as observableOf, take } from 'rxjs';
+import { of as observableOf } from 'rxjs';
 import { SearchConfigurationService } from '../../../../../../core/shared/search/search-configuration.service';
 import { SearchFilterService } from '../../../../../../core/shared/search/search-filter.service';
 import { SearchService } from '../../../../../../core/shared/search/search.service';
@@ -87,23 +87,6 @@ describe('SearchFacetOptionComponent', () => {
     fixture.detectChanges();
   });
 
-  describe('updateAddParams', () => {
-    it('should always reset the page to 1', (done: DoneFn) => {
-      spyOn(searchConfigurationService, 'selectNewAppliedFilterParams').and.returnValue(observableOf({
-        [mockFilterConfig.paramName]: [`${facetValue.value},equals`],
-        ['test-id.page']: 5,
-      }));
-
-      comp.updateAddParams().pipe(take(1)).subscribe((params: Params) => {
-        expect(params).toEqual({
-          [mockFilterConfig.paramName]: [`${facetValue.value},equals`],
-          ['test-id.page']: 1,
-        });
-        done();
-      });
-    });
-  });
-
   describe('when isVisible emits true', () => {
     it('the facet option should be visible', () => {
       comp.isVisible = observableOf(true);

src/app/shared/search/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.ts

@@ -92,13 +92,7 @@ export class SearchFacetOptionComponent implements OnInit {
    * Calculates the parameters that should change if this {@link filterValue} would be added to the active filters
    */
   updateAddParams(): Observable<Params> {
-    const page: string = this.paginationService.getPageParam(this.searchConfigService.paginationID);
+    return this.searchConfigService.selectNewAppliedFilterParams(this.filterConfig.name, this.getFacetValue());
-    return this.searchConfigService.selectNewAppliedFilterParams(this.filterConfig.name, this.getFacetValue()).pipe(
-      map((params: Params) => ({
-        ...params,
-        [page]: 1,
-      })),
-    );
   }
 
   /**

src/app/shared/search/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.spec.ts

@@ -2,9 +2,8 @@ import { NO_ERRORS_SCHEMA } from '@angular/core';
 import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
 import { FormsModule } from '@angular/forms';
 import { NoopAnimationsModule } from '@angular/platform-browser/animations';
-import { Params, Router } from '@angular/router';
+import { Router } from '@angular/router';
 import { TranslateModule } from '@ngx-translate/core';
-import { of as observableOf, take } from 'rxjs';
 import { SearchConfigurationService } from '../../../../../../core/shared/search/search-configuration.service';
 import { SearchFilterService } from '../../../../../../core/shared/search/search-filter.service';
 import { SearchService } from '../../../../../../core/shared/search/search.service';
@@ -81,20 +80,7 @@ describe('SearchFacetSelectedOptionComponent', () => {
     fixture.detectChanges();
   });
 
-  describe('updateRemoveParams', () => {
+  it('should create', () => {
-    it('should always reset the page to 1', (done: DoneFn) => {
+    expect(comp).toBeTruthy();
-      spyOn(searchConfigurationService, 'unselectAppliedFilterParams').and.returnValue(observableOf({
-        [mockFilterConfig.paramName]: [`${value1},equals`],
-        ['page-id.page']: 5,
-      }));
-
-      comp.updateRemoveParams().pipe(take(1)).subscribe((params: Params) => {
-        expect(params).toEqual({
-          [mockFilterConfig.paramName]: [`${value1},equals`],
-          ['page-id.page']: 1
-        });
-        done();
-      });
-    });
   });
 });

src/app/shared/search/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.ts

@@ -6,7 +6,6 @@ import { SearchService } from '../../../../../../core/shared/search/search.servi
 import { SearchConfigurationService } from '../../../../../../core/shared/search/search-configuration.service';
 import { currentPath } from '../../../../../utils/route.utils';
 import { AppliedFilter } from '../../../../models/applied-filter.model';
-import { map } from 'rxjs/operators';
 import { PaginationService } from '../../../../../../core/pagination/pagination.service';
 
 @Component({
@@ -64,13 +63,7 @@ export class SearchFacetSelectedOptionComponent implements OnInit {
    * Calculates the parameters that should change if this {@link selectedValue} would be removed from the active filters
    */
   updateRemoveParams(): Observable<Params> {
-    const page: string = this.paginationService.getPageParam(this.searchConfigService.paginationID);
+    return this.searchConfigService.unselectAppliedFilterParams(this.selectedValue.filter, this.selectedValue.value, this.selectedValue.operator);
-    return this.searchConfigService.unselectAppliedFilterParams(this.selectedValue.filter, this.selectedValue.value, this.selectedValue.operator).pipe(
-      map((params: Params) => ({
-        ...params,
-        [page]: 1,
-      })),
-    );
   }
 
   /**

src/app/shared/search/search-filters/search-filter/search-facet-filter/search-facet-filter.component.spec.ts

@@ -24,6 +24,7 @@ import { SEARCH_CONFIG_SERVICE } from '../../../../../my-dspace-page/my-dspace-p
 import { createSuccessfulRemoteDataObject$ } from '../../../../remote-data.utils';
 import { AppliedFilter } from '../../../models/applied-filter.model';
 import { FacetValues } from '../../../models/facet-values.model';
+import { SearchFilterServiceStub } from '../../../../testing/search-filter-service.stub';
 
 describe('SearchFacetFilterComponent', () => {
   let comp: SearchFacetFilterComponent;
@@ -64,37 +65,30 @@ describe('SearchFacetFilterComponent', () => {
 
   const searchLink = '/search';
   const selectedValues = [value1, value2];
-  let filterService;
+  let filterService: SearchFilterServiceStub;
-  let searchService;
+  let searchService: SearchServiceStub;
-  let router;
+  let router: RouterStub;
-  const page = observableOf(0);
+  let searchConfigService: SearchConfigurationServiceStub;
 
   beforeEach(waitForAsync(() => {
+    searchService = new SearchServiceStub(searchLink);
+    filterService = new SearchFilterServiceStub();
+    router = new RouterStub();
+    searchConfigService = new SearchConfigurationServiceStub();
+
     TestBed.configureTestingModule({
       imports: [TranslateModule.forRoot(), NoopAnimationsModule, FormsModule],
       declarations: [SearchFacetFilterComponent],
       providers: [
-        { provide: SearchService, useValue: new SearchServiceStub(searchLink) },
+        { provide: SearchService, useValue: searchService },
-        { provide: Router, useValue: new RouterStub() },
+        { provide: SearchFilterService, useValue: filterService },
+        { provide: Router, useValue: router },
         { provide: FILTER_CONFIG, useValue: new SearchFilterConfig() },
         { provide: RemoteDataBuildService, useValue: { aggregate: () => observableOf({}) } },
-        { provide: SEARCH_CONFIG_SERVICE, useValue: new SearchConfigurationServiceStub() },
+        { provide: SEARCH_CONFIG_SERVICE, useValue: searchConfigService },
         { provide: IN_PLACE_SEARCH, useValue: false },
         { provide: REFRESH_FILTER, useValue: new BehaviorSubject<boolean>(false) },
         { provide: CHANGE_APPLIED_FILTERS, useValue: new EventEmitter() },
-        {
-          provide: SearchFilterService, useValue: {
-            getSelectedValuesForFilter: () => observableOf(selectedValues),
-            isFilterActiveWithValue: (paramName: string, filterValue: string) => true,
-            getPage: (paramName: string) => page,
-            /* eslint-disable no-empty,@typescript-eslint/no-empty-function */
-            incrementPage: (filterName: string) => {
-            },
-            resetPage: (filterName: string) => {
-            }
-            /* eslint-enable no-empty, @typescript-eslint/no-empty-function */
-          }
-        }
       ],
       schemas: [NO_ERRORS_SCHEMA]
     }).overrideComponent(SearchFacetFilterComponent, {
@@ -106,10 +100,7 @@ describe('SearchFacetFilterComponent', () => {
     fixture = TestBed.createComponent(SearchFacetFilterComponent);
     comp = fixture.componentInstance; // SearchPageComponent test instance
     comp.filterConfig = mockFilterConfig;
-    filterService = (comp as any).filterService;
-    searchService = (comp as any).searchService;
     spyOn(searchService, 'getFacetValuesFor').and.returnValue(createSuccessfulRemoteDataObject$(values));
-    router = (comp as any).router;
     fixture.detectChanges();
   });
 
@@ -182,13 +173,14 @@ describe('SearchFacetFilterComponent', () => {
         })));
       fixture.detectChanges();
       spyOn(comp, 'getSearchLink').and.returnValue(searchUrl);
+      spyOn(searchConfigService, 'selectNewAppliedFilterParams').and.returnValue(observableOf({ [mockFilterConfig.paramName]: [...selectedValues.map((value) => `${value},equals`), `${testValue},equals`] }));
     });
 
     it('should call navigate on the router with the right searchlink and parameters when the filter is provided with a valid operator', () => {
       comp.onSubmit(testValue + ',equals');
+      expect(searchConfigService.selectNewAppliedFilterParams).toHaveBeenCalledWith(filterName1, testValue, 'equals');
       expect(router.navigate).toHaveBeenCalledWith(searchUrl.split('/'), {
         queryParams: { [mockFilterConfig.paramName]: [...selectedValues.map((value) => `${value},equals`), `${testValue},equals`] },
-        queryParamsHandling: 'merge'
       });
     });
 

src/app/shared/search/search-filters/search-filter/search-facet-filter/search-facet-filter.component.ts

@@ -1,12 +1,12 @@
 import { animate, state, style, transition, trigger } from '@angular/animations';
 import { Component, Inject, OnDestroy, OnInit, EventEmitter } from '@angular/core';
-import { Router } from '@angular/router';
+import { Router, Params } from '@angular/router';
 
 import { BehaviorSubject, combineLatest as observableCombineLatest, Observable, of as observableOf, Subscription } from 'rxjs';
 import { debounceTime, distinctUntilChanged, filter, map, mergeMap, switchMap, take, tap } from 'rxjs/operators';
 
 import { RemoteDataBuildService } from '../../../../../core/cache/builders/remote-data-build.service';
-import { hasNoValue, hasValue, isNotEmpty } from '../../../../empty.util';
+import { hasNoValue, hasValue } from '../../../../empty.util';
 import { FacetValue } from '../../../models/facet-value.model';
 import { SearchFilterConfig } from '../../../models/search-filter-config.model';
 import { SearchService } from '../../../../../core/shared/search/search.service';
@@ -17,7 +17,7 @@ import { InputSuggestion } from '../../../../input-suggestions/input-suggestions
 import { SearchOptions } from '../../../models/search-options.model';
 import { SEARCH_CONFIG_SERVICE } from '../../../../../my-dspace-page/my-dspace-page.component';
 import { currentPath } from '../../../../utils/route.utils';
-import { stripOperatorFromFilterValue, addOperatorToFilterValue } from '../../../search.utils';
+import { stripOperatorFromFilterValue } from '../../../search.utils';
 import { FacetValues } from '../../../models/facet-values.model';
 import { AppliedFilter } from '../../../models/applied-filter.model';
 
@@ -203,26 +203,18 @@ export class SearchFacetFilterComponent implements OnInit, OnDestroy {
 
   /**
    * Build the filter query using the value given and apply to the search.
-   * @param data The string from the input field
+   * @param data The string from the input field (containing operator)
    */
-  protected applyFilterValue(data) {
+  protected applyFilterValue(data: string): void {
     if (data.match(new RegExp(`^.+,(equals|query|authority)$`))) {
-      this.selectedAppliedFilters$.pipe(take(1)).subscribe((selectedValues: AppliedFilter[]) => {
+      const valueParts = data.split(',');
-        if (isNotEmpty(data)) {
+      this.subs.push(this.searchConfigService.selectNewAppliedFilterParams(this.filterConfig.name, valueParts.slice(0, valueParts.length - 1).join(), valueParts[valueParts.length - 1]).pipe(take(1)).subscribe((params: Params) => {
-          void this.router.navigate(this.getSearchLinkParts(), {
+        void this.router.navigate(this.getSearchLinkParts(), {
-            queryParams:
+          queryParams: params,
-              {
+        });
-                [this.filterConfig.paramName]: [
+        this.filter = '';
-                  ...selectedValues.map((appliedFilter: AppliedFilter) => addOperatorToFilterValue(appliedFilter.value, appliedFilter.operator)),
-                  data
-                ]
-              },
-            queryParamsHandling: 'merge'
-          });
-          this.filter = '';
-        }
         this.filterSearchResults$ = observableOf([]);
-      });
+      }));
     }
   }
 

src/app/shared/search/search-labels/search-label-range/search-label-range.component.spec.ts

@@ -12,8 +12,6 @@ import { SearchConfigurationService } from '../../../../core/shared/search/searc
 import { SearchConfigurationServiceStub } from '../../../testing/search-configuration-service.stub';
 import { PaginationService } from '../../../../core/pagination/pagination.service';
 import { PaginationServiceStub } from '../../../testing/pagination-service.stub';
-import { take } from 'rxjs/operators';
-import { of as observableOf } from 'rxjs';
 import { PaginationComponentOptions } from '../../../pagination/pagination-component-options.model';
 
 describe('SearchLabelRangeComponent', () => {
@@ -79,16 +77,7 @@ describe('SearchLabelRangeComponent', () => {
     fixture.detectChanges();
   });
 
-  describe('updateRemoveParams', () => {
+  it('should create', () => {
-    it('should always reset the page to 1', (done: DoneFn) => {
+    expect(comp).toBeTruthy();
-      spyOn(searchConfigurationService, 'unselectAppliedFilterParams').and.returnValue(observableOf(initialRouteParams));
-
-      comp.updateRemoveParams('f.dateIssued.max', '2000').pipe(take(1)).subscribe((params: Params) => {
-        expect(params).toEqual(Object.assign({}, initialRouteParams, {
-          'page-id.page': 1,
-        }));
-        done();
-      });
-    });
   });
 });

src/app/shared/search/search-labels/search-label-range/search-label-range.component.ts

@@ -6,7 +6,6 @@ import { currentPath } from '../../../utils/route.utils';
 import { AppliedFilter } from '../../models/applied-filter.model';
 import { renderSearchLabelFor } from '../search-label-loader/search-label-loader.decorator';
 import { SearchConfigurationService } from '../../../../core/shared/search/search-configuration.service';
-import { map } from 'rxjs/operators';
 import { PaginationService } from '../../../../core/pagination/pagination.service';
 
 /**
@@ -57,13 +56,7 @@ export class SearchLabelRangeComponent implements OnInit {
    * @param operator The {@link AppliedFilter}'s optional operator
    */
   updateRemoveParams(filterName: string, value: string, operator?: string): Observable<Params> {
-    const page: string = this.paginationService.getPageParam(this.searchConfigurationService.paginationID);
+    return this.searchConfigurationService.unselectAppliedFilterParams(filterName, value, operator);
-    return this.searchConfigurationService.unselectAppliedFilterParams(filterName, value, operator).pipe(
-      map((params: Params) => ({
-        ...params,
-        [page]: 1,
-      })),
-    );
   }
 
   /**

src/app/shared/search/search-labels/search-label/search-label.component.spec.ts

@@ -13,8 +13,6 @@ import { SearchConfigurationServiceStub } from '../../../testing/search-configur
 import { PaginationService } from '../../../../core/pagination/pagination.service';
 import { PaginationServiceStub } from '../../../testing/pagination-service.stub';
 import { PaginationComponentOptions } from '../../../pagination/pagination-component-options.model';
-import { of as observableOf } from 'rxjs';
-import { take } from 'rxjs/operators';
 
 describe('SearchLabelComponent', () => {
   let comp: SearchLabelComponent;
@@ -79,16 +77,7 @@ describe('SearchLabelComponent', () => {
     fixture.detectChanges();
   });
 
-  describe('updateRemoveParams', () => {
+  it('should create', () => {
-    it('should always reset the page to 1', (done: DoneFn) => {
+    expect(comp).toBeTruthy();
-      spyOn(searchConfigurationService, 'unselectAppliedFilterParams').and.returnValue(observableOf(initialRouteParams));
-
-      comp.updateRemoveParams().pipe(take(1)).subscribe((params: Params) => {
-        expect(params).toEqual(Object.assign({}, initialRouteParams, {
-          'page-id.page': 1,
-        }));
-        done();
-      });
-    });
   });
 });

src/app/shared/search/search-labels/search-label/search-label.component.ts

@@ -6,7 +6,6 @@ import { currentPath } from '../../../utils/route.utils';
 import { AppliedFilter } from '../../models/applied-filter.model';
 import { SearchConfigurationService } from '../../../../core/shared/search/search-configuration.service';
 import { renderSearchLabelFor } from '../search-label-loader/search-label-loader.decorator';
-import { map } from 'rxjs/operators';
 import { PaginationService } from '../../../../core/pagination/pagination.service';
 
 /**
@@ -49,13 +48,7 @@ export class SearchLabelComponent implements OnInit {
    * Calculates the parameters that should change if this {@link appliedFilter} would be removed from the active filters
    */
   updateRemoveParams(): Observable<Params> {
-    const page: string = this.paginationService.getPageParam(this.searchConfigurationService.paginationID);
+    return this.searchConfigurationService.unselectAppliedFilterParams(this.appliedFilter.filter, this.appliedFilter.value, this.appliedFilter.operator);
-    return this.searchConfigurationService.unselectAppliedFilterParams(this.appliedFilter.filter, this.appliedFilter.value, this.appliedFilter.operator).pipe(
-      map((params: Params) => ({
-        ...params,
-        [page]: 1,
-      })),
-    );
   }
 
   /**