Merge pull request DSpace#2183 from atmire/w2p-100302_Live-import-issues-7.5

Fix Live Import issues

Author: tdonohue

src/app/core/data/base/base-data.service.spec.ts

@@ -641,6 +641,62 @@ describe('BaseDataService', () => {
     });
   });
 
+  describe('hasCachedResponse', () => {
+    it('should return false when the request will be dispatched', (done) => {
+      const result = service.hasCachedResponse('test-href');
+
+      result.subscribe((hasCachedResponse) => {
+        expect(hasCachedResponse).toBeFalse();
+        done();
+      });
+    });
+
+    it('should return true when the request will not be dispatched', (done) => {
+      (requestService.shouldDispatchRequest as jasmine.Spy).and.returnValue(false);
+      const result = service.hasCachedResponse('test-href');
+
+      result.subscribe((hasCachedResponse) => {
+        expect(hasCachedResponse).toBeTrue();
+        done();
+      });
+    });
+  });
+
+  describe('hasCachedErrorResponse', () => {
+    it('should return false when no response is cached', (done) => {
+      spyOn(service,'hasCachedResponse').and.returnValue(observableOf(false));
+      const result = service.hasCachedErrorResponse('test-href');
+
+      result.subscribe((hasCachedErrorResponse) => {
+        expect(hasCachedErrorResponse).toBeFalse();
+        done();
+      });
+    });
+    it('should return false when no error response is cached', (done) => {
+      spyOn(service,'hasCachedResponse').and.returnValue(observableOf(true));
+      spyOn(rdbService,'buildSingle').and.returnValue(createSuccessfulRemoteDataObject$({}));
+
+      const result = service.hasCachedErrorResponse('test-href');
+
+      result.subscribe((hasCachedErrorResponse) => {
+        expect(hasCachedErrorResponse).toBeFalse();
+        done();
+      });
+    });
+
+    it('should return true when an error response is cached', (done) => {
+      spyOn(service,'hasCachedResponse').and.returnValue(observableOf(true));
+      spyOn(rdbService,'buildSingle').and.returnValue(createFailedRemoteDataObject$());
+
+      const result = service.hasCachedErrorResponse('test-href');
+
+      result.subscribe((hasCachedErrorResponse) => {
+        expect(hasCachedErrorResponse).toBeTrue();
+        done();
+      });
+    });
+  });
+
   describe('addDependency', () => {
     let addDependencySpy;
 

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

@@ -341,6 +341,48 @@ export class BaseDataService<T extends CacheableObject> implements HALDataServic
     }
   }
 
+  /**
+   * Checks for the provided href whether a response is already cached
+   * @param href$                       The url for which to check whether there is a cached response.
+   *                                    Can be a string or an Observable<string>
+   */
+  hasCachedResponse(href$: string | Observable<string>): Observable<boolean> {
+    if (isNotEmpty(href$)) {
+      if (typeof href$ === 'string') {
+        href$ = observableOf(href$);
+      }
+      return href$.pipe(
+        isNotEmptyOperator(),
+        take(1),
+        map((href: string) => {
+          const requestId = this.requestService.generateRequestId();
+          const request = new GetRequest(requestId, href);
+          return !this.requestService.shouldDispatchRequest(request, true);
+        }),
+      );
+    }
+    throw new Error(`Can't check whether there is a cached response for an empty href$`);
+  }
+
+  /**
+   * Checks for the provided href whether an ERROR response is currently cached
+   * @param href$                       The url for which to check whether there is a cached ERROR response.
+   *                                    Can be a string or an Observable<string>
+   */
+  hasCachedErrorResponse(href$: string | Observable<string>): Observable<boolean> {
+    return this.hasCachedResponse(href$).pipe(
+      switchMap((hasCachedResponse) => {
+        if (hasCachedResponse) {
+           return this.rdbService.buildSingle(href$).pipe(
+             getFirstCompletedRemoteData(),
+             map((rd => rd.hasFailed))
+           );
+        }
+        return observableOf(false);
+      })
+    );
+  }
+
   /**
    * Return the links to traverse from the root of the api to the
    * endpoint this DataService represents

src/app/core/data/external-source-data.service.spec.ts

@@ -5,6 +5,7 @@ import { ExternalSourceEntry } from '../shared/external-source-entry.model';
 import { of as observableOf } from 'rxjs';
 import { GetRequest } from './request.models';
 import { testSearchDataImplementation } from './base/search-data.spec';
+import { take } from 'rxjs/operators';
 
 describe('ExternalSourceService', () => {
   let service: ExternalSourceDataService;
@@ -64,19 +65,42 @@ describe('ExternalSourceService', () => {
   });
 
   describe('getExternalSourceEntries', () => {
-    let result;
 
-    beforeEach(() => {
-      result = service.getExternalSourceEntries('test');
-    });
+    describe('when no error response is cached', () => {
+      let result;
+      beforeEach(() => {
+        spyOn(service, 'hasCachedErrorResponse').and.returnValue(observableOf(false));
+        result = service.getExternalSourceEntries('test');
+      });
 
-    it('should send a GetRequest', () => {
-      expect(requestService.send).toHaveBeenCalledWith(jasmine.any(GetRequest), true);
+      it('should send a GetRequest', () => {
+        result.pipe(take(1)).subscribe();
+        expect(requestService.send).toHaveBeenCalledWith(jasmine.any(GetRequest), true);
+      });
+
+      it('should return the entries', () => {
+        result.subscribe((resultRD) => {
+          expect(resultRD.payload.page).toBe(entries);
+        });
+      });
     });
 
-    it('should return the entries', () => {
-      result.subscribe((resultRD) => {
-        expect(resultRD.payload.page).toBe(entries);
+    describe('when an error response is cached', () => {
+      let result;
+      beforeEach(() => {
+        spyOn(service, 'hasCachedErrorResponse').and.returnValue(observableOf(true));
+        result = service.getExternalSourceEntries('test');
+      });
+
+      it('should send a GetRequest', () => {
+        result.pipe(take(1)).subscribe();
+        expect(requestService.send).toHaveBeenCalledWith(jasmine.any(GetRequest), false);
+      });
+
+      it('should return the entries', () => {
+        result.subscribe((resultRD) => {
+          expect(resultRD.payload.page).toBe(entries);
+        });
       });
     });
   });

src/app/core/data/external-source-data.service.ts

@@ -74,7 +74,12 @@ export class ExternalSourceDataService extends IdentifiableDataService<ExternalS
     );
 
     // TODO create a dedicated ExternalSourceEntryDataService and move this entire method to it. Then the "as any"s won't be necessary
-    return this.findListByHref(href$, undefined, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow as any) as any;
+
+    return this.hasCachedErrorResponse(href$).pipe(
+      switchMap((hasCachedErrorResponse) => {
+        return this.findListByHref(href$, undefined, !hasCachedErrorResponse, reRequestOnStale, ...linksToFollow as any);
+      })
+    ) as any;
   }
 
   /**

src/app/shared/mocks/request.service.mock.ts

@@ -14,6 +14,7 @@ export function getMockRequestService(requestEntry$: Observable<RequestEntry> =
     removeByHrefSubstring: observableOf(true),
     setStaleByHrefSubstring: observableOf(true),
     setStaleByUUID: observableOf(true),
-    hasByHref$: observableOf(false)
+    hasByHref$: observableOf(false),
+    shouldDispatchRequest: true
   });
 }

src/app/submission/import-external/import-external-collection/submission-import-external-collection.component.html

@@ -9,7 +9,6 @@
     <ds-themed-collection-dropdown [ngClass]="{'d-none': isLoading()}"
                             (selectionChange)="selectObject($event)"
                             (searchComplete)="searchComplete()"
-                            (theOnlySelectable)="theOnlySelectable($event)"
                             [entityType]="entityType">
     </ds-themed-collection-dropdown>
   </div>

src/app/submission/import-external/import-external-collection/submission-import-external-collection.component.spec.ts

@@ -64,7 +64,7 @@ describe('SubmissionImportExternalCollectionComponent test suite', () => {
       compAsAny = null;
     });
 
-    it('should emit from selectedEvent on selectObject', () => {
+    it('should emit from selectedEvent on selectObject and set loading to true', () => {
       spyOn(comp.selectedEvent, 'emit').and.callThrough();
 
       const entry = {
@@ -79,6 +79,7 @@ describe('SubmissionImportExternalCollectionComponent test suite', () => {
       comp.selectObject(entry);
 
       expect(comp.selectedEvent.emit).toHaveBeenCalledWith(entry);
+      expect(comp.loading).toBeTrue();
     });
 
     it('should dismiss modal on closeCollectionModal', () => {

src/app/submission/import-external/import-external-collection/submission-import-external-collection.component.ts

@@ -38,6 +38,7 @@ export class SubmissionImportExternalCollectionComponent {
    * This method emits the selected Collection from the 'selectedEvent' variable.
    */
   public selectObject(object: CollectionListEntry): void {
+    this.loading = true;
     this.selectedEvent.emit(object);
   }
 

src/app/submission/import-external/submission-import-external.component.ts

@@ -15,7 +15,9 @@ import { Context } from '../../core/shared/context.model';
 import { PaginationComponentOptions } from '../../shared/pagination/pagination-component-options.model';
 import { RouteService } from '../../core/services/route.service';
 import { createSuccessfulRemoteDataObject } from '../../shared/remote-data.utils';
-import { SubmissionImportExternalPreviewComponent } from './import-external-preview/submission-import-external-preview.component';
+import {
+  SubmissionImportExternalPreviewComponent
+} from './import-external-preview/submission-import-external-preview.component';
 import { fadeIn } from '../../shared/animations/fade';
 import { PageInfo } from '../../core/shared/page-info.model';
 import { hasValue, isNotEmpty } from '../../shared/empty.util';