From 193e6925c63a17ea48b267ad1f2f04caf953e70d Mon Sep 17 00:00:00 2001 From: Nathan Jones Date: Tue, 17 Jul 2018 13:30:30 +0100 Subject: [PATCH 1/2] Ensure we don't worry about empty page request infos if we already have is list --- .../app/shared/monitors/pagination-monitor.ts | 97 ++++++++++--------- 1 file changed, 53 insertions(+), 44 deletions(-) diff --git a/src/frontend/app/shared/monitors/pagination-monitor.ts b/src/frontend/app/shared/monitors/pagination-monitor.ts index a922aaa082..475e08226f 100644 --- a/src/frontend/app/shared/monitors/pagination-monitor.ts +++ b/src/frontend/app/shared/monitors/pagination-monitor.ts @@ -14,7 +14,10 @@ import { tap, } from 'rxjs/operators'; -import { getAPIRequestDataState, selectEntities } from '../../store/selectors/api.selectors'; +import { + getAPIRequestDataState, + selectEntities, +} from '../../store/selectors/api.selectors'; import { selectPaginationState } from '../../store/selectors/pagination.selectors'; import { AppState } from './../../store/app-state'; import { ActionState } from './../../store/reducers/api-request-reducer/types'; @@ -22,16 +25,16 @@ import { PaginationEntityState } from './../../store/types/pagination.types'; export class PaginationMonitor { /** - * Emits the current page of entities. - */ + * Emits the current page of entities. + */ public currentPage$: Observable; /** - * Emits a boolean stating if the current page is fetching or not. - */ + * Emits a boolean stating if the current page is fetching or not. + */ public fetchingCurrentPage$: Observable; /** - * Emits a boolean stating if the current page has errored or not. - */ + * Emits a boolean stating if the current page has errored or not. + */ public currentPageError$: Observable; /** * All the information about the current pagination selection. @@ -41,13 +44,9 @@ export class PaginationMonitor { constructor( private store: Store, public paginationKey: string, - public schema: schema.Entity + public schema: schema.Entity, ) { - this.init( - store, - paginationKey, - schema - ); + this.init(store, paginationKey, schema); } /** @@ -60,9 +59,10 @@ export class PaginationMonitor { } const currentPage = pagination.ids[pagination.currentPage]; const hasPageIds = !!currentPage; - const requestInfo = pagination.pageRequests[pagination.clientPagination.currentPage]; + const requestInfo = + pagination.pageRequests[pagination.clientPagination.currentPage]; const hasPageRequestInfo = !!requestInfo; - const hasPage = hasPageIds && hasPageRequestInfo && !requestInfo.busy; + const hasPage = hasPageIds && (!hasPageRequestInfo || !requestInfo.busy); return hasPage; } @@ -78,12 +78,18 @@ export class PaginationMonitor { * Gets the request info for the current page. * @param pagination */ - private getCurrentPageRequestInfo(pagination: PaginationEntityState): ActionState { - if (!pagination || !pagination.pageRequests || !pagination.pageRequests[pagination.currentPage]) { + private getCurrentPageRequestInfo( + pagination: PaginationEntityState, + ): ActionState { + if ( + !pagination || + !pagination.pageRequests || + !pagination.pageRequests[pagination.currentPage] + ) { return { busy: false, error: false, - message: '' + message: '', }; } else { return pagination.pageRequests[pagination.currentPage]; @@ -94,17 +100,14 @@ export class PaginationMonitor { private init( store: Store, paginationKey: string, - schema: schema.Entity + schema: schema.Entity, ) { this.pagination$ = this.createPaginationObservable( store, schema.key, - paginationKey - ); - this.currentPage$ = this.createPageObservable( - this.pagination$, - schema + paginationKey, ); + this.currentPage$ = this.createPageObservable(this.pagination$, schema); this.currentPageError$ = this.createErrorObservable(this.pagination$); this.fetchingCurrentPage$ = this.createFetchingObservable(this.pagination$); } @@ -112,33 +115,34 @@ export class PaginationMonitor { private createPaginationObservable( store: Store, entityKey: string, - paginationKey: string + paginationKey: string, ) { - return store.select(selectPaginationState(entityKey, paginationKey)) - .pipe( - distinctUntilChanged(), - filter(pag => !!pag) - ); + return store.select(selectPaginationState(entityKey, paginationKey)).pipe( + distinctUntilChanged(), + filter(pag => !!pag), + ); } - - private createPageObservable( pagination$: Observable, - schema: schema.Entity + schema: schema.Entity, ) { - const entityObservable$ = this.store.select(selectEntities(this.schema.key)).pipe(distinctUntilChanged()); + const entityObservable$ = this.store + .select(selectEntities(this.schema.key)) + .pipe(distinctUntilChanged()); const allEntitiesObservable$ = this.store.select(getAPIRequestDataState); return pagination$.pipe( // Improve efficiency observeOn(asapScheduler), - filter((pagination) => this.hasPage(pagination)), + filter(pagination => this.hasPage(pagination)), distinctUntilChanged(this.isPageSameIsh), combineLatestOperator(entityObservable$), withLatestFrom(allEntitiesObservable$), map(([[pagination], allEntities]) => { const page = pagination.ids[pagination.currentPage] || []; - return page.length ? denormalize(page, [schema], allEntities).filter(ent => !!ent) : []; + return page.length + ? denormalize(page, [schema], allEntities).filter(ent => !!ent) + : []; }), tag('de-norming ' + schema.key), publishReplay(1), @@ -149,30 +153,35 @@ export class PaginationMonitor { private isPageSameIsh(x: PaginationEntityState, y: PaginationEntityState) { const samePage = x.currentPage === y.currentPage; // It's possible that we need to compare the whole page request object but busy will do for now. - const samePageBusyState = samePage && x.pageRequests[x.currentPage].busy === y.pageRequests[y.currentPage].busy; - const samePageIdList = samePage && x.ids[x.currentPage] === y.ids[y.currentPage]; + const samePageBusyState = + samePage && + x.pageRequests[x.currentPage].busy === y.pageRequests[y.currentPage].busy; + const samePageIdList = + samePage && x.ids[x.currentPage] === y.ids[y.currentPage]; return samePageIdList && samePageBusyState; } - private createErrorObservable(pagination$: Observable) { + private createErrorObservable( + pagination$: Observable, + ) { return pagination$.pipe( map(pagination => { const currentPageRequest = this.getCurrentPageRequestInfo(pagination); return !currentPageRequest.busy && currentPageRequest.error; - }) + }), ); } - private createFetchingObservable(pagination$: Observable) { + private createFetchingObservable( + pagination$: Observable, + ) { return pagination$.pipe( map(pagination => { const currentPageRequest = this.getCurrentPageRequestInfo(pagination); return currentPageRequest.busy; }), - distinctUntilChanged() + distinctUntilChanged(), ); } // ### Initialization methods end. - } - From e1a5d12ce567b4db06eb645e8f15b6beee5de252 Mon Sep 17 00:00:00 2001 From: Nathan Jones Date: Wed, 18 Jul 2018 15:56:57 +0100 Subject: [PATCH 2/2] Fix tests --- src/frontend/app/shared/monitors/pagination-monitor.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/frontend/app/shared/monitors/pagination-monitor.ts b/src/frontend/app/shared/monitors/pagination-monitor.ts index 475e08226f..77bf974194 100644 --- a/src/frontend/app/shared/monitors/pagination-monitor.ts +++ b/src/frontend/app/shared/monitors/pagination-monitor.ts @@ -155,7 +155,15 @@ export class PaginationMonitor { // It's possible that we need to compare the whole page request object but busy will do for now. const samePageBusyState = samePage && - x.pageRequests[x.currentPage].busy === y.pageRequests[y.currentPage].busy; + ( + x.pageRequests[x.currentPage] + && + x.pageRequests[x.currentPage].busy + ) === ( + y.pageRequests[y.currentPage] + && + y.pageRequests[y.currentPage].busy + ); const samePageIdList = samePage && x.ids[x.currentPage] === y.ids[y.currentPage]; return samePageIdList && samePageBusyState;