Merge branch 'master' into w2p-55990_Move-item-component

resources/i18n/en.json

@@ -163,7 +163,8 @@
     },
     "results": {
       "head": "Search Results",
-      "no-results": "There were no results for this search"
+      "no-results": "Your search returned no results. Having trouble finding what you're looking for? Try putting",
+      "no-results-link": "quotes around it"
     },
     "sidebar": {
       "close": "Back to results",

src/app/+item-page/field-components/metadata-field-wrapper/metadata-field-wrapper.component.html

@@ -1,6 +1,6 @@
-<div class="simple-view-element">
+<div class="simple-view-element" [class.d-none]="content.textContent.trim().length === 0">
-    <h5 class="simple-view-element-header" *ngIf="label">{{ label }}</h5>
+  <h5 class="simple-view-element-header" *ngIf="label">{{ label }}</h5>
-    <div class="simple-view-element-body">
+  <div #content class="simple-view-element-body">
-        <ng-content></ng-content>
+    <ng-content></ng-content>
-    </div>
+  </div>
 </div>

src/app/+item-page/field-components/metadata-field-wrapper/metadata-field-wrapper.component.spec.ts

@@ -0,0 +1,67 @@
+import { ComponentFixture, TestBed, async } from '@angular/core/testing';
+import { By } from '@angular/platform-browser';
+import { Component, DebugElement } from '@angular/core';
+
+import { MetadataFieldWrapperComponent } from './metadata-field-wrapper.component';
+
+@Component({
+    selector: 'ds-component-with-content',
+    template: '<ds-metadata-field-wrapper [label]="\'test label\'">\n' +
+      '    <div class="my-content">\n' +
+      '    <span></span>\n' +
+      '    </div>\n' +
+      '</ds-metadata-field-wrapper>'
+})
+class ContentComponent {}
+
+describe('MetadataFieldWrapperComponent', () => {
+  let component: MetadataFieldWrapperComponent;
+  let fixture: ComponentFixture<MetadataFieldWrapperComponent>;
+
+  beforeEach(async(() => {
+    TestBed.configureTestingModule({
+      declarations: [MetadataFieldWrapperComponent, ContentComponent]
+    }).compileComponents();
+  }));
+
+  beforeEach(() => {
+    fixture = TestBed.createComponent(MetadataFieldWrapperComponent);
+    component = fixture.componentInstance;
+  });
+
+  const wrapperSelector = '.simple-view-element';
+  const labelSelector = '.simple-view-element-header';
+  const contentSelector = '.my-content';
+
+  it('should create', () => {
+    expect(component).toBeDefined();
+  });
+
+  it('should not show the component when there is no content', () => {
+    component.label = 'test label';
+    fixture.detectChanges();
+    const parentNative = fixture.nativeElement;
+    const nativeWrapper = parentNative.querySelector(wrapperSelector);
+    expect(nativeWrapper.classList.contains('d-none')).toBe(true);
+  });
+
+  it('should not show the component when there is DOM content but no text', () => {
+    const parentFixture = TestBed.createComponent(ContentComponent);
+    parentFixture.detectChanges();
+    const parentNative = parentFixture.nativeElement;
+    const nativeWrapper = parentNative.querySelector(wrapperSelector);
+    expect(nativeWrapper.classList.contains('d-none')).toBe(true);
+  });
+
+  it('should show the component when there is text content', () => {
+    const parentFixture = TestBed.createComponent(ContentComponent);
+    parentFixture.detectChanges();
+    const parentNative = parentFixture.nativeElement;
+    const nativeContent = parentNative.querySelector(contentSelector);
+    nativeContent.textContent = 'lorem ipsum';
+    const nativeWrapper = parentNative.querySelector(wrapperSelector);
+    parentFixture.detectChanges();
+    expect(nativeWrapper.classList.contains('d-none')).toBe(false);
+  });
+
+});

src/app/+item-page/field-components/metadata-values/metadata-values.component.ts

@@ -1,4 +1,5 @@
 import { Component, Input } from '@angular/core';
+import { Metadatum } from '../../../core/shared/metadatum.model';
 
 /**
  * This component renders the configured 'values' into the ds-metadata-field-wrapper component.
@@ -11,7 +12,7 @@ import { Component, Input } from '@angular/core';
 })
 export class MetadataValuesComponent {
 
-  @Input() values: any;
+  @Input() values: Metadatum[];
 
   @Input() separator: string;
 

src/app/+search-page/search-results/search-results.component.html

@@ -7,5 +7,12 @@
   [hideGear]="true">
 </ds-viewable-collection></div>
 <ds-loading *ngIf="!searchResults || searchResults?.isLoading" message="{{'loading.search-results' | translate}}"></ds-loading>
-<ds-error *ngIf="searchResults?.hasFailed" message="{{'error.search-results' | translate}}"></ds-error>
+<ds-error *ngIf="searchResults?.hasFailed && (!searchResults?.error || searchResults?.error?.statusCode != 400)" message="{{'error.search-results' | translate}}"></ds-error>
-<ds-error *ngIf="searchResults?.payload?.page.length == 0" message="{{'search.results.no-results' | translate}}"></ds-error>
+<div *ngIf="searchResults?.payload?.page.length == 0 || searchResults?.error?.statusCode == 400">
+    {{ 'search.results.no-results' | translate }}
+    <a [routerLink]="['/search']"
+      [queryParams]="{ query: surroundStringWithQuotes(searchConfig?.query) }"
+      queryParamsHandling="merge">
+      {{"search.results.no-results-link" | translate}}
+    </a>
+</div> 

src/app/+search-page/search-results/search-results.component.spec.ts

@@ -1,40 +1,92 @@
 import { ComponentFixture, TestBed, async, tick, fakeAsync } from '@angular/core/testing';
 import { By } from '@angular/platform-browser';
-import { DebugElement, NO_ERRORS_SCHEMA } from '@angular/core';
+import { NoopAnimationsModule } from '@angular/platform-browser/animations';
+import { NO_ERRORS_SCHEMA } from '@angular/core';
 import { ResourceType } from '../../core/shared/resource-type';
 import { Community } from '../../core/shared/community.model';
 import { TranslateModule } from '@ngx-translate/core';
 import { SearchResultsComponent } from './search-results.component';
+import { QueryParamsDirectiveStub } from '../../shared/testing/query-params-directive-stub';
 
 describe('SearchResultsComponent', () => {
   let comp: SearchResultsComponent;
   let fixture: ComponentFixture<SearchResultsComponent>;
-  let heading: DebugElement;
 
   beforeEach(async(() => {
     TestBed.configureTestingModule({
-      imports: [TranslateModule.forRoot()],
+      imports: [TranslateModule.forRoot(), NoopAnimationsModule],
-      declarations: [SearchResultsComponent],
+      declarations: [
+        SearchResultsComponent,
+        QueryParamsDirectiveStub],
       schemas: [NO_ERRORS_SCHEMA]
     }).compileComponents();
   }));
 
   beforeEach(() => {
     fixture = TestBed.createComponent(SearchResultsComponent);
-    comp = fixture.componentInstance; // SearchFormComponent test instance
+    comp = fixture.componentInstance; // SearchResultsComponent test instance
-    heading = fixture.debugElement.query(By.css('heading'));
   });
 
-  it('should display heading when results are not empty', fakeAsync(() => {
+  it('should display results when results are not empty', () => {
-    (comp as any).searchResults = 'test';
+    (comp as any).searchResults = { hasSucceeded: true, isLoading: false, payload: { page: { length: 2 } } };
-    (comp as any).searchConfig = {pagination: ''};
+    (comp as any).searchConfig = {};
     fixture.detectChanges();
-    tick();
+    expect(fixture.debugElement.query(By.css('ds-viewable-collection'))).not.toBeNull();
-    expect(heading).toBeDefined();
+  });
-  }));
 
-  it('should not display heading when results is empty', () => {
+  it('should not display link when results are not empty', () => {
-    expect(heading).toBeNull();
+    (comp as any).searchResults = { hasSucceeded: true, isLoading: false, payload: { page: { length: 2 } } };
+    (comp as any).searchConfig = {};
+    fixture.detectChanges();
+    expect(fixture.debugElement.query(By.css('a'))).toBeNull();
+  });
+
+  it('should display error message if error is != 400', () => {
+    (comp as any).searchResults = { hasFailed: true, error: { statusCode: 500 } };
+    fixture.detectChanges();
+    expect(fixture.debugElement.query(By.css('ds-error'))).not.toBeNull();
+  });
+
+  it('should display link with new search where query is quoted if search return a error 400', () => {
+    (comp as any).searchResults = { hasFailed: true, error: { statusCode: 400 } };
+    (comp as any).searchConfig = { query: 'foobar' };
+    fixture.detectChanges();
+
+    const linkDes = fixture.debugElement.queryAll(By.directive(QueryParamsDirectiveStub));
+
+    // get attached link directive instances
+    // using each DebugElement's injector
+    const routerLinkQuery = linkDes.map((de) => de.injector.get(QueryParamsDirectiveStub));
+
+    expect(routerLinkQuery.length).toBe(1, 'should have 1 router link with query params');
+    expect(routerLinkQuery[0].queryParams.query).toBe('"foobar"', 'query params should be "foobar"');
+  });
+
+  it('should display link with new search where query is quoted if search result is empty', () => {
+    (comp as any).searchResults = { payload: { page: { length: 0 } } };
+    (comp as any).searchConfig = { query: 'foobar' };
+    fixture.detectChanges();
+
+    const linkDes = fixture.debugElement.queryAll(By.directive(QueryParamsDirectiveStub));
+
+    // get attached link directive instances
+    // using each DebugElement's injector
+    const routerLinkQuery = linkDes.map((de) => de.injector.get(QueryParamsDirectiveStub));
+
+    expect(routerLinkQuery.length).toBe(1, 'should have 1 router link with query params');
+    expect(routerLinkQuery[0].queryParams.query).toBe('"foobar"', 'query params should be "foobar"');
+  });
+
+  it('should add quotes around the given string', () => {
+    expect(comp.surroundStringWithQuotes('teststring')).toEqual('"teststring"');
+  });
+
+  it('should not add quotes around the given string if they are already there', () => {
+    expect(comp.surroundStringWithQuotes('"teststring"')).toEqual('"teststring"');
+  });
+
+  it('should not add quotes around a given empty string', () => {
+    expect(comp.surroundStringWithQuotes('')).toEqual('');
   });
 });
 

src/app/+search-page/search-results/search-results.component.ts

@@ -6,6 +6,7 @@ import { SearchOptions } from '../search-options.model';
 import { SearchResult } from '../search-result.model';
 import { PaginatedList } from '../../core/data/paginated-list';
 import { ViewMode } from '../../core/shared/view-mode.model';
+import { isNotEmpty } from '../../shared/empty.util';
 
 @Component({
   selector: 'ds-search-results',
@@ -35,4 +36,16 @@ export class SearchResultsComponent {
    */
   @Input() viewMode: ViewMode;
 
+  /**
+   * Method to change the given string by surrounding it by quotes if not already present.
+   */
+  surroundStringWithQuotes(input: string): string {
+    let result = input;
+
+    if (isNotEmpty(result) && !(result.startsWith('\"') && result.endsWith('\"'))) {
+      result = `"${result}"`;
+    }
+
+    return result;
+  }
 }

src/app/core/auth/auth.service.ts

@@ -340,14 +340,10 @@ export class AuthService {
     this.getRedirectUrl()
       .first()
       .subscribe((redirectUrl) => {
+
         if (isNotEmpty(redirectUrl)) {
           this.clearRedirectUrl();
-
+          this.router.onSameUrlNavigation = 'reload';
-          // override the route reuse strategy
-          this.router.routeReuseStrategy.shouldReuseRoute = () => {
-            return false;
-          };
-          this.router.navigated = false;
           const url = decodeURIComponent(redirectUrl);
           this.router.navigateByUrl(url);
           /* TODO Reenable hard redirect when REST API can handle x-forwarded-for, see https://github.com/DSpace/DSpace/pull/2207 */

src/app/core/cache/builders/remote-data-build.service.spec.ts

@@ -0,0 +1,59 @@
+import { RemoteDataBuildService } from './remote-data-build.service';
+import { Item } from '../../shared/item.model';
+import { PaginatedList } from '../../data/paginated-list';
+import { PageInfo } from '../../shared/page-info.model';
+import { RemoteData } from '../../data/remote-data';
+import { Observable } from 'rxjs/Observable';
+
+const pageInfo = new PageInfo();
+const array = [
+  Object.assign(new Item(), {
+    metadata: [
+      {
+        key: 'dc.title',
+        language: 'en_US',
+        value: 'Item nr 1'
+      }]
+  }),
+  Object.assign(new Item(), {
+    metadata: [
+      {
+        key: 'dc.title',
+        language: 'en_US',
+        value: 'Item nr 2'
+      }]
+  })
+];
+const paginatedList = new PaginatedList(pageInfo, array);
+const arrayRD = new RemoteData(false, false, true, undefined, array);
+const paginatedListRD = new RemoteData(false, false, true, undefined, paginatedList);
+
+describe('RemoteDataBuildService', () => {
+  let service: RemoteDataBuildService;
+
+  beforeEach(() => {
+    service = new RemoteDataBuildService(undefined, undefined, undefined);
+  });
+
+  describe('when toPaginatedList is called', () => {
+    let expected: RemoteData<PaginatedList<Item>>;
+
+    beforeEach(() => {
+      expected = paginatedListRD;
+    });
+
+    it('should return the correct remoteData of a paginatedList when the input is a (remoteData of an) array', () => {
+      const result = (service as any).toPaginatedList(Observable.of(arrayRD), pageInfo);
+      result.subscribe((resultRD) => {
+        expect(resultRD).toEqual(expected);
+      });
+    });
+
+    it('should return the correct remoteData of a paginatedList when the input is a (remoteData of a) paginated list', () => {
+      const result = (service as any).toPaginatedList(Observable.of(paginatedListRD), pageInfo);
+      result.subscribe((resultRD) => {
+        expect(resultRD).toEqual(expected);
+      });
+    });
+  });
+});

src/app/core/cache/builders/remote-data-build.service.ts

@@ -1,6 +1,6 @@
 import { Injectable } from '@angular/core';
 import { Observable } from 'rxjs/Observable';
-import { distinctUntilChanged, flatMap, map, startWith } from 'rxjs/operators';
+import { distinctUntilChanged, filter, flatMap, map, startWith, switchMap } from 'rxjs/operators';
 import { hasValue, hasValueOperator, isEmpty, isNotEmpty } from '../../../shared/empty.util';
 import { PaginatedList } from '../../data/paginated-list';
 import { RemoteData } from '../../data/remote-data';
@@ -190,7 +190,7 @@ export class RemoteDataBuildService {
         }
 
         if (hasValue(normalized[relationship].page)) {
-          links[relationship] = this.aggregatePaginatedList(result, normalized[relationship].pageInfo);
+          links[relationship] = this.toPaginatedList(result, normalized[relationship].pageInfo);
         } else {
           links[relationship] = result;
         }
@@ -254,8 +254,14 @@ export class RemoteDataBuildService {
       })
   }
 
-  aggregatePaginatedList<T>(input: Observable<RemoteData<T[]>>, pageInfo: PageInfo): Observable<RemoteData<PaginatedList<T>>> {
+  private toPaginatedList<T>(input: Observable<RemoteData<T[] | PaginatedList<T>>>, pageInfo: PageInfo): Observable<RemoteData<PaginatedList<T>>> {
-    return input.map((rd) => Object.assign(rd, {payload: new PaginatedList(pageInfo, rd.payload)}));
+    return input.map((rd: RemoteData<T[] | PaginatedList<T>>) => {
+      if (Array.isArray(rd.payload)) {
+        return Object.assign(rd, { payload: new PaginatedList(pageInfo, rd.payload) })
+      } else {
+        return Object.assign(rd, { payload: new PaginatedList(pageInfo, rd.payload.page) });
+      }
+    });
   }
 
 }

src/app/core/data/base-response-parsing.service.ts

@@ -6,7 +6,6 @@ import { ObjectCacheService } from '../cache/object-cache.service';
 import { GlobalConfig } from '../../../config/global-config.interface';
 import { GenericConstructor } from '../shared/generic-constructor';
 import { PaginatedList } from './paginated-list';
-import { NormalizedObject } from '../cache/models/normalized-object.model';
 import { ResourceType } from '../shared/resource-type';
 import { RESTURLCombiner } from '../url-combiner/rest-url-combiner';
 
@@ -15,7 +14,7 @@ function isObjectLevel(halObj: any) {
 }
 
 function isPaginatedResponse(halObj: any) {
-  return isNotEmpty(halObj.page) && hasValue(halObj._embedded);
+  return hasValue(halObj.page) && hasValue(halObj._embedded);
 }
 
 /* tslint:disable:max-classes-per-file */
@@ -130,7 +129,7 @@ export abstract class BaseResponseParsingService {
   }
 
   processPageInfo(payload: any): PageInfo {
-    if (isNotEmpty(payload.page)) {
+    if (hasValue(payload.page)) {
       const pageObj = Object.assign({}, payload.page, { _links: payload._links });
       const pageInfoObject = new DSpaceRESTv2Serializer(PageInfo).deserialize(pageObj);
       if (pageInfoObject.currentPage >= 0) {

src/app/core/metadata/metadata.service.spec.ts

@@ -34,6 +34,9 @@ import { MockItem } from '../../shared/mocks/mock-item';
 import { MockTranslateLoader } from '../../shared/mocks/mock-translate-loader';
 import { BrowseService } from '../browse/browse.service';
 import { HALEndpointService } from '../shared/hal-endpoint.service';
+import { PaginatedList } from '../data/paginated-list';
+import { PageInfo } from '../shared/page-info.model';
+import { EmptyError } from 'rxjs/util/EmptyError';
 
 /* tslint:disable:max-classes-per-file */
 @Component({
@@ -181,6 +184,22 @@ describe('MetadataService', () => {
     expect(tagStore.get('description')[0].content).toEqual('This is a dummy item component for testing!');
   }));
 
+  describe('when the item has no bitstreams', () => {
+
+    beforeEach(() => {
+      spyOn(MockItem, 'getFiles').and.returnValue(Observable.of([]));
+    });
+
+    it('processRemoteData should not produce an EmptyError', fakeAsync(() => {
+      spyOn(itemDataService, 'findById').and.returnValue(mockRemoteData(MockItem));
+      spyOn(metadataService, 'processRemoteData').and.callThrough();
+      router.navigate(['/items/0ec7ff22-f211-40ab-a69e-c819b0b1f357']);
+      tick();
+      expect(metadataService.processRemoteData).not.toThrow(new EmptyError());
+    }));
+
+  });
+
   const mockRemoteData = (mockItem: Item): Observable<RemoteData<Item>> => {
     return Observable.of(new RemoteData<Item>(
       false,

src/app/core/metadata/metadata.service.ts

@@ -269,8 +269,11 @@ export class MetadataService {
   private setCitationPdfUrlTag(): void {
     if (this.currentObject.value instanceof Item) {
       const item = this.currentObject.value as Item;
-      item.getFiles().filter((files) => isNotEmpty(files)).first().subscribe((bitstreams: Bitstream[]) => {
+      item.getFiles()
-        for (const bitstream of bitstreams) {
+        .first((files) => isNotEmpty(files))
+        .catch((error) => { console.debug(error); return [] })
+        .subscribe((bitstreams: Bitstream[]) => {
+          for (const bitstream of bitstreams) {
           bitstream.format.first()
             .map((rd: RemoteData<BitstreamFormat>) => rd.payload)
             .filter((format: BitstreamFormat) => hasValue(format))

src/app/core/shared/item.model.ts

@@ -1,4 +1,5 @@
 import { Observable } from 'rxjs/Observable';
+import { filter, map, startWith, tap } from 'rxjs/operators';
 
 import { DSpaceObject } from './dspace-object.model';
 import { Collection } from './collection.model';

src/app/core/shared/operators.ts

@@ -59,7 +59,7 @@ export const toDSpaceObjectListRD = () =>
     source.pipe(
       map((rd: RemoteData<PaginatedList<SearchResult<T>>>) => {
         const dsoPage: T[] = rd.payload.page.map((searchResult: SearchResult<T>) => searchResult.dspaceObject);
-        const payload = Object.assign(rd.payload, { page: dsoPage }) as PaginatedList<T>;
+        const payload = Object.assign(rd.payload, { page: dsoPage }) as any;
         return Object.assign(rd, {payload: payload});
       })
     );

src/app/shared/loading/loading.component.ts

@@ -3,6 +3,7 @@ import { Component, Input, OnDestroy, OnInit } from '@angular/core';
 import { TranslateService } from '@ngx-translate/core';
 
 import { Subscription } from 'rxjs/Subscription';
+import { hasValue } from '../empty.util';
 
 @Component({
   selector: 'ds-loading',
@@ -28,7 +29,7 @@ export class LoadingComponent implements OnDestroy, OnInit {
   }
 
   ngOnDestroy() {
-    if (this.subscription !== undefined) {
+    if (hasValue(this.subscription)) {
       this.subscription.unsubscribe();
     }
   }

src/app/shared/testing/query-params-directive-stub.ts

@@ -0,0 +1,10 @@
+import { Directive, Input } from '@angular/core';
+
+/* tslint:disable:directive-class-suffix */
+@Directive({
+  // tslint:disable-next-line:directive-selector
+  selector: '[queryParams]',
+})
+export class QueryParamsDirectiveStub {
+  @Input('queryParams') queryParams: any;
+}

src/app/shared/testing/test-module.ts

@@ -0,0 +1,15 @@
+import { NgModule } from '@angular/core';
+import { QueryParamsDirectiveStub } from './query-params-directive-stub';
+
+/**
+ * This module isn't used. It serves to prevent the AoT compiler
+ * complaining about components/pipes/directives that were
+ * created only for use in tests.
+ * See https://github.com/angular/angular/issues/13590
+ */
+@NgModule({
+  declarations: [
+    QueryParamsDirectiveStub
+  ]
+})
+export class TestModule {}