From 99683cce6f027ef8bb1877ece0ee01363addf5cd Mon Sep 17 00:00:00 2001
From: Lotte Hofstede <lotte_hofstede@hotmail.com>
Date: Tue, 19 Sep 2017 16:35:42 +0200
Subject: [PATCH] 44239: pagination optimizations

---
 .../top-level-community-list.component.html   |   2 +-
 .../top-level-community-list.component.ts     |   2 -
 .../object-list/object-list.component.html    |   3 +-
 src/app/search-page/search-page.component.ts  |  39 +++-
 .../search-results.compontent.ts              |   4 +-
 .../object-list/object-list.component.ts      |   6 +
 .../pagination/pagination.component.html      |   4 +-
 .../shared/pagination/pagination.component.ts | 214 ++++++++++--------
 .../search-form/search-form.component.html    |   6 +-
 .../search-form/search-form.component.ts      |  20 +-
 10 files changed, 176 insertions(+), 124 deletions(-)

diff --git a/src/app/home/top-level-community-list/top-level-community-list.component.html b/src/app/home/top-level-community-list/top-level-community-list.component.html
index a5450b6cce..772eb66012 100644
--- a/src/app/home/top-level-community-list/top-level-community-list.component.html
+++ b/src/app/home/top-level-community-list/top-level-community-list.component.html
@@ -2,5 +2,5 @@
   <h2>{{'home.top-level-communities.head' | translate}}</h2>
   <p class="lead">{{'home.top-level-communities.help' | translate}}</p>
   <ds-object-list [config]="config" [sortConfig]="sortConfig"
-                  [objects]="topLevelCommunities" [hideGear]="false" (paginationChange)="updatePage()"></ds-object-list>
+                  [objects]="topLevelCommunities" [hideGear]="true" (paginationChange)="updatePage($event)"></ds-object-list>
 </div>
diff --git a/src/app/home/top-level-community-list/top-level-community-list.component.ts b/src/app/home/top-level-community-list/top-level-community-list.component.ts
index 2362d7ddd7..43ad4859df 100644
--- a/src/app/home/top-level-community-list/top-level-community-list.component.ts
+++ b/src/app/home/top-level-community-list/top-level-community-list.component.ts
@@ -24,9 +24,7 @@ export class TopLevelCommunityListComponent {
     this.config.pageSizeOptions = [4];
     this.config.pageSize = 4;
     this.sortConfig = new SortOptions();
-  }
 
-  ngOnInit(): void {
     this.updatePage({
       page: 1,
       pageSize: this.config.pageSize,
diff --git a/src/app/object-list/object-list.component.html b/src/app/object-list/object-list.component.html
index 8489b24d2b..0765e00476 100644
--- a/src/app/object-list/object-list.component.html
+++ b/src/app/object-list/object-list.component.html
@@ -7,7 +7,8 @@
                (pageChange)="onPageChange($event)"
                (pageSizeChange)="onPageSizeChange($event)"
                (sortDirectionChange)="onSortDirectionChange($event)"
-               (sortFieldChange)="onSortDirectionChange($event)">
+               (sortFieldChange)="onSortDirectionChange($event)"
+               (paginationChange)="onPaginationChange($event)">
     <ul *ngIf="objects.hasSucceeded | async"> <!--class="list-unstyled"-->
         <li *ngFor="let object of (objects.payload | async) | paginate: { itemsPerPage: (pageInfo | async)?.elementsPerPage, currentPage: (pageInfo | async)?.currentPage, totalItems: (pageInfo | async)?.totalElements }">
             <ds-wrapper-list-element [object]="object"></ds-wrapper-list-element>
diff --git a/src/app/search-page/search-page.component.ts b/src/app/search-page/search-page.component.ts
index 0aad198a4f..eec01d0a0c 100644
--- a/src/app/search-page/search-page.component.ts
+++ b/src/app/search-page/search-page.component.ts
@@ -37,6 +37,13 @@ export class SearchPageComponent implements OnInit, OnDestroy {
               private route: ActivatedRoute,
               private communityService: CommunityDataService,) {
     this.scopeList = communityService.findAll();
+    // Initial pagination config
+    const pagination: PaginationComponentOptions = new PaginationComponentOptions();
+    pagination.id = 'search-results-pagination';
+    pagination.currentPage = 1;
+    pagination.pageSize = 10;
+    const sort: SortOptions = new SortOptions();
+    this.searchOptions = { pagination: pagination, sort: sort };
   }
 
   ngOnInit(): void {
@@ -47,17 +54,21 @@ export class SearchPageComponent implements OnInit, OnDestroy {
           this.currentParams = params;
           this.query = params.query || '';
           this.scope = params.scope;
-          this.page = +params.page || 1;
-          // Prepare search parameters
-          const pagination: PaginationComponentOptions = new PaginationComponentOptions();
-          pagination.id = 'search-results-pagination';
-          pagination.currentPage = this.page;
-          pagination.pageSize = +params.pageSize || 10;
-          const sort: SortOptions = new SortOptions(params.sortField, params.sortDirection);
-          // Create search options
-          this.searchOptions = { pagination: pagination, sort: sort };
-          // Resolve search results
-          this.results = this.service.search(this.query, params.scope, this.searchOptions);
+          const page = +params.page  || this.searchOptions.pagination.currentPage;
+          const pageSize = +params.pageSize  || this.searchOptions.pagination.pageSize;
+          const sortDirection = +params.page || this.searchOptions.sort.direction;
+          const pagination = Object.assign({},
+            this.searchOptions.pagination,
+            { currentPage: page, pageSize: pageSize }
+          );
+          const sort = Object.assign({},
+            this.searchOptions.sort,
+            { direction: sortDirection, field: params.sortField }
+          );
+          this.updateSearchResults({
+            pagination: pagination,
+            sort: sort
+          });
           if (isNotEmpty(this.scope)) {
             this.scopeObject = this.communityService.findById(this.scope);
           } else {
@@ -67,6 +78,12 @@ export class SearchPageComponent implements OnInit, OnDestroy {
       );
   }
 
+  private updateSearchResults(searchOptions) {
+    // Resolve search results
+    this.results = this.service.search(this.query, this.scope, searchOptions);
+
+  }
+
   ngOnDestroy() {
     this.sub.unsubscribe();
   }
diff --git a/src/app/search-page/search-results/search-results.compontent.ts b/src/app/search-page/search-results/search-results.compontent.ts
index 8ca741a68d..505a3eeae4 100644
--- a/src/app/search-page/search-results/search-results.compontent.ts
+++ b/src/app/search-page/search-results/search-results.compontent.ts
@@ -1,9 +1,7 @@
-import { Component, OnInit, Input, Output, EventEmitter } from '@angular/core';
+import { Component, Input } from '@angular/core';
 import { RemoteData } from '../../core/data/remote-data';
 import { SearchResult } from '../../search/search-result.model';
 import { DSpaceObject } from '../../core/shared/dspace-object.model';
-import { SortOptions, SortDirection } from '../../core/cache/models/sort-options.model';
-import { PaginationComponentOptions } from '../../shared/pagination/pagination-component-options.model';
 import { SearchOptions } from '../../search/search-options.model';
 
 /**
diff --git a/src/app/shared/object-list/object-list.component.ts b/src/app/shared/object-list/object-list.component.ts
index 20b6ec0873..c3018b957c 100644
--- a/src/app/shared/object-list/object-list.component.ts
+++ b/src/app/shared/object-list/object-list.component.ts
@@ -52,6 +52,8 @@ export class ObjectListComponent implements OnChanges, OnInit {
    */
   @Output() sortDirectionChange: EventEmitter<SortDirection> = new EventEmitter<SortDirection>();
 
+  @Output() paginationChange: EventEmitter<SortDirection> = new EventEmitter<any>();
+
   /**
    * An event fired when the sort field is changed.
    * Event's payload equals to the newly selected sort field.
@@ -95,4 +97,8 @@ export class ObjectListComponent implements OnChanges, OnInit {
     this.sortFieldChange.emit(event);
   }
 
+  onPaginationChange(event) {
+    this.paginationChange.emit(event);
+  }
+
 }
diff --git a/src/app/shared/pagination/pagination.component.html b/src/app/shared/pagination/pagination.component.html
index d1a3028b8a..4c8dec81bd 100644
--- a/src/app/shared/pagination/pagination.component.html
+++ b/src/app/shared/pagination/pagination.component.html
@@ -10,9 +10,9 @@
           <button class="btn btn-outline-primary" id="paginationControls" ngbDropdownToggle><i class="fa fa-cog" aria-hidden="true"></i></button>
           <div class="dropdown-menu dropdown-menu-right" id="paginationControlsDropdownMenu" aria-labelledby="paginationControls" ngbDropdownMenu>
             <h6 class="dropdown-header">{{ 'pagination.results-per-page' | translate}}</h6>
-            <button class="dropdown-item" *ngFor="let item of pageSizeOptions" (click)="setPageSize(item)"><i class="fa fa-check {{(item != paginationOptions.pageSize) ? 'invisible' : ''}}" aria-hidden="true"></i> {{item}} </button>
+            <button class="dropdown-item" *ngFor="let item of pageSizeOptions" (click)="doPageSizeChange(item)"><i class="fa fa-check {{(item != pageSize) ? 'invisible' : ''}}" aria-hidden="true"></i> {{item}} </button>
             <h6 class="dropdown-header">{{ 'pagination.sort-direction' | translate}}</h6>
-            <button class="dropdown-item" *ngFor="let direction of (sortDirections | dsKeys)" (click)="setSortDirection(direction.key)"><i class="fa fa-check {{(direction.key != sortOptions.direction) ? 'invisible' : ''}}" aria-hidden="true"></i> {{direction.value}} </button>
+            <button class="dropdown-item" *ngFor="let direction of (sortDirections | dsKeys)" (click)="doSortDirectionChange(direction.key)"><i class="fa fa-check {{(direction.key != sortDirection) ? 'invisible' : ''}}" aria-hidden="true"></i> {{direction.value}} </button>
           </div>
         </div>
       </div>
diff --git a/src/app/shared/pagination/pagination.component.ts b/src/app/shared/pagination/pagination.component.ts
index 80cf8b10ae..0f81005880 100644
--- a/src/app/shared/pagination/pagination.component.ts
+++ b/src/app/shared/pagination/pagination.component.ts
@@ -3,10 +3,9 @@ import {
   Component,
   EventEmitter,
   Input,
-  OnChanges,
   OnDestroy,
   OnInit,
-  Output, SimpleChanges,
+  Output,
   ViewEncapsulation
 } from '@angular/core'
 
@@ -21,7 +20,7 @@ import { HostWindowService } from '../host-window.service';
 import { HostWindowState } from '../host-window.reducer';
 import { PaginationComponentOptions } from './pagination-component-options.model';
 import { SortDirection, SortOptions } from '../../core/cache/models/sort-options.model';
-import { hasValue, isUndefined, isNotEmpty, hasNoValue } from '../empty.util';
+import { hasNoValue, hasValue, isEmpty, isNotEmpty } from '../empty.util';
 import { PageInfo } from '../../core/shared/page-info.model';
 
 /**
@@ -110,7 +109,7 @@ export class PaginationComponent implements OnDestroy, OnInit {
   /**
    * Current URL query parameters
    */
-  public currentQueryParams = {};
+  public currentQueryParams: any;
 
   /**
    * An observable of HostWindowState type
@@ -186,18 +185,24 @@ export class PaginationComponent implements OnDestroy, OnInit {
     // Listen to changes
     this.subs.push(this.route.queryParams
       .subscribe((queryParams) => {
-        if (isNotEmpty(queryParams)) {
-          // No need to rewrite empty search queries - we have the initial configuration
-          this.validateParams(queryParams.page, queryParams.pageSize, queryParams.sortDirection, queryParams.sortField);
+        if (isEmpty(queryParams)) {
+          this.initializeConfig();
+        } else {
           this.currentQueryParams = queryParams;
-        } else if (hasValue(this.currentPage)) {
-          // When going back to the base /search base url - don't use the last this.currentPage value anymore
-          this.doPageChange(this.paginationOptions.currentPage);
+          const fixedProperties = this.validateParams(queryParams);
+          if (isNotEmpty(fixedProperties)) {
+            this.fixRoute(fixedProperties);
+          }
+          this.setFields();
+          this.setShowingDetail();
         }
-        this.setShowingDetail();
       }));
   }
 
+  private fixRoute(fixedProperties) {
+    this.updateRoute(fixedProperties);
+  }
+
   /**
    * Method provided by Angular. Invoked when the instance is destroyed.
    */
@@ -215,6 +220,12 @@ export class PaginationComponent implements OnDestroy, OnInit {
     this.pageSize = this.paginationOptions.pageSize;
     this.sortDirection = this.sortOptions.direction;
     this.sortField = this.sortOptions.field;
+    this.currentQueryParams = {
+      page: this.currentPage,
+      pageSize: this.pageSize,
+      sortDirection: this.sortDirection,
+      sortField: this.sortField
+    };
     this.setShowingDetail();
   }
 
@@ -237,9 +248,49 @@ export class PaginationComponent implements OnDestroy, OnInit {
    *    The page being navigated to.
    */
   public doPageChange(page: number) {
+    this.updateRoute({ page: page });
+  }
+
+  /**
+   * Method to set set new page size and update route parameters
+   *
+   * @param pageSize
+   *    The new page size.
+   */
+  public doPageSizeChange(pageSize: number) {
+    this.updateRoute({ page: 1, pageSize: pageSize });
+  }
+
+  /**
+   * Method to set set new sort direction and update route parameters
+   *
+   * @param sortDirection
+   *    The new sort direction.
+   */
+  public doSortDirectionChange(sortDirection: SortDirection) {
+    this.updateRoute({ page: 1, sortDirection: sortDirection });
+  }
+
+  /**
+   * Method to set set new sort field and update route parameters
+   *
+   * @param sortField
+   *    The new sort field.
+   */
+  public doSortFieldChange(field: string) {
+    this.updateRoute({ page: 1, sortField: field });
+  }
+
+  /**
+   * Method to set set new page and update route parameters
+   *
+   * @param page
+   *    The page being navigated to.
+   */
+  public setPage(page: number) {
     this.currentPage = page;
-    this.updateRoute();
     this.pageChange.emit(page);
+    this.emitPaginationChange();
   }
 
   /**
@@ -250,9 +301,8 @@ export class PaginationComponent implements OnDestroy, OnInit {
    */
   public setPageSize(pageSize: number) {
     this.pageSize = pageSize;
-    this.doPageChange(1);
-    this.updateRoute();
     this.pageSizeChange.emit(pageSize);
+    this.emitPaginationChange();
   }
 
   /**
@@ -263,9 +313,8 @@ export class PaginationComponent implements OnDestroy, OnInit {
    */
   public setSortDirection(sortDirection: SortDirection) {
     this.sortDirection = sortDirection;
-    this.doPageChange(1);
-    this.updateRoute();
     this.sortDirectionChange.emit(sortDirection);
+    this.emitPaginationChange();
   }
 
   /**
@@ -276,23 +325,25 @@ export class PaginationComponent implements OnDestroy, OnInit {
    */
   public setSortField(field: string) {
     this.sortField = field;
-    this.doPageChange(1);
-    this.updateRoute();
     this.sortFieldChange.emit(field);
+    this.emitPaginationChange();
+  }
+
+  private emitPaginationChange() {
+    this.paginationChange.emit({
+      page: this.currentPage,
+      pageSize: this.pageSize,
+      sortDirection: this.sortDirection,
+      sortField: this.sortField
+    });
   }
 
   /**
    * Method to update the route parameters
    */
-  private updateRoute() {
+  private updateRoute(params: {}) {
     this.router.navigate([], {
-      queryParams: Object.assign({}, this.currentQueryParams, {
-        pageId: this.id,
-        page: this.currentPage,
-        pageSize: this.pageSize,
-        sortDirection: this.sortDirection,
-        sortField: this.sortField
-      })
+      queryParams: Object.assign({}, this.currentQueryParams, params)
     });
   }
 
@@ -324,83 +375,58 @@ export class PaginationComponent implements OnDestroy, OnInit {
    * @param pageSize
    *    The page size to validate
    */
-  private validateParams(page: any, pageSize: any, sortDirection: any, sortField: any) {
-    const originalPageInfo = {
-      id: this.id,
-      page: page,
-      pageSize: pageSize,
-      sortDirection: sortDirection,
-      sortField: sortField
-    };
-    // tslint:disable-next-line:triple-equals
-    const filteredPageInfo = {
-      id: this.id,
-      page: this.validatePage(page),
-      pageSize: this.validatePageSize(pageSize),
-      sortDirection: sortDirection,
-      sortField: sortField
-    };
-
-    // let filteredPageSize = this.pageSizeOptions.find((x) => x === pageSize);
-    if (JSON.stringify(originalPageInfo) !== JSON.stringify(filteredPageInfo)) {
-      // filteredPageSize = (filteredPageSize) ? filteredPageSize : this.pageSize;
-      this.router.navigate([], {
-        queryParams: filteredPageInfo
-      });
-    } else {
-      let hasChanged = false;
-      // (+) converts string to a number
-      if (this.currentPage !== +page) {
-        this.currentPage = +page;
-        this.pageChange.emit(this.currentPage);
-        hasChanged = true;
-      }
-
-      if (this.pageSize !== +pageSize) {
-        this.pageSize = +pageSize;
-        this.pageSizeChange.emit(this.pageSize);
-        hasChanged = true;
-      }
-
-      if (this.sortDirection !== +sortDirection) {
-        this.sortDirection = +sortDirection;
-        this.sortDirectionChange.emit(this.sortDirection);
-        hasChanged = true;
-      }
-
-      if (this.sortField !== sortField) {
-        this.sortField = sortField;
-        this.sortFieldChange.emit(this.sortField);
-        hasChanged = true;
-      }
-
-      if (hasChanged) {
-        this.paginationChange.emit({
-          page: page,
-          pageSize: pageSize,
-          sortDirection: sortDirection,
-          sortField: sortField
-        });
-      }
-      this.cdRef.detectChanges();
+  private validateParams(params: any): any {
+    const validPage = this.validatePage(params.page);
+    const filteredSize = this.validatePageSize(params.pageSize);
+    const fixedFields: any = {};
+    if (+params.page !== validPage) {
+      fixedFields.page = validPage;
     }
+    if (+params.pageSize !== filteredSize) {
+      fixedFields.pageSize = filteredSize;
+    }
+    return fixedFields;
   }
 
-  private validatePage(page: any): string {
-    let result = this.currentPage
+  private setFields() {
+    // (+) converts string to a number
+    const page = this.currentQueryParams.page;
+    if (this.currentPage !== +page) {
+      this.setPage(+page);
+    }
+
+    const pageSize = this.currentQueryParams.pageSize;
+    if (this.pageSize !== +pageSize) {
+      this.setPageSize(+pageSize);
+    }
+
+    const sortDirection = this.currentQueryParams.sortDirection;
+    if (this.sortDirection !== +sortDirection) {
+      this.setSortDirection(+sortDirection);
+    }
+
+    const sortField = this.currentQueryParams.sortField;
+    if (this.sortField !== sortField) {
+      this.setSortField(sortField);
+    }
+    this.cdRef.detectChanges();
+  }
+
+  private validatePage(page: any): number {
+    let result = this.currentPage;
     if (isNumeric(page)) {
-      result = page;
+      result = +page;
     }
-    return result + '';
+    return result;
   }
 
-  private validatePageSize(pageSize: any): string {
-    const filteredPageSize = this.pageSizeOptions.find((x) => x === pageSize);
-    let result = this.pageSize
+  private validatePageSize(pageSize: any): number {
+    const filteredPageSize = this.pageSizeOptions.find((x) => x === +pageSize);
+    let result = this.pageSize;
     if (filteredPageSize) {
-      result = pageSize;
+      result = +pageSize;
     }
-    return result + '';
+    return result;
   }
 
   /**
diff --git a/src/app/shared/search-form/search-form.component.html b/src/app/shared/search-form/search-form.component.html
index ae4132e950..dfdc20024a 100644
--- a/src/app/shared/search-form/search-form.component.html
+++ b/src/app/shared/search-form/search-form.component.html
@@ -1,11 +1,11 @@
 <form #form="ngForm" (ngSubmit)="onSubmit(form.value)" class="row">
-<div class="col-12 col-sm-3">
-  <select ngif="isNotEmpty(scopes | async)" [(ngModel)]="selectedId" name="scope" class="form-control" aria-label="Search scope" [compareWith]="byId">
+<div *ngIf="isNotEmpty(scopes | async)" class="col-12 col-sm-3">
+  <select [(ngModel)]="selectedId" name="scope" class="form-control" aria-label="Search scope" [compareWith]="byId">
     <option value>{{'search.form.search_dspace' | translate}}</option>
     <option *ngFor="let scopeOption of scopes | async" [value]="scopeOption.id">{{scopeOption?.name ? scopeOption.name : 'search.form.search_dspace' | translate}}</option>
   </select>
 </div>
-    <div class="col-12 col-sm-9">
+    <div [ngClass]="{'col-sm-9': isNotEmpty(scopes | async)}" class="col-12">
     <div class="form-group input-group">
         <input type="text" [(ngModel)]="query" name="query" class="form-control" aria-label="Search input">
         <span class="input-group-btn">
diff --git a/src/app/shared/search-form/search-form.component.ts b/src/app/shared/search-form/search-form.component.ts
index 3d06515f31..4bfbf2b62b 100644
--- a/src/app/shared/search-form/search-form.component.ts
+++ b/src/app/shared/search-form/search-form.component.ts
@@ -31,13 +31,15 @@ export class SearchFormComponent implements OnInit {
   }
 
   ngOnInit(): void {
-    this.scopes
-      .filter((scopes: DSpaceObject[]) => isEmpty(scopes))
-      .subscribe((scopes: DSpaceObject[]) => {
-          this.scopeOptions = scopes
-            .map((scope: DSpaceObject) => scope.id);
-        }
-      );
+    if (this.scopes) {
+      this.scopes
+        .filter((scopes: DSpaceObject[]) => isEmpty(scopes))
+        .subscribe((scopes: DSpaceObject[]) => {
+            this.scopeOptions = scopes
+              .map((scope: DSpaceObject) => scope.id);
+          }
+        );
+    }
   }
 
   constructor(private router: Router) {
@@ -61,6 +63,10 @@ export class SearchFormComponent implements OnInit {
     ;
   }
 
+  private isNotEmpty(object: any) {
+    return isNotEmpty(object);
+  }
+
   byId(id1: string, id2: string) {
     if (isEmpty(id1) && isEmpty(id2)) {
       return true;