Merge pull request #1419 from 4Science/CST-4882

[CST-4882] The search loads indefinitely if the written query is invalid
This commit is contained in:
Tim Donohue
2021-12-14 10:39:01 -06:00
committed by GitHub
14 changed files with 70 additions and 25 deletions

View File

@@ -4,7 +4,7 @@
<ds-search-sidebar *ngIf="!(isXsOrSm$ | async)" class="col-3 sidebar-md-sticky"
id="search-sidebar"
[configurationList]="(configurationList$ | async)"
[resultCount]="(resultsRD$ | async)?.payload.totalElements"
[resultCount]="(resultsRD$ | async)?.payload?.totalElements"
[viewModeList]="viewModeList"
[searchOptions]="(searchOptions$ | async)"
[sortOptions]="(sortOptions$ | async)"
@@ -27,7 +27,7 @@
<ds-search-sidebar *ngIf="(isXsOrSm$ | async)" class="col-12"
id="search-sidebar-sm"
[configurationList]="(configurationList$ | async)"
[resultCount]="(resultsRD$ | async)?.payload.totalElements"
[resultCount]="(resultsRD$ | async)?.payload?.totalElements"
(toggleSidebar)="closeSidebar()"
[ngClass]="{'active': !(isSidebarCollapsed() | async)}"
[searchOptions]="(searchOptions$ | async)"

View File

@@ -19,7 +19,7 @@ import { PaginatedSearchOptions } from '../shared/search/paginated-search-option
import { SearchService } from '../core/shared/search/search.service';
import { SidebarService } from '../shared/sidebar/sidebar.service';
import { hasValue } from '../shared/empty.util';
import { getFirstSucceededRemoteData } from '../core/shared/operators';
import { getFirstCompletedRemoteData } from '../core/shared/operators';
import { MyDSpaceResponseParsingService } from '../core/data/mydspace-response-parsing.service';
import { SearchConfigurationOption } from '../shared/search/search-switch-configuration/search-configuration-option.model';
import { RoleType } from '../core/roles/role-types';
@@ -30,7 +30,7 @@ import { MyDSpaceRequest } from '../core/data/request.models';
import { SearchResult } from '../shared/search/search-result.model';
import { Context } from '../core/shared/context.model';
import { SortOptions } from '../core/cache/models/sort-options.model';
import { RouteService } from '../core/services/route.service';
import { SearchObjects } from '../shared/search/search-objects.model';
export const MYDSPACE_ROUTE = '/mydspace';
export const SEARCH_CONFIG_SERVICE: InjectionToken<SearchConfigurationService> = new InjectionToken<SearchConfigurationService>('searchConfigurationService');
@@ -111,8 +111,7 @@ export class MyDSpacePageComponent implements OnInit {
constructor(private service: SearchService,
private sidebarService: SidebarService,
private windowService: HostWindowService,
@Inject(SEARCH_CONFIG_SERVICE) public searchConfigService: MyDSpaceConfigurationService,
private routeService: RouteService) {
@Inject(SEARCH_CONFIG_SERVICE) public searchConfigService: MyDSpaceConfigurationService) {
this.isXsOrSm$ = this.windowService.isXsOrSm();
this.service.setServiceOptions(MyDSpaceResponseParsingService, MyDSpaceRequest);
}
@@ -134,8 +133,8 @@ export class MyDSpacePageComponent implements OnInit {
this.searchOptions$ = this.searchConfigService.paginatedSearchOptions;
this.sub = this.searchOptions$.pipe(
tap(() => this.resultsRD$.next(null)),
switchMap((options: PaginatedSearchOptions) => this.service.search(options).pipe(getFirstSucceededRemoteData())))
.subscribe((results) => {
switchMap((options: PaginatedSearchOptions) => this.service.search(options).pipe(getFirstCompletedRemoteData())))
.subscribe((results: RemoteData<SearchObjects<DSpaceObject>>) => {
this.resultsRD$.next(results);
});

View File

@@ -10,5 +10,5 @@
</ds-viewable-collection>
</div>
<ds-loading *ngIf="isLoading()" message="{{'loading.mydspace-results' | translate}}"></ds-loading>
<ds-error *ngIf="searchResults?.hasFailed && (!searchResults?.errorMessage || searchResults?.statusCode != 400)" message="{{'error.search-results' | translate}}"></ds-error>
<ds-error *ngIf="showError()" message="{{errorMessageLabel() | translate}}"></ds-error>
<h3 *ngIf="searchResults?.payload?.page.length == 0" class="text-center text-muted" ><span>{{'mydspace.results.no-results' | translate}}</span></h3>

View File

@@ -40,9 +40,19 @@ describe('MyDSpaceResultsComponent', () => {
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 } };
it('should display error message if error is 500', () => {
(comp as any).searchResults = { hasFailed: true, statusCode: 500 };
fixture.detectChanges();
expect(comp.showError()).toBeTrue();
expect(comp.errorMessageLabel()).toBe('error.search-results');
expect(fixture.debugElement.query(By.css('ds-error'))).not.toBeNull();
});
it('should display error message if error is 422', () => {
(comp as any).searchResults = { hasFailed: true, statusCode: 422 };
fixture.detectChanges();
expect(comp.showError()).toBeTrue();
expect(comp.errorMessageLabel()).toBe('error.invalid-search-query');
expect(fixture.debugElement.query(By.css('ds-error'))).not.toBeNull();
});

View File

@@ -58,4 +58,12 @@ export class MyDSpaceResultsComponent {
isLoading() {
return !this.searchResults || isEmpty(this.searchResults) || this.searchResults.isLoading;
}
showError(): boolean {
return this.searchResults?.hasFailed && (!this.searchResults?.errorMessage || this.searchResults?.statusCode !== 400);
}
errorMessageLabel(): string {
return (this.searchResults?.statusCode === 422) ? 'error.invalid-search-query' : 'error.search-results';
}
}

View File

@@ -8,7 +8,7 @@ import { pushInOut } from '../shared/animations/push';
import { HostWindowService } from '../shared/host-window.service';
import { SidebarService } from '../shared/sidebar/sidebar.service';
import { hasValue, isEmpty } from '../shared/empty.util';
import { getFirstSucceededRemoteData } from '../core/shared/operators';
import { getFirstCompletedRemoteData } from '../core/shared/operators';
import { RouteService } from '../core/services/route.service';
import { SEARCH_CONFIG_SERVICE } from '../my-dspace-page/my-dspace-page.component';
import { PaginatedSearchOptions } from '../shared/search/paginated-search-options.model';
@@ -126,8 +126,8 @@ export class SearchComponent implements OnInit {
this.searchOptions$ = this.getSearchOptions();
this.sub = this.searchOptions$.pipe(
switchMap((options) => this.service.search(
options, undefined, true, true, followLink<Item>('thumbnail', { isOptional: true })
).pipe(getFirstSucceededRemoteData(), startWith(undefined))
options, undefined, false, true, followLink<Item>('thumbnail', { isOptional: true })
).pipe(getFirstCompletedRemoteData(), startWith(undefined))
)
).subscribe((results) => {
this.resultsRD$.next(results);

View File

@@ -1,3 +1,4 @@
<div>
<label>{{ message }}</label>
</div>
<ds-alert [type]="AlertTypeEnum.Error" [dismissible]="false">
<!-- Using [innerHTML] instead of {{message}} allows to render HTML code -->
<span [innerHTML]="message"></span>
</ds-alert>

View File

@@ -36,7 +36,7 @@ describe('ErrorComponent (inline template)', () => {
comp = fixture.componentInstance; // ErrorComponent test instance
// query for the message <label> by CSS element selector
de = fixture.debugElement.query(By.css('label'));
de = fixture.debugElement.query(By.css('ds-alert'));
el = de.nativeElement;
});

View File

@@ -3,6 +3,7 @@ import { Component, Input } from '@angular/core';
import { TranslateService } from '@ngx-translate/core';
import { Subscription } from 'rxjs';
import { AlertType } from '../alert/aletr-type';
@Component({
selector: 'ds-error',
@@ -13,6 +14,12 @@ export class ErrorComponent {
@Input() message = 'Error...';
/**
* The AlertType enumeration
* @type {AlertType}
*/
public AlertTypeEnum = AlertType;
private subscription: Subscription;
constructor(private translate: TranslateService) {

View File

@@ -169,7 +169,7 @@ export class SearchFilterComponent implements OnInit {
return this.searchService.getFacetValuesFor(this.filter, 1, options).pipe(
filter((RD) => !RD.isLoading),
map((valuesRD) => {
return valuesRD.payload.totalElements > 0;
return valuesRD.payload?.totalElements > 0;
}),);
}
));

View File

@@ -16,11 +16,11 @@
</ds-viewable-collection>
</div>
<ds-loading
*ngIf="hasNoValue(searchResults) || hasNoValue(searchResults.payload) || searchResults.isLoading"
*ngIf="!showError() && (hasNoValue(searchResults) || hasNoValue(searchResults.payload) || searchResults.isLoading)"
message="{{'loading.search-results' | translate}}"></ds-loading>
<ds-error
*ngIf="searchResults?.hasFailed && (!searchResults?.errorMessage || searchResults?.statusCode != 400)"
message="{{'error.search-results' | translate}}"></ds-error>
*ngIf="showError()"
message="{{errorMessageLabel() | translate}}"></ds-error>
<div *ngIf="searchResults?.payload?.page.length == 0 || searchResults?.statusCode == 400">
{{ 'search.results.no-results' | translate }}
<a [routerLink]="['/search']"

View File

@@ -45,9 +45,19 @@ describe('SearchResultsComponent', () => {
expect(fixture.debugElement.query(By.css('a'))).toBeNull();
});
it('should display error message if error is != 400', () => {
it('should display error message if error is 500', () => {
(comp as any).searchResults = createFailedRemoteDataObject('Error', 500);
fixture.detectChanges();
expect(comp.showError()).toBeTrue();
expect(comp.errorMessageLabel()).toBe('error.search-results');
expect(fixture.debugElement.query(By.css('ds-error'))).not.toBeNull();
});
it('should display error message if error is 422', () => {
(comp as any).searchResults = createFailedRemoteDataObject('Error', 422);
fixture.detectChanges();
expect(comp.showError()).toBeTrue();
expect(comp.errorMessageLabel()).toBe('error.invalid-search-query');
expect(fixture.debugElement.query(By.css('ds-error'))).not.toBeNull();
});

View File

@@ -78,6 +78,14 @@ export class SearchResultsComponent {
@Output() selectObject: EventEmitter<ListableObject> = new EventEmitter<ListableObject>();
showError(): boolean {
return this.searchResults?.hasFailed && (!this.searchResults?.errorMessage || this.searchResults?.statusCode !== 400);
}
errorMessageLabel(): string {
return (this.searchResults?.statusCode === 422) ? 'error.invalid-search-query' : 'error.search-results';
}
/**
* Method to change the given string by surrounding it by quotes if not already present.
*/

View File

@@ -1331,6 +1331,8 @@
"error.search-results": "Error fetching search results",
"error.invalid-search-query": "Search query is not valid. Please check <a href=\"https://solr.apache.org/guide/query-syntax-and-parsing.html\" target=\"_blank\">Solr query syntax</a> best practices for further information about this error.",
"error.sub-collections": "Error fetching sub-collections",
"error.sub-communities": "Error fetching sub-communities",