diff --git a/resources/i18n/en.json b/resources/i18n/en.json index b6a23068d7..169ff0efe9 100644 --- a/resources/i18n/en.json +++ b/resources/i18n/en.json @@ -92,7 +92,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", diff --git a/src/app/+search-page/search-results/search-results.component.html b/src/app/+search-page/search-results/search-results.component.html index ed6fc18d9c..4915b552c3 100644 --- a/src/app/+search-page/search-results/search-results.component.html +++ b/src/app/+search-page/search-results/search-results.component.html @@ -7,5 +7,12 @@ [hideGear]="true"> - - + +
+ {{ 'search.results.no-results' | translate }} + + {{"search.results.no-results-link" | translate}} + +
diff --git a/src/app/+search-page/search-results/search-results.component.spec.ts b/src/app/+search-page/search-results/search-results.component.spec.ts index 4f299c5c50..54463d916d 100644 --- a/src/app/+search-page/search-results/search-results.component.spec.ts +++ b/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; - let heading: DebugElement; beforeEach(async(() => { TestBed.configureTestingModule({ - imports: [TranslateModule.forRoot()], - declarations: [SearchResultsComponent], + imports: [TranslateModule.forRoot(), NoopAnimationsModule], + declarations: [ + SearchResultsComponent, + QueryParamsDirectiveStub], schemas: [NO_ERRORS_SCHEMA] }).compileComponents(); })); beforeEach(() => { fixture = TestBed.createComponent(SearchResultsComponent); - comp = fixture.componentInstance; // SearchFormComponent test instance - heading = fixture.debugElement.query(By.css('heading')); + comp = fixture.componentInstance; // SearchResultsComponent test instance }); - it('should display heading when results are not empty', fakeAsync(() => { - (comp as any).searchResults = 'test'; - (comp as any).searchConfig = {pagination: ''}; + it('should display results when results are not empty', () => { + (comp as any).searchResults = { hasSucceeded: true, isLoading: false, payload: { page: { length: 2 } } }; + (comp as any).searchConfig = {}; fixture.detectChanges(); - tick(); - expect(heading).toBeDefined(); - })); + expect(fixture.debugElement.query(By.css('ds-viewable-collection'))).not.toBeNull(); + }); - it('should not display heading when results is empty', () => { - expect(heading).toBeNull(); + it('should not display link when results are not empty', () => { + (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(''); }); }); diff --git a/src/app/+search-page/search-results/search-results.component.ts b/src/app/+search-page/search-results/search-results.component.ts index 6399243f92..ae0abfcd27 100644 --- a/src/app/+search-page/search-results/search-results.component.ts +++ b/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; + } } diff --git a/src/app/core/cache/builders/remote-data-build.service.spec.ts b/src/app/core/cache/builders/remote-data-build.service.spec.ts new file mode 100644 index 0000000000..bab628a7fe --- /dev/null +++ b/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>; + + 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); + }); + }); + }); +}); diff --git a/src/app/core/cache/builders/remote-data-build.service.ts b/src/app/core/cache/builders/remote-data-build.service.ts index fe7f56220f..3a612df8a2 100644 --- a/src/app/core/cache/builders/remote-data-build.service.ts +++ b/src/app/core/cache/builders/remote-data-build.service.ts @@ -1,7 +1,7 @@ import { combineLatest as observableCombineLatest, - of as observableOf, Observable, + of as observableOf, race as observableRace } from 'rxjs'; import { Injectable } from '@angular/core'; @@ -22,10 +22,10 @@ import { ResponseCacheService } from '../response-cache.service'; import { getMapsTo, getRelationMetadata, getRelationships } from './build-decorators'; import { PageInfo } from '../../shared/page-info.model'; import { + filterSuccessfulResponses, getRequestFromSelflink, getResourceLinksFromResponse, - getResponseFromSelflink, - filterSuccessfulResponses + getResponseFromSelflink } from '../../shared/operators'; @Injectable() @@ -198,7 +198,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; } @@ -261,8 +261,14 @@ export class RemoteDataBuildService { })) } - aggregatePaginatedList(input: Observable>, pageInfo: PageInfo): Observable>> { - return input.pipe(map((rd) => Object.assign(rd, { payload: new PaginatedList(pageInfo, rd.payload) }))); + private toPaginatedList(input: Observable>>, pageInfo: PageInfo): Observable>> { + return input.map((rd: RemoteData>) => { + 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) }); + } + }); } } diff --git a/src/app/core/data/base-response-parsing.service.ts b/src/app/core/data/base-response-parsing.service.ts index fdf5b4eb97..63468295d4 100644 --- a/src/app/core/data/base-response-parsing.service.ts +++ b/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) { diff --git a/src/app/core/metadata/metadata.service.spec.ts b/src/app/core/metadata/metadata.service.spec.ts index 50ce4711ff..1493a543f1 100644 --- a/src/app/core/metadata/metadata.service.spec.ts +++ b/src/app/core/metadata/metadata.service.spec.ts @@ -32,6 +32,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({ @@ -179,6 +182,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> => { return observableOf(new RemoteData( false, diff --git a/src/app/core/metadata/metadata.service.ts b/src/app/core/metadata/metadata.service.ts index ede66f6952..3c2302b03f 100644 --- a/src/app/core/metadata/metadata.service.ts +++ b/src/app/core/metadata/metadata.service.ts @@ -259,11 +259,14 @@ export class MetadataService { private setCitationPdfUrlTag(): void { if (this.currentObject.value instanceof Item) { const item = this.currentObject.value as Item; - item.getFiles().pipe(filter((files) => isNotEmpty(files)),first(),).subscribe((bitstreams: Bitstream[]) => { - for (const bitstream of bitstreams) { - bitstream.format.pipe(first(), - map((rd: RemoteData) => rd.payload), - filter((format: BitstreamFormat) => hasValue(format)),) + item.getFiles() + .first((files) => isNotEmpty(files)) + .catch((error) => { console.debug(error); return [] }) + .subscribe((bitstreams: Bitstream[]) => { + for (const bitstream of bitstreams) { + bitstream.format.first() + .map((rd: RemoteData) => rd.payload) + .filter((format: BitstreamFormat) => hasValue(format)) .subscribe((format: BitstreamFormat) => { if (format.mimetype === 'application/pdf') { this.addMetaTag('citation_pdf_url', bitstream.content); diff --git a/src/app/core/shared/operators.ts b/src/app/core/shared/operators.ts index 476119399b..9622bb91e1 100644 --- a/src/app/core/shared/operators.ts +++ b/src/app/core/shared/operators.ts @@ -59,7 +59,7 @@ export const toDSpaceObjectListRD = () => source.pipe( map((rd: RemoteData>>) => { const dsoPage: T[] = rd.payload.page.map((searchResult: SearchResult) => searchResult.dspaceObject); - const payload = Object.assign(rd.payload, { page: dsoPage }) as PaginatedList; + const payload = Object.assign(rd.payload, { page: dsoPage }) as any; return Object.assign(rd, {payload: payload}); }) ); diff --git a/src/app/shared/loading/loading.component.ts b/src/app/shared/loading/loading.component.ts index b827bcc683..fec05b4720 100644 --- a/src/app/shared/loading/loading.component.ts +++ b/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'; +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(); } } diff --git a/src/app/shared/testing/query-params-directive-stub.ts b/src/app/shared/testing/query-params-directive-stub.ts new file mode 100644 index 0000000000..c19c5e6a5f --- /dev/null +++ b/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; +} diff --git a/src/app/shared/testing/test-module.ts b/src/app/shared/testing/test-module.ts new file mode 100644 index 0000000000..03d22640d3 --- /dev/null +++ b/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 {}