Merge pull request #4388 from atmire/backport-4060-submission-form-getting-stuck-in-loop-contribute-8

[Port dspace-8_x] Fix infinite loading submission forms

Author: tdonohue

src/app/core/submission/submission-rest.service.spec.ts

@@ -1,4 +1,5 @@
 import { getTestScheduler } from 'jasmine-marbles';
+import { of } from 'rxjs';
 import { TestScheduler } from 'rxjs/testing';
 
 import { FormFieldMetadataValueObject } from '../../shared/form/builder/models/form-field-metadata-value.model';
@@ -13,6 +14,7 @@ import {
   SubmissionRequest,
 } from '../data/request.models';
 import { RequestService } from '../data/request.service';
+import { RequestEntry } from '../data/request-entry.model';
 import { SubmissionRestService } from './submission-rest.service';
 
 describe('SubmissionRestService test suite', () => {
@@ -38,7 +40,9 @@ describe('SubmissionRestService test suite', () => {
   }
 
   beforeEach(() => {
-    requestService = getMockRequestService();
+    requestService = getMockRequestService(of(Object.assign(new RequestEntry(), {
+      request: new SubmissionRequest('mock-request-uuid', 'mock-request-href'),
+    })));
     rdbService = getMockRemoteDataBuildService();
     scheduler = getTestScheduler();
     halService = new HALEndpointServiceStub(resourceEndpointURL);
@@ -62,7 +66,7 @@ describe('SubmissionRestService test suite', () => {
       scheduler.schedule(() => service.getDataById(resourceEndpoint, resourceScope).subscribe());
       scheduler.flush();
 
-      expect(requestService.send).toHaveBeenCalledWith(expected);
+      expect(requestService.send).toHaveBeenCalledWith(expected, false);
     });
   });
 

src/app/core/submission/submission-rest.service.ts

@@ -1,19 +1,23 @@
 import { Injectable } from '@angular/core';
-import { Observable } from 'rxjs';
+import {
+  Observable,
+  skipWhile,
+} from 'rxjs';
 import {
   distinctUntilChanged,
   filter,
   map,
   mergeMap,
+  switchMap,
   tap,
 } from 'rxjs/operators';
 
 import {
   hasValue,
+  hasValueOperator,
   isNotEmpty,
 } from '../../shared/empty.util';
 import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service';
-import { ErrorResponse } from '../cache/response.models';
 import { RemoteData } from '../data/remote-data';
 import {
   DeleteRequest,
@@ -24,15 +28,30 @@ import {
   SubmissionRequest,
 } from '../data/request.models';
 import { RequestService } from '../data/request.service';
-import { RequestError } from '../data/request-error.model';
-import { RestRequest } from '../data/rest-request.model';
 import { HttpOptions } from '../dspace-rest/dspace-rest.service';
 import { HALEndpointService } from '../shared/hal-endpoint.service';
 import { getFirstCompletedRemoteData } from '../shared/operators';
 import { SubmitDataResponseDefinitionObject } from '../shared/submit-data-response-definition.model';
 import { URLCombiner } from '../url-combiner/url-combiner';
 import { SubmissionResponse } from './submission-response.model';
 
+/**
+ * Retrieve the first emitting payload's dataDefinition, or throw an error if the request failed
+ */
+export const getFirstDataDefinition = () =>
+  (source: Observable<RemoteData<SubmissionResponse>>): Observable<SubmitDataResponseDefinitionObject> =>
+    source.pipe(
+      getFirstCompletedRemoteData(),
+      map((response: RemoteData<SubmissionResponse>) => {
+        if (response.hasFailed) {
+          throw new Error(response.errorMessage);
+        } else {
+          return hasValue(response?.payload?.dataDefinition) ? response.payload.dataDefinition : [response.payload];
+        }
+      }),
+      distinctUntilChanged(),
+    );
+
 /**
  * The service handling all submission REST requests
  */
@@ -56,15 +75,7 @@ export class SubmissionRestService {
    */
   protected fetchRequest(requestId: string): Observable<SubmitDataResponseDefinitionObject> {
     return this.rdbService.buildFromRequestUUID<SubmissionResponse>(requestId).pipe(
-      getFirstCompletedRemoteData(),
-      map((response: RemoteData<SubmissionResponse>) => {
-        if (response.hasFailed) {
-          throw new ErrorResponse({ statusText: response.errorMessage, statusCode: response.statusCode } as RequestError);
-        } else {
-          return hasValue(response.payload) ? response.payload.dataDefinition : response.payload;
-        }
-      }),
-      distinctUntilChanged(),
+      getFirstDataDefinition(),
     );
   }
 
@@ -116,21 +127,52 @@ export class SubmissionRestService {
    *    The endpoint link name
    * @param id
    *    The submission Object to retrieve
+   * @param useCachedVersionIfAvailable
+   *    If this is true, the request will only be sent if there's no valid & cached version. Defaults to false
    * @return Observable<SubmitDataResponseDefinitionObject>
    *     server response
    */
-  public getDataById(linkName: string, id: string): Observable<SubmitDataResponseDefinitionObject> {
-    const requestId = this.requestService.generateRequestId();
+  public getDataById(linkName: string, id: string, useCachedVersionIfAvailable = false): Observable<SubmitDataResponseDefinitionObject> {
     return this.halService.getEndpoint(linkName).pipe(
       map((endpointURL: string) => this.getEndpointByIDHref(endpointURL, id)),
       filter((href: string) => isNotEmpty(href)),
       distinctUntilChanged(),
-      map((endpointURL: string) => new SubmissionRequest(requestId, endpointURL)),
-      tap((request: RestRequest) => {
-        this.requestService.send(request);
+      mergeMap((endpointURL: string) => {
+        this.sendGetDataRequest(endpointURL, useCachedVersionIfAvailable);
+        const startTime: number = new Date().getTime();
+        return this.requestService.getByHref(endpointURL).pipe(
+          map((requestEntry) => requestEntry?.request?.uuid),
+          hasValueOperator(),
+          distinctUntilChanged(),
+          switchMap((requestId) => this.rdbService.buildFromRequestUUID<SubmissionResponse>(requestId)),
+          // This skip ensures that if a stale object is present in the cache when you do a
+          // call it isn't immediately returned, but we wait until the remote data for the new request
+          // is created. If useCachedVersionIfAvailable is false it also ensures you don't get a
+          // cached completed object
+          skipWhile((rd: RemoteData<SubmissionResponse>) => rd.isStale || (!useCachedVersionIfAvailable && rd.lastUpdated < startTime)),
+          tap((rd: RemoteData<SubmissionResponse>) => {
+            if (hasValue(rd) && rd.isStale) {
+              this.sendGetDataRequest(endpointURL, useCachedVersionIfAvailable);
+            }
+          }),
+        );
       }),
-      mergeMap(() => this.fetchRequest(requestId)),
-      distinctUntilChanged());
+      getFirstDataDefinition(),
+    );
+  }
+
+  /**
+   * Send a GET SubmissionRequest
+   *
+   * @param href
+   *    Endpoint URL of the submission data
+   * @param useCachedVersionIfAvailable
+   *    If this is true, the request will only be sent if there's no valid & cached version. Defaults to false
+   */
+  private sendGetDataRequest(href: string, useCachedVersionIfAvailable = false) {
+    const requestId = this.requestService.generateRequestId();
+    const request = new SubmissionRequest(requestId, href);
+    this.requestService.send(request, useCachedVersionIfAvailable);
   }
 
   /**

src/app/submission/form/submission-form.component.spec.ts

@@ -47,7 +47,7 @@ import { SubmissionFormSectionAddComponent } from './section-add/submission-form
 import { SubmissionFormComponent } from './submission-form.component';
 import { ThemedSubmissionUploadFilesComponent } from './submission-upload-files/themed-submission-upload-files.component';
 
-describe('SubmissionFormComponent Component', () => {
+describe('SubmissionFormComponent', () => {
 
   let comp: SubmissionFormComponent;
   let compAsAny: any;
@@ -227,7 +227,6 @@ describe('SubmissionFormComponent Component', () => {
       });
       scheduler.flush();
 
-      expect(comp.collectionId).toEqual(submissionObjectNew.collection.id);
       expect(comp.submissionDefinition).toEqual(submissionObjectNew.submissionDefinition);
       expect(comp.definitionId).toEqual(submissionObjectNew.submissionDefinition.name);
       expect(comp.sections).toEqual(submissionObjectNew.sections);
@@ -265,7 +264,6 @@ describe('SubmissionFormComponent Component', () => {
       });
       scheduler.flush();
 
-      expect(comp.collectionId).toEqual('45f2f3f1-ba1f-4f36-908a-3f1ea9a557eb');
       expect(submissionServiceStub.resetSubmissionObject).not.toHaveBeenCalled();
       done();
     });

src/app/submission/form/submission-form.component.ts

@@ -286,13 +286,12 @@ export class SubmissionFormComponent implements OnChanges, OnDestroy {
    *    new submission object
    */
   onCollectionChange(submissionObject: SubmissionObject) {
-    this.collectionId = (submissionObject.collection as Collection).id;
     if (this.definitionId !== (submissionObject.submissionDefinition as SubmissionDefinitionsModel).name) {
       this.sections = submissionObject.sections;
       this.submissionDefinition = (submissionObject.submissionDefinition as SubmissionDefinitionsModel);
       this.definitionId = this.submissionDefinition.name;
       this.submissionService.resetSubmissionObject(
-        this.collectionId,
+        (submissionObject.collection as Collection).id,
         this.submissionId,
         submissionObject._links.self.href,
         this.submissionDefinition,

src/app/workflowitems-edit-page/workflow-item-page.resolver.ts

@@ -9,8 +9,8 @@ import { Observable } from 'rxjs';
 import { RemoteData } from '../core/data/remote-data';
 import { getFirstCompletedRemoteData } from '../core/shared/operators';
 import { WorkflowItem } from '../core/submission/models/workflowitem.model';
+import { SUBMISSION_LINKS_TO_FOLLOW } from '../core/submission/resolver/submission-links-to-follow';
 import { WorkflowItemDataService } from '../core/submission/workflowitem-data.service';
-import { followLink } from '../shared/utils/follow-link-config.model';
 
 export const workflowItemPageResolver: ResolveFn<RemoteData<WorkflowItem>> = (
   route: ActivatedRouteSnapshot,
@@ -21,7 +21,7 @@ export const workflowItemPageResolver: ResolveFn<RemoteData<WorkflowItem>> = (
     route.params.id,
     true,
     false,
-    followLink('item'),
+    ...SUBMISSION_LINKS_TO_FOLLOW,
   ).pipe(
     getFirstCompletedRemoteData(),
   );

src/app/workspaceitems-edit-page/workspace-item-page.resolver.ts

@@ -9,8 +9,8 @@ import { Observable } from 'rxjs';
 import { RemoteData } from '../core/data/remote-data';
 import { getFirstCompletedRemoteData } from '../core/shared/operators';
 import { WorkspaceItem } from '../core/submission/models/workspaceitem.model';
+import { SUBMISSION_LINKS_TO_FOLLOW } from '../core/submission/resolver/submission-links-to-follow';
 import { WorkspaceitemDataService } from '../core/submission/workspaceitem-data.service';
-import { followLink } from '../shared/utils/follow-link-config.model';
 
 /**
  * Method for resolving a workflow item based on the parameters in the current route
@@ -28,7 +28,7 @@ export const workspaceItemPageResolver: ResolveFn<RemoteData<WorkspaceItem>> = (
   return workspaceItemService.findById(route.params.id,
     true,
     false,
-    followLink('item'),
+    ...SUBMISSION_LINKS_TO_FOLLOW,
   ).pipe(
     getFirstCompletedRemoteData(),
   );