From e73f4ca57a69c1b5e356b7cacba09790dee4b446 Mon Sep 17 00:00:00 2001 From: Yana De Pauw Date: Mon, 8 Mar 2021 16:47:16 +0100 Subject: [PATCH] 76654: Remove console.logs, fix some issues --- ...-drag-and-drop-bitstream-list.component.ts | 5 +- src/app/core/pagination/pagination.service.ts | 186 ++++++++++-------- ...nated-drag-and-drop-list.component.spec.ts | 2 +- ...-paginated-drag-and-drop-list.component.ts | 10 +- .../pagination/pagination.component.spec.ts | 2 - .../shared/pagination/pagination.component.ts | 5 +- .../submission-import-external.component.ts | 3 - 7 files changed, 117 insertions(+), 96 deletions(-) diff --git a/src/app/+item-page/edit-item-page/item-bitstreams/item-edit-bitstream-bundle/paginated-drag-and-drop-bitstream-list/paginated-drag-and-drop-bitstream-list.component.ts b/src/app/+item-page/edit-item-page/item-bitstreams/item-edit-bitstream-bundle/paginated-drag-and-drop-bitstream-list/paginated-drag-and-drop-bitstream-list.component.ts index 0941a4dfd6..f3f00abf92 100644 --- a/src/app/+item-page/edit-item-page/item-bitstreams/item-edit-bitstream-bundle/paginated-drag-and-drop-bitstream-list/paginated-drag-and-drop-bitstream-list.component.ts +++ b/src/app/+item-page/edit-item-page/item-bitstreams/item-edit-bitstream-bundle/paginated-drag-and-drop-bitstream-list/paginated-drag-and-drop-bitstream-list.component.ts @@ -11,6 +11,7 @@ import { followLink } from '../../../../../shared/utils/follow-link-config.model import { ObjectValuesPipe } from '../../../../../shared/utils/object-values-pipe'; import { RequestService } from '../../../../../core/data/request.service'; import { PaginationService } from '../../../../../core/pagination/pagination.service'; +import { PaginationComponentOptions } from '../../../../../shared/pagination/pagination-component-options.model'; @Component({ selector: 'ds-paginated-drag-and-drop-bitstream-list', @@ -52,8 +53,8 @@ export class PaginatedDragAndDropBitstreamListComponent extends AbstractPaginate */ initializeObjectsRD(): void { this.objectsRD$ = this.currentPage$.pipe( - switchMap((page: number) => { - const paginatedOptions = new PaginatedSearchOptions({pagination: Object.assign({}, this.options, { currentPage: page })}); + switchMap((page: PaginationComponentOptions) => { + const paginatedOptions = new PaginatedSearchOptions({pagination: Object.assign({}, page)}); return this.bundleService.getBitstreamsEndpoint(this.bundle.id, paginatedOptions).pipe( switchMap((href) => this.requestService.hasByHref$(href)), switchMap(() => this.bundleService.getBitstreams( diff --git a/src/app/core/pagination/pagination.service.ts b/src/app/core/pagination/pagination.service.ts index 29e57214c1..45a1ffee96 100644 --- a/src/app/core/pagination/pagination.service.ts +++ b/src/app/core/pagination/pagination.service.ts @@ -14,23 +14,31 @@ import { isNumeric } from 'rxjs/internal-compatibility'; @Injectable({ providedIn: 'root', }) +/** + * Service to manage the pagination of different components + */ export class PaginationService { private defaultSortOptions = new SortOptions('id', SortDirection.ASC); + private clearParams = {}; + constructor(protected routeService: RouteService, protected router: Router ) { } /** - * @returns {Observable} Emits the current pagination settings + * Method to retrieve the current pagination settings for an ID based on the router params and default options + * @param paginationId - The id to check the pagination for + * @param defaultPagination - The default pagination values to be used when no route info is present + * @returns {Observable} Retrieves the current pagination settings based on the router params */ getCurrentPagination(paginationId: string, defaultPagination: PaginationComponentOptions): Observable { const page$ = this.routeService.getQueryParameterValue(`p.${paginationId}`); const size$ = this.routeService.getQueryParameterValue(`rpp.${paginationId}`); - return observableCombineLatest([page$, size$]).pipe(map(([page, size]) => { - console.log(page, size); + return observableCombineLatest([page$, size$]).pipe( + map(([page, size]) => { return Object.assign(new PaginationComponentOptions(), defaultPagination, { currentPage: this.convertToNumeric(page, defaultPagination.currentPage), pageSize: this.getBestMatchPageSize(size, defaultPagination) @@ -40,7 +48,11 @@ export class PaginationService { } /** - * @returns {Observable} Emits the current sorting settings + * Method to retrieve the current sort options for an ID based on the router params and default options + * @param paginationId - The id to check the sort options for + * @param defaultSort - The default sort options to be used when no route info is present + * @param ignoreDefault - Indicate whether the default should be ignored + * @returns {Observable} Retrieves the current sort options based on the router params */ getCurrentSort(paginationId: string, defaultSort: SortOptions, ignoreDefault?: boolean): Observable { if (!ignoreDefault && (isEmpty(defaultSort) || !hasValue(defaultSort))) { @@ -56,6 +68,13 @@ export class PaginationService { ); } + /** + * Method to retrieve the current find list options for an ID based on the router params and default options + * @param paginationId - The id to check the find list options for + * @param defaultFindList - The default find list options to be used when no route info is present + * @param ignoreDefault - Indicate whether the default should be ignored + * @returns {Observable} Retrieves the current find list options based on the router params + */ getFindListOptions(paginationId: string, defaultFindList: FindListOptions, ignoreDefault?: boolean): Observable { const paginationComponentOptions = new PaginationComponentOptions(); paginationComponentOptions.currentPage = defaultFindList.currentPage; @@ -74,10 +93,94 @@ export class PaginationService { })); } + /** + * Reset the current page for the provided pagination ID to 1. + * @param paginationId - The pagination id for which to reset the page + */ resetPage(paginationId: string) { this.updateRoute(paginationId, {page: 1}); } + + /** + * Update the route with the provided information + * @param paginationId - The pagination ID for which to update the route with info + * @param params - The page related params to update in the route + * @param extraParams - Addition params unrelated to the pagination that need to be added to the route + * @param retainScrollPosition - Scroll to the pagination component after updating the route instead of the top of the page + */ + updateRoute(paginationId: string, params: { + page?: number + pageSize?: number + sortField?: string + sortDirection?: SortDirection + }, extraParams?, retainScrollPosition?: boolean) { + + this.updateRouteWithUrl(paginationId, [], params, extraParams, retainScrollPosition); + } + + /** + * Update the route with the provided information + * @param paginationId - The pagination ID for which to update the route with info + * @param url - The url to navigate to + * @param params - The page related params to update in the route + * @param extraParams - Addition params unrelated to the pagination that need to be added to the route + * @param retainScrollPosition - Scroll to the pagination component after updating the route instead of the top of the page + */ + updateRouteWithUrl(paginationId: string, url: string[], params: { + page?: number + pageSize?: number + sortField?: string + sortDirection?: SortDirection + }, extraParams?, retainScrollPosition?: boolean) { + this.getCurrentRouting(paginationId).subscribe((currentFindListOptions) => { + const currentParametersWithIdName = this.getParametersWithIdName(paginationId, currentFindListOptions); + const parametersWithIdName = this.getParametersWithIdName(paginationId, params); + if (isNotEmpty(difference(parametersWithIdName, currentParametersWithIdName)) || isNotEmpty(extraParams) || isNotEmpty(this.clearParams)) { + const queryParams = Object.assign({}, this.clearParams, currentParametersWithIdName, + parametersWithIdName, extraParams); + console.log(queryParams, this.clearParams); + if (retainScrollPosition) { + this.router.navigate(url, { + queryParams: queryParams, + queryParamsHandling: 'merge', + fragment: `p-${paginationId}` + }); + } else { + this.router.navigate(url, { + queryParams: queryParams, + queryParamsHandling: 'merge' + }); + } + this.clearParams = {}; + console.log('postcear', this.clearParams); + } + }); + } + + /** + * Add the params to be cleared to the clearParams variable. + * When the updateRoute or updateRouteWithUrl these params will be removed from the route pagination + * @param paginationId - The ID for which to clear the params + */ + clearPagination(paginationId: string) { + const params = {}; + params[`p.${paginationId}`] = null; + params[`rpp.${paginationId}`] = null; + params[`sf.${paginationId}`] = null; + params[`sd.${paginationId}`] = null; + + Object.assign(this.clearParams, params); + } + + /** + * Retrieve the page parameter for the provided id + * @param paginationId - The ID for which to retrieve the page param + */ + getPageParam(paginationId: string) { + return `p.${paginationId}`; + } + private getCurrentRouting(paginationId: string) { return this.getFindListOptions(paginationId, {}, true).pipe( take(1), @@ -92,81 +195,6 @@ export class PaginationService { ); } - updateRoute(paginationId: string, params: { - page?: number - pageSize?: number - sortField?: string - sortDirection?: SortDirection - }, extraParams?, retainScrollPosition?: boolean) { - this.getCurrentRouting(paginationId).subscribe((currentFindListOptions) => { - console.log('currentFindListOptions',currentFindListOptions ); - const currentParametersWithIdName = this.getParametersWithIdName(paginationId, currentFindListOptions); - const parametersWithIdName = this.getParametersWithIdName(paginationId, params); - if (isNotEmpty(difference(parametersWithIdName, currentParametersWithIdName)) || isNotEmpty(extraParams)) { - const queryParams = Object.assign({}, currentParametersWithIdName, - parametersWithIdName, extraParams); - if (retainScrollPosition) { - this.router.navigate([], { - queryParams: queryParams, - queryParamsHandling: 'merge', - fragment: `p-${paginationId}` - }); - } else { - this.router.navigate([], { - queryParams: queryParams, - queryParamsHandling: 'merge' - }); - } - } - }); - } - - updateRouteWithUrl(paginationId: string, url: string[], params: { - page?: number - pageSize?: number - sortField?: string - sortDirection?: SortDirection - }, extraParams?, retainScrollPosition?: boolean) { - console.log(retainScrollPosition); - this.getCurrentRouting(paginationId).subscribe((currentFindListOptions) => { - const currentParametersWithIdName = this.getParametersWithIdName(paginationId, currentFindListOptions); - const parametersWithIdName = this.getParametersWithIdName(paginationId, params); - if (isNotEmpty(difference(parametersWithIdName, currentParametersWithIdName)) || isNotEmpty(extraParams)) { - const queryParams = Object.assign({}, currentParametersWithIdName, - parametersWithIdName, extraParams); - if (retainScrollPosition) { - this.router.navigate(url, { - queryParams: queryParams, - queryParamsHandling: 'merge', - fragment: `p-${paginationId}` - }); - } else { - this.router.navigate(url, { - queryParams: queryParams, - queryParamsHandling: 'merge' - }); - } - } - }); - } - - clearPagination(paginationId: string) { - const params = {}; - params[`p.${paginationId}`] = null; - params[`rpp.${paginationId}`] = null; - params[`sf.${paginationId}`] = null; - params[`sd.${paginationId}`] = null; - - this.router.navigate([], { - queryParams: params, - queryParamsHandling: 'merge' - }); - } - - getPageParam(paginationId: string) { - return `p.${paginationId}`; - } - private getParametersWithIdName(paginationId: string, params: { page?: number pageSize?: number diff --git a/src/app/shared/pagination-drag-and-drop/abstract-paginated-drag-and-drop-list.component.spec.ts b/src/app/shared/pagination-drag-and-drop/abstract-paginated-drag-and-drop-list.component.spec.ts index 4c29e2af9d..53a4619071 100644 --- a/src/app/shared/pagination-drag-and-drop/abstract-paginated-drag-and-drop-list.component.spec.ts +++ b/src/app/shared/pagination-drag-and-drop/abstract-paginated-drag-and-drop-list.component.spec.ts @@ -118,7 +118,7 @@ describe('AbstractPaginatedDragAndDropListComponent', () => { it('should send out a dropObject event with the expected processed paginated indexes', () => { expect(component.dropObject.emit).toHaveBeenCalledWith(Object.assign({ - fromIndex: ((component.currentPage$.value - 1) * component.pageSize) + event.previousIndex, + fromIndex: ((component.currentPage$.value.currentPage - 1) * component.pageSize) + event.previousIndex, toIndex: ((hoverPage - 1) * component.pageSize), finish: jasmine.anything() })); diff --git a/src/app/shared/pagination-drag-and-drop/abstract-paginated-drag-and-drop-list.component.ts b/src/app/shared/pagination-drag-and-drop/abstract-paginated-drag-and-drop-list.component.ts index b861104197..f9b9ff8e90 100644 --- a/src/app/shared/pagination-drag-and-drop/abstract-paginated-drag-and-drop-list.component.ts +++ b/src/app/shared/pagination-drag-and-drop/abstract-paginated-drag-and-drop-list.component.ts @@ -82,7 +82,7 @@ export abstract class AbstractPaginatedDragAndDropListComponent(1); + currentPage$ = new BehaviorSubject(this.options); /** * Whether or not we should display a loading animation @@ -144,7 +144,7 @@ export abstract class AbstractPaginatedDragAndDropListComponent { - this.currentPage$.next(currentPagination.currentPage); + this.currentPage$.next(currentPagination); }); } @@ -187,8 +187,8 @@ export abstract class AbstractPaginatedDragAndDropListComponent) { const dragIndex = event.previousIndex; let dropIndex = event.currentIndex; - const dragPage = this.currentPage$.value - 1; - let dropPage = this.currentPage$.value - 1; + const dragPage = this.currentPage$.value.currentPage - 1; + let dropPage = this.currentPage$.value.currentPage - 1; // Check if the user is hovering over any of the pagination's pages at the time of dropping the object const droppedOnElement = this.elRef.nativeElement.querySelector('.page-item:hover'); diff --git a/src/app/shared/pagination/pagination.component.spec.ts b/src/app/shared/pagination/pagination.component.spec.ts index bad54d6e83..66308df67f 100644 --- a/src/app/shared/pagination/pagination.component.spec.ts +++ b/src/app/shared/pagination/pagination.component.spec.ts @@ -38,8 +38,6 @@ import { BehaviorSubject, of as observableOf } from 'rxjs'; function expectPages(fixture: ComponentFixture, pagesDef: string[]): void { const de = fixture.debugElement.query(By.css('.pagination')); const pages = de.nativeElement.querySelectorAll('li'); - console.log('pages', pages.length, pagesDef.length); - console.log(pages); expect(pages.length).toEqual(pagesDef.length); diff --git a/src/app/shared/pagination/pagination.component.ts b/src/app/shared/pagination/pagination.component.ts index 4a181d7090..520dcda651 100644 --- a/src/app/shared/pagination/pagination.component.ts +++ b/src/app/shared/pagination/pagination.component.ts @@ -176,7 +176,7 @@ export class PaginationComponent implements OnDestroy, OnInit { })); this.checkConfig(this.paginationOptions); this.initializeConfig(); - } + } /** * Method provided by Angular. Invoked when the instance is destroyed. @@ -195,14 +195,11 @@ export class PaginationComponent implements OnDestroy, OnInit { this.id = this.paginationOptions.id || null; this.pageSizeOptions = this.paginationOptions.pageSizeOptions; this.currentPage$ = this.paginationService.getCurrentPagination(this.id, this.paginationOptions).pipe( - tap((v) => console.log('currentPage', v)), map((currentPagination) => currentPagination.currentPage) ); this.pageSize$ = this.paginationService.getCurrentPagination(this.id, this.paginationOptions).pipe( map((currentPagination) => currentPagination.pageSize) ); - this.pageSize$.subscribe((v) => console.log('this.pageSize$', v)); - this.currentPage$.subscribe((v) => console.log('this.currentPage$', v)); let sortOptions; if (this.sortOptions) { diff --git a/src/app/submission/import-external/submission-import-external.component.ts b/src/app/submission/import-external/submission-import-external.component.ts index 2c407a1edb..9dabf4ceb7 100644 --- a/src/app/submission/import-external/submission-import-external.component.ts +++ b/src/app/submission/import-external/submission-import-external.component.ts @@ -124,7 +124,6 @@ export class SubmissionImportExternalComponent implements OnInit, OnDestroy { this.reload$.next({query: query, source: source}); this.retrieveExternalSources(); })); - this.reload$.subscribe((v) => console.log('this.reload$', v)); } /** @@ -169,7 +168,6 @@ export class SubmissionImportExternalComponent implements OnInit, OnDestroy { */ private retrieveExternalSources(): void { this.reload$.subscribe((sourceQueryObject: {source: string, query: string}) => { - console.log('ping?', sourceQueryObject); const source = sourceQueryObject.source; const query = sourceQueryObject.query; if (isNotEmpty(source) && isNotEmpty(query)) { @@ -178,7 +176,6 @@ export class SubmissionImportExternalComponent implements OnInit, OnDestroy { this.isLoading$.next(true); this.subs.push( this.searchConfigService.paginatedSearchOptions.pipe( - tap((v) => console.log('searchpag?', v)), filter((searchOptions) => searchOptions.query === query), mergeMap((searchOptions) => this.externalService.getExternalSourceEntries(this.routeData.sourceId, searchOptions).pipe( getFinishedRemoteData(),