78991: Clean up SearchOptions.toRestUrl

* Extract "selective encoding" ops into separate methods
* Add comments to clarify regex
* Use hasValue() instead of just checking on 'match'
* Leave only the last comma of filter values unencoded

src/app/shared/search/paginated-search-options.model.spec.ts

@@ -10,10 +10,10 @@ describe('PaginatedSearchOptions', () => {
   const pageOptions = Object.assign(new PaginationComponentOptions(), { pageSize: 40, page: 1 });
   const filters = [
     new SearchFilter('f.test', ['value']),
-    new SearchFilter('f.example', ['another value', 'second value']),
+    new SearchFilter('f.example', ['another value', 'second value']), // should be split into two arguments, spaces should be URI-encoded
-    new SearchFilter('f.range', ['[2002 TO 2021]'], 'equals'),
+    new SearchFilter('f.range', ['[2002 TO 2021]'], 'equals'),        // value should be URI-encoded, ',equals' should not
   ];
-  const fixedFilter = 'f.fixed=1234 5678,equals';
+  const fixedFilter = 'f.fixed=1234,5678,equals';                     // '=' and ',equals' should not be URI-encoded
   const query = 'search query';
   const scope = '0fde1ecb-82cc-425a-b600-ac3576d76b47';
   const baseUrl = 'www.rest.com';
@@ -37,7 +37,7 @@ describe('PaginatedSearchOptions', () => {
         'sort=test.field,DESC&' +
         'page=0&' +
         'size=40&' +
-        'f.fixed=1234%205678,equals&' +
+        'f.fixed=1234%2C5678,equals&' +
         'query=search%20query&' +
         'scope=0fde1ecb-82cc-425a-b600-ac3576d76b47&' +
         'dsoType=ITEM&' +

src/app/shared/search/search-options.model.spec.ts

@@ -8,10 +8,10 @@ describe('SearchOptions', () => {
 
   const filters = [
     new SearchFilter('f.test', ['value']),
-    new SearchFilter('f.example', ['another value', 'second value']),
+    new SearchFilter('f.example', ['another value', 'second value']), // should be split into two arguments, spaces should be URI-encoded
-    new SearchFilter('f.range', ['[2002 TO 2021]'], 'equals'),
+    new SearchFilter('f.range', ['[2002 TO 2021]'], 'equals'),        // value should be URI-encoded, ',equals' should not
   ];
-  const fixedFilter = 'f.fixed=1234 5678,equals';
+  const fixedFilter = 'f.fixed=1234,5678,equals';                    // '=' and ',equals' should not be URI-encoded
   const query = 'search query';
   const scope = '0fde1ecb-82cc-425a-b600-ac3576d76b47';
   const baseUrl = 'www.rest.com';
@@ -30,7 +30,7 @@ describe('SearchOptions', () => {
     it('should generate a string with all parameters that are present', () => {
       const outcome = options.toRestUrl(baseUrl);
       expect(outcome).toEqual('www.rest.com?' +
-        'f.fixed=1234%205678,equals&' +
+        'f.fixed=1234%2C5678,equals&' +
         'query=search%20query&' +
         'scope=0fde1ecb-82cc-425a-b600-ac3576d76b47&' +
         'dsoType=ITEM&' +

src/app/shared/search/search-options.model.ts

@@ -1,4 +1,4 @@
-import { isNotEmpty } from '../empty.util';
+import { hasValue, isNotEmpty } from '../empty.util';
 import { URLCombiner } from '../../core/url-combiner/url-combiner';
 import { SearchFilter } from './search-filter.model';
 import { DSpaceObjectType } from '../../core/shared/dspace-object-type.model';
@@ -41,16 +41,7 @@ export class SearchOptions {
       args.push(`configuration=${encodeURIComponent(this.configuration)}`);
     }
     if (isNotEmpty(this.fixedFilter)) {
-      let fixedFilter: string;
+      args.push(this.encodedFixedFilter);
-      const match = this.fixedFilter.match(/^([^=]+=)(.+)$/);
-
-      if (match) {
-        fixedFilter = match[1] + encodeURIComponent(match[2]).replace(/%2C/g, ',');
-      } else {
-        fixedFilter = encodeURIComponent(this.fixedFilter);
-      }
-
-      args.push(fixedFilter);
     }
     if (isNotEmpty(this.query)) {
       args.push(`query=${encodeURIComponent(this.query)}`);
@@ -67,9 +58,7 @@ export class SearchOptions {
       this.filters.forEach((filter: SearchFilter) => {
         filter.values.forEach((value) => {
           const filterValue = value.includes(',') ? `${value}` : value + (filter.operator ? ',' + filter.operator : '');
-
+          args.push(`${filter.key}=${this.encodeFilterQueryValue(filterValue)}`);
-          // we don't want commas to get URI-encoded
-          args.push(`${filter.key}=${encodeURIComponent(filterValue).replace(/%2C/g, ',')}`);
         });
       });
     }
@@ -78,4 +67,28 @@ export class SearchOptions {
     }
     return url;
   }
+
+  get encodedFixedFilter(): string {
+    // expected format: 'arg=value'
+    //  -> split the query agument into (arg=)(value) and only encode 'value'
+    const match = this.fixedFilter.match(/^([^=]+=)(.+)$/);
+
+    if (hasValue(match)) {
+      return match[1] + this.encodeFilterQueryValue(match[2]);
+    } else {
+      return this.encodeFilterQueryValue(this.fixedFilter);
+    }
+  }
+
+  encodeFilterQueryValue(filterQueryValue: string): string {
+    // expected format: 'value' or 'value,operator'
+    //  -> split into (value)(,operator) and only encode 'value'
+    const match = filterQueryValue.match(/^(.*)(,\w+)$/);
+
+    if (hasValue(match)) {
+      return encodeURIComponent(match[1]) + match[2];
+    } else {
+      return encodeURIComponent(filterQueryValue);
+    }
+  }
 }