79252: Fix dso-selector issues

2025-10-07 01:54:15 +00:00 · 2021-05-06 18:03:46 +02:00
parent a69033917d
commit 86bf7ff4f2
6 changed files with 100 additions and 51 deletions
--- a/src/app/shared/dso-selector/dso-selector/authorized-collection-selector/authorized-collection-selector.component.spec.ts
+++ b/src/app/shared/dso-selector/dso-selector/authorized-collection-selector/authorized-collection-selector.component.spec.ts
@@ -10,6 +10,7 @@ import { createSuccessfulRemoteDataObject$ } from '../../../remote-data.utils';
 import { createPaginatedList } from '../../../testing/utils.test';
 import { Collection } from '../../../../core/shared/collection.model';
 import { DSpaceObjectType } from '../../../../core/shared/dspace-object-type.model';
+import { NotificationsService } from '../../../notifications/notifications.service';

 describe('AuthorizedCollectionSelectorComponent', () => {
  let component: AuthorizedCollectionSelectorComponent;
@@ -18,6 +19,8 @@ describe('AuthorizedCollectionSelectorComponent', () => {
  let collectionService;
  let collection;

+  let notificationsService: NotificationsService;
+
  beforeEach(waitForAsync(() => {
    collection = Object.assign(new Collection(), {
      id: 'authorized-collection'
@@ -25,12 +28,14 @@ describe('AuthorizedCollectionSelectorComponent', () => {
    collectionService = jasmine.createSpyObj('collectionService', {
      getAuthorizedCollection: createSuccessfulRemoteDataObject$(createPaginatedList([collection]))
    });
+    notificationsService = jasmine.createSpyObj('notificationsService', ['error']);
    TestBed.configureTestingModule({
      declarations: [AuthorizedCollectionSelectorComponent, VarDirective],
      imports: [TranslateModule.forRoot(), RouterTestingModule.withRoutes([])],
      providers: [
        { provide: SearchService, useValue: {} },
-        { provide: CollectionDataService, useValue: collectionService }
+        { provide: CollectionDataService, useValue: collectionService },
+        { provide: NotificationsService, useValue: notificationsService },
      ],
      schemas: [NO_ERRORS_SCHEMA]
    }).compileComponents();
@@ -45,10 +50,10 @@ describe('AuthorizedCollectionSelectorComponent', () => {

  describe('search', () => {
    it('should call getAuthorizedCollection and return the authorized collection in a SearchResult', (done) => {
-      component.search('', 1).subscribe((result) => {
+      component.search('', 1).subscribe((resultRD) => {
        expect(collectionService.getAuthorizedCollection).toHaveBeenCalled();
-        expect(result.page.length).toEqual(1);
-        expect(result.page[0].indexableObject).toEqual(collection);
+        expect(resultRD.payload.page.length).toEqual(1);
+        expect(resultRD.payload.page[0].indexableObject).toEqual(collection);
        done();
      });
    });
--- a/src/app/shared/dso-selector/dso-selector/authorized-collection-selector/authorized-collection-selector.component.ts
+++ b/src/app/shared/dso-selector/dso-selector/authorized-collection-selector/authorized-collection-selector.component.ts
@@ -3,13 +3,17 @@ import { DSOSelectorComponent } from '../dso-selector.component';
 import { SearchService } from '../../../../core/shared/search/search.service';
 import { CollectionDataService } from '../../../../core/data/collection-data.service';
 import { Observable } from 'rxjs/internal/Observable';
-import { getFirstSucceededRemoteDataPayload } from '../../../../core/shared/operators';
+import { getFirstCompletedRemoteData, getFirstSucceededRemoteDataPayload } from '../../../../core/shared/operators';
 import { map } from 'rxjs/operators';
 import { CollectionSearchResult } from '../../../object-collection/shared/collection-search-result.model';
 import { SearchResult } from '../../../search/search-result.model';
 import { DSpaceObject } from '../../../../core/shared/dspace-object.model';
 import { buildPaginatedList, PaginatedList } from '../../../../core/data/paginated-list.model';
 import { followLink } from '../../../utils/follow-link-config.model';
+import { RemoteData } from '../../../../core/data/remote-data';
+import { hasValue } from '../../../empty.util';
+import { NotificationsService } from '../../../notifications/notifications.service';
+import { TranslateService } from '@ngx-translate/core';

@Component({
  selector: 'ds-authorized-collection-selector',
@@ -21,8 +25,10 @@ import { followLink } from '../../../utils/follow-link-config.model';
 */
 export class AuthorizedCollectionSelectorComponent extends DSOSelectorComponent {
  constructor(protected searchService: SearchService,
-              protected collectionDataService: CollectionDataService) {
-    super(searchService);
+              protected collectionDataService: CollectionDataService,
+              protected notifcationsService: NotificationsService,
+              protected translate: TranslateService) {
+    super(searchService, notifcationsService, translate);
  }

  /**
@@ -37,13 +43,15 @@ export class AuthorizedCollectionSelectorComponent extends DSOSelectorComponent
   * @param query Query to search objects for
   * @param page  Page to retrieve
   */
-  search(query: string, page: number): Observable<PaginatedList<SearchResult<DSpaceObject>>> {
+  search(query: string, page: number): Observable<RemoteData<PaginatedList<SearchResult<DSpaceObject>>>> {
    return this.collectionDataService.getAuthorizedCollection(query, Object.assign({
      currentPage: page,
      elementsPerPage: this.defaultPagination.pageSize
    }),true, false, followLink('parentCommunity')).pipe(
-      getFirstSucceededRemoteDataPayload(),
-      map((list) => buildPaginatedList(list.pageInfo, list.page.map((col) => Object.assign(new CollectionSearchResult(), { indexableObject: col }))))
+      getFirstCompletedRemoteData(),
+      map((rd) => Object.assign(new RemoteData(null, null, null, null), rd, {
+        payload: hasValue(rd.payload) ? buildPaginatedList(rd.payload.pageInfo, rd.payload.page.map((col) => Object.assign(new CollectionSearchResult(), { indexableObject: col }))) : null,
+      }))
    );
  }
 }
--- a/src/app/shared/dso-selector/dso-selector/dso-selector.component.html
+++ b/src/app/shared/dso-selector/dso-selector/dso-selector.component.html
@@ -10,24 +10,26 @@
  <div
    infiniteScroll
    [infiniteScrollDistance]="1"
-    [infiniteScrollThrottle]="300"
+    [infiniteScrollThrottle]="0"
    [infiniteScrollContainer]="'.scrollable-menu'"
    [fromRoot]="true"
    (scrolled)="onScrollDown()">
-    <button class="list-group-item list-group-item-action border-0 disabled"
-            *ngIf="listEntries.length == 0">
+    <ng-container *ngIf="listEntries">
+      <button class="list-group-item list-group-item-action border-0 disabled"
+              *ngIf="listEntries.length == 0">
        {{'dso-selector.no-results' | translate: { type: typesString } }}
-    </button>
-    <button *ngFor="let listEntry of listEntries"
-            class="list-group-item list-group-item-action border-0 list-entry"
-            [ngClass]="{'bg-primary': listEntry.indexableObject.id === currentDSOId}"
-            title="{{ listEntry.indexableObject.name }}"
-            dsHoverClass="ds-hover"
-            (click)="onSelect.emit(listEntry.indexableObject)" #listEntryElement>
-      <ds-listable-object-component-loader [object]="listEntry" [viewMode]="viewMode"
-                                           [linkType]=linkTypes.None [context]="getContext(listEntry.indexableObject.id)"></ds-listable-object-component-loader>
-    </button>
-    <button *ngIf="hasNextPage"
+      </button>
+      <button *ngFor="let listEntry of listEntries"
+              class="list-group-item list-group-item-action border-0 list-entry"
+              [ngClass]="{'bg-primary': listEntry.indexableObject.id === currentDSOId}"
+              title="{{ listEntry.indexableObject.name }}"
+              dsHoverClass="ds-hover"
+              (click)="onSelect.emit(listEntry.indexableObject)" #listEntryElement>
+        <ds-listable-object-component-loader [object]="listEntry" [viewMode]="viewMode"
+                                             [linkType]=linkTypes.None [context]="getContext(listEntry.indexableObject.id)"></ds-listable-object-component-loader>
+      </button>
+    </ng-container>
+    <button *ngIf="loading"
            class="list-group-item list-group-item-action border-0 list-entry">
      <ds-loading [showMessage]="false"></ds-loading>
    </button>
--- a/src/app/shared/dso-selector/dso-selector/dso-selector.component.spec.ts
+++ b/src/app/shared/dso-selector/dso-selector/dso-selector.component.spec.ts
@@ -6,10 +6,11 @@ import { SearchService } from '../../../core/shared/search/search.service';
 import { DSpaceObjectType } from '../../../core/shared/dspace-object-type.model';
 import { ItemSearchResult } from '../../object-collection/shared/item-search-result.model';
 import { Item } from '../../../core/shared/item.model';
-import { createSuccessfulRemoteDataObject$ } from '../../remote-data.utils';
+import { createFailedRemoteDataObject$, createSuccessfulRemoteDataObject$ } from '../../remote-data.utils';
 import { PaginatedSearchOptions } from '../../search/paginated-search-options.model';
 import { hasValue } from '../../empty.util';
 import { createPaginatedList } from '../../testing/utils.test';
+import { NotificationsService } from '../../notifications/notifications.service';

 describe('DSOSelectorComponent', () => {
  let component: DSOSelectorComponent;
@@ -59,12 +60,17 @@ describe('DSOSelectorComponent', () => {
    });
  }

+  let notificationsService: NotificationsService;
+
  beforeEach(waitForAsync(() => {
+    notificationsService = jasmine.createSpyObj('notificationsService', ['error']);
+
    TestBed.configureTestingModule({
      imports: [TranslateModule.forRoot()],
      declarations: [DSOSelectorComponent],
      providers: [
        { provide: SearchService, useValue: searchService },
+        { provide: NotificationsService, useValue: notificationsService },
      ],
      schemas: [NO_ERRORS_SCHEMA]
    }).compileComponents();
@@ -104,4 +110,15 @@ describe('DSOSelectorComponent', () => {
      });
    });
  });
+
+  describe('when search returns an error', () => {
+    beforeEach(() => {
+      spyOn(searchService, 'search').and.returnValue(createFailedRemoteDataObject$());
+      component.ngOnInit();
+    });
+
+    it('should display an error notification', () => {
+      expect(notificationsService.error).toHaveBeenCalled();
+    });
+  });
 });
--- a/src/app/shared/dso-selector/dso-selector/dso-selector.component.ts
+++ b/src/app/shared/dso-selector/dso-selector/dso-selector.component.ts
@@ -27,10 +27,13 @@ import { DSpaceObjectType } from '../../../core/shared/dspace-object-type.model'
 import { DSpaceObject } from '../../../core/shared/dspace-object.model';
 import { ViewMode } from '../../../core/shared/view-mode.model';
 import { Context } from '../../../core/shared/context.model';
-import { getFirstSucceededRemoteDataPayload } from '../../../core/shared/operators';
-import { hasValue, isEmpty, isNotEmpty } from '../../empty.util';
+import { getFirstCompletedRemoteData, getFirstSucceededRemoteDataPayload } from '../../../core/shared/operators';
+import { hasNoValue, hasValue, isEmpty, isNotEmpty } from '../../empty.util';
 import { PaginatedList, buildPaginatedList } from '../../../core/data/paginated-list.model';
 import { SearchResult } from '../../search/search-result.model';
+import { RemoteData } from '../../../core/data/remote-data';
+import { NotificationsService } from '../../notifications/notifications.service';
+import { TranslateService } from '@ngx-translate/core';

@Component({
  selector: 'ds-dso-selector',
@@ -78,7 +81,7 @@ export class DSOSelectorComponent implements OnInit, OnDestroy {
  /**
   * List with search results of DSpace objects for the current query
   */
-  listEntries: SearchResult<DSpaceObject>[] = [];
+  listEntries: SearchResult<DSpaceObject>[] = null;

  /**
   * The current page to load
@@ -93,9 +96,9 @@ export class DSOSelectorComponent implements OnInit, OnDestroy {
  hasNextPage = false;

  /**
-   * Whether or not the list should be reset next time it receives a page to load
+   * Whether or not new results are currently loading
   */
-  resetList = false;
+  loading = false;

  /**
   * List of element references to all elements
@@ -123,7 +126,9 @@ export class DSOSelectorComponent implements OnInit, OnDestroy {
   */
  public subs: Subscription[] = [];

-  constructor(protected searchService: SearchService) {
+  constructor(protected searchService: SearchService,
+              protected notifcationsService: NotificationsService,
+              protected translate: TranslateService) {
  }

  /**
@@ -136,7 +141,7 @@ export class DSOSelectorComponent implements OnInit, OnDestroy {
    // Create an observable searching for the current DSO (return empty list if there's no current DSO)
    let currentDSOResult$;
    if (isNotEmpty(this.currentDSOId)) {
-      currentDSOResult$ = this.search(this.getCurrentDSOQuery(), 1);
+      currentDSOResult$ = this.search(this.getCurrentDSOQuery(), 1).pipe(getFirstSucceededRemoteDataPayload());
    } else {
      currentDSOResult$ = observableOf(buildPaginatedList(undefined, []));
    }
@@ -152,31 +157,41 @@ export class DSOSelectorComponent implements OnInit, OnDestroy {
      this.currentPage$
    ).pipe(
      switchMap(([currentDSOResult, query, page]: [PaginatedList<SearchResult<DSpaceObject>>, string, number]) => {
+        this.loading = true;
        if (page === 1) {
          // The first page is loading, this means we should reset the list instead of adding to it
-          this.resetList = true;
+          this.listEntries = null;
        }
        return this.search(query, page).pipe(
-          map((list) => {
-            // If it's the first page and no query is entered, add the current DSO to the start of the list
-            // If no query is entered, filter out the current DSO from the results, as it'll be displayed at the start of the list already
-            list.page = [
-              ...((isEmpty(query) && page === 1) ? currentDSOResult.page : []),
-              ...list.page.filter((result) => isNotEmpty(query) || result.indexableObject.id !== this.currentDSOId)
-            ];
-            return list;
+          map((rd) => {
+            if (rd.hasSucceeded) {
+              // If it's the first page and no query is entered, add the current DSO to the start of the list
+              // If no query is entered, filter out the current DSO from the results, as it'll be displayed at the start of the list already
+              rd.payload.page = [
+                ...((isEmpty(query) && page === 1) ? currentDSOResult.page : []),
+                ...rd.payload.page.filter((result) => isNotEmpty(query) || result.indexableObject.id !== this.currentDSOId)
+              ];
+            } else if (rd.hasFailed) {
+              this.notifcationsService.error(this.translate.instant('dso-selector.error.title', { type: this.typesString }), rd.errorMessage);
+            }
+            return rd;
          })
        );
      })
-    ).subscribe((list) => {
-      if (this.resetList) {
-        this.listEntries = list.page;
-        this.resetList = false;
+    ).subscribe((rd) => {
+      this.loading = false;
+      if (rd.hasSucceeded) {
+        if (hasNoValue(this.listEntries)) {
+          this.listEntries = rd.payload.page;
+        } else {
+          this.listEntries.push(...rd.payload.page);
+        }
+        // Check if there are more pages available after the current one
+        this.hasNextPage = rd.payload.totalElements > this.listEntries.length;
      } else {
-        this.listEntries.push(...list.page);
+        this.listEntries = null;
+        this.hasNextPage = false;
      }
-      // Check if there are more pages available after the current one
-      this.hasNextPage = list.totalElements > this.listEntries.length;
    }));
  }

@@ -192,7 +207,7 @@ export class DSOSelectorComponent implements OnInit, OnDestroy {
   * @param query Query to search objects for
   * @param page  Page to retrieve
   */
-  search(query: string, page: number): Observable<PaginatedList<SearchResult<DSpaceObject>>> {
+  search(query: string, page: number): Observable<RemoteData<PaginatedList<SearchResult<DSpaceObject>>>> {
    return this.searchService.search(
      new PaginatedSearchOptions({
        query: query,
@@ -202,7 +217,7 @@ export class DSOSelectorComponent implements OnInit, OnDestroy {
        })
      })
    ).pipe(
-      getFirstSucceededRemoteDataPayload()
+      getFirstCompletedRemoteData()
    );
  }

@@ -210,7 +225,7 @@ export class DSOSelectorComponent implements OnInit, OnDestroy {
   * When the user reaches the bottom of the page (or almost) and there's a next page available, increase the current page
   */
  onScrollDown() {
-    if (this.hasNextPage) {
+    if (this.hasNextPage && !this.loading) {
      this.currentPage$.next(this.currentPage$.value + 1);
    }
  }
--- a/src/assets/i18n/en.json5
+++ b/src/assets/i18n/en.json5
@@ -1168,6 +1168,8 @@

  "dso-selector.edit.item.head": "Edit item",

+  "dso-selector.error.title": "An error occurred searching for a {{ type }}",
+
  "dso-selector.export-metadata.dspaceobject.head": "Export metadata from",

  "dso-selector.no-results": "No {{ type }} found",