111731: Made the SearchLabelComponent use the AppliedFilter to display the label instead of value

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

@@ -7,12 +7,15 @@ import { PaginatedSearchOptions } from '../../../shared/search/models/paginated-
 import { SearchFilter } from '../../../shared/search/models/search-filter.model';
 import { combineLatest as observableCombineLatest, Observable, of as observableOf } from 'rxjs';
 import { PaginationServiceStub } from '../../../shared/testing/pagination-service.stub';
-import { map } from 'rxjs/operators';
+import { map, take } from 'rxjs/operators';
 import { RemoteData } from '../../data/remote-data';
 import { createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils';
 import { getMockRequestService } from '../../../shared/mocks/request.service.mock';
 import { RequestEntry } from '../../data/request-entry.model';
 import { SearchObjects } from '../../../shared/search/models/search-objects.model';
+import { Params } from '@angular/router';
+import { addOperatorToFilterValue } from '../../../shared/search/search.utils';
+import { AppliedFilter } from '../../../shared/search/models/applied-filter.model';
 
 describe('SearchConfigurationService', () => {
   let service: SearchConfigurationService;
@@ -44,7 +47,7 @@ describe('SearchConfigurationService', () => {
   const paginationService = new PaginationServiceStub();
 
 
-  const activatedRoute: any = new ActivatedRouteStub();
+  const activatedRoute: ActivatedRouteStub = new ActivatedRouteStub();
   const linkService: any = {};
   const requestService: any = getMockRequestService();
   const halService: any = {
@@ -70,7 +73,7 @@ describe('SearchConfigurationService', () => {
     }
   };
   beforeEach(() => {
-    service = new SearchConfigurationService(routeService, paginationService as any, activatedRoute, linkService, halService, requestService, rdb);
+    service = new SearchConfigurationService(routeService, paginationService as any, activatedRoute as any, linkService, halService, requestService, rdb);
   });
 
   describe('when the scope is called', () => {
@@ -279,4 +282,62 @@ describe('SearchConfigurationService', () => {
       expect((service as any).requestService.send).toHaveBeenCalledWith(jasmine.objectContaining({ href: requestUrl }), true);
     });
   });
+
+  describe('getParamsWithoutAppliedFilter', () => {
+    let appliedFilter: AppliedFilter;
+
+    beforeEach(() => {
+      appliedFilter = Object.assign(new AppliedFilter(), {
+        filter: 'author',
+        operator: 'authority',
+        value: '1282121b-5394-4689-ab93-78d537764052',
+        label: 'Odinson, Thor',
+      });
+      activatedRoute.testParams = {
+        'query': '',
+        'spc.page': '1',
+        'f.author': addOperatorToFilterValue(appliedFilter.value, appliedFilter.operator),
+        'f.has_content_in_original_bundle': addOperatorToFilterValue('true', 'equals'),
+        'f.dateIssued.max': '2000',
+      };
+    });
+
+    it('should return all params except the applied filter', (done: DoneFn) => {
+      service.getParamsWithoutAppliedFilter(appliedFilter.filter, appliedFilter.value, appliedFilter.operator).pipe(take(1)).subscribe((params: Params) => {
+        expect(params).toEqual({
+          'query': '',
+          'spc.page': '1',
+          'f.has_content_in_original_bundle': addOperatorToFilterValue('true', 'equals'),
+          'f.dateIssued.max': '2000',
+        });
+        done();
+      });
+    });
+
+    it('should return all params except the applied filter even when multiple filters of the same type are selected', (done: DoneFn) => {
+      activatedRoute.testParams['f.author'] = [addOperatorToFilterValue(appliedFilter.value, appliedFilter.operator), addOperatorToFilterValue('71b91a28-c280-4352-a199-bd7fc3312501', 'authority')];
+      service.getParamsWithoutAppliedFilter(appliedFilter.filter, appliedFilter.value, appliedFilter.operator).pipe(take(1)).subscribe((params: Params) => {
+        expect(params).toEqual({
+          'query': '',
+          'spc.page': '1',
+          'f.author': [addOperatorToFilterValue('71b91a28-c280-4352-a199-bd7fc3312501', 'authority')],
+          'f.has_content_in_original_bundle': addOperatorToFilterValue('true', 'equals'),
+          'f.dateIssued.max': '2000',
+        });
+        done();
+      });
+    });
+
+    it('should be able to remove AppliedFilter without operator', (done: DoneFn) => {
+      service.getParamsWithoutAppliedFilter('dateIssued.max', '2000').pipe(take(1)).subscribe((params: Params) => {
+        expect(params).toEqual({
+          'query': '',
+          'spc.page': '1',
+          'f.author': addOperatorToFilterValue(appliedFilter.value, appliedFilter.operator),
+          'f.has_content_in_original_bundle': addOperatorToFilterValue('true', 'equals'),
+        });
+        done();
+      });
+    });
+  });
 });

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

@@ -28,6 +28,7 @@ import { FacetConfigResponseParsingService } from '../../data/facet-config-respo
 import { ViewMode } from '../view-mode.model';
 import { SearchFilterConfig } from '../../../shared/search/models/search-filter-config.model';
 import { FacetConfigResponse } from '../../../shared/search/models/facet-config-response.model';
+import { addOperatorToFilterValue } from '../../../shared/search/search.utils';
 
 /**
  * Service that performs all actions that have to do with the current search configuration
@@ -525,6 +526,23 @@ export class SearchConfigurationService implements OnDestroy {
     );
   }
 
+  getParamsWithoutAppliedFilter(filterName: string, value: string, operator?: string): Observable<Params> {
+    return this.route.queryParams.pipe(
+      map((params: Params) => {
+        const newParams: Params = Object.assign({}, params);
+        const queryParamValues: string | string[] = newParams[`f.${filterName}`];
+        const excludeValue = hasValue(operator) ? addOperatorToFilterValue(value, operator) : value;
+
+        if (queryParamValues === excludeValue) {
+          delete newParams[`f.${filterName}`];
+        } else if (queryParamValues?.includes(excludeValue)) {
+          newParams[`f.${filterName}`] = (queryParamValues as string[])
+            .filter((paramValue: string) => paramValue !== excludeValue);
+        }
+        return newParams;
+      }),
+    );
+  }
 
   /**
    * @returns {Observable<Params>} Emits the current view mode as a partial SearchOptions object

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

@@ -1,6 +1,6 @@
 <a class="badge badge-primary mr-1 mb-1 text-capitalize"
    [routerLink]="searchLink"
-   [queryParams]="(removeParameters | async)" queryParamsHandling="merge">
-    {{('search.filters.applied.' + key) | translate}}: {{'search.filters.' + filterName + '.' + value | translate: {default: normalizeFilterValue(value)} }}
+   [queryParams]="(removeParameters | async)">
+    {{('search.filters.applied.f.' + appliedFilter.filter) | translate}}: {{'search.filters.' + appliedFilter.filter + '.' + appliedFilter.label | translate: {default: appliedFilter.label} }}
     <span> ×</span>
 </a>

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

@@ -1,99 +1,71 @@
 import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
-import { NoopAnimationsModule } from '@angular/platform-browser/animations';
 import { TranslateModule } from '@ngx-translate/core';
-import { ChangeDetectionStrategy, NO_ERRORS_SCHEMA } from '@angular/core';
-import { FormsModule } from '@angular/forms';
-import { Observable, of as observableOf } from 'rxjs';
-import { Params, Router } from '@angular/router';
+import { Params, ActivatedRoute } from '@angular/router';
 import { SearchLabelComponent } from './search-label.component';
-import { ObjectKeysPipe } from '../../../utils/object-keys-pipe';
-import { SEARCH_CONFIG_SERVICE } from '../../../../my-dspace-page/my-dspace-page.component';
 import { SearchServiceStub } from '../../../testing/search-service.stub';
-import { SearchConfigurationServiceStub } from '../../../testing/search-configuration-service.stub';
 import { SearchService } from '../../../../core/shared/search/search.service';
-import { PaginationComponentOptions } from '../../../pagination/pagination-component-options.model';
-import { SortDirection, SortOptions } from '../../../../core/cache/models/sort-options.model';
-import { PaginationService } from '../../../../core/pagination/pagination.service';
+import { ActivatedRouteStub } from '../../../testing/active-router.stub';
+import { AppliedFilter } from '../../models/applied-filter.model';
+import { addOperatorToFilterValue } from '../../search.utils';
+import { RouterTestingModule } from '@angular/router/testing';
 import { SearchConfigurationService } from '../../../../core/shared/search/search-configuration.service';
-import { PaginationServiceStub } from '../../../testing/pagination-service.stub';
-import { FindListOptions } from '../../../../core/data/find-list-options.model';
+import { SearchConfigurationServiceStub } from '../../../testing/search-configuration-service.stub';
 
 describe('SearchLabelComponent', () => {
   let comp: SearchLabelComponent;
   let fixture: ComponentFixture<SearchLabelComponent>;
 
+  let route: ActivatedRouteStub;
+  let searchConfigurationService: SearchConfigurationServiceStub;
+
   const searchLink = '/search';
-  let searchService;
+  let appliedFilter: AppliedFilter;
+  let initialRouteParams: Params;
 
-  const key1 = 'author';
-  const key2 = 'subject';
-  const value1 = 'Test, Author';
-  const normValue1 = 'Test, Author';
-  const value2 = 'TestSubject';
-  const value3 = 'Test, Authority,authority';
-  const normValue3 = 'Test, Authority';
-  const filter1 = [key1, value1];
-  const filter2 = [key2, value2];
-  const mockFilters = [
-    filter1,
-    filter2
-  ];
+  function init(): void {
+    appliedFilter = Object.assign(new AppliedFilter(), {
+      filter: 'author',
+      operator: 'authority',
+      value: '1282121b-5394-4689-ab93-78d537764052',
+      label: 'Odinson, Thor',
+    });
+    initialRouteParams = {
+      'query': '',
+      'spc.page': '1',
+      'f.author': addOperatorToFilterValue(appliedFilter.value, appliedFilter.operator),
+      'f.has_content_in_original_bundle': addOperatorToFilterValue('true', 'equals'),
+    };
+  }
 
-  const pagination = Object.assign(new PaginationComponentOptions(), { id: 'page-id', currentPage: 1, pageSize: 20 });
-  const paginationService = new PaginationServiceStub(pagination);
+  beforeEach(waitForAsync(async () => {
+    init();
+    route = new ActivatedRouteStub(initialRouteParams);
+    searchConfigurationService = new SearchConfigurationServiceStub();
 
-  beforeEach(waitForAsync(() => {
-    TestBed.configureTestingModule({
-      imports: [TranslateModule.forRoot(), NoopAnimationsModule, FormsModule],
-      declarations: [SearchLabelComponent, ObjectKeysPipe],
-      providers: [
-        { provide: SearchService, useValue: new SearchServiceStub(searchLink) },
-        { provide: SEARCH_CONFIG_SERVICE, useValue: new SearchConfigurationServiceStub() },
-        { provide: SearchConfigurationService, useValue: new SearchConfigurationServiceStub() },
-        { provide: PaginationService, useValue: paginationService },
-        { provide: Router, useValue: {} }
-        // { provide: SearchConfigurationService, useValue: {getCurrentFrontendFilters : () =>  observableOf({})} }
+    await TestBed.configureTestingModule({
+      imports: [
+        RouterTestingModule,
+        TranslateModule.forRoot(),
+      ],
+      declarations: [
+        SearchLabelComponent,
+      ],
+      providers: [
+        { provide: SearchConfigurationService, useValue: searchConfigurationService },
+        { provide: SearchService, useValue: new SearchServiceStub(searchLink) },
+        { provide: ActivatedRoute, useValue: route },
       ],
-      schemas: [NO_ERRORS_SCHEMA]
-    }).overrideComponent(SearchLabelComponent, {
-      set: { changeDetection: ChangeDetectionStrategy.Default }
     }).compileComponents();
   }));
 
   beforeEach(() => {
     fixture = TestBed.createComponent(SearchLabelComponent);
     comp = fixture.componentInstance;
-    searchService = (comp as any).searchService;
-    comp.key = key1;
-    comp.value = value1;
-    (comp as any).appliedFilters = observableOf(mockFilters);
+    comp.appliedFilter = appliedFilter;
     fixture.detectChanges();
   });
 
-  describe('when getRemoveParams is called', () => {
-    let obs: Observable<Params>;
-
-    beforeEach(() => {
-      obs = comp.getRemoveParams();
-    });
-
-    it('should return all params but the provided filter', () => {
-      obs.subscribe((params) => {
-        // Should contain only filter2 and page: length == 2
-        expect(Object.keys(params).length).toBe(2);
-      });
-    });
-  });
-
-  describe('when normalizeFilterValue is called', () => {
-    it('should return properly filter value', () => {
-      let result: string;
-
-      result = comp.normalizeFilterValue(value1);
-      expect(result).toBe(normValue1);
-
-      result = comp.normalizeFilterValue(value3);
-      expect(result).toBe(normValue3);
-    });
+  it('should create', () => {
+    expect(comp).toBeTruthy();
   });
 });

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

@@ -1,11 +1,9 @@
 import { Component, Input, OnInit } from '@angular/core';
 import { Observable } from 'rxjs';
 import { Params, Router } from '@angular/router';
-import { map } from 'rxjs/operators';
-import { hasValue, isNotEmpty } from '../../../empty.util';
 import { SearchService } from '../../../../core/shared/search/search.service';
 import { currentPath } from '../../../utils/route.utils';
-import { PaginationService } from '../../../../core/pagination/pagination.service';
+import { AppliedFilter } from '../../models/applied-filter.model';
 import { SearchConfigurationService } from '../../../../core/shared/search/search-configuration.service';
 
 @Component({
@@ -17,50 +15,24 @@ import { SearchConfigurationService } from '../../../../core/shared/search/searc
  * Component that represents the label containing the currently active filters
  */
 export class SearchLabelComponent implements OnInit {
-  @Input() key: string;
-  @Input() value: string;
   @Input() inPlaceSearch: boolean;
-  @Input() appliedFilters: Observable<Params>;
+  @Input() appliedFilter: AppliedFilter;
   searchLink: string;
   removeParameters: Observable<Params>;
 
-  /**
-   * The name of the filter without the f. prefix
-   */
-  filterName: string;
-
   /**
    * Initialize the instance variable
    */
   constructor(
-    private searchService: SearchService,
-    private paginationService: PaginationService,
-    private searchConfigurationService: SearchConfigurationService,
-    private router: Router) {
+    protected searchConfigurationService: SearchConfigurationService,
+    protected searchService: SearchService,
+    protected router: Router,
+  ) {
   }
 
   ngOnInit(): void {
     this.searchLink = this.getSearchLink();
-    this.removeParameters = this.getRemoveParams();
-    this.filterName = this.getFilterName();
-  }
-
-  /**
-   * Calculates the parameters that should change if a given value for the given filter would be removed from the active filters
-   * @returns {Observable<Params>} The changed filter parameters
-   */
-  getRemoveParams(): Observable<Params> {
-    return this.appliedFilters.pipe(
-      map((filters) => {
-        const field: string = Object.keys(filters).find((f) => f === this.key);
-        const newValues = hasValue(filters[field]) ? filters[field].filter((v) => v !== this.value) : null;
-        const page = this.paginationService.getPageParam(this.searchConfigurationService.paginationID);
-        return {
-          [field]: isNotEmpty(newValues) ? newValues : null,
-          [page]: 1
-        };
-      })
-    );
+    this.removeParameters = this.searchConfigurationService.getParamsWithoutAppliedFilter(this.appliedFilter.filter, this.appliedFilter.value, this.appliedFilter.operator);
   }
 
   /**
@@ -73,20 +45,4 @@ export class SearchLabelComponent implements OnInit {
     return this.searchService.getSearchLink();
   }
 
-  /**
-   * TODO to review after https://github.com/DSpace/dspace-angular/issues/368 is resolved
-   * Strips authority operator from filter value
-   * e.g. 'test ,authority' => 'test'
-   *
-   * @param value
-   */
-  normalizeFilterValue(value: string) {
-    // const pattern = /,[^,]*$/g;
-    const pattern = /,authority*$/g;
-    return value.replace(pattern, '');
-  }
-
-  private getFilterName(): string {
-    return this.key.startsWith('f.') ? this.key.substring(2) : this.key;
-  }
 }

src/app/shared/search/search-labels/search-labels.component.html

@@ -1,5 +1,5 @@
 <div class="labels">
   <ng-container *ngFor="let appliedFilters of (appliedFilters | keyvalue)">
-    <ng-container *ngFor="let appliedFilter of appliedFilters.value">{{appliedFilter.label}}</ng-container>
+    <ds-search-label *ngFor="let appliedFilter of appliedFilters.value" [inPlaceSearch]="inPlaceSearch" [appliedFilter]="appliedFilter"></ds-search-label>
   </ng-container>
 </div>

src/app/shared/testing/search-configuration-service.stub.ts

@@ -1,6 +1,5 @@
-import { BehaviorSubject, of as observableOf } from 'rxjs';
-import { SearchConfig } from '../../core/shared/search/search-filters/search-config.model';
-import { SortDirection, SortOptions } from '../../core/cache/models/sort-options.model';
+import { BehaviorSubject, of as observableOf, Observable } from 'rxjs';
+import { Params } from '@angular/router';
 
 export class SearchConfigurationServiceStub {
 
@@ -33,15 +32,8 @@ export class SearchConfigurationServiceStub {
     return observableOf([{value: 'test', label: 'test'}]);
   }
 
-  getConfigurationSearchConfigObservable() {
-    return observableOf(new SearchConfig());
+  getParamsWithoutAppliedFilter(_filterName: string, _value: string, _operator?: string): Observable<Params> {
+    return observableOf({});
   }
 
-  getConfigurationSortOptionsObservable() {
-    return observableOf([new SortOptions('score', SortDirection.ASC), new SortOptions('score', SortDirection.DESC)]);
-  }
-
-  initializeSortOptionsFromConfiguration() {
-   /* empty */
-  }
 }