From 0e7e6a4832f0f8d9c6521eeef7fad712e7ee69f6 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Wed, 6 Mar 2019 18:36:57 +0100 Subject: [PATCH 01/10] replace pathSelector by ngrx memoized selectors --- src/app/core/cache/object-cache.service.ts | 43 ++++--- .../core/cache/server-sync-buffer.effects.ts | 3 +- src/app/core/core.reducers.ts | 6 +- src/app/core/core.selectors.ts | 4 + .../object-updates/object-updates.service.ts | 3 +- src/app/core/data/request.service.spec.ts | 2 +- src/app/core/data/request.service.ts | 121 +++++++++--------- src/app/core/index/index.reducer.spec.ts | 4 +- src/app/core/index/index.reducer.ts | 25 ++-- src/app/core/index/index.selectors.ts | 45 +++++++ src/app/core/shared/selectors.ts | 17 --- 11 files changed, 154 insertions(+), 119 deletions(-) create mode 100644 src/app/core/core.selectors.ts create mode 100644 src/app/core/index/index.selectors.ts delete mode 100644 src/app/core/shared/selectors.ts diff --git a/src/app/core/cache/object-cache.service.ts b/src/app/core/cache/object-cache.service.ts index d4d52b404f1..9be895c41a1 100644 --- a/src/app/core/cache/object-cache.service.ts +++ b/src/app/core/cache/object-cache.service.ts @@ -1,36 +1,39 @@ +import { Injectable } from '@angular/core'; +import { createSelector, MemoizedSelector, select, Store } from '@ngrx/store'; +import { applyPatch, Operation } from 'fast-json-patch'; import { combineLatest as observableCombineLatest, Observable } from 'rxjs'; import { distinctUntilChanged, filter, map, mergeMap, take, } from 'rxjs/operators'; -import { Injectable } from '@angular/core'; -import { MemoizedSelector, select, Store } from '@ngrx/store'; -import { IndexName } from '../index/index.reducer'; - -import { CacheableObject, ObjectCacheEntry } from './object-cache.reducer'; +import { hasNoValue, hasValue, isNotEmpty } from '../../shared/empty.util'; +import { CoreState } from '../core.reducers'; +import { coreSelector } from '../core.selectors'; +import { RestRequestMethod } from '../data/rest-request-method'; +import { selfLinkFromUuidSelector } from '../index/index.selectors'; +import { GenericConstructor } from '../shared/generic-constructor'; +import { NormalizedObjectFactory } from './models/normalized-object-factory'; +import { NormalizedObject } from './models/normalized-object.model'; import { AddPatchObjectCacheAction, AddToObjectCacheAction, ApplyPatchObjectCacheAction, RemoveFromObjectCacheAction } from './object-cache.actions'; -import { hasNoValue, isNotEmpty } from '../../shared/empty.util'; -import { GenericConstructor } from '../shared/generic-constructor'; -import { coreSelector, CoreState } from '../core.reducers'; -import { pathSelector } from '../shared/selectors'; -import { NormalizedObjectFactory } from './models/normalized-object-factory'; -import { NormalizedObject } from './models/normalized-object.model'; -import { applyPatch, Operation } from 'fast-json-patch'; + +import { CacheableObject, ObjectCacheEntry, ObjectCacheState } from './object-cache.reducer'; import { AddToSSBAction } from './server-sync-buffer.actions'; -import { RestRequestMethod } from '../data/rest-request-method'; -function selfLinkFromUuidSelector(uuid: string): MemoizedSelector { - return pathSelector(coreSelector, 'index', IndexName.OBJECT, uuid); -} +const objectCacheSelector = createSelector( + coreSelector, + (state: CoreState) => state['cache/object'] +); -function entryFromSelfLinkSelector(selfLink: string): MemoizedSelector { - return pathSelector(coreSelector, 'cache/object', selfLink); -} +const entryFromSelfLinkSelector = + (selfLink: string): MemoizedSelector => createSelector( + objectCacheSelector, + (state: ObjectCacheState) => state[selfLink], + ); -/** + /** * A service to interact with the object cache */ @Injectable() diff --git a/src/app/core/cache/server-sync-buffer.effects.ts b/src/app/core/cache/server-sync-buffer.effects.ts index 0d7392e555b..036b18d280d 100644 --- a/src/app/core/cache/server-sync-buffer.effects.ts +++ b/src/app/core/cache/server-sync-buffer.effects.ts @@ -1,6 +1,7 @@ import { delay, exhaustMap, map, switchMap, take } from 'rxjs/operators'; import { Inject, Injectable } from '@angular/core'; import { Actions, Effect, ofType } from '@ngrx/effects'; +import { coreSelector } from '../core.selectors'; import { AddToSSBAction, CommitSSBAction, @@ -9,7 +10,7 @@ import { } from './server-sync-buffer.actions'; import { GLOBAL_CONFIG } from '../../../config'; import { GlobalConfig } from '../../../config/global-config.interface'; -import { coreSelector, CoreState } from '../core.reducers'; +import { CoreState } from '../core.reducers'; import { Action, createSelector, MemoizedSelector, select, Store } from '@ngrx/store'; import { ServerSyncBufferEntry, ServerSyncBufferState } from './server-sync-buffer.reducer'; import { combineLatest as observableCombineLatest, of as observableOf } from 'rxjs'; diff --git a/src/app/core/core.reducers.ts b/src/app/core/core.reducers.ts index e0ddb4a9de1..abfc3b69cc8 100644 --- a/src/app/core/core.reducers.ts +++ b/src/app/core/core.reducers.ts @@ -4,7 +4,7 @@ import { } from '@ngrx/store'; import { objectCacheReducer, ObjectCacheState } from './cache/object-cache.reducer'; -import { indexReducer, IndexState } from './index/index.reducer'; +import { indexReducer, MetaIndexState } from './index/index.reducer'; import { requestReducer, RequestState } from './data/request.reducer'; import { authReducer, AuthState } from './auth/auth.reducer'; import { serverSyncBufferReducer, ServerSyncBufferState } from './cache/server-sync-buffer.reducer'; @@ -18,7 +18,7 @@ export interface CoreState { 'cache/syncbuffer': ServerSyncBufferState, 'cache/object-updates': ObjectUpdatesState 'data/request': RequestState, - 'index': IndexState, + 'index': MetaIndexState, 'auth': AuthState, } @@ -30,5 +30,3 @@ export const coreReducers: ActionReducerMap = { 'index': indexReducer, 'auth': authReducer, }; - -export const coreSelector = createFeatureSelector('core'); diff --git a/src/app/core/core.selectors.ts b/src/app/core/core.selectors.ts new file mode 100644 index 00000000000..2ad701eba12 --- /dev/null +++ b/src/app/core/core.selectors.ts @@ -0,0 +1,4 @@ +import { createFeatureSelector } from '@ngrx/store'; +import { CoreState } from './core.reducers'; + +export const coreSelector = createFeatureSelector('core'); diff --git a/src/app/core/data/object-updates/object-updates.service.ts b/src/app/core/data/object-updates/object-updates.service.ts index 85e17b5b2f2..a13fb9487b2 100644 --- a/src/app/core/data/object-updates/object-updates.service.ts +++ b/src/app/core/data/object-updates/object-updates.service.ts @@ -1,6 +1,7 @@ import { Injectable } from '@angular/core'; import { createSelector, MemoizedSelector, select, Store } from '@ngrx/store'; -import { coreSelector, CoreState } from '../../core.reducers'; +import { CoreState } from '../../core.reducers'; +import { coreSelector } from '../../core.selectors'; import { FieldState, FieldUpdates, diff --git a/src/app/core/data/request.service.spec.ts b/src/app/core/data/request.service.spec.ts index b28436f3a83..642d83544bb 100644 --- a/src/app/core/data/request.service.spec.ts +++ b/src/app/core/data/request.service.spec.ts @@ -22,7 +22,7 @@ import { ActionsSubject, Store } from '@ngrx/store'; import { TestScheduler } from 'rxjs/testing'; import { BehaviorSubject } from 'rxjs/internal/BehaviorSubject'; import { MockStore } from '../../shared/testing/mock-store'; -import { IndexState } from '../index/index.reducer'; +import { MetaIndexState } from '../index/index.reducer'; describe('RequestService', () => { let scheduler: TestScheduler; diff --git a/src/app/core/data/request.service.ts b/src/app/core/data/request.service.ts index 93a7a10506e..745a8ea0006 100644 --- a/src/app/core/data/request.service.ts +++ b/src/app/core/data/request.service.ts @@ -1,90 +1,87 @@ import { merge as observableMerge, Observable, of as observableOf } from 'rxjs'; import { - distinctUntilChanged, filter, - find, - first, map, mergeMap, - reduce, - startWith, switchMap, take, - tap } from 'rxjs/operators'; import { race as observableRace } from 'rxjs'; import { Injectable } from '@angular/core'; import { createSelector, MemoizedSelector, select, Store } from '@ngrx/store'; -import { hasNoValue, hasValue, isNotEmpty, isNotUndefined } from '../../shared/empty.util'; +import { AppState } from '../../app.reducer'; +import { hasNoValue, hasValue, isNotEmpty } from '../../shared/empty.util'; import { CacheableObject } from '../cache/object-cache.reducer'; import { ObjectCacheService } from '../cache/object-cache.service'; import { DSOSuccessResponse, RestResponse } from '../cache/response.models'; -import { coreSelector, CoreState } from '../core.reducers'; -import { IndexName, IndexState } from '../index/index.reducer'; -import { pathSelector } from '../shared/selectors'; +import { CoreState } from '../core.reducers'; +import { coreSelector } from '../core.selectors'; +import { + IndexName, IndexState, + MetaIndexState +} from '../index/index.reducer'; +import { + originalRequestUUIDFromRequestUUIDSelector, requestIndexSelector, + uuidFromHrefSelector +} from '../index/index.selectors'; import { UUIDService } from '../shared/uuid.service'; import { RequestConfigureAction, RequestExecuteAction, RequestRemoveAction } from './request.actions'; import { GetRequest, RestRequest } from './request.models'; -import { RequestEntry } from './request.reducer'; +import { RequestEntry, RequestState } from './request.reducer'; import { CommitSSBAction } from '../cache/server-sync-buffer.actions'; import { RestRequestMethod } from './rest-request-method'; import { getResponseFromEntry } from '../shared/operators'; import { AddToIndexAction, RemoveFromIndexBySubstringAction } from '../index/index.actions'; -@Injectable() -export class RequestService { - private requestsOnTheirWayToTheStore: string[] = []; +const requestCacheSelector = createSelector( + coreSelector, + (state: CoreState) => state['data/request'] +); - constructor(private objectCache: ObjectCacheService, - private uuidService: UUIDService, - private store: Store, - private indexStore: Store) { - } - - private entryFromUUIDSelector(uuid: string): MemoizedSelector { - return pathSelector(coreSelector, 'data/request', uuid); +const entryFromUUIDSelector = (uuid: string): MemoizedSelector => createSelector( + requestCacheSelector, + (state: RequestState) => { + return hasValue(state) ? state[uuid] : undefined; } +); - private uuidFromHrefSelector(href: string): MemoizedSelector { - return pathSelector(coreSelector, 'index', IndexName.REQUEST, href); - } +/** + * Create a selector that fetches a list of request UUIDs from a given index substate of which the request href + * contains a given substring + * @param selector MemoizedSelector to start from + * @param name The name of the index substate we're fetching request UUIDs from + * @param href Substring that the request's href should contain + */ +const uuidsFromHrefSubstringSelector = + (selector: MemoizedSelector, href: string): MemoizedSelector => createSelector( + selector, + (state: IndexState) => getUuidsFromHrefSubstring(state, href) + ); - private originalUUIDFromUUIDSelector(uuid: string): MemoizedSelector { - return pathSelector(coreSelector, 'index', IndexName.UUID_MAPPING, uuid); +/** + * Fetch a list of request UUIDs from a given index substate of which the request href contains a given substring + * @param state The IndexState + * @param href Substring that the request's href should contain + */ +const getUuidsFromHrefSubstring = (state: IndexState, href: string): string[] => { + let result = []; + if (isNotEmpty(state)) { + result = Object.values(state) + .filter((value: string) => value.startsWith(href)); } + return result; +}; - /** - * Create a selector that fetches a list of request UUIDs from a given index substate of which the request href - * contains a given substring - * @param selector MemoizedSelector to start from - * @param name The name of the index substate we're fetching request UUIDs from - * @param href Substring that the request's href should contain - */ - private uuidsFromHrefSubstringSelector(selector: MemoizedSelector, name: string, href: string): MemoizedSelector { - return createSelector(selector, (state: IndexState) => this.getUuidsFromHrefSubstring(state, name, href)); - } +@Injectable() +export class RequestService { + private requestsOnTheirWayToTheStore: string[] = []; - /** - * Fetch a list of request UUIDs from a given index substate of which the request href contains a given substring - * @param state The IndexState - * @param name The name of the index substate we're fetching request UUIDs from - * @param href Substring that the request's href should contain - */ - private getUuidsFromHrefSubstring(state: IndexState, name: string, href: string): string[] { - let result = []; - if (isNotEmpty(state)) { - const subState = state[name]; - if (isNotEmpty(subState)) { - for (const value in subState) { - if (value.indexOf(href) > -1) { - result = [...result, subState[value]]; - } - } - } - } - return result; + constructor(private objectCache: ObjectCacheService, + private uuidService: UUIDService, + private store: Store, + private indexStore: Store) { } generateRequestId(): string { @@ -110,11 +107,11 @@ export class RequestService { getByUUID(uuid: string): Observable { return observableRace( - this.store.pipe(select(this.entryFromUUIDSelector(uuid))), + this.store.pipe(select(entryFromUUIDSelector(uuid))), this.store.pipe( - select(this.originalUUIDFromUUIDSelector(uuid)), + select(originalRequestUUIDFromRequestUUIDSelector(uuid)), mergeMap((originalUUID) => { - return this.store.pipe(select(this.entryFromUUIDSelector(originalUUID))) + return this.store.pipe(select(entryFromUUIDSelector(originalUUID))) }, )) ); @@ -122,7 +119,7 @@ export class RequestService { getByHref(href: string): Observable { return this.store.pipe( - select(this.uuidFromHrefSelector(href)), + select(uuidFromHrefSelector(href)), mergeMap((uuid: string) => this.getByUUID(uuid)) ); } @@ -159,7 +156,7 @@ export class RequestService { */ removeByHrefSubstring(href: string) { this.store.pipe( - select(this.uuidsFromHrefSubstringSelector(pathSelector(coreSelector, 'index'), IndexName.REQUEST, href)), + select(uuidsFromHrefSubstringSelector(requestIndexSelector, href)), take(1) ).subscribe((uuids: string[]) => { for (const uuid of uuids) { @@ -230,7 +227,7 @@ export class RequestService { */ private trackRequestsOnTheirWayToTheStore(request: GetRequest) { this.requestsOnTheirWayToTheStore = [...this.requestsOnTheirWayToTheStore, request.href]; - this.store.pipe(select(this.entryFromUUIDSelector(request.href)), + this.store.pipe(select(entryFromUUIDSelector(request.href)), filter((re: RequestEntry) => hasValue(re)), take(1) ).subscribe((re: RequestEntry) => { diff --git a/src/app/core/index/index.reducer.spec.ts b/src/app/core/index/index.reducer.spec.ts index d1403ac5bf0..ef46c760c6e 100644 --- a/src/app/core/index/index.reducer.spec.ts +++ b/src/app/core/index/index.reducer.spec.ts @@ -1,6 +1,6 @@ import * as deepFreeze from 'deep-freeze'; -import { IndexName, indexReducer, IndexState } from './index.reducer'; +import { IndexName, indexReducer, MetaIndexState } from './index.reducer'; import { AddToIndexAction, RemoveFromIndexBySubstringAction, RemoveFromIndexByValueAction } from './index.actions'; class NullAction extends AddToIndexAction { @@ -17,7 +17,7 @@ describe('requestReducer', () => { const key2 = '1911e8a4-6939-490c-b58b-a5d70f8d91fb'; const val1 = 'https://dspace7.4science.it/dspace-spring-rest/api/core/items/567a639f-f5ff-4126-807c-b7d0910808c8'; const val2 = 'https://dspace7.4science.it/dspace-spring-rest/api/core/items/1911e8a4-6939-490c-b58b-a5d70f8d91fb'; - const testState: IndexState = { + const testState: MetaIndexState = { [IndexName.OBJECT]: { [key1]: val1 },[IndexName.REQUEST]: { diff --git a/src/app/core/index/index.reducer.ts b/src/app/core/index/index.reducer.ts index 3597c786d8d..d95748ef8a5 100644 --- a/src/app/core/index/index.reducer.ts +++ b/src/app/core/index/index.reducer.ts @@ -1,8 +1,9 @@ import { + AddToIndexAction, IndexAction, IndexActionTypes, - AddToIndexAction, - RemoveFromIndexByValueAction, RemoveFromIndexBySubstringAction + RemoveFromIndexBySubstringAction, + RemoveFromIndexByValueAction } from './index.actions'; export enum IndexName { @@ -11,16 +12,18 @@ export enum IndexName { UUID_MAPPING = 'get-request/configured-to-cache-uuid' } -export type IndexState = { - [name in IndexName]: { - [key: string]: string - } +export interface IndexState { + [key: string]: string +} + +export type MetaIndexState = { + [name in IndexName]: IndexState } // Object.create(null) ensures the object has no default js properties (e.g. `__proto__`) -const initialState: IndexState = Object.create(null); +const initialState: MetaIndexState = Object.create(null); -export function indexReducer(state = initialState, action: IndexAction): IndexState { +export function indexReducer(state = initialState, action: IndexAction): MetaIndexState { switch (action.type) { case IndexActionTypes.ADD: { @@ -41,7 +44,7 @@ export function indexReducer(state = initialState, action: IndexAction): IndexSt } } -function addToIndex(state: IndexState, action: AddToIndexAction): IndexState { +function addToIndex(state: MetaIndexState, action: AddToIndexAction): MetaIndexState { const subState = state[action.payload.name]; const newSubState = Object.assign({}, subState, { [action.payload.key]: action.payload.value @@ -52,7 +55,7 @@ function addToIndex(state: IndexState, action: AddToIndexAction): IndexState { return obs; } -function removeFromIndexByValue(state: IndexState, action: RemoveFromIndexByValueAction): IndexState { +function removeFromIndexByValue(state: MetaIndexState, action: RemoveFromIndexByValueAction): MetaIndexState { const subState = state[action.payload.name]; const newSubState = Object.create(null); for (const value in subState) { @@ -70,7 +73,7 @@ function removeFromIndexByValue(state: IndexState, action: RemoveFromIndexByValu * @param state The IndexState to remove values from * @param action The RemoveFromIndexByValueAction containing the necessary information to remove the values */ -function removeFromIndexBySubstring(state: IndexState, action: RemoveFromIndexByValueAction): IndexState { +function removeFromIndexBySubstring(state: MetaIndexState, action: RemoveFromIndexByValueAction): MetaIndexState { const subState = state[action.payload.name]; const newSubState = Object.create(null); for (const value in subState) { diff --git a/src/app/core/index/index.selectors.ts b/src/app/core/index/index.selectors.ts new file mode 100644 index 00000000000..51821120ca3 --- /dev/null +++ b/src/app/core/index/index.selectors.ts @@ -0,0 +1,45 @@ +import { createSelector, MemoizedSelector } from '@ngrx/store'; +import { AppState } from '../../app.reducer'; +import { hasValue } from '../../shared/empty.util'; +import { CoreState } from '../core.reducers'; +import { coreSelector } from '../core.selectors'; +import { IndexName, IndexState, MetaIndexState } from './index.reducer'; + +export const metaIndexSelector: MemoizedSelector = createSelector( + coreSelector, + (state: CoreState) => state.index +); +export const objectIndexSelector: MemoizedSelector = createSelector( + metaIndexSelector, + (state: MetaIndexState) => state[IndexName.OBJECT] +); +export const requestIndexSelector: MemoizedSelector = createSelector( + metaIndexSelector, + (state: MetaIndexState) => state[IndexName.REQUEST] +); +export const requestUUIDIndexSelector: MemoizedSelector = createSelector( + metaIndexSelector, + (state: MetaIndexState) => state[IndexName.UUID_MAPPING] +); +export const selfLinkFromUuidSelector = + (uuid: string): MemoizedSelector => createSelector( + objectIndexSelector, + (state: IndexState) => hasValue(state) ? state[uuid] : undefined + ); +export const uuidFromHrefSelector = + (href: string): MemoizedSelector => createSelector( + requestIndexSelector, + (state: IndexState) => hasValue(state) ? state[href] : undefined + ); +/** + * If a request wasn't sent to the server because the result was already cached, + * this selector allows you to find the UUID of the cached request based on the + * UUID of the new request + * + * @param uuid The uuid of the new request + */ +export const originalRequestUUIDFromRequestUUIDSelector = + (uuid: string): MemoizedSelector => createSelector( + requestUUIDIndexSelector, + (state: IndexState) => hasValue(state) ? state[uuid] : undefined + ); diff --git a/src/app/core/shared/selectors.ts b/src/app/core/shared/selectors.ts deleted file mode 100644 index 7bd35d39c16..00000000000 --- a/src/app/core/shared/selectors.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { createSelector, MemoizedSelector } from '@ngrx/store'; -import { hasNoValue, isEmpty } from '../../shared/empty.util'; - -export function pathSelector(selector: MemoizedSelector, ...path: string[]): MemoizedSelector { - return createSelector(selector, (state: any) => getSubState(state, path)); -} - -function getSubState(state: any, path: string[]) { - const current = path[0]; - const remainingPath = path.slice(1); - const subState = state[current]; - if (hasNoValue(subState) || isEmpty(remainingPath)) { - return subState; - } else { - return getSubState(subState, remainingPath); - } -} From db9861b4951a76b6277e62028ecbcfa6c56ffff6 Mon Sep 17 00:00:00 2001 From: lotte Date: Mon, 11 Mar 2019 16:28:42 +0100 Subject: [PATCH 02/10] Added search filter optimalisations --- .../search-boolean-filter.component.html | 2 +- .../search-facet-filter.component.ts | 8 ++++++++ .../search-filter/search-filter.service.ts | 14 +++++++++----- .../search-hierarchy-filter.component.html | 2 +- .../search-range-filter.component.html | 2 +- .../search-text-filter.component.html | 2 +- .../search-filters/search-filters.component.html | 2 +- .../search-filters/search-filters.component.ts | 16 +++++++++++++--- src/app/shared/services/route.service.ts | 2 +- 9 files changed, 36 insertions(+), 14 deletions(-) diff --git a/src/app/+search-page/search-filters/search-filter/search-boolean-filter/search-boolean-filter.component.html b/src/app/+search-page/search-filters/search-filter/search-boolean-filter/search-boolean-filter.component.html index 32d9ea6e774..216683c1225 100644 --- a/src/app/+search-page/search-filters/search-filter/search-boolean-filter/search-boolean-filter.component.html +++ b/src/app/+search-page/search-filters/search-filter/search-boolean-filter/search-boolean-filter.component.html @@ -8,7 +8,7 @@
- + diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter/search-facet-filter.component.ts b/src/app/+search-page/search-filters/search-filter/search-facet-filter/search-facet-filter.component.ts index 1675dd051a5..f4f835c152e 100644 --- a/src/app/+search-page/search-filters/search-filter/search-facet-filter/search-facet-filter.component.ts +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter/search-facet-filter.component.ts @@ -290,6 +290,14 @@ export class SearchFacetFilterComponent implements OnInit, OnDestroy { getDisplayValue(facet: FacetValue, query: string): string { return new EmphasizePipe().transform(facet.value, query) + ' (' + facet.count + ')'; } + + + /** + * Prevent unnecessary rerendering + */ + trackUpdate(index, value: FacetValue) { + return value ? value.search : undefined; + } } export const facetLoad = trigger('facetLoad', [ diff --git a/src/app/+search-page/search-filters/search-filter/search-filter.service.ts b/src/app/+search-page/search-filters/search-filter/search-filter.service.ts index bf21eab3675..34c34ee4cc8 100644 --- a/src/app/+search-page/search-filters/search-filter/search-filter.service.ts +++ b/src/app/+search-page/search-filters/search-filter/search-filter.service.ts @@ -1,6 +1,6 @@ import { combineLatest as observableCombineLatest, Observable } from 'rxjs'; import { Injectable, InjectionToken } from '@angular/core'; -import { map } from 'rxjs/operators'; +import { distinctUntilChanged, map } from 'rxjs/operators'; import { SearchFiltersState, SearchFilterState } from './search-filter.reducer'; import { createSelector, MemoizedSelector, select, Store } from '@ngrx/store'; import { @@ -65,7 +65,9 @@ export class SearchFilterService { return observableCombineLatest(values$, prefixValues$).pipe( map(([values, prefixValues]) => { - if (isNotEmpty(values)) { + console.log('getSelectedValuesForFilter ', values, prefixValues); + + if (isNotEmpty(values)) { return values; } return prefixValues; @@ -88,13 +90,14 @@ export class SearchFilterService { } else { return false; } - }) + }), + distinctUntilChanged() ); } /** * Request the current page of a given filter - * @param {string} filterName The filtername for which the page state is checked + * @param {string} filterName The filter name for which the page state is checked * @returns {Observable} Emits the current page state of the given filter, if it's unavailable, return 1 */ getPage(filterName: string): Observable { @@ -106,7 +109,8 @@ export class SearchFilterService { } else { return 1; } - })); + }), + distinctUntilChanged()); } /** diff --git a/src/app/+search-page/search-filters/search-filter/search-hierarchy-filter/search-hierarchy-filter.component.html b/src/app/+search-page/search-filters/search-filter/search-hierarchy-filter/search-hierarchy-filter.component.html index 812f5437162..5434f0e0f42 100644 --- a/src/app/+search-page/search-filters/search-filter/search-hierarchy-filter/search-hierarchy-filter.component.html +++ b/src/app/+search-page/search-filters/search-filter/search-hierarchy-filter/search-hierarchy-filter.component.html @@ -8,7 +8,7 @@
- + diff --git a/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.html b/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.html index 352c1710c05..1cc0556ed52 100644 --- a/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.html +++ b/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.html @@ -24,7 +24,7 @@
- + diff --git a/src/app/+search-page/search-filters/search-filter/search-text-filter/search-text-filter.component.html b/src/app/+search-page/search-filters/search-filter/search-text-filter/search-text-filter.component.html index fcc2393b935..32f6fe2153c 100644 --- a/src/app/+search-page/search-filters/search-filter/search-text-filter/search-text-filter.component.html +++ b/src/app/+search-page/search-filters/search-filter/search-text-filter/search-text-filter.component.html @@ -9,7 +9,7 @@
- + diff --git a/src/app/+search-page/search-filters/search-filters.component.html b/src/app/+search-page/search-filters/search-filters.component.html index 0522c1fba0e..de725f09583 100644 --- a/src/app/+search-page/search-filters/search-filters.component.html +++ b/src/app/+search-page/search-filters/search-filters.component.html @@ -1,6 +1,6 @@

{{"search.filters.head" | translate}}

-
+
diff --git a/src/app/+search-page/search-filters/search-filters.component.ts b/src/app/+search-page/search-filters/search-filters.component.ts index f16faff1f35..0d32df48677 100644 --- a/src/app/+search-page/search-filters/search-filters.component.ts +++ b/src/app/+search-page/search-filters/search-filters.component.ts @@ -1,6 +1,6 @@ import { Observable, of as observableOf } from 'rxjs'; -import { filter, map, mergeMap, startWith, switchMap } from 'rxjs/operators'; +import { filter, first, map, mergeMap, startWith, switchMap, tap } from 'rxjs/operators'; import { Component } from '@angular/core'; import { SearchService } from '../search-service/search.service'; import { RemoteData } from '../../core/data/remote-data'; @@ -9,6 +9,7 @@ import { SearchConfigurationService } from '../search-service/search-configurati import { isNotEmpty } from '../../shared/empty.util'; import { SearchFilterService } from './search-filter/search-filter.service'; import { getSucceededRemoteData } from '../../core/shared/operators'; +import { FieldUpdate } from '../../core/data/object-updates/object-updates.reducer'; @Component({ selector: 'ds-search-filters', @@ -59,11 +60,13 @@ export class SearchFiltersComponent { */ isActive(filterConfig: SearchFilterConfig): Observable { return this.filterService.getSelectedValuesForFilter(filterConfig).pipe( - mergeMap((isActive) => { + switchMap((isActive) => { + console.log('selected fires'); if (isNotEmpty(isActive)) { return observableOf(true); } else { return this.searchConfigService.searchOptions.pipe( + first(), switchMap((options) => { return this.searchService.getFacetValuesFor(filterConfig, 1, options).pipe( filter((RD) => !RD.isLoading), @@ -73,6 +76,13 @@ export class SearchFiltersComponent { } )) } - }),startWith(true),); + }), tap(t => console.log(t)), startWith(true)); + } + + /** + * Prevent unnecessary rerendering + */ + trackUpdate(index, config: SearchFilterConfig) { + return config ? config.name : undefined; } } diff --git a/src/app/shared/services/route.service.ts b/src/app/shared/services/route.service.ts index 9dd9a0f1641..478d92c21ed 100644 --- a/src/app/shared/services/route.service.ts +++ b/src/app/shared/services/route.service.ts @@ -16,7 +16,7 @@ export class RouteService { getQueryParameterValues(paramName: string): Observable { return this.route.queryParamMap.pipe( map((params) => [...params.getAll(paramName)]), - distinctUntilChanged() + distinctUntilChanged((a, b) => JSON.stringify(a) === JSON.stringify(b)) ); } From fc73fc1d3ea8b497390aafe7a7e55a61e5c6eef0 Mon Sep 17 00:00:00 2001 From: lotte Date: Wed, 20 Mar 2019 16:57:55 +0100 Subject: [PATCH 03/10] optimizations links in facets --- package.json | 1 + .../search-boolean-filter.component.html | 20 +---- .../search-facet-option.component.html | 9 ++ .../search-facet-option.component.ts | 78 ++++++++++++++++ .../search-facet-range-option.component.html | 8 ++ .../search-facet-range-option.component.ts | 88 +++++++++++++++++++ ...earch-facet-selected-option.component.html | 6 ++ .../search-facet-selected-option.component.ts | 67 ++++++++++++++ ...search-facet-filter-wrapper.component.html | 2 +- .../search-facet-filter-wrapper.component.ts | 6 +- .../search-facet-filter.component.ts | 4 + .../search-filter/search-filter.actions.ts | 20 ++--- .../search-filter.component.html | 6 +- .../search-filter/search-filter.component.ts | 75 ++++++++++------ .../search-filter/search-filter.reducer.ts | 37 +++----- .../search-filter.service.spec.ts | 20 ++--- .../search-filter/search-filter.service.ts | 40 ++++----- .../search-hierarchy-filter.component.html | 13 +-- .../search-range-filter.component.html | 9 +- .../search-range-filter.component.ts | 30 ++----- .../search-text-filter.component.html | 14 +-- .../search-filters.component.html | 2 +- .../search-filters.component.ts | 32 +------ src/app/+search-page/search-page.component.ts | 1 - src/app/+search-page/search-page.module.ts | 9 ++ src/app/shared/services/route.service.ts | 9 +- src/main.browser.ts | 1 + yarn.lock | 43 +++++++++ 28 files changed, 445 insertions(+), 205 deletions(-) create mode 100644 src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.html create mode 100644 src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.ts create mode 100644 src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.html create mode 100644 src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.ts create mode 100644 src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.html create mode 100644 src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.ts diff --git a/package.json b/package.json index fd037de3f73..7f319d97160 100644 --- a/package.json +++ b/package.json @@ -119,6 +119,7 @@ "pem": "1.12.3", "reflect-metadata": "0.1.12", "rxjs": "6.2.2", + "rxjs-spy": "^7.5.1", "sortablejs": "1.7.0", "text-mask-core": "5.0.1", "ts-loader": "^5.2.1", diff --git a/src/app/+search-page/search-filters/search-filter/search-boolean-filter/search-boolean-filter.component.html b/src/app/+search-page/search-filters/search-filter/search-boolean-filter/search-boolean-filter.component.html index 216683c1225..06154916b2a 100644 --- a/src/app/+search-page/search-filters/search-filter/search-boolean-filter/search-boolean-filter.component.html +++ b/src/app/+search-page/search-filters/search-filter/search-boolean-filter/search-boolean-filter.component.html @@ -1,24 +1,10 @@
- - - {{value}} - +
diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.html b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.html new file mode 100644 index 00000000000..881fb98dcb0 --- /dev/null +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.html @@ -0,0 +1,9 @@ + + + {{filterValue.value}} + + {{filterValue.count}} + + \ No newline at end of file diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.ts b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.ts new file mode 100644 index 00000000000..c5896f92b63 --- /dev/null +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.ts @@ -0,0 +1,78 @@ +import { Observable, of as observableOf } from 'rxjs'; +import { map } from 'rxjs/operators'; +import { Component, Input, OnInit } from '@angular/core'; +import { Router } from '@angular/router'; +import { FacetValue } from '../../../../search-service/facet-value.model'; +import { SearchFilterConfig } from '../../../../search-service/search-filter-config.model'; +import { SearchService } from '../../../../search-service/search.service'; +import { SearchFilterService } from '../../search-filter.service'; + +@Component({ + selector: 'ds-search-facet-option', + templateUrl: './search-facet-option.component.html', +}) + +/** + * Represents a single option in a filter facet + */ +export class SearchFacetOptionComponent implements OnInit { + /** + * A single value for this component + */ + @Input() filterValue: FacetValue; + @Input() filterConfig: SearchFilterConfig; + + /** + * Emits the active values for this filter + */ + selectedValues$: Observable; + + isVisible: Observable; + + addQueryParams; + + constructor(protected searchService: SearchService, + protected filterService: SearchFilterService, + protected router: Router + ) { + } + + /** + * Initializes all observable instance variables and starts listening to them + */ + ngOnInit(): void { + this.selectedValues$ = this.filterService.getSelectedValuesForFilter(this.filterConfig); + this.isVisible = this.isChecked().pipe(map((checked: boolean) => !checked)); + this.addQueryParams = this.getAddParams(); + } + + /** + * Checks if a value for this filter is currently active + */ + private isChecked(): Observable { + return this.filterService.isFilterActiveWithValue(this.filterConfig.paramName, this.filterValue.value); + } + + /** + * @returns {string} The base path to the search page + */ + getSearchLink() { + return this.searchService.getSearchLink(); + } + + /** + * Calculates the parameters that should change if a given value for this filter would be added to the active filters + * @param {string} value The value that is added for this filter + * @returns {Observable} The changed filter parameters + */ + private getAddParams(): Observable { + return this.selectedValues$.pipe(map((selectedValues) => { + return { + [this.filterConfig.paramName]: [...selectedValues, this.filterValue.value], + page: 1 + }; + })); + } + +} + diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.html b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.html new file mode 100644 index 00000000000..92e90a00cc9 --- /dev/null +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.html @@ -0,0 +1,8 @@ + + {{filterValue.value}} + + {{filterValue.count}} + + \ No newline at end of file diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.ts b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.ts new file mode 100644 index 00000000000..0b38ca18450 --- /dev/null +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.ts @@ -0,0 +1,88 @@ +import { Observable, of as observableOf } from 'rxjs'; +import { map } from 'rxjs/operators'; +import { Component, Input, OnInit } from '@angular/core'; +import { Router } from '@angular/router'; +import { FacetValue } from '../../../../search-service/facet-value.model'; +import { SearchFilterConfig } from '../../../../search-service/search-filter-config.model'; +import { SearchService } from '../../../../search-service/search.service'; +import { SearchFilterService } from '../../search-filter.service'; +import { + RANGE_FILTER_MAX_SUFFIX, + RANGE_FILTER_MIN_SUFFIX +} from '../../search-range-filter/search-range-filter.component'; + +const rangeDelimiter = '-'; + +@Component({ + selector: 'ds-search-facet-range-option', + templateUrl: './search-facet-range-option.component.html', +}) + +/** + * Represents a single option in a filter facet + */ +export class SearchFacetRangeOptionComponent implements OnInit { + /** + * A single value for this component + */ + @Input() filterValue: FacetValue; + @Input() filterConfig: SearchFilterConfig; + + /** + * Emits the active values for this filter + */ + selectedValues$: Observable; + + isVisible: Observable; + + changeQueryParams; + + constructor(protected searchService: SearchService, + protected filterService: SearchFilterService, + protected router: Router + ) { + } + + /** + * Initializes all observable instance variables and starts listening to them + */ + ngOnInit(): void { + this.selectedValues$ = this.filterService.getSelectedValuesForFilter(this.filterConfig); + this.isVisible = this.isChecked().pipe(map((checked: boolean) => !checked)); + this.changeQueryParams = this.getChangeParams(); + } + + /** + * Checks if a value for this filter is currently active + */ + private isChecked(): Observable { + return this.filterService.isFilterActiveWithValue(this.filterConfig.paramName, this.filterValue.value); + } + + /** + * @returns {string} The base path to the search page + */ + getSearchLink() { + return this.searchService.getSearchLink(); + } + + + /** + * Calculates the parameters that should change if a given values for this range filter would be changed + * @param {string} value The values that are changed for this filter + * @returns {Observable} The changed filter parameters + */ + getChangeParams() { + const parts = this.filterValue.value.split(rangeDelimiter); + const min = parts.length > 1 ? parts[0].trim() : this.filterValue.value; + const max = parts.length > 1 ? parts[1].trim() : this.filterValue.value; + return observableOf( + { + [this.filterConfig.paramName + RANGE_FILTER_MIN_SUFFIX]: [min], + [this.filterConfig.paramName + RANGE_FILTER_MAX_SUFFIX]: [max], + page: 1 + }); + } + +} + diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.html b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.html new file mode 100644 index 00000000000..7bed6207706 --- /dev/null +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.html @@ -0,0 +1,6 @@ + + + {{selectedValue}} + \ No newline at end of file diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.ts b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.ts new file mode 100644 index 00000000000..2fdc490d68d --- /dev/null +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.ts @@ -0,0 +1,67 @@ +import { Observable, of as observableOf } from 'rxjs'; +import { map } from 'rxjs/operators'; +import { Component, Input, OnInit } from '@angular/core'; +import { Router } from '@angular/router'; +import { FacetValue } from '../../../../search-service/facet-value.model'; +import { SearchFilterConfig } from '../../../../search-service/search-filter-config.model'; +import { SearchService } from '../../../../search-service/search.service'; +import { SearchFilterService } from '../../search-filter.service'; + +@Component({ + selector: 'ds-search-facet-selected-option', + templateUrl: './search-facet-selected-option.component.html', +}) + +/** + * Represents a single option in a filter facet + */ +export class SearchFacetSelectedOptionComponent implements OnInit { + /** + * A single value for this component + */ + @Input() selectedValue: string; + @Input() filterConfig: SearchFilterConfig; + + /** + * Emits the active values for this filter + */ + selectedValues$: Observable; + + removeQueryParams; + + constructor(protected searchService: SearchService, + protected filterService: SearchFilterService, + protected router: Router + ) { + } + + /** + * Initializes all observable instance variables and starts listening to them + */ + ngOnInit(): void { + this.selectedValues$ = this.filterService.getSelectedValuesForFilter(this.filterConfig); + this.removeQueryParams = this.getRemoveParams(); + } + + /** + * @returns {string} The base path to the search page + */ + getSearchLink() { + return this.searchService.getSearchLink(); + } + + /** + * Calculates the parameters that should change if a given value for this filter would be removed from the active filters + * @param {string} value The value that is removed for this filter + * @returns {Observable} The changed filter parameters + */ + private getRemoveParams(): Observable { + return this.selectedValues$.pipe(map((selectedValues) => { + return { + [this.filterConfig.paramName]: selectedValues.filter((v) => v !== this.selectedValue), + page: 1 + }; + })); + } +} + diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter-wrapper/search-facet-filter-wrapper.component.html b/src/app/+search-page/search-filters/search-filter/search-facet-filter-wrapper/search-facet-filter-wrapper.component.html index b7e03af4733..4a325d9b3ca 100644 --- a/src/app/+search-page/search-filters/search-filter/search-facet-filter-wrapper/search-facet-filter-wrapper.component.html +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter-wrapper/search-facet-filter-wrapper.component.html @@ -1 +1 @@ - + \ No newline at end of file diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter-wrapper/search-facet-filter-wrapper.component.ts b/src/app/+search-page/search-filters/search-filter/search-facet-filter-wrapper/search-facet-filter-wrapper.component.ts index bc088777fa4..5b733b52cb4 100644 --- a/src/app/+search-page/search-filters/search-filter/search-facet-filter-wrapper/search-facet-filter-wrapper.component.ts +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter-wrapper/search-facet-filter-wrapper.component.ts @@ -3,6 +3,8 @@ import { renderFilterType } from '../search-filter-type-decorator'; import { FilterType } from '../../../search-service/filter-type.model'; import { SearchFilterConfig } from '../../../search-service/search-filter-config.model'; import { FILTER_CONFIG } from '../search-filter.service'; +import { GenericConstructor } from '../../../../core/shared/generic-constructor'; +import { SearchFacetFilterComponent } from '../search-facet-filter/search-facet-filter.component'; @Component({ selector: 'ds-search-facet-filter-wrapper', @@ -18,6 +20,7 @@ export class SearchFacetFilterWrapperComponent implements OnInit { */ @Input() filterConfig: SearchFilterConfig; + searchFilter: GenericConstructor; /** * Injector to inject a child component with the @Input parameters */ @@ -30,6 +33,7 @@ export class SearchFacetFilterWrapperComponent implements OnInit { * Initialize and add the filter config to the injector */ ngOnInit(): void { + this.searchFilter = this.getSearchFilter(); this.objectInjector = Injector.create({ providers: [ { provide: FILTER_CONFIG, useFactory: () => (this.filterConfig), deps: [] } @@ -41,7 +45,7 @@ export class SearchFacetFilterWrapperComponent implements OnInit { /** * Find the correct component based on the filter config's type */ - getSearchFilter() { + private getSearchFilter() { const type: FilterType = this.filterConfig.type; return renderFilterType(type); } diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter/search-facet-filter.component.ts b/src/app/+search-page/search-filters/search-filter/search-facet-filter/search-facet-filter.component.ts index f4f835c152e..74c2794313b 100644 --- a/src/app/+search-page/search-filters/search-filter/search-facet-filter/search-facet-filter.component.ts +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter/search-facet-filter.component.ts @@ -85,8 +85,11 @@ export class SearchFacetFilterComponent implements OnInit, OnDestroy { * Initializes all observable instance variables and starts listening to them */ ngOnInit(): void { + console.log('renderSearchFacetFilterComponent') + this.filterValues$ = new BehaviorSubject(new RemoteData(true, false, undefined, undefined, undefined)); this.currentPage = this.getCurrentPage().pipe(distinctUntilChanged()); + this.selectedValues = this.filterService.getSelectedValuesForFilter(this.filterConfig); const searchOptions = this.searchConfigService.searchOptions; this.subs.push(this.searchConfigService.searchOptions.subscribe(() => this.updateFilterValueList())); @@ -190,6 +193,7 @@ export class SearchFacetFilterComponent implements OnInit, OnDestroy { * @param data The string from the input field */ onSubmit(data: any) { + console.log('onsubmit'); this.selectedValues.pipe(take(1)).subscribe((selectedValues) => { if (isNotEmpty(data)) { this.router.navigate([this.getSearchLink()], { diff --git a/src/app/+search-page/search-filters/search-filter/search-filter.actions.ts b/src/app/+search-page/search-filters/search-filter/search-filter.actions.ts index 2e556b32d6b..838597e03dd 100644 --- a/src/app/+search-page/search-filters/search-filter/search-filter.actions.ts +++ b/src/app/+search-page/search-filters/search-filter/search-filter.actions.ts @@ -1,6 +1,7 @@ import { Action } from '@ngrx/store'; import { type } from '../../../shared/ngrx/type'; +import { SearchFilterConfig } from '../../search-service/search-filter-config.model'; /** * For each action type in an action group, make a simple @@ -12,9 +13,8 @@ import { type } from '../../../shared/ngrx/type'; */ export const SearchFilterActionTypes = { COLLAPSE: type('dspace/search-filter/COLLAPSE'), - INITIAL_COLLAPSE: type('dspace/search-filter/INITIAL_COLLAPSE'), + INITIALIZE: type('dspace/search-filter/INITIALIZE'), EXPAND: type('dspace/search-filter/EXPAND'), - INITIAL_EXPAND: type('dspace/search-filter/INITIAL_EXPAND'), TOGGLE: type('dspace/search-filter/TOGGLE'), DECREMENT_PAGE: type('dspace/search-filter/DECREMENT_PAGE'), INCREMENT_PAGE: type('dspace/search-filter/INCREMENT_PAGE'), @@ -66,15 +66,13 @@ export class SearchFilterToggleAction extends SearchFilterAction { /** * Used to set the initial state of a filter to collapsed */ -export class SearchFilterInitialCollapseAction extends SearchFilterAction { - type = SearchFilterActionTypes.INITIAL_COLLAPSE; -} - -/** - * Used to set the initial state of a filter to expanded - */ -export class SearchFilterInitialExpandAction extends SearchFilterAction { - type = SearchFilterActionTypes.INITIAL_EXPAND; +export class SearchFilterInitializeAction extends SearchFilterAction { + type = SearchFilterActionTypes.INITIALIZE; + initiallyExpanded; + constructor(filter: SearchFilterConfig) { + super(filter.name); + this.initiallyExpanded = filter.isOpenByDefault; + } } /** diff --git a/src/app/+search-page/search-filters/search-filter/search-filter.component.html b/src/app/+search-page/search-filters/search-filter/search-filter.component.html index 1013bf7e28f..e2c128593fe 100644 --- a/src/app/+search-page/search-filters/search-filter/search-filter.component.html +++ b/src/app/+search-page/search-filters/search-filter/search-filter.component.html @@ -1,7 +1,7 @@ -
+
{{'search.filters.filter.' + filter.name + '.head'| translate}}
-
+ [ngClass]="(collapsed$ | async) ? 'fa-plus' : 'fa-minus'">
+
\ No newline at end of file diff --git a/src/app/+search-page/search-filters/search-filter/search-filter.component.ts b/src/app/+search-page/search-filters/search-filter/search-filter.component.ts index dcc01f2b46b..eaf318655f5 100644 --- a/src/app/+search-page/search-filters/search-filter/search-filter.component.ts +++ b/src/app/+search-page/search-filters/search-filter/search-filter.component.ts @@ -1,11 +1,12 @@ - -import { take } from 'rxjs/operators'; +import { filter, first, map, startWith, switchMap, take } from 'rxjs/operators'; import { Component, Input, OnInit } from '@angular/core'; import { SearchFilterConfig } from '../../search-service/search-filter-config.model'; import { SearchFilterService } from './search-filter.service'; -import { Observable } from 'rxjs'; +import { BehaviorSubject, Observable, of as observableOf } from 'rxjs'; import { slide } from '../../../shared/animations/slide'; import { isNotEmpty } from '../../../shared/empty.util'; +import { SearchService } from '../../search-service/search.service'; +import { SearchConfigurationService } from '../../search-service/search-configuration.service'; @Component({ selector: 'ds-search-filter', @@ -26,9 +27,13 @@ export class SearchFilterComponent implements OnInit { /** * True when the filter is 100% collapsed in the UI */ - collapsed; + closed = true; + collapsed$: Observable; + + selectedValues$: Observable; + active$: Observable; - constructor(private filterService: SearchFilterService) { + constructor(private filterService: SearchFilterService, private searchService: SearchService, private searchConfigService: SearchConfigurationService) { } /** @@ -37,11 +42,13 @@ export class SearchFilterComponent implements OnInit { * Else, the filter should initially be collapsed */ ngOnInit() { - this.getSelectedValues().pipe(take(1)).subscribe((isActive) => { - if (this.filter.isOpenByDefault || isNotEmpty(isActive)) { - this.initialExpand(); - } else { - this.initialCollapse(); + this.selectedValues$ = this.getSelectedValues(); + this.active$ = this.isActive(); + this.collapsed$ = this.isCollapsed(); + this.initializeFilter(); + this.selectedValues$.pipe(take(1)).subscribe((selectedValues) => { + if (isNotEmpty(selectedValues)) { + this.filterService.expand(this.filter.name); } }); } @@ -57,30 +64,21 @@ export class SearchFilterComponent implements OnInit { * Checks if the filter is currently collapsed * @returns {Observable} Emits true when the current state of the filter is collapsed, false when it's expanded */ - isCollapsed(): Observable { + private isCollapsed(): Observable { return this.filterService.isCollapsed(this.filter.name); } /** - * Changes the initial state to collapsed - */ - initialCollapse() { - this.filterService.initialCollapse(this.filter.name); - this.collapsed = true; - } - - /** - * Changes the initial state to expanded + * Sets the initial state of the filter */ - initialExpand() { - this.filterService.initialExpand(this.filter.name); - this.collapsed = false; + initializeFilter() { + this.filterService.initializeFilter(this.filter); } /** * @returns {Observable} Emits a list of all values that are currently active for this filter */ - getSelectedValues(): Observable { + private getSelectedValues(): Observable { return this.filterService.getSelectedValuesForFilter(this.filter); } @@ -90,7 +88,7 @@ export class SearchFilterComponent implements OnInit { */ finishSlide(event: any): void { if (event.fromState === 'collapsed') { - this.collapsed = false; + this.closed = false; } } @@ -100,7 +98,32 @@ export class SearchFilterComponent implements OnInit { */ startSlide(event: any): void { if (event.toState === 'collapsed') { - this.collapsed = true; + this.closed = true; } } + + /** + * Check if a given filter is supposed to be shown or not + * @returns {Observable} Emits true whenever a given filter config should be shown + */ + private isActive(): Observable { + return this.selectedValues$.pipe( + switchMap((isActive) => { + if (isNotEmpty(isActive)) { + return observableOf(true); + } else { + return this.searchConfigService.searchOptions.pipe( + first(), + switchMap((options) => { + return this.searchService.getFacetValuesFor(this.filter, 1, options).pipe( + filter((RD) => !RD.isLoading), + map((valuesRD) => { + return valuesRD.payload.totalElements > 0 + }),) + } + )) + } + }), + startWith(true)); + } } diff --git a/src/app/+search-page/search-filters/search-filter/search-filter.reducer.ts b/src/app/+search-page/search-filters/search-filter/search-filter.reducer.ts index f7e064fcc7a..187bcd50d05 100644 --- a/src/app/+search-page/search-filters/search-filter/search-filter.reducer.ts +++ b/src/app/+search-page/search-filters/search-filter/search-filter.reducer.ts @@ -1,5 +1,9 @@ -import { SearchFilterAction, SearchFilterActionTypes } from './search-filter.actions'; -import { isEmpty } from '../../../shared/empty.util'; +import { + SearchFilterAction, + SearchFilterActionTypes, + SearchFilterInitializeAction +} from './search-filter.actions'; +import { isEmpty, isNotUndefined } from '../../../shared/empty.util'; /** * Interface that represents the state for a single filters @@ -28,27 +32,14 @@ export function filterReducer(state = initialState, action: SearchFilterAction): switch (action.type) { - case SearchFilterActionTypes.INITIAL_COLLAPSE: { - if (isEmpty(state) || isEmpty(state[action.filterName])) { - return Object.assign({}, state, { - [action.filterName]: { - filterCollapsed: true, - page: 1 - } - }); - } - return state; - } - - case SearchFilterActionTypes.INITIAL_EXPAND: { - if (isEmpty(state) || isEmpty(state[action.filterName])) { - return Object.assign({}, state, { - [action.filterName]: { - filterCollapsed: false, - page: 1 - } - }); - } + case SearchFilterActionTypes.INITIALIZE: { + const initAction = (action as SearchFilterInitializeAction); + return Object.assign({}, state, { + [action.filterName]: { + filterCollapsed: !initAction.initiallyExpanded, + page: 1 + } + }); return state; } diff --git a/src/app/+search-page/search-filters/search-filter/search-filter.service.spec.ts b/src/app/+search-page/search-filters/search-filter/search-filter.service.spec.ts index 156e8d47ea0..306f8cdb96c 100644 --- a/src/app/+search-page/search-filters/search-filter/search-filter.service.spec.ts +++ b/src/app/+search-page/search-filters/search-filter/search-filter.service.spec.ts @@ -5,8 +5,7 @@ import { SearchFilterDecrementPageAction, SearchFilterExpandAction, SearchFilterIncrementPageAction, - SearchFilterInitialCollapseAction, - SearchFilterInitialExpandAction, + SearchFilterInitializeAction, SearchFilterResetPageAction, SearchFilterToggleAction } from './search-filter.actions'; @@ -62,25 +61,16 @@ describe('SearchFilterService', () => { service = new SearchFilterService(store, routeServiceStub); }); - describe('when the initialCollapse method is triggered', () => { + describe('when the initializeFilter method is triggered', () => { beforeEach(() => { - service.initialCollapse(mockFilterConfig.name); + service.initializeFilter(mockFilterConfig); }); - it('SearchFilterInitialCollapseAction should be dispatched to the store', () => { - expect(store.dispatch).toHaveBeenCalledWith(new SearchFilterInitialCollapseAction(mockFilterConfig.name)); + it('SearchFilterInitializeAction should be dispatched to the store', () => { + expect(store.dispatch).toHaveBeenCalledWith(new SearchFilterInitializeAction(mockFilterConfig)); }); }); - describe('when the initialExpand method is triggered', () => { - beforeEach(() => { - service.initialExpand(mockFilterConfig.name); - }); - - it('SearchFilterInitialExpandAction should be dispatched to the store', () => { - expect(store.dispatch).toHaveBeenCalledWith(new SearchFilterInitialExpandAction(mockFilterConfig.name)); - }); - }); describe('when the collapse method is triggered', () => { beforeEach(() => { diff --git a/src/app/+search-page/search-filters/search-filter/search-filter.service.ts b/src/app/+search-page/search-filters/search-filter/search-filter.service.ts index 34c34ee4cc8..5ce1e8e6473 100644 --- a/src/app/+search-page/search-filters/search-filter/search-filter.service.ts +++ b/src/app/+search-page/search-filters/search-filter/search-filter.service.ts @@ -1,6 +1,6 @@ import { combineLatest as observableCombineLatest, Observable } from 'rxjs'; import { Injectable, InjectionToken } from '@angular/core'; -import { distinctUntilChanged, map } from 'rxjs/operators'; +import { distinctUntilChanged, map, tap } from 'rxjs/operators'; import { SearchFiltersState, SearchFilterState } from './search-filter.reducer'; import { createSelector, MemoizedSelector, select, Store } from '@ngrx/store'; import { @@ -8,8 +8,7 @@ import { SearchFilterDecrementPageAction, SearchFilterExpandAction, SearchFilterIncrementPageAction, - SearchFilterInitialCollapseAction, - SearchFilterInitialExpandAction, + SearchFilterInitializeAction, SearchFilterResetPageAction, SearchFilterToggleAction } from './search-filter.actions'; @@ -17,7 +16,9 @@ import { hasValue, isNotEmpty, } from '../../../shared/empty.util'; import { SearchFilterConfig } from '../../search-service/search-filter-config.model'; import { RouteService } from '../../../shared/services/route.service'; import { Params } from '@angular/router'; - +import { tag } from 'rxjs-spy/operators'; +import { create, detect } from "rxjs-spy"; +const spy = create(); const filterStateSelector = (state: SearchFiltersState) => state.searchFilter; export const FILTER_CONFIG: InjectionToken = new InjectionToken('filterConfig'); @@ -58,16 +59,21 @@ export class SearchFilterService { * @returns {Observable} Emits the active filters for the given filter configuration */ getSelectedValuesForFilter(filterConfig: SearchFilterConfig): Observable { - const values$ = this.routeService.getQueryParameterValues(filterConfig.paramName); + const values$ = this.routeService.getQueryParameterValues(filterConfig.paramName).pipe( + tag("parameter") + ); const prefixValues$ = this.routeService.getQueryParamsWithPrefix(filterConfig.paramName + '.').pipe( - map((params: Params) => [].concat(...Object.values(params))) + map((params: Params) => [].concat(...Object.values(params))), + tag("prefix-tag") + + ); + spy.log(); + detect('prefix-tag'); return observableCombineLatest(values$, prefixValues$).pipe( map(([values, prefixValues]) => { - console.log('getSelectedValuesForFilter ', values, prefixValues); - - if (isNotEmpty(values)) { + if (isNotEmpty(values)) { return values; } return prefixValues; @@ -138,19 +144,11 @@ export class SearchFilterService { } /** - * Dispatches an initial collapse action to the store for a given filter - * @param {string} filterName The filter for which the action is dispatched - */ - public initialCollapse(filterName: string): void { - this.store.dispatch(new SearchFilterInitialCollapseAction(filterName)); - } - - /** - * Dispatches an initial expand action to the store for a given filter - * @param {string} filterName The filter for which the action is dispatched + * Dispatches an initialize action to the store for a given filter + * @param {SearchFilterConfig} filter The filter for which the action is dispatched */ - public initialExpand(filterName: string): void { - this.store.dispatch(new SearchFilterInitialExpandAction(filterName)); + public initializeFilter(filter: SearchFilterConfig): void { + this.store.dispatch(new SearchFilterInitializeAction(filter)); } /** diff --git a/src/app/+search-page/search-filters/search-filter/search-hierarchy-filter/search-hierarchy-filter.component.html b/src/app/+search-page/search-filters/search-filter/search-hierarchy-filter/search-hierarchy-filter.component.html index 5434f0e0f42..3acc868a17b 100644 --- a/src/app/+search-page/search-filters/search-filter/search-hierarchy-filter/search-hierarchy-filter.component.html +++ b/src/app/+search-page/search-filters/search-filter/search-hierarchy-filter/search-hierarchy-filter.component.html @@ -8,17 +8,8 @@
diff --git a/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.html b/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.html index 1cc0556ed52..a30233ece33 100644 --- a/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.html +++ b/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.html @@ -25,14 +25,7 @@ diff --git a/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.ts b/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.ts index 6cb04c6c1f3..2b6797f5a1e 100644 --- a/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.ts +++ b/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.ts @@ -28,10 +28,9 @@ import { SearchConfigurationService } from '../../../search-service/search-confi * The route parameter 'id' is used to request the item it represents. * All fields of the item that should be displayed, are defined in its template. */ -const minSuffix = '.min'; -const maxSuffix = '.max'; +export const RANGE_FILTER_MIN_SUFFIX = '.min'; +export const RANGE_FILTER_MAX_SUFFIX = '.max'; const dateFormats = ['YYYY', 'YYYY-MM', 'YYYY-MM-DD']; -const rangeDelimiter = '-'; @Component({ selector: 'ds-search-range-filter', @@ -85,8 +84,8 @@ export class SearchRangeFilterComponent extends SearchFacetFilterComponent imple super.ngOnInit(); this.min = moment(this.filterConfig.minValue, dateFormats).year() || this.min; this.max = moment(this.filterConfig.maxValue, dateFormats).year() || this.max; - const iniMin = this.route.getQueryParameterValue(this.filterConfig.paramName + minSuffix).pipe(startWith(undefined)); - const iniMax = this.route.getQueryParameterValue(this.filterConfig.paramName + maxSuffix).pipe(startWith(undefined)); + const iniMin = this.route.getQueryParameterValue(this.filterConfig.paramName + RANGE_FILTER_MIN_SUFFIX).pipe(startWith(undefined)); + const iniMax = this.route.getQueryParameterValue(this.filterConfig.paramName + RANGE_FILTER_MAX_SUFFIX).pipe(startWith(undefined)); this.sub = observableCombineLatest(iniMin, iniMax).pipe( map(([min, max]) => { const minimum = hasValue(min) ? min : this.min; @@ -96,22 +95,7 @@ export class SearchRangeFilterComponent extends SearchFacetFilterComponent imple ).subscribe((minmax) => this.range = minmax); } - /** - * Calculates the parameters that should change if a given values for this range filter would be changed - * @param {string} value The values that are changed for this filter - * @returns {Observable} The changed filter parameters - */ - getChangeParams(value: string) { - const parts = value.split(rangeDelimiter); - const min = parts.length > 1 ? parts[0].trim() : value; - const max = parts.length > 1 ? parts[1].trim() : value; - return observableOf( - { - [this.filterConfig.paramName + minSuffix]: [min], - [this.filterConfig.paramName + maxSuffix]: [max], - page: 1 - }); - } + /** * Submits new custom range values to the range filter from the widget @@ -122,8 +106,8 @@ export class SearchRangeFilterComponent extends SearchFacetFilterComponent imple this.router.navigate([this.getSearchLink()], { queryParams: { - [this.filterConfig.paramName + minSuffix]: newMin, - [this.filterConfig.paramName + maxSuffix]: newMax + [this.filterConfig.paramName + RANGE_FILTER_MIN_SUFFIX]: newMin, + [this.filterConfig.paramName + RANGE_FILTER_MAX_SUFFIX]: newMax }, queryParamsHandling: 'merge' }); diff --git a/src/app/+search-page/search-filters/search-filter/search-text-filter/search-text-filter.component.html b/src/app/+search-page/search-filters/search-filter/search-text-filter/search-text-filter.component.html index 32f6fe2153c..45034507a2e 100644 --- a/src/app/+search-page/search-filters/search-filter/search-text-filter/search-text-filter.component.html +++ b/src/app/+search-page/search-filters/search-filter/search-text-filter/search-text-filter.component.html @@ -10,15 +10,8 @@ @@ -40,6 +33,5 @@ (submitSuggestion)="onSubmit($event)" (clickSuggestion)="onClick($event)" (findSuggestions)="findSuggestions($event)" - ngDefaultControl - > + ngDefaultControl>
diff --git a/src/app/+search-page/search-filters/search-filters.component.html b/src/app/+search-page/search-filters/search-filters.component.html index de725f09583..ab2a96d752b 100644 --- a/src/app/+search-page/search-filters/search-filters.component.html +++ b/src/app/+search-page/search-filters/search-filters.component.html @@ -1,7 +1,7 @@

{{"search.filters.head" | translate}}

- +
{{"search.filters.reset" | translate}} \ No newline at end of file diff --git a/src/app/+search-page/search-filters/search-filters.component.ts b/src/app/+search-page/search-filters/search-filters.component.ts index 0d32df48677..1dd747e908d 100644 --- a/src/app/+search-page/search-filters/search-filters.component.ts +++ b/src/app/+search-page/search-filters/search-filters.component.ts @@ -1,15 +1,13 @@ -import { Observable, of as observableOf } from 'rxjs'; +import { Observable } from 'rxjs'; -import { filter, first, map, mergeMap, startWith, switchMap, tap } from 'rxjs/operators'; +import { map } from 'rxjs/operators'; import { Component } from '@angular/core'; import { SearchService } from '../search-service/search.service'; import { RemoteData } from '../../core/data/remote-data'; import { SearchFilterConfig } from '../search-service/search-filter-config.model'; import { SearchConfigurationService } from '../search-service/search-configuration.service'; -import { isNotEmpty } from '../../shared/empty.util'; import { SearchFilterService } from './search-filter/search-filter.service'; import { getSucceededRemoteData } from '../../core/shared/operators'; -import { FieldUpdate } from '../../core/data/object-updates/object-updates.reducer'; @Component({ selector: 'ds-search-filters', @@ -53,32 +51,6 @@ export class SearchFiltersComponent { return this.searchService.getSearchLink(); } - /** - * Check if a given filter is supposed to be shown or not - * @param {SearchFilterConfig} filter The filter to check for - * @returns {Observable} Emits true whenever a given filter config should be shown - */ - isActive(filterConfig: SearchFilterConfig): Observable { - return this.filterService.getSelectedValuesForFilter(filterConfig).pipe( - switchMap((isActive) => { - console.log('selected fires'); - if (isNotEmpty(isActive)) { - return observableOf(true); - } else { - return this.searchConfigService.searchOptions.pipe( - first(), - switchMap((options) => { - return this.searchService.getFacetValuesFor(filterConfig, 1, options).pipe( - filter((RD) => !RD.isLoading), - map((valuesRD) => { - return valuesRD.payload.totalElements > 0 - }),) - } - )) - } - }), tap(t => console.log(t)), startWith(true)); - } - /** * Prevent unnecessary rerendering */ diff --git a/src/app/+search-page/search-page.component.ts b/src/app/+search-page/search-page.component.ts index 816e3d67bf6..0c572a3a843 100644 --- a/src/app/+search-page/search-page.component.ts +++ b/src/app/+search-page/search-page.component.ts @@ -62,7 +62,6 @@ export class SearchPageComponent implements OnInit { constructor(private service: SearchService, private sidebarService: SearchSidebarService, private windowService: HostWindowService, - private filterService: SearchFilterService, private searchConfigService: SearchConfigurationService) { this.isXsOrSm$ = this.windowService.isXsOrSm(); } diff --git a/src/app/+search-page/search-page.module.ts b/src/app/+search-page/search-page.module.ts index 0c8a4ee3060..b441367f1b7 100644 --- a/src/app/+search-page/search-page.module.ts +++ b/src/app/+search-page/search-page.module.ts @@ -28,6 +28,9 @@ import { SearchFacetFilterWrapperComponent } from './search-filters/search-filte import { SearchBooleanFilterComponent } from './search-filters/search-filter/search-boolean-filter/search-boolean-filter.component'; import { SearchHierarchyFilterComponent } from './search-filters/search-filter/search-hierarchy-filter/search-hierarchy-filter.component'; import { SearchConfigurationService } from './search-service/search-configuration.service'; +import { SearchFacetOptionComponent } from './search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component'; +import { SearchFacetSelectedOptionComponent } from './search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component'; +import { SearchFacetRangeOptionComponent } from './search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component'; const effects = [ SearchSidebarEffects @@ -63,6 +66,9 @@ const effects = [ SearchTextFilterComponent, SearchHierarchyFilterComponent, SearchBooleanFilterComponent, + SearchFacetOptionComponent, + SearchFacetSelectedOptionComponent, + SearchFacetRangeOptionComponent ], providers: [ SearchService, @@ -82,6 +88,9 @@ const effects = [ SearchTextFilterComponent, SearchHierarchyFilterComponent, SearchBooleanFilterComponent, + SearchFacetOptionComponent, + SearchFacetSelectedOptionComponent, + SearchFacetRangeOptionComponent ] }) diff --git a/src/app/shared/services/route.service.ts b/src/app/shared/services/route.service.ts index 478d92c21ed..b6c7747cfbd 100644 --- a/src/app/shared/services/route.service.ts +++ b/src/app/shared/services/route.service.ts @@ -1,4 +1,4 @@ -import { distinctUntilChanged, map } from 'rxjs/operators'; +import { distinctUntilChanged, map, tap } from 'rxjs/operators'; import { Injectable } from '@angular/core'; import { Observable } from 'rxjs'; import { @@ -6,6 +6,7 @@ import { Router, } from '@angular/router'; import { isNotEmpty } from '../empty.util'; +import { detect } from 'rxjs-spy'; @Injectable() export class RouteService { @@ -14,6 +15,7 @@ export class RouteService { } getQueryParameterValues(paramName: string): Observable { + console.log('called'); return this.route.queryParamMap.pipe( map((params) => [...params.getAll(paramName)]), distinctUntilChanged((a, b) => JSON.stringify(a) === JSON.stringify(b)) @@ -44,6 +46,7 @@ export class RouteService { getQueryParamsWithPrefix(prefix: string): Observable { return this.route.queryParamMap.pipe( map((qparams) => { + console.log('map'); const params = {}; qparams.keys .filter((key) => key.startsWith(prefix)) @@ -52,6 +55,8 @@ export class RouteService { }); return params; }), - distinctUntilChanged((a, b) => JSON.stringify(a) === JSON.stringify(b))); + distinctUntilChanged((a, b) => { console.log('changed?', a, b, JSON.stringify(a) === JSON.stringify(b)); return JSON.stringify(a) === JSON.stringify(b)}), + tap((t) => console.log('changed')) + ); } } diff --git a/src/main.browser.ts b/src/main.browser.ts index 8409a964853..7b0fbf9d65b 100644 --- a/src/main.browser.ts +++ b/src/main.browser.ts @@ -12,6 +12,7 @@ import { BrowserAppModule } from './modules/app/browser-app.module'; import { ENV_CONFIG } from './config'; + if (ENV_CONFIG.production) { enableProdMode(); } diff --git a/yarn.lock b/yarn.lock index d71d9b91869..208ed60c2f6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -243,6 +243,10 @@ "@types/connect" "*" "@types/node" "*" +"@types/circular-json@^0.4.0": + version "0.4.0" + resolved "https://registry.yarnpkg.com/@types/circular-json/-/circular-json-0.4.0.tgz#7401f7e218cfe87ad4c43690da5658b9acaf51be" + "@types/connect@*": version "3.4.32" resolved "https://registry.yarnpkg.com/@types/connect/-/connect-3.4.32.tgz#aa0e9616b9435ccad02bc52b5b454ffc2c70ba28" @@ -370,6 +374,10 @@ dependencies: "@types/node" "*" +"@types/stacktrace-js@^0.0.32": + version "0.0.32" + resolved "https://registry.yarnpkg.com/@types/stacktrace-js/-/stacktrace-js-0.0.32.tgz#d23e4a36a5073d39487fbea8234cc6186862d389" + "@types/strip-bom@^3.0.0": version "3.0.0" resolved "https://registry.yarnpkg.com/@types/strip-bom/-/strip-bom-3.0.0.tgz#14a8ec3956c2e81edb7520790aecf21c290aebd2" @@ -1600,6 +1608,10 @@ cipher-base@^1.0.0, cipher-base@^1.0.1, cipher-base@^1.0.3: inherits "^2.0.1" safe-buffer "^5.0.1" +circular-json@^0.5.0: + version "0.5.9" + resolved "https://registry.yarnpkg.com/circular-json/-/circular-json-0.5.9.tgz#932763ae88f4f7dead7a0d09c8a51a4743a53b1d" + circular-json@^0.5.5: version "0.5.5" resolved "https://registry.yarnpkg.com/circular-json/-/circular-json-0.5.5.tgz#64182ef359042d37cd8e767fc9de878b1e9447d3" @@ -2605,6 +2617,12 @@ error-ex@^1.2.0, error-ex@^1.3.1: dependencies: is-arrayish "^0.2.1" +error-stack-parser@^2.0.1: + version "2.0.2" + resolved "https://registry.yarnpkg.com/error-stack-parser/-/error-stack-parser-2.0.2.tgz#4ae8dbaa2bf90a8b450707b9149dcabca135520d" + dependencies: + stackframe "^1.0.4" + es-abstract@^1.4.3, es-abstract@^1.5.1: version "1.12.0" resolved "https://registry.yarnpkg.com/es-abstract/-/es-abstract-1.12.0.tgz#9dbbdd27c6856f0001421ca18782d786bf8a6165" @@ -7237,6 +7255,16 @@ rx@^4.1.0: version "4.1.0" resolved "https://registry.yarnpkg.com/rx/-/rx-4.1.0.tgz#a5f13ff79ef3b740fe30aa803fb09f98805d4782" +rxjs-spy@^7.5.1: + version "7.5.1" + resolved "https://registry.yarnpkg.com/rxjs-spy/-/rxjs-spy-7.5.1.tgz#1a9ef50bc8d7dd00d9ecf3c54c00929231eaf319" + dependencies: + "@types/circular-json" "^0.4.0" + "@types/stacktrace-js" "^0.0.32" + circular-json "^0.5.0" + error-stack-parser "^2.0.1" + stacktrace-gps "^3.0.2" + rxjs@6.2.2, rxjs@^6.0.0, rxjs@^6.1.0: version "6.2.2" resolved "https://registry.yarnpkg.com/rxjs/-/rxjs-6.2.2.tgz#eb75fa3c186ff5289907d06483a77884586e1cf9" @@ -7681,6 +7709,10 @@ source-map@0.5.0: version "0.5.0" resolved "https://registry.yarnpkg.com/source-map/-/source-map-0.5.0.tgz#0fe96503ac86a5adb5de63f4e412ae4872cdbe86" +source-map@0.5.6: + version "0.5.6" + resolved "https://registry.yarnpkg.com/source-map/-/source-map-0.5.6.tgz#75ce38f52bf0733c5a7f0c118d81334a2bb5f412" + source-map@0.7.3: version "0.7.3" resolved "https://registry.yarnpkg.com/source-map/-/source-map-0.7.3.tgz#5302f8169031735226544092e64981f751750383" @@ -7801,6 +7833,17 @@ ssri@^5.2.4: dependencies: safe-buffer "^5.1.1" +stackframe@^1.0.4: + version "1.0.4" + resolved "https://registry.yarnpkg.com/stackframe/-/stackframe-1.0.4.tgz#357b24a992f9427cba6b545d96a14ed2cbca187b" + +stacktrace-gps@^3.0.2: + version "3.0.2" + resolved "https://registry.yarnpkg.com/stacktrace-gps/-/stacktrace-gps-3.0.2.tgz#33f8baa4467323ab2bd1816efa279942ba431ccc" + dependencies: + source-map "0.5.6" + stackframe "^1.0.4" + static-extend@^0.1.1: version "0.1.2" resolved "https://registry.yarnpkg.com/static-extend/-/static-extend-0.1.2.tgz#60809c39cbff55337226fd5e0b520f341f1fb5c6" From 037063ea63b007c2bf6e2d06b0e373eb9379d8dd Mon Sep 17 00:00:00 2001 From: lotte Date: Thu, 21 Mar 2019 15:52:34 +0100 Subject: [PATCH 04/10] Added more optimalisations and fixed tests --- .../search-boolean-filter.component.html | 5 +- .../search-facet-option.component.html | 6 +-- .../search-facet-option.component.ts | 43 +++++++++-------- .../search-facet-range-option.component.html | 4 +- .../search-facet-range-option.component.ts | 41 ++++++++-------- ...earch-facet-selected-option.component.html | 2 +- .../search-facet-selected-option.component.ts | 45 +++++++++++------ .../search-facet-filter.component.spec.ts | 14 ------ .../search-facet-filter.component.ts | 48 ++++--------------- .../search-filter.component.spec.ts | 33 +++++-------- .../search-filter.reducer.spec.ts | 24 +++++----- .../search-filter.service.spec.ts | 1 - .../search-filter/search-filter.service.ts | 16 ++----- .../search-hierarchy-filter.component.html | 10 +--- .../search-range-filter.component.html | 4 +- .../search-range-filter.component.spec.ts | 10 ---- .../search-range-filter.component.ts | 2 - .../search-text-filter.component.html | 16 ++----- .../search-configuration.service.spec.ts | 4 +- .../search-configuration.service.ts | 4 +- src/app/core/cache/object-cache.service.ts | 2 +- src/app/shared/services/route.service.ts | 5 +- src/main.browser.ts | 1 - 23 files changed, 132 insertions(+), 208 deletions(-) diff --git a/src/app/+search-page/search-filters/search-filter/search-boolean-filter/search-boolean-filter.component.html b/src/app/+search-page/search-filters/search-filter/search-boolean-filter/search-boolean-filter.component.html index 06154916b2a..968bf9e4209 100644 --- a/src/app/+search-page/search-filters/search-filter/search-boolean-filter/search-boolean-filter.component.html +++ b/src/app/+search-page/search-filters/search-filter/search-boolean-filter/search-boolean-filter.component.html @@ -1,10 +1,9 @@
- +
- - +
diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.html b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.html index 881fb98dcb0..9f5a88b71b8 100644 --- a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.html +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.html @@ -1,6 +1,6 @@ - + {{filterValue.value}} diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.ts b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.ts index c5896f92b63..9fa59a0c1e2 100644 --- a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.ts +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.ts @@ -1,11 +1,13 @@ -import { Observable, of as observableOf } from 'rxjs'; -import { map } from 'rxjs/operators'; -import { Component, Input, OnInit } from '@angular/core'; +import { combineLatest as observableCombineLatest, Observable, Subscription } from 'rxjs'; +import { map, take } from 'rxjs/operators'; +import { Component, Input, OnDestroy, OnInit } from '@angular/core'; import { Router } from '@angular/router'; import { FacetValue } from '../../../../search-service/facet-value.model'; import { SearchFilterConfig } from '../../../../search-service/search-filter-config.model'; import { SearchService } from '../../../../search-service/search.service'; import { SearchFilterService } from '../../search-filter.service'; +import { SearchConfigurationService } from '../../../../search-service/search-configuration.service'; +import { hasValue } from '../../../../../shared/empty.util'; @Component({ selector: 'ds-search-facet-option', @@ -15,24 +17,22 @@ import { SearchFilterService } from '../../search-filter.service'; /** * Represents a single option in a filter facet */ -export class SearchFacetOptionComponent implements OnInit { +export class SearchFacetOptionComponent implements OnInit, OnDestroy { /** * A single value for this component */ @Input() filterValue: FacetValue; @Input() filterConfig: SearchFilterConfig; - - /** - * Emits the active values for this filter - */ - selectedValues$: Observable; + @Input() selectedValues$: Observable; isVisible: Observable; addQueryParams; + sub: Subscription; constructor(protected searchService: SearchService, protected filterService: SearchFilterService, + protected searchConfigService: SearchConfigurationService, protected router: Router ) { } @@ -41,9 +41,11 @@ export class SearchFacetOptionComponent implements OnInit { * Initializes all observable instance variables and starts listening to them */ ngOnInit(): void { - this.selectedValues$ = this.filterService.getSelectedValuesForFilter(this.filterConfig); this.isVisible = this.isChecked().pipe(map((checked: boolean) => !checked)); - this.addQueryParams = this.getAddParams(); + this.sub = observableCombineLatest(this.selectedValues$, this.searchConfigService.searchOptions) + .subscribe(([selectedValues, searchOptions]) => { + this.updateAddParams(selectedValues) + }); } /** @@ -63,16 +65,17 @@ export class SearchFacetOptionComponent implements OnInit { /** * Calculates the parameters that should change if a given value for this filter would be added to the active filters * @param {string} value The value that is added for this filter - * @returns {Observable} The changed filter parameters */ - private getAddParams(): Observable { - return this.selectedValues$.pipe(map((selectedValues) => { - return { - [this.filterConfig.paramName]: [...selectedValues, this.filterValue.value], - page: 1 - }; - })); + private updateAddParams(selectedValues: string[]): void { + this.addQueryParams = { + [this.filterConfig.paramName]: [...selectedValues, this.filterValue.value], + page: 1 + }; } + ngOnDestroy(): void { + if (hasValue(this.sub)) { + this.sub.unsubscribe(); + } + } } - diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.html b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.html index 92e90a00cc9..b485fe0fd08 100644 --- a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.html +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.html @@ -1,6 +1,6 @@ - + [queryParams]="changeQueryParams" queryParamsHandling="merge"> {{filterValue.value}} {{filterValue.count}} diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.ts b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.ts index 0b38ca18450..2f6a49f8b98 100644 --- a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.ts +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.ts @@ -1,6 +1,6 @@ -import { Observable, of as observableOf } from 'rxjs'; +import { Observable, Subscription } from 'rxjs'; import { map } from 'rxjs/operators'; -import { Component, Input, OnInit } from '@angular/core'; +import { Component, Input, OnDestroy, OnInit } from '@angular/core'; import { Router } from '@angular/router'; import { FacetValue } from '../../../../search-service/facet-value.model'; import { SearchFilterConfig } from '../../../../search-service/search-filter-config.model'; @@ -10,6 +10,8 @@ import { RANGE_FILTER_MAX_SUFFIX, RANGE_FILTER_MIN_SUFFIX } from '../../search-range-filter/search-range-filter.component'; +import { SearchConfigurationService } from '../../../../search-service/search-configuration.service'; +import { hasValue } from '../../../../../shared/empty.util'; const rangeDelimiter = '-'; @@ -21,24 +23,21 @@ const rangeDelimiter = '-'; /** * Represents a single option in a filter facet */ -export class SearchFacetRangeOptionComponent implements OnInit { +export class SearchFacetRangeOptionComponent implements OnInit, OnDestroy { /** * A single value for this component */ @Input() filterValue: FacetValue; @Input() filterConfig: SearchFilterConfig; - /** - * Emits the active values for this filter - */ - selectedValues$: Observable; - isVisible: Observable; changeQueryParams; + sub: Subscription; constructor(protected searchService: SearchService, protected filterService: SearchFilterService, + protected searchConfigService: SearchConfigurationService, protected router: Router ) { } @@ -47,9 +46,10 @@ export class SearchFacetRangeOptionComponent implements OnInit { * Initializes all observable instance variables and starts listening to them */ ngOnInit(): void { - this.selectedValues$ = this.filterService.getSelectedValuesForFilter(this.filterConfig); this.isVisible = this.isChecked().pipe(map((checked: boolean) => !checked)); - this.changeQueryParams = this.getChangeParams(); + this.sub = this.searchConfigService.searchOptions.subscribe(() => { + this.updateChangeParams() + }); } /** @@ -66,23 +66,24 @@ export class SearchFacetRangeOptionComponent implements OnInit { return this.searchService.getSearchLink(); } - /** * Calculates the parameters that should change if a given values for this range filter would be changed * @param {string} value The values that are changed for this filter - * @returns {Observable} The changed filter parameters */ - getChangeParams() { + updateChangeParams(): void { const parts = this.filterValue.value.split(rangeDelimiter); const min = parts.length > 1 ? parts[0].trim() : this.filterValue.value; const max = parts.length > 1 ? parts[1].trim() : this.filterValue.value; - return observableOf( - { - [this.filterConfig.paramName + RANGE_FILTER_MIN_SUFFIX]: [min], - [this.filterConfig.paramName + RANGE_FILTER_MAX_SUFFIX]: [max], - page: 1 - }); + this.changeQueryParams = { + [this.filterConfig.paramName + RANGE_FILTER_MIN_SUFFIX]: [min], + [this.filterConfig.paramName + RANGE_FILTER_MAX_SUFFIX]: [max], + page: 1 + }; } + ngOnDestroy(): void { + if (hasValue(this.sub)) { + this.sub.unsubscribe(); + } + } } - diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.html b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.html index 7bed6207706..ba43bae100c 100644 --- a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.html +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.html @@ -1,6 +1,6 @@ + [queryParams]="removeQueryParams" queryParamsHandling="merge"> {{selectedValue}} \ No newline at end of file diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.ts b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.ts index 2fdc490d68d..55030a4a84d 100644 --- a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.ts +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.ts @@ -1,11 +1,19 @@ -import { Observable, of as observableOf } from 'rxjs'; -import { map } from 'rxjs/operators'; -import { Component, Input, OnInit } from '@angular/core'; +import { + combineLatest as observableCombineLatest, + Observable, + of as observableOf, + Subscription +} from 'rxjs'; +import { delay, map } from 'rxjs/operators'; +import { Component, Input, OnChanges, OnDestroy, OnInit, SimpleChanges } from '@angular/core'; import { Router } from '@angular/router'; import { FacetValue } from '../../../../search-service/facet-value.model'; import { SearchFilterConfig } from '../../../../search-service/search-filter-config.model'; import { SearchService } from '../../../../search-service/search.service'; import { SearchFilterService } from '../../search-filter.service'; +import { hasValue } from '../../../../../shared/empty.util'; +import { SearchOptions } from '../../../../search-options.model'; +import { SearchConfigurationService } from '../../../../search-service/search-configuration.service'; @Component({ selector: 'ds-search-facet-selected-option', @@ -15,7 +23,7 @@ import { SearchFilterService } from '../../search-filter.service'; /** * Represents a single option in a filter facet */ -export class SearchFacetSelectedOptionComponent implements OnInit { +export class SearchFacetSelectedOptionComponent implements OnInit, OnDestroy { /** * A single value for this component */ @@ -25,12 +33,14 @@ export class SearchFacetSelectedOptionComponent implements OnInit { /** * Emits the active values for this filter */ - selectedValues$: Observable; + @Input() selectedValues$: Observable; removeQueryParams; + sub: Subscription; constructor(protected searchService: SearchService, protected filterService: SearchFilterService, + protected searchConfigService: SearchConfigurationService, protected router: Router ) { } @@ -39,8 +49,10 @@ export class SearchFacetSelectedOptionComponent implements OnInit { * Initializes all observable instance variables and starts listening to them */ ngOnInit(): void { - this.selectedValues$ = this.filterService.getSelectedValuesForFilter(this.filterConfig); - this.removeQueryParams = this.getRemoveParams(); + this.sub = observableCombineLatest(this.selectedValues$, this.searchConfigService.searchOptions) + .subscribe(([selectedValues, searchOptions]) => { + this.updateRemoveParams(selectedValues) + }); } /** @@ -55,13 +67,16 @@ export class SearchFacetSelectedOptionComponent implements OnInit { * @param {string} value The value that is removed for this filter * @returns {Observable} The changed filter parameters */ - private getRemoveParams(): Observable { - return this.selectedValues$.pipe(map((selectedValues) => { - return { - [this.filterConfig.paramName]: selectedValues.filter((v) => v !== this.selectedValue), - page: 1 - }; - })); + private updateRemoveParams(selectedValues: string[]): void { + this.removeQueryParams = { + [this.filterConfig.paramName]: selectedValues.filter((v) => v !== this.selectedValue), + page: 1 + }; } -} + ngOnDestroy(): void { + if (hasValue(this.sub)) { + this.sub.unsubscribe(); + } + } +} diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter/search-facet-filter.component.spec.ts b/src/app/+search-page/search-filters/search-filter/search-facet-filter/search-facet-filter.component.spec.ts index 498c41dd6cc..cb3d4730b4f 100644 --- a/src/app/+search-page/search-filters/search-filter/search-facet-filter/search-facet-filter.component.spec.ts +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter/search-facet-filter.component.spec.ts @@ -120,20 +120,6 @@ describe('SearchFacetFilterComponent', () => { }); }); - describe('when the getAddParams method is called wih a value', () => { - it('should return the selectedValue list with the new parameter value', () => { - const result = comp.getAddParams(value3); - result.subscribe((r) => expect(r[mockFilterConfig.paramName]).toEqual([value1, value2, value3])); - }); - }); - - describe('when the getRemoveParams method is called wih a value', () => { - it('should return the selectedValue list with the parameter value left out', () => { - const result = comp.getRemoveParams(value1); - result.subscribe((r) => expect(r[mockFilterConfig.paramName]).toEqual([value2])); - }); - }); - describe('when the showMore method is called', () => { beforeEach(() => { spyOn(filterService, 'incrementPage'); diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter/search-facet-filter.component.ts b/src/app/+search-page/search-filters/search-filter/search-facet-filter/search-facet-filter.component.ts index 74c2794313b..62275ee3c48 100644 --- a/src/app/+search-page/search-filters/search-filter/search-facet-filter/search-facet-filter.component.ts +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter/search-facet-filter.component.ts @@ -22,6 +22,7 @@ import { FILTER_CONFIG, SearchFilterService } from '../search-filter.service'; import { SearchConfigurationService } from '../../../search-service/search-configuration.service'; import { getSucceededRemoteData } from '../../../../core/shared/operators'; import { InputSuggestion } from '../../../../shared/input-suggestions/input-suggestions.model'; +import { SearchOptions } from '../../../search-options.model'; @Component({ selector: 'ds-search-facet-filter', @@ -65,13 +66,14 @@ export class SearchFacetFilterComponent implements OnInit, OnDestroy { /** * Emits the active values for this filter */ - selectedValues: Observable; + selectedValues$: Observable; private collapseNextUpdate = true; /** * State of the requested facets used to time the animation */ animationState = 'loading'; + searchOptions$: Observable; constructor(protected searchService: SearchService, protected filterService: SearchFilterService, @@ -85,15 +87,13 @@ export class SearchFacetFilterComponent implements OnInit, OnDestroy { * Initializes all observable instance variables and starts listening to them */ ngOnInit(): void { - console.log('renderSearchFacetFilterComponent') - this.filterValues$ = new BehaviorSubject(new RemoteData(true, false, undefined, undefined, undefined)); this.currentPage = this.getCurrentPage().pipe(distinctUntilChanged()); - this.selectedValues = this.filterService.getSelectedValuesForFilter(this.filterConfig); - const searchOptions = this.searchConfigService.searchOptions; - this.subs.push(this.searchConfigService.searchOptions.subscribe(() => this.updateFilterValueList())); - const facetValues = observableCombineLatest(searchOptions, this.currentPage).pipe( + this.selectedValues$ = this.filterService.getSelectedValuesForFilter(this.filterConfig); + this.searchOptions$ = this.searchConfigService.searchOptions; + this.subs.push(this.searchOptions$.subscribe(() => this.updateFilterValueList())); + const facetValues = observableCombineLatest(this.searchOptions$, this.currentPage).pipe( map(([options, page]) => { return { options, page } }), @@ -193,8 +193,7 @@ export class SearchFacetFilterComponent implements OnInit, OnDestroy { * @param data The string from the input field */ onSubmit(data: any) { - console.log('onsubmit'); - this.selectedValues.pipe(take(1)).subscribe((selectedValues) => { + this.selectedValues$.pipe(take(1)).subscribe((selectedValues) => { if (isNotEmpty(data)) { this.router.navigate([this.getSearchLink()], { queryParams: @@ -219,34 +218,6 @@ export class SearchFacetFilterComponent implements OnInit, OnDestroy { return hasValue(o); } - /** - * Calculates the parameters that should change if a given value for this filter would be removed from the active filters - * @param {string} value The value that is removed for this filter - * @returns {Observable} The changed filter parameters - */ - getRemoveParams(value: string): Observable { - return this.selectedValues.pipe(map((selectedValues) => { - return { - [this.filterConfig.paramName]: selectedValues.filter((v) => v !== value), - page: 1 - }; - })); - } - - /** - * Calculates the parameters that should change if a given value for this filter would be added to the active filters - * @param {string} value The value that is added for this filter - * @returns {Observable} The changed filter parameters - */ - getAddParams(value: string): Observable { - return this.selectedValues.pipe(map((selectedValues) => { - return { - [this.filterConfig.paramName]: [...selectedValues, value], - page: 1 - }; - })); - } - /** * Unsubscribe from all subscriptions */ @@ -263,7 +234,7 @@ export class SearchFacetFilterComponent implements OnInit, OnDestroy { */ findSuggestions(data): void { if (isNotEmpty(data)) { - this.searchConfigService.searchOptions.pipe(take(1)).subscribe( + this.searchOptions$.pipe(take(1)).subscribe( (options) => { this.filterSearchResults = this.searchService.getFacetValuesFor(this.filterConfig, 1, options, data.toLowerCase()) .pipe( @@ -295,7 +266,6 @@ export class SearchFacetFilterComponent implements OnInit, OnDestroy { return new EmphasizePipe().transform(facet.value, query) + ' (' + facet.count + ')'; } - /** * Prevent unnecessary rerendering */ diff --git a/src/app/+search-page/search-filters/search-filter/search-filter.component.spec.ts b/src/app/+search-page/search-filters/search-filter/search-filter.component.spec.ts index caa5a6febc2..30ef349675a 100644 --- a/src/app/+search-page/search-filters/search-filter/search-filter.component.spec.ts +++ b/src/app/+search-page/search-filters/search-filter/search-filter.component.spec.ts @@ -10,6 +10,7 @@ import { SearchService } from '../../search-service/search.service'; import { SearchFilterComponent } from './search-filter.component'; import { SearchFilterConfig } from '../../search-service/search-filter-config.model'; import { FilterType } from '../../search-service/filter-type.model'; +import { SearchConfigurationService } from '../../search-service/search-configuration.service'; describe('SearchFilterComponent', () => { let comp: SearchFilterComponent; @@ -33,9 +34,7 @@ describe('SearchFilterComponent', () => { }, expand: (filter) => { }, - initialCollapse: (filter) => { - }, - initialExpand: (filter) => { + initializeFilter: (filter) => { }, getSelectedValuesForFilter: (filter) => { return observableOf([filterName1, filterName2, filterName3]) @@ -55,6 +54,8 @@ describe('SearchFilterComponent', () => { getFacetValuesFor: (filter) => mockResults }; + const searchConfigServiceStub = {}; + beforeEach(async(() => { TestBed.configureTestingModule({ imports: [TranslateModule.forRoot(), RouterTestingModule.withRoutes([]), NoopAnimationsModule], @@ -65,6 +66,7 @@ describe('SearchFilterComponent', () => { provide: SearchFilterService, useValue: mockFilterService }, + { provide: SearchConfigurationService, useValue: searchConfigServiceStub }, ], schemas: [NO_ERRORS_SCHEMA] }).overrideComponent(SearchFilterComponent, { @@ -91,32 +93,21 @@ describe('SearchFilterComponent', () => { }); }); - describe('when the initialCollapse method is triggered', () => { - beforeEach(() => { - spyOn(filterService, 'initialCollapse'); - comp.initialCollapse(); - }); - - it('should call initialCollapse with the correct filter configuration name', () => { - expect(filterService.initialCollapse).toHaveBeenCalledWith(mockFilterConfig.name) - }); - }); - - describe('when the initialExpand method is triggered', () => { + describe('when the initializeFilter method is triggered', () => { beforeEach(() => { - spyOn(filterService, 'initialExpand'); - comp.initialExpand(); + spyOn(filterService, 'initializeFilter'); + comp.initializeFilter(); }); it('should call initialCollapse with the correct filter configuration name', () => { - expect(filterService.initialExpand).toHaveBeenCalledWith(mockFilterConfig.name) + expect(filterService.initializeFilter).toHaveBeenCalledWith(mockFilterConfig) }); }); describe('when getSelectedValues is called', () => { let valuesObservable: Observable; beforeEach(() => { - valuesObservable = comp.getSelectedValues(); + valuesObservable = (comp as any).getSelectedValues(); }); it('should return an observable containing the existing filters', () => { @@ -141,7 +132,7 @@ describe('SearchFilterComponent', () => { let isActive: Observable; beforeEach(() => { filterService.isCollapsed = () => observableOf(true); - isActive = comp.isCollapsed(); + isActive = (comp as any).isCollapsed(); }); it('should return an observable containing true', () => { @@ -156,7 +147,7 @@ describe('SearchFilterComponent', () => { let isActive: Observable; beforeEach(() => { filterService.isCollapsed = () => observableOf(false); - isActive = comp.isCollapsed(); + isActive = (comp as any).isCollapsed(); }); it('should return an observable containing false', () => { diff --git a/src/app/+search-page/search-filters/search-filter/search-filter.reducer.spec.ts b/src/app/+search-page/search-filters/search-filter/search-filter.reducer.spec.ts index 8fbfbf2e651..2f3268fba59 100644 --- a/src/app/+search-page/search-filters/search-filter/search-filter.reducer.spec.ts +++ b/src/app/+search-page/search-filters/search-filter/search-filter.reducer.spec.ts @@ -1,10 +1,8 @@ import * as deepFreeze from 'deep-freeze'; import { SearchFilterCollapseAction, SearchFilterExpandAction, SearchFilterIncrementPageAction, - SearchFilterInitialCollapseAction, - SearchFilterInitialExpandAction, SearchFilterToggleAction, - SearchFilterDecrementPageAction, SearchFilterResetPageAction + SearchFilterDecrementPageAction, SearchFilterResetPageAction, SearchFilterInitializeAction } from './search-filter.actions'; import { filterReducer } from './search-filter.reducer'; @@ -98,35 +96,39 @@ describe('filterReducer', () => { filterReducer(state, action); }); - it('should set filterCollapsed to true in response to the INITIAL_COLLAPSE action when no state has been set for this filter', () => { + it('should set filterCollapsed to true in response to the INITIALIZE action with isOpenByDefault to false when no state has been set for this filter', () => { const state = {}; state[filterName2] = { filterCollapsed: false, page: 1 }; - const action = new SearchFilterInitialCollapseAction(filterName1); + const filterConfig = {isOpenByDefault: false, name: filterName1} as any; + const action = new SearchFilterInitializeAction(filterConfig); const newState = filterReducer(state, action); expect(newState[filterName1].filterCollapsed).toEqual(true); }); - it('should set filterCollapsed to true in response to the INITIAL_EXPAND action when no state has been set for this filter', () => { + it('should set filterCollapsed to false in response to the INITIALIZE action with isOpenByDefault to true when no state has been set for this filter', () => { const state = {}; state[filterName2] = { filterCollapsed: true, page: 1 }; - const action = new SearchFilterInitialExpandAction(filterName1); + const filterConfig = {isOpenByDefault: true, name: filterName1} as any; + const action = new SearchFilterInitializeAction(filterConfig); const newState = filterReducer(state, action); expect(newState[filterName1].filterCollapsed).toEqual(false); }); - it('should not change the state in response to the INITIAL_COLLAPSE action when the state has already been set for this filter', () => { + it('should not change the state in response to the INITIALIZE action with isOpenByDefault to false when the state has already been set for this filter', () => { const state = {}; state[filterName1] = { filterCollapsed: false, page: 1 }; - const action = new SearchFilterInitialCollapseAction(filterName1); + const filterConfig = { isOpenByDefault: true, name: filterName1 } as any; + const action = new SearchFilterInitializeAction(filterConfig); const newState = filterReducer(state, action); expect(newState).toEqual(state); }); - it('should not change the state in response to the INITIAL_EXPAND action when the state has already been set for this filter', () => { + it('should not change the state in response to the INITIALIZE action with isOpenByDefault to true when the state has already been set for this filter', () => { const state = {}; state[filterName1] = { filterCollapsed: true, page: 1 }; - const action = new SearchFilterInitialExpandAction(filterName1); + const filterConfig = { isOpenByDefault: false, name: filterName1 } as any; + const action = new SearchFilterInitializeAction(filterConfig); const newState = filterReducer(state, action); expect(newState).toEqual(state); }); diff --git a/src/app/+search-page/search-filters/search-filter/search-filter.service.spec.ts b/src/app/+search-page/search-filters/search-filter/search-filter.service.spec.ts index 306f8cdb96c..19239d899cf 100644 --- a/src/app/+search-page/search-filters/search-filter/search-filter.service.spec.ts +++ b/src/app/+search-page/search-filters/search-filter/search-filter.service.spec.ts @@ -71,7 +71,6 @@ describe('SearchFilterService', () => { }); }); - describe('when the collapse method is triggered', () => { beforeEach(() => { service.collapse(mockFilterConfig.name); diff --git a/src/app/+search-page/search-filters/search-filter/search-filter.service.ts b/src/app/+search-page/search-filters/search-filter/search-filter.service.ts index 5ce1e8e6473..bed4b1777f6 100644 --- a/src/app/+search-page/search-filters/search-filter/search-filter.service.ts +++ b/src/app/+search-page/search-filters/search-filter/search-filter.service.ts @@ -1,6 +1,6 @@ import { combineLatest as observableCombineLatest, Observable } from 'rxjs'; import { Injectable, InjectionToken } from '@angular/core'; -import { distinctUntilChanged, map, tap } from 'rxjs/operators'; +import { distinctUntilChanged, map } from 'rxjs/operators'; import { SearchFiltersState, SearchFilterState } from './search-filter.reducer'; import { createSelector, MemoizedSelector, select, Store } from '@ngrx/store'; import { @@ -16,9 +16,8 @@ import { hasValue, isNotEmpty, } from '../../../shared/empty.util'; import { SearchFilterConfig } from '../../search-service/search-filter-config.model'; import { RouteService } from '../../../shared/services/route.service'; import { Params } from '@angular/router'; -import { tag } from 'rxjs-spy/operators'; -import { create, detect } from "rxjs-spy"; -const spy = create(); +import { SearchOptions } from '../../search-options.model'; +// const spy = create(); const filterStateSelector = (state: SearchFiltersState) => state.searchFilter; export const FILTER_CONFIG: InjectionToken = new InjectionToken('filterConfig'); @@ -59,17 +58,10 @@ export class SearchFilterService { * @returns {Observable} Emits the active filters for the given filter configuration */ getSelectedValuesForFilter(filterConfig: SearchFilterConfig): Observable { - const values$ = this.routeService.getQueryParameterValues(filterConfig.paramName).pipe( - tag("parameter") - ); + const values$ = this.routeService.getQueryParameterValues(filterConfig.paramName); const prefixValues$ = this.routeService.getQueryParamsWithPrefix(filterConfig.paramName + '.').pipe( map((params: Params) => [].concat(...Object.values(params))), - tag("prefix-tag") - - ); - spy.log(); - detect('prefix-tag'); return observableCombineLatest(values$, prefixValues$).pipe( map(([values, prefixValues]) => { diff --git a/src/app/+search-page/search-filters/search-filter/search-hierarchy-filter/search-hierarchy-filter.component.html b/src/app/+search-page/search-filters/search-filter/search-hierarchy-filter/search-hierarchy-filter.component.html index 3acc868a17b..b6ae0ada639 100644 --- a/src/app/+search-page/search-filters/search-filter/search-hierarchy-filter/search-hierarchy-filter.component.html +++ b/src/app/+search-page/search-filters/search-filter/search-hierarchy-filter/search-hierarchy-filter.component.html @@ -1,15 +1,9 @@
- - - {{value}} - +
- - +
diff --git a/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.html b/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.html index a30233ece33..9d35cc518af 100644 --- a/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.html +++ b/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.html @@ -24,9 +24,7 @@
- - - +
diff --git a/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.spec.ts b/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.spec.ts index 6f3450e18e1..930ea8c9fb2 100644 --- a/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.spec.ts +++ b/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.spec.ts @@ -106,16 +106,6 @@ describe('SearchRangeFilterComponent', () => { fixture.detectChanges(); }); - describe('when the getChangeParams method is called wih a value', () => { - it('should return the selectedValue list with the new parameter value', () => { - const result$ = comp.getChangeParams(value3); - result$.subscribe((result) => { - expect(result[mockFilterConfig.paramName + minSuffix]).toEqual(['1990']); - expect(result[mockFilterConfig.paramName + maxSuffix]).toEqual(['1992']); - }); - }); - }); - describe('when the onSubmit method is called with data', () => { const searchUrl = '/search/path'; // const data = { [mockFilterConfig.paramName + minSuffix]: '1900', [mockFilterConfig.paramName + maxSuffix]: '1950' }; diff --git a/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.ts b/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.ts index 2b6797f5a1e..0bc30676c16 100644 --- a/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.ts +++ b/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.ts @@ -95,8 +95,6 @@ export class SearchRangeFilterComponent extends SearchFacetFilterComponent imple ).subscribe((minmax) => this.range = minmax); } - - /** * Submits new custom range values to the range filter from the widget */ diff --git a/src/app/+search-page/search-filters/search-filter/search-text-filter/search-text-filter.component.html b/src/app/+search-page/search-filters/search-filter/search-text-filter/search-text-filter.component.html index 45034507a2e..25ff8e46d34 100644 --- a/src/app/+search-page/search-filters/search-filter/search-text-filter/search-text-filter.component.html +++ b/src/app/+search-page/search-filters/search-filter/search-text-filter/search-text-filter.component.html @@ -1,19 +1,9 @@
- - - {{value}} - - + +
- - - - - - +
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 af8897c93b7..f1f4ef8bdce 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 @@ -117,7 +117,7 @@ describe('SearchConfigurationService', () => { describe('when subscribeToSearchOptions is called', () => { beforeEach(() => { - service.subscribeToSearchOptions(defaults) + (service as any).subscribeToSearchOptions(defaults) }); it('should call all getters it needs, but not call any others', () => { expect(service.getCurrentPagination).not.toHaveBeenCalled(); @@ -131,7 +131,7 @@ describe('SearchConfigurationService', () => { describe('when subscribeToPaginatedSearchOptions is called', () => { beforeEach(() => { - service.subscribeToPaginatedSearchOptions(defaults); + (service as any).subscribeToPaginatedSearchOptions(defaults); }); it('should call all getters it needs', () => { expect(service.getCurrentPagination).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 292f26724db..7ba1ebd75f9 100644 --- a/src/app/+search-page/search-service/search-configuration.service.ts +++ b/src/app/+search-page/search-service/search-configuration.service.ts @@ -186,7 +186,7 @@ export class SearchConfigurationService implements OnDestroy { * @param {SearchOptions} defaults Default values for when no parameters are available * @returns {Subscription} The subscription to unsubscribe from */ - subscribeToSearchOptions(defaults: SearchOptions): Subscription { + private subscribeToSearchOptions(defaults: SearchOptions): Subscription { return observableMerge( this.getScopePart(defaults.scope), this.getQueryPart(defaults.query), @@ -204,7 +204,7 @@ export class SearchConfigurationService implements OnDestroy { * @param {PaginatedSearchOptions} defaults Default values for when no parameters are available * @returns {Subscription} The subscription to unsubscribe from */ - subscribeToPaginatedSearchOptions(defaults: PaginatedSearchOptions): Subscription { + private subscribeToPaginatedSearchOptions(defaults: PaginatedSearchOptions): Subscription { return observableMerge( this.getPaginationPart(defaults.pagination), this.getSortPart(defaults.sort), diff --git a/src/app/core/cache/object-cache.service.ts b/src/app/core/cache/object-cache.service.ts index 9be895c41a1..ec91d0f37e4 100644 --- a/src/app/core/cache/object-cache.service.ts +++ b/src/app/core/cache/object-cache.service.ts @@ -33,7 +33,7 @@ const entryFromSelfLinkSelector = (state: ObjectCacheState) => state[selfLink], ); - /** +/** * A service to interact with the object cache */ @Injectable() diff --git a/src/app/shared/services/route.service.ts b/src/app/shared/services/route.service.ts index b6c7747cfbd..6350605bcea 100644 --- a/src/app/shared/services/route.service.ts +++ b/src/app/shared/services/route.service.ts @@ -15,7 +15,6 @@ export class RouteService { } getQueryParameterValues(paramName: string): Observable { - console.log('called'); return this.route.queryParamMap.pipe( map((params) => [...params.getAll(paramName)]), distinctUntilChanged((a, b) => JSON.stringify(a) === JSON.stringify(b)) @@ -46,7 +45,6 @@ export class RouteService { getQueryParamsWithPrefix(prefix: string): Observable { return this.route.queryParamMap.pipe( map((qparams) => { - console.log('map'); const params = {}; qparams.keys .filter((key) => key.startsWith(prefix)) @@ -55,8 +53,7 @@ export class RouteService { }); return params; }), - distinctUntilChanged((a, b) => { console.log('changed?', a, b, JSON.stringify(a) === JSON.stringify(b)); return JSON.stringify(a) === JSON.stringify(b)}), - tap((t) => console.log('changed')) + distinctUntilChanged((a, b) => JSON.stringify(a) === JSON.stringify(b)), ); } } diff --git a/src/main.browser.ts b/src/main.browser.ts index 7b0fbf9d65b..8409a964853 100644 --- a/src/main.browser.ts +++ b/src/main.browser.ts @@ -12,7 +12,6 @@ import { BrowserAppModule } from './modules/app/browser-app.module'; import { ENV_CONFIG } from './config'; - if (ENV_CONFIG.production) { enableProdMode(); } From d820991bebe041cc81516b16f16960ed207be3a5 Mon Sep 17 00:00:00 2001 From: lotte Date: Thu, 21 Mar 2019 16:16:24 +0100 Subject: [PATCH 05/10] solved conflicts with master --- src/app/core/data/request.service.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/core/data/request.service.ts b/src/app/core/data/request.service.ts index 46ef4daad3e..c85a0ff4233 100644 --- a/src/app/core/data/request.service.ts +++ b/src/app/core/data/request.service.ts @@ -7,9 +7,8 @@ import { AppState } from '../../app.reducer'; import { hasValue, isNotEmpty } from '../../shared/empty.util'; import { CacheableObject } from '../cache/object-cache.reducer'; import { ObjectCacheService } from '../cache/object-cache.service'; -import { coreSelector, CoreState } from '../core.reducers'; +import { CoreState } from '../core.reducers'; import { IndexName, IndexState, MetaIndexState } from '../index/index.reducer'; -import { pathSelector } from '../shared/selectors'; import { originalRequestUUIDFromRequestUUIDSelector, requestIndexSelector, @@ -27,6 +26,7 @@ import { RequestEntry, RequestState } from './request.reducer'; import { CommitSSBAction } from '../cache/server-sync-buffer.actions'; import { RestRequestMethod } from './rest-request-method'; import { AddToIndexAction, RemoveFromIndexBySubstringAction } from '../index/index.actions'; +import { coreSelector } from '../core.selectors'; const requestCacheSelector = createSelector( coreSelector, From b62e5786ab7f9bc7b1936e8e90ad4c6f9f5078c3 Mon Sep 17 00:00:00 2001 From: lotte Date: Thu, 21 Mar 2019 17:20:33 +0100 Subject: [PATCH 06/10] added typedoc --- .../search-facet-option.component.ts | 23 ++++++++++++++- .../search-facet-range-option.component.ts | 20 +++++++++++-- .../search-facet-selected-option.component.ts | 21 +++++++++++--- .../search-facet-filter-wrapper.component.ts | 3 ++ .../search-facet-filter.component.ts | 8 ++++++ .../search-filter/search-filter.actions.ts | 2 +- .../search-filter/search-filter.component.ts | 11 ++++++++ .../search-range-filter.component.ts | 28 ++++++++++--------- src/app/core/cache/object-cache.service.ts | 7 +++++ src/app/core/core.selectors.ts | 3 ++ src/app/core/data/request.service.ts | 10 +++++++ src/app/shared/services/route.service.ts | 24 ++++++++++++++++ 12 files changed, 139 insertions(+), 21 deletions(-) diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.ts b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.ts index 9fa59a0c1e2..7a6a51e99d9 100644 --- a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.ts +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.ts @@ -22,12 +22,30 @@ export class SearchFacetOptionComponent implements OnInit, OnDestroy { * A single value for this component */ @Input() filterValue: FacetValue; + + /** + * The filter configuration for this facet option + */ @Input() filterConfig: SearchFilterConfig; + + /** + * Emits the active values for this filter + */ @Input() selectedValues$: Observable; + /** + * Emits true when this option should be visible and false when it should be invisible + */ isVisible: Observable; + /** + * UI parameters when this filter is added + */ addQueryParams; + + /** + * Subscription to unsubscribe from on destroy + */ sub: Subscription; constructor(protected searchService: SearchService, @@ -64,7 +82,7 @@ export class SearchFacetOptionComponent implements OnInit, OnDestroy { /** * Calculates the parameters that should change if a given value for this filter would be added to the active filters - * @param {string} value The value that is added for this filter + * @param {string[]} selectedValues The values that are currently selected for this filter */ private updateAddParams(selectedValues: string[]): void { this.addQueryParams = { @@ -73,6 +91,9 @@ export class SearchFacetOptionComponent implements OnInit, OnDestroy { }; } + /** + * Make sure the subscription is unsubscribed from when this component is destroyed + */ ngOnDestroy(): void { if (hasValue(this.sub)) { this.sub.unsubscribe(); diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.ts b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.ts index 2f6a49f8b98..22e6f04d9a3 100644 --- a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.ts +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.ts @@ -21,18 +21,32 @@ const rangeDelimiter = '-'; }) /** - * Represents a single option in a filter facet + * Represents a single option in a range filter facet */ export class SearchFacetRangeOptionComponent implements OnInit, OnDestroy { /** * A single value for this component */ @Input() filterValue: FacetValue; + + /** + * The filter configuration for this facet option + */ @Input() filterConfig: SearchFilterConfig; + /** + * Emits true when this option should be visible and false when it should be invisible + */ isVisible: Observable; + /** + * UI parameters when this filter is changed + */ changeQueryParams; + + /** + * Subscription to unsubscribe from on destroy + */ sub: Subscription; constructor(protected searchService: SearchService, @@ -68,7 +82,6 @@ export class SearchFacetRangeOptionComponent implements OnInit, OnDestroy { /** * Calculates the parameters that should change if a given values for this range filter would be changed - * @param {string} value The values that are changed for this filter */ updateChangeParams(): void { const parts = this.filterValue.value.split(rangeDelimiter); @@ -81,6 +94,9 @@ export class SearchFacetRangeOptionComponent implements OnInit, OnDestroy { }; } + /** + * Make sure the subscription is unsubscribed from when this component is destroyed + */ ngOnDestroy(): void { if (hasValue(this.sub)) { this.sub.unsubscribe(); diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.ts b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.ts index 55030a4a84d..2eb17681df7 100644 --- a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.ts +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.ts @@ -21,13 +21,17 @@ import { SearchConfigurationService } from '../../../../search-service/search-co }) /** - * Represents a single option in a filter facet + * Represents a single selected option in a filter facet */ export class SearchFacetSelectedOptionComponent implements OnInit, OnDestroy { /** - * A single value for this component + * The value for this component */ @Input() selectedValue: string; + + /** + * The filter configuration for this facet option + */ @Input() filterConfig: SearchFilterConfig; /** @@ -35,7 +39,14 @@ export class SearchFacetSelectedOptionComponent implements OnInit, OnDestroy { */ @Input() selectedValues$: Observable; + /** + * UI parameters when this filter is removed + */ removeQueryParams; + + /** + * Subscription to unsubscribe from on destroy + */ sub: Subscription; constructor(protected searchService: SearchService, @@ -64,8 +75,7 @@ export class SearchFacetSelectedOptionComponent implements OnInit, OnDestroy { /** * Calculates the parameters that should change if a given value for this filter would be removed from the active filters - * @param {string} value The value that is removed for this filter - * @returns {Observable} The changed filter parameters + * @param {string[]} selectedValues The values that are currently selected for this filter */ private updateRemoveParams(selectedValues: string[]): void { this.removeQueryParams = { @@ -74,6 +84,9 @@ export class SearchFacetSelectedOptionComponent implements OnInit, OnDestroy { }; } + /** + * Make sure the subscription is unsubscribed from when this component is destroyed + */ ngOnDestroy(): void { if (hasValue(this.sub)) { this.sub.unsubscribe(); diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter-wrapper/search-facet-filter-wrapper.component.ts b/src/app/+search-page/search-filters/search-filter/search-facet-filter-wrapper/search-facet-filter-wrapper.component.ts index 5b733b52cb4..6369a7691ed 100644 --- a/src/app/+search-page/search-filters/search-filter/search-facet-filter-wrapper/search-facet-filter-wrapper.component.ts +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter-wrapper/search-facet-filter-wrapper.component.ts @@ -20,6 +20,9 @@ export class SearchFacetFilterWrapperComponent implements OnInit { */ @Input() filterConfig: SearchFilterConfig; + /** + * The constructor of the search facet filter that should be rendered, based on the filter config's type + */ searchFilter: GenericConstructor; /** * Injector to inject a child component with the @Input parameters diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter/search-facet-filter.component.ts b/src/app/+search-page/search-filters/search-filter/search-facet-filter/search-facet-filter.component.ts index 62275ee3c48..367947a3777 100644 --- a/src/app/+search-page/search-filters/search-filter/search-facet-filter/search-facet-filter.component.ts +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter/search-facet-filter.component.ts @@ -73,6 +73,10 @@ export class SearchFacetFilterComponent implements OnInit, OnDestroy { * State of the requested facets used to time the animation */ animationState = 'loading'; + + /** + * Emits all current search options available in the search URL + */ searchOptions$: Observable; constructor(protected searchService: SearchService, @@ -207,6 +211,10 @@ export class SearchFacetFilterComponent implements OnInit, OnDestroy { ) } + /** + * On click, set the input's value to the clicked data + * @param data The value of the option that was clicked + */ onClick(data: any) { this.filter = data; } diff --git a/src/app/+search-page/search-filters/search-filter/search-filter.actions.ts b/src/app/+search-page/search-filters/search-filter/search-filter.actions.ts index 838597e03dd..f7f80eefff0 100644 --- a/src/app/+search-page/search-filters/search-filter/search-filter.actions.ts +++ b/src/app/+search-page/search-filters/search-filter/search-filter.actions.ts @@ -64,7 +64,7 @@ export class SearchFilterToggleAction extends SearchFilterAction { } /** - * Used to set the initial state of a filter to collapsed + * Used to set the initial state of a filter */ export class SearchFilterInitializeAction extends SearchFilterAction { type = SearchFilterActionTypes.INITIALIZE; diff --git a/src/app/+search-page/search-filters/search-filter/search-filter.component.ts b/src/app/+search-page/search-filters/search-filter/search-filter.component.ts index eaf318655f5..3a5e4bfc552 100644 --- a/src/app/+search-page/search-filters/search-filter/search-filter.component.ts +++ b/src/app/+search-page/search-filters/search-filter/search-filter.component.ts @@ -28,9 +28,20 @@ export class SearchFilterComponent implements OnInit { * True when the filter is 100% collapsed in the UI */ closed = true; + + /** + * Emits true when the filter is currently collapsed in the store + */ collapsed$: Observable; + /** + * Emits all currently selected values for this filter + */ selectedValues$: Observable; + + /** + * Emits true when the current filter is supposed to be shown + */ active$: Observable; constructor(private filterService: SearchFilterService, private searchService: SearchService, private searchConfigService: SearchConfigurationService) { diff --git a/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.ts b/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.ts index 0bc30676c16..ebdb7975003 100644 --- a/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.ts +++ b/src/app/+search-page/search-filters/search-filter/search-range-filter/search-range-filter.component.ts @@ -1,9 +1,4 @@ -import { - of as observableOf, - combineLatest as observableCombineLatest, - Observable, - Subscription -} from 'rxjs'; +import { combineLatest as observableCombineLatest, Subscription } from 'rxjs'; import { map, startWith } from 'rxjs/operators'; import { isPlatformBrowser } from '@angular/common'; import { Component, Inject, OnDestroy, OnInit, PLATFORM_ID } from '@angular/core'; @@ -24,14 +19,25 @@ import { hasValue } from '../../../../shared/empty.util'; import { SearchConfigurationService } from '../../../search-service/search-configuration.service'; /** - * This component renders a simple item page. - * The route parameter 'id' is used to request the item it represents. - * All fields of the item that should be displayed, are defined in its template. + * The suffix for a range filters' minimum in the frontend URL */ export const RANGE_FILTER_MIN_SUFFIX = '.min'; + +/** + * The suffix for a range filters' maximum in the frontend URL + */ export const RANGE_FILTER_MAX_SUFFIX = '.max'; + +/** + * The date formats that are possible to appear in a date filter + */ const dateFormats = ['YYYY', 'YYYY-MM', 'YYYY-MM-DD']; +/** + * This component renders a simple item page. + * The route parameter 'id' is used to request the item it represents. + * All fields of the item that should be displayed, are defined in its template. + */ @Component({ selector: 'ds-search-range-filter', styleUrls: ['./search-range-filter.component.scss'], @@ -130,8 +136,4 @@ export class SearchRangeFilterComponent extends SearchFacetFilterComponent imple this.sub.unsubscribe(); } } - - out(call) { - console.log(call); - } } diff --git a/src/app/core/cache/object-cache.service.ts b/src/app/core/cache/object-cache.service.ts index 1fdc2eaaf8b..483de65b980 100644 --- a/src/app/core/cache/object-cache.service.ts +++ b/src/app/core/cache/object-cache.service.ts @@ -22,11 +22,18 @@ import { import { CacheableObject, ObjectCacheEntry, ObjectCacheState } from './object-cache.reducer'; import { AddToSSBAction } from './server-sync-buffer.actions'; +/** + * The base selector function to select the object cache in the store + */ const objectCacheSelector = createSelector( coreSelector, (state: CoreState) => state['cache/object'] ); +/** + * Selector function to select an object entry by self link from the cache + * @param selfLink The self link of the object + */ const entryFromSelfLinkSelector = (selfLink: string): MemoizedSelector => createSelector( objectCacheSelector, diff --git a/src/app/core/core.selectors.ts b/src/app/core/core.selectors.ts index 2ad701eba12..60365be7c26 100644 --- a/src/app/core/core.selectors.ts +++ b/src/app/core/core.selectors.ts @@ -1,4 +1,7 @@ import { createFeatureSelector } from '@ngrx/store'; import { CoreState } from './core.reducers'; +/** + * Base selector to select the core state from the store + */ export const coreSelector = createFeatureSelector('core'); diff --git a/src/app/core/data/request.service.ts b/src/app/core/data/request.service.ts index c85a0ff4233..97bc4ea1a2d 100644 --- a/src/app/core/data/request.service.ts +++ b/src/app/core/data/request.service.ts @@ -28,11 +28,18 @@ import { RestRequestMethod } from './rest-request-method'; import { AddToIndexAction, RemoveFromIndexBySubstringAction } from '../index/index.actions'; import { coreSelector } from '../core.selectors'; +/** + * The base selector function to select the request state in the store + */ const requestCacheSelector = createSelector( coreSelector, (state: CoreState) => state['data/request'] ); +/** + * Selector function to select a request entry by uuid from the cache + * @param uuid The uuid of the request + */ const entryFromUUIDSelector = (uuid: string): MemoizedSelector => createSelector( requestCacheSelector, (state: RequestState) => { @@ -67,6 +74,9 @@ const getUuidsFromHrefSubstring = (state: IndexState, href: string): string[] => return result; }; +/** + * A service to interact with the request state in the store + */ @Injectable() export class RequestService { private requestsOnTheirWayToTheStore: string[] = []; diff --git a/src/app/shared/services/route.service.ts b/src/app/shared/services/route.service.ts index 6350605bcea..3e62064d916 100644 --- a/src/app/shared/services/route.service.ts +++ b/src/app/shared/services/route.service.ts @@ -8,12 +8,19 @@ import { import { isNotEmpty } from '../empty.util'; import { detect } from 'rxjs-spy'; +/** + * Service to keep track of the current query parameters + */ @Injectable() export class RouteService { constructor(private route: ActivatedRoute) { } + /** + * Retrieves all query parameter values based on a parameter name + * @param paramName The name of the parameter to look for + */ getQueryParameterValues(paramName: string): Observable { return this.route.queryParamMap.pipe( map((params) => [...params.getAll(paramName)]), @@ -21,6 +28,10 @@ export class RouteService { ); } + /** + * Retrieves a single query parameter values based on a parameter name + * @param paramName The name of the parameter to look for + */ getQueryParameterValue(paramName: string): Observable { return this.route.queryParamMap.pipe( map((params) => params.get(paramName)), @@ -28,6 +39,10 @@ export class RouteService { ); } + /** + * Checks if the query parameter currently exists in the route + * @param paramName The name of the parameter to look for + */ hasQueryParam(paramName: string): Observable { return this.route.queryParamMap.pipe( map((params) => params.has(paramName)), @@ -35,6 +50,11 @@ export class RouteService { ); } + /** + * Checks if the query parameter with a specific value currently exists in the route + * @param paramName The name of the parameter to look for + * @param paramValue The value of the parameter to look for + */ hasQueryParamWithValue(paramName: string, paramValue: string): Observable { return this.route.queryParamMap.pipe( map((params) => params.getAll(paramName).indexOf(paramValue) > -1), @@ -42,6 +62,10 @@ export class RouteService { ); } + /** + * Retrieves all query parameters of which the parameter name starts with the given prefix + * @param prefix The prefix of the parameter name to look for + */ getQueryParamsWithPrefix(prefix: string): Observable { return this.route.queryParamMap.pipe( map((qparams) => { From bbf181e522466b744fd1b2369e711e13ace9bb46 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Thu, 21 Mar 2019 18:49:58 +0100 Subject: [PATCH 07/10] add TypeDocs about the IndexReducer and its selectors --- src/app/core/index/index.reducer.ts | 59 +++++++++++++++++++++++++-- src/app/core/index/index.selectors.ts | 57 ++++++++++++++++++++++++-- 2 files changed, 109 insertions(+), 7 deletions(-) diff --git a/src/app/core/index/index.reducer.ts b/src/app/core/index/index.reducer.ts index d95748ef8a5..b4cd8aa84b7 100644 --- a/src/app/core/index/index.reducer.ts +++ b/src/app/core/index/index.reducer.ts @@ -6,16 +6,34 @@ import { RemoveFromIndexByValueAction } from './index.actions'; +/** + * An enum containing all index names + */ export enum IndexName { + // Contains all objects in the object cache indexed by UUID OBJECT = 'object/uuid-to-self-link', + + // contains all requests in the request cache indexed by UUID REQUEST = 'get-request/href-to-uuid', + + /** + * Contains the UUIDs of requests that were sent to the server and + * have their responses cached, indexed by the UUIDs of requests that + * weren't sent because the response they requested was already cached + */ UUID_MAPPING = 'get-request/configured-to-cache-uuid' } +/** + * The state of a single index + */ export interface IndexState { [key: string]: string } +/** + * The state that contains all indices + */ export type MetaIndexState = { [name in IndexName]: IndexState } @@ -23,6 +41,16 @@ export type MetaIndexState = { // Object.create(null) ensures the object has no default js properties (e.g. `__proto__`) const initialState: MetaIndexState = Object.create(null); +/** + * The Index Reducer + * + * @param state + * the current state + * @param action + * the action to perform on the state + * @return MetaIndexState + * the new state + */ export function indexReducer(state = initialState, action: IndexAction): MetaIndexState { switch (action.type) { @@ -44,6 +72,16 @@ export function indexReducer(state = initialState, action: IndexAction): MetaInd } } +/** + * Add an entry to a given index + * + * @param state + * The MetaIndexState that contains all indices + * @param action + * The AddToIndexAction containing the value to add, and the index to add it to + * @return MetaIndexState + * the new state + */ function addToIndex(state: MetaIndexState, action: AddToIndexAction): MetaIndexState { const subState = state[action.payload.name]; const newSubState = Object.assign({}, subState, { @@ -55,6 +93,16 @@ function addToIndex(state: MetaIndexState, action: AddToIndexAction): MetaIndexS return obs; } +/** + * Remove a entries that contain a given value from a given index + * + * @param state + * The MetaIndexState that contains all indices + * @param action + * The RemoveFromIndexByValueAction containing the value to remove, and the index to remove it from + * @return MetaIndexState + * the new state + */ function removeFromIndexByValue(state: MetaIndexState, action: RemoveFromIndexByValueAction): MetaIndexState { const subState = state[action.payload.name]; const newSubState = Object.create(null); @@ -69,9 +117,14 @@ function removeFromIndexByValue(state: MetaIndexState, action: RemoveFromIndexBy } /** - * Remove values from the IndexState's substate that contain a given substring - * @param state The IndexState to remove values from - * @param action The RemoveFromIndexByValueAction containing the necessary information to remove the values + * Remove entries that contain a given substring from a given index + * + * @param state + * The MetaIndexState that contains all indices + * @param action + * The RemoveFromIndexByValueAction the substring to remove, and the index to remove it from + * @return MetaIndexState + * the new state */ function removeFromIndexBySubstring(state: MetaIndexState, action: RemoveFromIndexByValueAction): MetaIndexState { const subState = state[action.payload.name]; diff --git a/src/app/core/index/index.selectors.ts b/src/app/core/index/index.selectors.ts index 51821120ca3..3c7b331a926 100644 --- a/src/app/core/index/index.selectors.ts +++ b/src/app/core/index/index.selectors.ts @@ -5,38 +5,87 @@ import { CoreState } from '../core.reducers'; import { coreSelector } from '../core.selectors'; import { IndexName, IndexState, MetaIndexState } from './index.reducer'; +/** + * Return the MetaIndexState based on the CoreSate + * + * @returns + * a MemoizedSelector to select the MetaIndexState + */ export const metaIndexSelector: MemoizedSelector = createSelector( coreSelector, (state: CoreState) => state.index ); + +/** + * Return the object index based on the MetaIndexState + * It contains all objects in the object cache indexed by UUID + * + * @returns + * a MemoizedSelector to select the object index + */ export const objectIndexSelector: MemoizedSelector = createSelector( metaIndexSelector, (state: MetaIndexState) => state[IndexName.OBJECT] ); + +/** + * Return the request index based on the MetaIndexState + * + * @returns + * a MemoizedSelector to select the request index + */ export const requestIndexSelector: MemoizedSelector = createSelector( metaIndexSelector, (state: MetaIndexState) => state[IndexName.REQUEST] ); + +/** + * Return the request UUID mapping index based on the MetaIndexState + * + * @returns + * a MemoizedSelector to select the request UUID mapping + */ export const requestUUIDIndexSelector: MemoizedSelector = createSelector( metaIndexSelector, (state: MetaIndexState) => state[IndexName.UUID_MAPPING] ); + +/** + * Return the self link of an object in the object-cache based on its UUID + * + * @param uuid + * the UUID for which you want to find the matching self link + * @returns + * a MemoizedSelector to select the self link + */ export const selfLinkFromUuidSelector = (uuid: string): MemoizedSelector => createSelector( objectIndexSelector, (state: IndexState) => hasValue(state) ? state[uuid] : undefined ); + +/** + * Return the UUID of a GET request based on its href + * + * @param href + * the href of the GET request + * @returns + * a MemoizedSelector to select the UUID + */ export const uuidFromHrefSelector = (href: string): MemoizedSelector => createSelector( requestIndexSelector, (state: IndexState) => hasValue(state) ? state[href] : undefined ); + /** - * If a request wasn't sent to the server because the result was already cached, - * this selector allows you to find the UUID of the cached request based on the - * UUID of the new request + * Return the UUID of a cached request based on the UUID of a request + * that wasn't sent because the response was already cached * - * @param uuid The uuid of the new request + * @param uuid + * The UUID of the new request + * @returns + * a MemoizedSelector to select the UUID of the cached request */ export const originalRequestUUIDFromRequestUUIDSelector = (uuid: string): MemoizedSelector => createSelector( From a788999c75981e9cf974d965e8adf422ccf37e5f Mon Sep 17 00:00:00 2001 From: lotte Date: Fri, 22 Mar 2019 11:40:07 +0100 Subject: [PATCH 08/10] Added tests for new components --- .../search-facet-option.component.html | 2 +- .../search-facet-range-option.component.ts | 2 +- .../search-facet-selected-option.component.ts | 12 ++---------- 3 files changed, 4 insertions(+), 12 deletions(-) diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.html b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.html index 9f5a88b71b8..7ab7ffd0cad 100644 --- a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.html +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.html @@ -1,4 +1,4 @@ - diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.ts b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.ts index 22e6f04d9a3..b7f02ad18b3 100644 --- a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.ts +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.ts @@ -83,7 +83,7 @@ export class SearchFacetRangeOptionComponent implements OnInit, OnDestroy { /** * Calculates the parameters that should change if a given values for this range filter would be changed */ - updateChangeParams(): void { + private updateChangeParams(): void { const parts = this.filterValue.value.split(rangeDelimiter); const min = parts.length > 1 ? parts[0].trim() : this.filterValue.value; const max = parts.length > 1 ? parts[1].trim() : this.filterValue.value; diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.ts b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.ts index 2eb17681df7..5137bf8ffc5 100644 --- a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.ts +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.ts @@ -1,18 +1,10 @@ -import { - combineLatest as observableCombineLatest, - Observable, - of as observableOf, - Subscription -} from 'rxjs'; -import { delay, map } from 'rxjs/operators'; -import { Component, Input, OnChanges, OnDestroy, OnInit, SimpleChanges } from '@angular/core'; +import { combineLatest as observableCombineLatest, Observable, Subscription } from 'rxjs'; +import { Component, Input, OnDestroy, OnInit } from '@angular/core'; import { Router } from '@angular/router'; -import { FacetValue } from '../../../../search-service/facet-value.model'; import { SearchFilterConfig } from '../../../../search-service/search-filter-config.model'; import { SearchService } from '../../../../search-service/search.service'; import { SearchFilterService } from '../../search-filter.service'; import { hasValue } from '../../../../../shared/empty.util'; -import { SearchOptions } from '../../../../search-options.model'; import { SearchConfigurationService } from '../../../../search-service/search-configuration.service'; @Component({ From c07766f5be75df01502aabf5ff4b7253ffa48933 Mon Sep 17 00:00:00 2001 From: lotte Date: Fri, 22 Mar 2019 11:41:42 +0100 Subject: [PATCH 09/10] Added missing test files --- .../search-facet-option.component.spec.ts | 121 +++++++++++++++++ ...earch-facet-range-option.component.spec.ts | 125 ++++++++++++++++++ ...ch-facet-selected-option.component.spec.ts | 95 +++++++++++++ 3 files changed, 341 insertions(+) create mode 100644 src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.spec.ts create mode 100644 src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.spec.ts create mode 100644 src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.spec.ts diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.spec.ts b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.spec.ts new file mode 100644 index 00000000000..f1dbedfb40f --- /dev/null +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-option/search-facet-option.component.spec.ts @@ -0,0 +1,121 @@ +import { ChangeDetectionStrategy, NO_ERRORS_SCHEMA } from '@angular/core'; +import { async, ComponentFixture, TestBed } from '@angular/core/testing'; +import { TranslateModule } from '@ngx-translate/core'; +import { NoopAnimationsModule } from '@angular/platform-browser/animations'; +import { SearchFacetOptionComponent } from './search-facet-option.component'; +import { SearchFilterConfig } from '../../../../search-service/search-filter-config.model'; +import { FilterType } from '../../../../search-service/filter-type.model'; +import { FacetValue } from '../../../../search-service/facet-value.model'; +import { FormsModule } from '@angular/forms'; +import { of as observableOf } from 'rxjs'; +import { SearchService } from '../../../../search-service/search.service'; +import { SearchServiceStub } from '../../../../../shared/testing/search-service-stub'; +import { Router } from '@angular/router'; +import { RouterStub } from '../../../../../shared/testing/router-stub'; +import { SearchConfigurationService } from '../../../../search-service/search-configuration.service'; +import { SearchFilterService } from '../../search-filter.service'; +import { By } from '@angular/platform-browser'; + +describe('SearchFacetOptionComponent', () => { + let comp: SearchFacetOptionComponent; + let fixture: ComponentFixture; + const filterName1 = 'test name'; + const value1 = 'testvalue1'; + const value2 = 'test2'; + const value3 = 'another value3'; + const mockFilterConfig = Object.assign(new SearchFilterConfig(), { + name: filterName1, + type: FilterType.range, + hasFacets: false, + isOpenByDefault: false, + pageSize: 2, + minValue: 200, + maxValue: 3000, + }); + const value: FacetValue = { + value: value2, + count: 20, + search: '' + }; + + const searchLink = '/search'; + const selectedValues = [value1]; + const selectedValues$ = observableOf(selectedValues); + let filterService; + let searchService; + let router; + const page = observableOf(0); + + beforeEach(async(() => { + TestBed.configureTestingModule({ + imports: [TranslateModule.forRoot(), NoopAnimationsModule, FormsModule], + declarations: [SearchFacetOptionComponent], + providers: [ + { provide: SearchService, useValue: new SearchServiceStub(searchLink) }, + { provide: Router, useValue: new RouterStub() }, + { + provide: SearchConfigurationService, useValue: { + searchOptions: observableOf({}) + } + }, + { + provide: SearchFilterService, useValue: { + getSelectedValuesForFilter: () => selectedValues, + isFilterActiveWithValue: (paramName: string, filterValue: string) => observableOf(true), + getPage: (paramName: string) => page, + /* tslint:disable:no-empty */ + incrementPage: (filterName: string) => { + }, + resetPage: (filterName: string) => { + } + /* tslint:enable:no-empty */ + } + } + ], + schemas: [NO_ERRORS_SCHEMA] + }).overrideComponent(SearchFacetOptionComponent, { + set: { changeDetection: ChangeDetectionStrategy.Default } + }).compileComponents(); + })); + + beforeEach(() => { + fixture = TestBed.createComponent(SearchFacetOptionComponent); + comp = fixture.componentInstance; // SearchPageComponent test instance + filterService = (comp as any).filterService; + searchService = (comp as any).searchService; + router = (comp as any).router; + comp.filterValue = value; + comp.selectedValues$ = selectedValues$; + comp.filterConfig = mockFilterConfig; + fixture.detectChanges(); + }); + + describe('when the updateAddParams method is called wih a value', () => { + it('should update the addQueryParams with the new parameter values', () => { + comp.addQueryParams = {}; + (comp as any).updateAddParams(selectedValues); + expect(comp.addQueryParams).toEqual({ + [mockFilterConfig.paramName]: [value1, value.value], + page: 1 + }); + }); + }); + + describe('when isVisible emits true', () => { + it('the facet option should be visible', () => { + comp.isVisible = observableOf(true); + fixture.detectChanges(); + const linkEl = fixture.debugElement.query(By.css('a')); + expect(linkEl).not.toBeNull(); + }); + }); + + describe('when isVisible emits false', () => { + it('the facet option should not be visible', () => { + comp.isVisible = observableOf(false); + fixture.detectChanges(); + const linkEl = fixture.debugElement.query(By.css('a')); + expect(linkEl).toBeNull(); + }); + }); +}); diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.spec.ts b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.spec.ts new file mode 100644 index 00000000000..218730263bb --- /dev/null +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-range-option/search-facet-range-option.component.spec.ts @@ -0,0 +1,125 @@ +import { ChangeDetectionStrategy, NO_ERRORS_SCHEMA } from '@angular/core'; +import { async, ComponentFixture, TestBed } from '@angular/core/testing'; +import { TranslateModule } from '@ngx-translate/core'; +import { NoopAnimationsModule } from '@angular/platform-browser/animations'; +import { SearchFilterConfig } from '../../../../search-service/search-filter-config.model'; +import { FilterType } from '../../../../search-service/filter-type.model'; +import { FacetValue } from '../../../../search-service/facet-value.model'; +import { FormsModule } from '@angular/forms'; +import { of as observableOf } from 'rxjs'; +import { SearchService } from '../../../../search-service/search.service'; +import { SearchServiceStub } from '../../../../../shared/testing/search-service-stub'; +import { Router } from '@angular/router'; +import { RouterStub } from '../../../../../shared/testing/router-stub'; +import { SearchConfigurationService } from '../../../../search-service/search-configuration.service'; +import { SearchFilterService } from '../../search-filter.service'; +import { By } from '@angular/platform-browser'; +import { SearchFacetRangeOptionComponent } from './search-facet-range-option.component'; +import { + RANGE_FILTER_MAX_SUFFIX, + RANGE_FILTER_MIN_SUFFIX +} from '../../search-range-filter/search-range-filter.component'; + +describe('SearchFacetRangeOptionComponent', () => { + let comp: SearchFacetRangeOptionComponent; + let fixture: ComponentFixture; + const filterName1 = 'test name'; + const value2 = '20 - 30'; + const mockFilterConfig = Object.assign(new SearchFilterConfig(), { + name: filterName1, + type: FilterType.range, + hasFacets: false, + isOpenByDefault: false, + pageSize: 2, + minValue: 200, + maxValue: 3000, + }); + const value: FacetValue = { + value: value2, + count: 20, + search: '' + }; + + const searchLink = '/search'; + let filterService; + let searchService; + let router; + const page = observableOf(0); + + beforeEach(async(() => { + TestBed.configureTestingModule({ + imports: [TranslateModule.forRoot(), NoopAnimationsModule, FormsModule], + declarations: [SearchFacetRangeOptionComponent], + providers: [ + { provide: SearchService, useValue: new SearchServiceStub(searchLink) }, + { provide: Router, useValue: new RouterStub() }, + { + provide: SearchConfigurationService, useValue: { + searchOptions: observableOf({}) + } + }, + { + provide: SearchFilterService, useValue: { + isFilterActiveWithValue: (paramName: string, filterValue: string) => observableOf(true), + getPage: (paramName: string) => page, + /* tslint:disable:no-empty */ + incrementPage: (filterName: string) => { + }, + resetPage: (filterName: string) => { + } + /* tslint:enable:no-empty */ + } + } + ], + schemas: [NO_ERRORS_SCHEMA] + }).overrideComponent(SearchFacetRangeOptionComponent, { + set: { changeDetection: ChangeDetectionStrategy.Default } + }).compileComponents(); + })); + + beforeEach(() => { + fixture = TestBed.createComponent(SearchFacetRangeOptionComponent); + comp = fixture.componentInstance; // SearchFacetRangeOptionComponent test instance + filterService = (comp as any).filterService; + searchService = (comp as any).searchService; + router = (comp as any).router; + comp.filterValue = value; + comp.filterConfig = mockFilterConfig; + fixture.detectChanges(); + }); + + describe('when the updateChangeParams method is called wih a value', () => { + it('should update the changeQueryParams with the new parameter values', () => { + comp.changeQueryParams = {}; + comp.filterValue = { + value: '50-60', + count: 20, + search: '' + }; + (comp as any).updateChangeParams(); + expect(comp.changeQueryParams).toEqual({ + [mockFilterConfig.paramName + RANGE_FILTER_MIN_SUFFIX]: ['50'], + [mockFilterConfig.paramName + RANGE_FILTER_MAX_SUFFIX]: ['60'], + page: 1 + }); + }); + }); + + describe('when isVisible emits true', () => { + it('the facet option should be visible', () => { + comp.isVisible = observableOf(true); + fixture.detectChanges(); + const linkEl = fixture.debugElement.query(By.css('a')); + expect(linkEl).not.toBeNull(); + }); + }); + + describe('when isVisible emits false', () => { + it('the facet option should not be visible', () => { + comp.isVisible = observableOf(false); + fixture.detectChanges(); + const linkEl = fixture.debugElement.query(By.css('a')); + expect(linkEl).toBeNull(); + }); + }); +}); diff --git a/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.spec.ts b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.spec.ts new file mode 100644 index 00000000000..545ba1d66bf --- /dev/null +++ b/src/app/+search-page/search-filters/search-filter/search-facet-filter-options/search-facet-selected-option/search-facet-selected-option.component.spec.ts @@ -0,0 +1,95 @@ +import { ChangeDetectionStrategy, NO_ERRORS_SCHEMA } from '@angular/core'; +import { async, ComponentFixture, TestBed } from '@angular/core/testing'; +import { TranslateModule } from '@ngx-translate/core'; +import { NoopAnimationsModule } from '@angular/platform-browser/animations'; +import { SearchFilterConfig } from '../../../../search-service/search-filter-config.model'; +import { FilterType } from '../../../../search-service/filter-type.model'; +import { FormsModule } from '@angular/forms'; +import { of as observableOf } from 'rxjs'; +import { SearchService } from '../../../../search-service/search.service'; +import { SearchServiceStub } from '../../../../../shared/testing/search-service-stub'; +import { Router } from '@angular/router'; +import { RouterStub } from '../../../../../shared/testing/router-stub'; +import { SearchConfigurationService } from '../../../../search-service/search-configuration.service'; +import { SearchFilterService } from '../../search-filter.service'; +import { SearchFacetSelectedOptionComponent } from './search-facet-selected-option.component'; + +describe('SearchFacetSelectedOptionComponent', () => { + let comp: SearchFacetSelectedOptionComponent; + let fixture: ComponentFixture; + const filterName1 = 'test name'; + const value1 = 'testvalue1'; + const value2 = 'test2'; + const mockFilterConfig = Object.assign(new SearchFilterConfig(), { + name: filterName1, + type: FilterType.range, + hasFacets: false, + isOpenByDefault: false, + pageSize: 2, + minValue: 200, + maxValue: 3000, + }); + + const searchLink = '/search'; + const selectedValues = [value1, value2]; + const selectedValues$ = observableOf(selectedValues); + let filterService; + let searchService; + let router; + const page = observableOf(0); + + beforeEach(async(() => { + TestBed.configureTestingModule({ + imports: [TranslateModule.forRoot(), NoopAnimationsModule, FormsModule], + declarations: [SearchFacetSelectedOptionComponent], + providers: [ + { provide: SearchService, useValue: new SearchServiceStub(searchLink) }, + { provide: Router, useValue: new RouterStub() }, + { + provide: SearchConfigurationService, useValue: { + searchOptions: observableOf({}) + } + }, + { + provide: SearchFilterService, useValue: { + getSelectedValuesForFilter: () => selectedValues, + isFilterActiveWithValue: (paramName: string, filterValue: string) => observableOf(true), + getPage: (paramName: string) => page, + /* tslint:disable:no-empty */ + incrementPage: (filterName: string) => { + }, + resetPage: (filterName: string) => { + } + /* tslint:enable:no-empty */ + } + } + ], + schemas: [NO_ERRORS_SCHEMA] + }).overrideComponent(SearchFacetSelectedOptionComponent, { + set: { changeDetection: ChangeDetectionStrategy.Default } + }).compileComponents(); + })); + + beforeEach(() => { + fixture = TestBed.createComponent(SearchFacetSelectedOptionComponent); + comp = fixture.componentInstance; // SearchFacetSelectedOptionComponent test instance + filterService = (comp as any).filterService; + searchService = (comp as any).searchService; + router = (comp as any).router; + comp.selectedValue = value2; + comp.selectedValues$ = selectedValues$; + comp.filterConfig = mockFilterConfig; + fixture.detectChanges(); + }); + + describe('when the updateRemoveParams method is called wih a value', () => { + it('should update the removeQueryParams with the new parameter values', () => { + comp.removeQueryParams = {}; + (comp as any).updateRemoveParams(selectedValues); + expect(comp.removeQueryParams).toEqual({ + [mockFilterConfig.paramName]: [value1], + page: 1 + }); + }); + }); +}); From 281a4cd1d7b77ff8c9f51f5cb3e254f497821521 Mon Sep 17 00:00:00 2001 From: lotte Date: Tue, 26 Mar 2019 09:36:07 +0100 Subject: [PATCH 10/10] Fixed issues when there are no values for a specific facet --- .../search-filters/search-filter/search-filter.component.html | 2 +- .../search-filters/search-filter/search-filter.component.scss | 2 +- .../search-filters/search-filter/search-filter.component.ts | 1 - .../+search-page/search-filters/search-filters.component.html | 2 +- src/app/shared/error/error.component.ts | 1 - 5 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/app/+search-page/search-filters/search-filter/search-filter.component.html b/src/app/+search-page/search-filters/search-filter/search-filter.component.html index e2c128593fe..5c4db44d245 100644 --- a/src/app/+search-page/search-filters/search-filter/search-filter.component.html +++ b/src/app/+search-page/search-filters/search-filter/search-filter.component.html @@ -1,4 +1,4 @@ -
+
{{'search.filters.filter.' + filter.name + '.head'| translate}}
diff --git a/src/app/+search-page/search-filters/search-filter/search-filter.component.scss b/src/app/+search-page/search-filters/search-filter/search-filter.component.scss index 4e45f49468e..1db5e9a1b23 100644 --- a/src/app/+search-page/search-filters/search-filter/search-filter.component.scss +++ b/src/app/+search-page/search-filters/search-filter/search-filter.component.scss @@ -1,7 +1,7 @@ @import '../../../../styles/variables.scss'; @import '../../../../styles/mixins.scss'; -:host { +:host .facet-filter { border: 1px solid map-get($theme-colors, light); cursor: pointer; .search-filter-wrapper.closed { diff --git a/src/app/+search-page/search-filters/search-filter/search-filter.component.ts b/src/app/+search-page/search-filters/search-filter/search-filter.component.ts index 3a5e4bfc552..14ba8f0b76c 100644 --- a/src/app/+search-page/search-filters/search-filter/search-filter.component.ts +++ b/src/app/+search-page/search-filters/search-filter/search-filter.component.ts @@ -124,7 +124,6 @@ export class SearchFilterComponent implements OnInit { return observableOf(true); } else { return this.searchConfigService.searchOptions.pipe( - first(), switchMap((options) => { return this.searchService.getFacetValuesFor(this.filter, 1, options).pipe( filter((RD) => !RD.isLoading), diff --git a/src/app/+search-page/search-filters/search-filters.component.html b/src/app/+search-page/search-filters/search-filters.component.html index ab2a96d752b..895765f6ac5 100644 --- a/src/app/+search-page/search-filters/search-filters.component.html +++ b/src/app/+search-page/search-filters/search-filters.component.html @@ -1,7 +1,7 @@

{{"search.filters.head" | translate}}

- +
{{"search.filters.reset" | translate}} \ No newline at end of file diff --git a/src/app/shared/error/error.component.ts b/src/app/shared/error/error.component.ts index 6900869183a..bcb30c5fdc2 100644 --- a/src/app/shared/error/error.component.ts +++ b/src/app/shared/error/error.component.ts @@ -32,5 +32,4 @@ export class ErrorComponent { this.subscription.unsubscribe(); } } - }