From f4826ca71e5fa43ea652f980f3899ef41aa65169 Mon Sep 17 00:00:00 2001 From: lotte Date: Thu, 2 Aug 2018 11:34:34 +0200 Subject: [PATCH] search page switchmap fix --- src/app/+search-page/search-page.component.ts | 13 ++-- .../search-configuration.service.spec.ts | 63 +++++++++---------- .../search-configuration.service.ts | 25 ++++---- src/app/shared/services/route.service.ts | 1 - 4 files changed, 47 insertions(+), 55 deletions(-) diff --git a/src/app/+search-page/search-page.component.ts b/src/app/+search-page/search-page.component.ts index 52e59766d9..2326355472 100644 --- a/src/app/+search-page/search-page.component.ts +++ b/src/app/+search-page/search-page.component.ts @@ -77,14 +77,11 @@ export class SearchPageComponent implements OnInit { */ ngOnInit(): void { this.searchOptions$ = this.searchConfigService.paginatedSearchOptions; - let subscription; - this.sub = this.searchOptions$.subscribe((searchOptions) => { - if (hasValue(subscription)) { - /** Make sure no race condition is possible with a previous result coming back from the server later */ - subscription.unsubscribe(); - } - subscription = this.service.search(searchOptions).filter((rd) => !rd.isLoading).first().subscribe((results) => this.resultsRD$.next(results)) - }); + this.sub = this.searchOptions$ + .switchMap((options) => this.service.search(options).filter((rd) => !rd.isLoading).first()) + .subscribe((results) => { + this.resultsRD$.next(results); + }); this.scopeListRD$ = this.searchConfigService.getCurrentScope('').pipe( flatMap((scopeId) => this.service.getScopes(scopeId)) ); diff --git a/src/app/+search-page/search-service/search-configuration.service.spec.ts b/src/app/+search-page/search-service/search-configuration.service.spec.ts index d7a41c4759..af1b19f06a 100644 --- a/src/app/+search-page/search-service/search-configuration.service.spec.ts +++ b/src/app/+search-page/search-service/search-configuration.service.spec.ts @@ -1,13 +1,11 @@ import { SearchConfigurationService } from './search-configuration.service'; -import { Observable } from 'rxjs/Observable'; import { ActivatedRouteStub } from '../../shared/testing/active-router-stub'; -import { RemoteData } from '../../core/data/remote-data'; import { PaginationComponentOptions } from '../../shared/pagination/pagination-component-options.model'; import { SortDirection, SortOptions } from '../../core/cache/models/sort-options.model'; import { PaginatedSearchOptions } from '../paginated-search-options.model'; -import { cold, hot } from 'jasmine-marbles'; +import { Observable } from 'rxjs/Observable'; -fdescribe('SearchConfigurationService', () => { +describe('SearchConfigurationService', () => { let service: SearchConfigurationService; const value1 = 'random value'; const value2 = 'another value'; @@ -25,8 +23,8 @@ fdescribe('SearchConfigurationService', () => { const backendFilters = { 'f.author': ['another value'], 'f.date': ['[2013 TO 2018]'] }; const spy = jasmine.createSpyObj('RouteService', { - getQueryParameterValue: cold('a', {a: [value1, value2]}), - getQueryParamsWithPrefix: cold('a', {a: prefixFilter}), + getQueryParameterValue: Observable.of([value1, value2]), + getQueryParamsWithPrefix: Observable.of(prefixFilter) }); const activatedRoute: any = new ActivatedRouteStub(); @@ -97,38 +95,39 @@ fdescribe('SearchConfigurationService', () => { expect((service as any).routeService.getQueryParameterValue).toHaveBeenCalledWith('pageSize'); }); }); - fdescribe('when updateSearchOptions or updatePaginatedSearchOptions is called', () => { + describe('when subscribeToSearchOptions or subscribeToPaginatedSearchOptions is called', () => { beforeEach(() => { - spyOn(service, 'getPaginationPart'); - spyOn(service, 'getSortPart'); - spyOn(service, 'getScopePart'); - spyOn(service, 'getQueryPart'); - spyOn(service, 'getFiltersPart'); + spyOn(service, 'getCurrentPagination').and.callThrough(); + spyOn(service, 'getCurrentSort').and.callThrough(); + spyOn(service, 'getCurrentScope').and.callThrough(); + spyOn(service, 'getCurrentQuery').and.callThrough(); + spyOn(service, 'getCurrentFilters').and.callThrough(); }); - describe('when getPaginatedSearchOptions is called', () => { + + describe('when subscribeToSearchOptions is called', () => { beforeEach(() => { - service.subscribeToSearchOptions(defaults); + service.subscribeToSearchOptions(defaults) + }); + it('should call all getters it needs, but not call any others', () => { + expect(service.getCurrentPagination).not.toHaveBeenCalled(); + expect(service.getCurrentSort).not.toHaveBeenCalled(); + expect(service.getCurrentScope).toHaveBeenCalled(); + expect(service.getCurrentQuery).toHaveBeenCalled(); + expect(service.getCurrentFilters).toHaveBeenCalled(); + }); + }); + + describe('when subscribeToPaginatedSearchOptions is called', () => { + beforeEach(() => { + service.subscribeToPaginatedSearchOptions(defaults); }); it('should call all getters it needs', () => { - - // expect(service.getCurrentPagination).toHaveBeenCalled(); - // expect(service.getCurrentSort).toHaveBeenCalled(); - expect(service.getScopePart).toHaveBeenCalled(); - expect(service.getQueryPart).toHaveBeenCalled(); - expect(service.getFiltersPart).toHaveBeenCalled(); + expect(service.getCurrentPagination).toHaveBeenCalled(); + expect(service.getCurrentSort).toHaveBeenCalled(); + expect(service.getCurrentScope).toHaveBeenCalled(); + expect(service.getCurrentQuery).toHaveBeenCalled(); + expect(service.getCurrentFilters).toHaveBeenCalled(); }); }); - // describe('when searchOptions is called', () => { - // beforeEach(() => { - // service.searchOptions; - // }); - // it('should call all getters it needs', () => { - // expect(service.getCurrentPagination).not.toHaveBeenCalled(); - // expect(service.getCurrentSort).not.toHaveBeenCalled(); - // expect(service.getCurrentScope).toHaveBeenCalled(); - // expect(service.getCurrentQuery).toHaveBeenCalled(); - // expect(service.getCurrentFilters).toHaveBeenCalled(); - // }); - // }); }); }); diff --git a/src/app/+search-page/search-service/search-configuration.service.ts b/src/app/+search-page/search-service/search-configuration.service.ts index 6aeb856e87..95c4c60d65 100644 --- a/src/app/+search-page/search-service/search-configuration.service.ts +++ b/src/app/+search-page/search-service/search-configuration.service.ts @@ -1,7 +1,6 @@ import { SortDirection, SortOptions } from '../../core/cache/models/sort-options.model'; import { PaginationComponentOptions } from '../../shared/pagination/pagination-component-options.model'; import { SearchOptions } from '../search-options.model'; -import { distinctUntilChanged, map } from 'rxjs/operators'; import { Observable } from 'rxjs/Observable'; import { ActivatedRoute, Params } from '@angular/router'; import { PaginatedSearchOptions } from '../paginated-search-options.model'; @@ -17,7 +16,11 @@ import { Subscription } from 'rxjs/Subscription'; */ @Injectable() export class SearchConfigurationService implements OnDestroy { - private defaultPagination = Object.assign(new PaginationComponentOptions(), { id: 'search-page-configuration', pageSize: 10, currentPage: 1 }); + private defaultPagination = Object.assign(new PaginationComponentOptions(), { + id: 'search-page-configuration', + pageSize: 10, + currentPage: 1 + }); private defaultSort = new SortOptions('score', SortDirection.DESC); @@ -118,7 +121,7 @@ export class SearchConfigurationService implements OnDestroy { }); } - /** + /** * @returns {Observable} Emits the current active filters with their values as they are displayed in the frontend URL */ getCurrentFrontendFilters(): Observable { @@ -126,12 +129,6 @@ export class SearchConfigurationService implements OnDestroy { } subscribeToSearchOptions(defaults: SearchOptions): Subscription { - console.log('scope: ', this.getScopePart(defaults.scope)); - console.log('query: ', this.getQueryPart(defaults.query)); - console.log('filters: ', this.getFiltersPart()); - this.getScopePart(defaults.scope).subscribe((y) => console.log('scope: ' + JSON.stringify(y))); - this.getQueryPart(defaults.query).subscribe((y) => console.log('query: ' + JSON.stringify(y))); - this.getFiltersPart().subscribe((y) => console.log('filters: ' + JSON.stringify(y))); return Observable.merge( this.getScopePart(defaults.scope), this.getQueryPart(defaults.query), @@ -179,7 +176,7 @@ export class SearchConfigurationService implements OnDestroy { }); } - getScopePart(defaultScope: string): Observable { + private getScopePart(defaultScope: string): Observable { return this.getCurrentScope(defaultScope).map((scope) => { return { scope } }); @@ -188,7 +185,7 @@ export class SearchConfigurationService implements OnDestroy { /** * @returns {Observable} Emits the current query string */ - getQueryPart(defaultQuery: string): Observable { + private getQueryPart(defaultQuery: string): Observable { return this.getCurrentQuery(defaultQuery).map((query) => { return { query } }); @@ -197,7 +194,7 @@ export class SearchConfigurationService implements OnDestroy { /** * @returns {Observable} Emits the current pagination settings */ - getPaginationPart(defaultPagination: PaginationComponentOptions): Observable { + private getPaginationPart(defaultPagination: PaginationComponentOptions): Observable { return this.getCurrentPagination(defaultPagination).map((pagination) => { return { pagination } }); @@ -206,7 +203,7 @@ export class SearchConfigurationService implements OnDestroy { /** * @returns {Observable} Emits the current sorting settings */ - getSortPart(defaultSort: SortOptions): Observable { + private getSortPart(defaultSort: SortOptions): Observable { return this.getCurrentSort(defaultSort).map((sort) => { return { sort } }); @@ -215,7 +212,7 @@ export class SearchConfigurationService implements OnDestroy { /** * @returns {Observable} Emits the current active filters with their values as they are sent to the backend */ - getFiltersPart(): Observable { + private getFiltersPart(): Observable { return this.getCurrentFilters().map((filters) => { return { filters } }); diff --git a/src/app/shared/services/route.service.ts b/src/app/shared/services/route.service.ts index 4b237d7930..7c24c6f6c1 100644 --- a/src/app/shared/services/route.service.ts +++ b/src/app/shared/services/route.service.ts @@ -17,7 +17,6 @@ export class RouteService { } getQueryParameterValue(paramName: string): Observable { - this.route.queryParamMap.map((map) => map.get(paramName)).distinctUntilChanged().subscribe((t) => console.log('paramName: ' + paramName + 'values: ' + t)); return this.route.queryParamMap.map((map) => map.get(paramName)).distinctUntilChanged(); }