From da7723c27bfe4e56cccb7429d04dc187c92a0a85 Mon Sep 17 00:00:00 2001
From: Kristof De Langhe <kristof.delanghe@atmire.com>
Date: Wed, 30 Jan 2019 11:09:29 +0100
Subject: [PATCH] 59415: BrowseEntrySearchOptions to enforce correct use of
 parameters

---
 .../browse-by-metadata-page.component.ts      | 35 +++++++++++--------
 .../browse-by-title-page.component.ts         |  8 ++---
 .../browse-entry-search-options.model.ts      | 17 +++++++++
 src/app/core/browse/browse.service.ts         | 17 +++------
 4 files changed, 45 insertions(+), 32 deletions(-)
 create mode 100644 src/app/core/browse/browse-entry-search-options.model.ts

diff --git a/src/app/+browse-by/+browse-by-metadata-page/browse-by-metadata-page.component.ts b/src/app/+browse-by/+browse-by-metadata-page/browse-by-metadata-page.component.ts
index 975dc33d48..48f9c8b6ca 100644
--- a/src/app/+browse-by/+browse-by-metadata-page/browse-by-metadata-page.component.ts
+++ b/src/app/+browse-by/+browse-by-metadata-page/browse-by-metadata-page.component.ts
@@ -11,6 +11,7 @@ import { hasValue, isNotEmpty } from '../../shared/empty.util';
 import { BrowseService } from '../../core/browse/browse.service';
 import { BrowseEntry } from '../../core/shared/browse-entry.model';
 import { Item } from '../../core/shared/item.model';
+import { BrowseEntrySearchOptions } from '../../core/browse/browse-entry-search-options.model';
 
 @Component({
   selector: 'ds-browse-by-metadata-page',
@@ -76,10 +77,7 @@ export class BrowseByMetadataPageComponent implements OnInit {
   }
 
   ngOnInit(): void {
-    this.updatePage({
-      pagination: this.paginationConfig,
-      sort: this.sortConfig
-    });
+    this.updatePage(new BrowseEntrySearchOptions(null, this.paginationConfig, this.sortConfig));
     this.subs.push(
       observableCombineLatest(
         this.route.params,
@@ -107,8 +105,8 @@ export class BrowseByMetadataPageComponent implements OnInit {
    *                          sort: SortOptions,
    *                          scope: string }
    */
-  updatePage(searchOptions) {
-    this.browseEntries$ = this.browseService.getBrowseEntriesFor(searchOptions.metadata, searchOptions);
+  updatePage(searchOptions: BrowseEntrySearchOptions) {
+    this.browseEntries$ = this.browseService.getBrowseEntriesFor(searchOptions);
     this.items$ = undefined;
   }
 
@@ -121,8 +119,8 @@ export class BrowseByMetadataPageComponent implements OnInit {
    *                          scope: string }
    * @param value          The value of the browse-entry to display items for
    */
-  updatePageWithItems(searchOptions, value: string) {
-    this.items$ = this.browseService.getBrowseItemsFor(searchOptions.metadata, value, searchOptions);
+  updatePageWithItems(searchOptions: BrowseEntrySearchOptions, value: string) {
+    this.items$ = this.browseService.getBrowseItemsFor(value, searchOptions);
   }
 
   ngOnDestroy(): void {
@@ -131,26 +129,33 @@ export class BrowseByMetadataPageComponent implements OnInit {
 
 }
 
+/**
+ * Function to transform query and url parameters into searchOptions used to fetch browse entries or items
+ * @param params            URL and query parameters
+ * @param paginationConfig  Pagination configuration
+ * @param sortConfig        Sorting configuration
+ * @param metadata          Optional metadata definition to fetch browse entries/items for
+ */
 export function browseParamsToOptions(params: any,
                                       paginationConfig: PaginationComponentOptions,
                                       sortConfig: SortOptions,
-                                      metadata?: string): any {
-  return {
-    metadata: metadata,
-    pagination: Object.assign({},
+                                      metadata?: string): BrowseEntrySearchOptions {
+  return new BrowseEntrySearchOptions(
+    metadata,
+    Object.assign({},
       paginationConfig,
       {
         currentPage: +params.page || paginationConfig.currentPage,
         pageSize: +params.pageSize || paginationConfig.pageSize
       }
     ),
-    sort: Object.assign({},
+    Object.assign({},
       sortConfig,
       {
         direction: params.sortDirection || sortConfig.direction,
         field: params.sortField || sortConfig.field
       }
     ),
-    scope: params.scope
-  };
+    params.scope
+  );
 }
diff --git a/src/app/+browse-by/+browse-by-title-page/browse-by-title-page.component.ts b/src/app/+browse-by/+browse-by-title-page/browse-by-title-page.component.ts
index ace585ea5a..4af7495dad 100644
--- a/src/app/+browse-by/+browse-by-title-page/browse-by-title-page.component.ts
+++ b/src/app/+browse-by/+browse-by-title-page/browse-by-title-page.component.ts
@@ -12,6 +12,7 @@ import { ActivatedRoute, PRIMARY_OUTLET, UrlSegmentGroup } from '@angular/router
 import { hasValue } from '../../shared/empty.util';
 import { Collection } from '../../core/shared/collection.model';
 import { browseParamsToOptions } from '../+browse-by-metadata-page/browse-by-metadata-page.component';
+import { BrowseEntrySearchOptions } from '../../core/browse/browse-entry-search-options.model';
 
 @Component({
   selector: 'ds-browse-by-title-page',
@@ -53,10 +54,7 @@ export class BrowseByTitlePageComponent implements OnInit {
   }
 
   ngOnInit(): void {
-    this.updatePage({
-      pagination: this.paginationConfig,
-      sort: this.sortConfig
-    });
+    this.updatePage(new BrowseEntrySearchOptions(null, this.paginationConfig, this.sortConfig));
     this.subs.push(
       observableCombineLatest(
         this.route.params,
@@ -75,7 +73,7 @@ export class BrowseByTitlePageComponent implements OnInit {
    *                        { pagination: PaginationComponentOptions,
    *                          sort: SortOptions }
    */
-  updatePage(searchOptions) {
+  updatePage(searchOptions: BrowseEntrySearchOptions) {
     this.items$ = this.itemDataService.findAll({
       currentPage: searchOptions.pagination.currentPage,
       elementsPerPage: searchOptions.pagination.pageSize,
diff --git a/src/app/core/browse/browse-entry-search-options.model.ts b/src/app/core/browse/browse-entry-search-options.model.ts
new file mode 100644
index 0000000000..94e2013ce0
--- /dev/null
+++ b/src/app/core/browse/browse-entry-search-options.model.ts
@@ -0,0 +1,17 @@
+import { PaginationComponentOptions } from '../../shared/pagination/pagination-component-options.model';
+import { SortOptions } from '../cache/models/sort-options.model';
+
+/**
+ * A class that defines the search options to be used for fetching browse entries or items
+ * - metadataDefinition:  The metadata definition to fetch entries or items for
+ * - pagination:          The pagination options to use
+ * - sort:                The sorting options to use
+ * - scope:               An optional scope to limit the results within a specific collection or community
+ */
+export class BrowseEntrySearchOptions {
+  constructor(public metadataDefinition: string,
+              public pagination: PaginationComponentOptions,
+              public sort: SortOptions,
+              public scope?: string) {
+  }
+}
diff --git a/src/app/core/browse/browse.service.ts b/src/app/core/browse/browse.service.ts
index 8e2c288ae8..815570f348 100644
--- a/src/app/core/browse/browse.service.ts
+++ b/src/app/core/browse/browse.service.ts
@@ -37,6 +37,7 @@ import {
 import { URLCombiner } from '../url-combiner/url-combiner';
 import { Item } from '../shared/item.model';
 import { DSpaceObject } from '../shared/dspace-object.model';
+import { BrowseEntrySearchOptions } from './browse-entry-search-options.model';
 
 @Injectable()
 export class BrowseService {
@@ -87,13 +88,9 @@ export class BrowseService {
     return this.rdb.toRemoteDataObservable(requestEntry$, responseCache$, payload$);
   }
 
-  getBrowseEntriesFor(definitionID: string, options: {
-    pagination?: PaginationComponentOptions;
-    sort?: SortOptions;
-    scope?: string;
-  } = {}): Observable<RemoteData<PaginatedList<BrowseEntry>>> {
+  getBrowseEntriesFor(options: BrowseEntrySearchOptions): Observable<RemoteData<PaginatedList<BrowseEntry>>> {
     const request$ = this.getBrowseDefinitions().pipe(
-      getBrowseDefinitionLinks(definitionID),
+      getBrowseDefinitionLinks(options.metadataDefinition),
       hasValueOperator(),
       map((_links: any) => _links.entries),
       hasValueOperator(),
@@ -146,13 +143,9 @@ export class BrowseService {
    *                                    sort: SortOptions }
    * @returns {Observable<RemoteData<PaginatedList<Item>>>}
    */
-  getBrowseItemsFor(definitionID: string, filterValue: string, options: {
-    pagination?: PaginationComponentOptions;
-    sort?: SortOptions;
-    scope?: string;
-  } = {}): Observable<RemoteData<PaginatedList<Item>>> {
+  getBrowseItemsFor(filterValue: string, options: BrowseEntrySearchOptions): Observable<RemoteData<PaginatedList<Item>>> {
     const request$ = this.getBrowseDefinitions().pipe(
-      getBrowseDefinitionLinks(definitionID),
+      getBrowseDefinitionLinks(options.metadataDefinition),
       hasValueOperator(),
       map((_links: any) => _links.items),
       hasValueOperator(),
-- 
GitLab